Replace all `random()` calls with a function called `frr_weak_random()`
and make it clear that it is only supposed to be used for weak random
applications.
Use the annotation described by the Coverity Scan documentation to
ignore `random()` call warnings.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
This is a full rewrite of the "back end" logging code. It now uses a
lock-free list to iterate over logging targets, and the targets
themselves are as lock-free as possible. (syslog() may have a hidden
internal mutex in the C library; the file/fd targets use a single
write() call which should ensure atomicity kernel-side.)
Note that some functionality is lost in this patch:
- Solaris printstack() backtraces are ditched (unlikely to come back)
- the `log-filter` machinery is gone (re-added in followup commit)
- `terminal monitor` is temporarily stubbed out. The old code had a
race condition with VTYs going away. It'll likely come back rewritten
and with vtysh support.
- The `zebra_ext_log` hook is gone. Instead, it's now much easier to
add a "proper" logging target.
v2: TLS buffer to get some actual performance
Signed-off-by: David Lamparter <equinox@diac24.net>
When watchfrr starts up, it first tries to connect to daemons. This is
expected to fail if we are just starting up FRR, but we log it as an
error, and it shows up red in journalctl. Similarly when we fork
background commands that is also logged as an error. This is scaring
users, let's change these to info.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
The vrrpd one conflicts with the standalone vrrpd package; also we're
installing daemons to /usr/lib/frr on some systems so they're not on
PATH.
Signed-off-by: David Lamparter <equinox@diac24.net>
Allow systemd to be informed about operational state so operators can
infer a bit about what is going on with FRR from the systemd status
cli.
sharpd@robot ~/frr4> systemctl status frr
● frr.service - FRRouting
Loaded: loaded (/usr/lib/systemd/system/frr.service; enabled; vendor preset: disabled)
Active: active (running) since Thu 2019-10-03 21:09:04 EDT; 7s ago
Docs: https://frrouting.readthedocs.io/en/latest/setup.html
Process: 32455 ExecStart=/usr/lib/frr/frrinit.sh start (code=exited, status=0/SUCCESS)
Status: "FRR Operational"
Tasks: 12 (limit: 4915)
Memory: 76.5M
CGroup: /system.slice/frr.service
├─32468 /usr/lib/frr/watchfrr -d zebra bgpd staticd
├─32487 /usr/lib/frr/zebra -d -A 127.0.0.1 -s 90000000
├─32492 /usr/lib/frr/bgpd -d -A 127.0.0.1
└─32500 /usr/lib/frr/staticd -d -A 127.0.0.1
Please note the `Status: ...` line above.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
There is a fairly common state we are seeing where watchfrr
has decided that something is not right and is printing out
a `wtf` message. At this point I am not sure what is going on
or how we are getting here, but let's add a bit more data dump
to the message so that we can figure out what is going on.
This is mainly being done because at this point in time I have no
clue the what/how of how we got here and I cannot reproduce.
Maybe by adding more useful information here I can figure out what is
going on.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com.
Allow an end user who is debugging behavior, with say gdb, to turn
off watchfrr and it's attempts to keep control of a daemons up/responsiveness
With code change:
donna.cumulusnetworks.com# show watchfrr
watchfrr global phase: Idle
zebra Up
bgpd Up/Ignoring Timeout
staticd Up
Now grab bgpd with gdb:
sharpd@donna ~/frr4> date ; sudo gdb -p 27893
Mon 16 Sep 2019 01:44:57 PM EDT
GNU gdb (GDB) Fedora 8.3-6.fc30
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word".
Attaching to process 27893
[New LWP 27894]
[New LWP 27895]
[New LWP 27896]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
0x00007f1787a3e5c7 in poll () from /lib64/libc.so.6
Missing separate debuginfos, use: dnf debuginfo-install glibc-2.29-15.fc30.x86_64 gperftools-libs-2.7-5.fc30.x86_64 json-c-0.13.1-4.fc30.x86_64 libcap-2.26-5.fc30.x86_64 libgcc-9.1.1-1.fc30.x86_64 libgcrypt-1.8.4-3.fc30.x86_64 libgpg-error-1.33-2.fc30.x86_64 libstdc++-9.1.1-1.fc30.x86_64 libxcrypt-4.4.6-2.fc30.x86_64 libyang-0.16.105-1.fc30.x86_64 lua-libs-5.3.5-5.fc30.x86_64 lz4-libs-1.8.3-2.fc30.x86_64 pcre-8.43-2.fc30.x86_64 xz-libs-5.2.4-5.fc30.x86_64
(gdb)
In another window we can see when watchfrr thinks it's not responding:
donna.cumulusnetworks.com# show watchfrr
watchfrr global phase: Idle
zebra Up
bgpd Unresponsive/Ignoring Timeout
staticd Up
Finally exit gdb and watchfrr now believes bgpd is good to go again:
donna.cumulusnetworks.com# show watchfrr
watchfrr global phase: Idle
zebra Up
bgpd Up/Ignoring Timeout
staticd Up
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
When the user specifies -N namespace allow it to influence the
frr_vtydir(DAEMON_VTY_DIR) to have namespace in it's path
like so: $frrstate_dir/<namespace>
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
In a variety of places we are using DAEMON_VTY_DIR, convert
to use frr_vtydir. This will allow us in a future commit
to have the -N namespace option be automatically used.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
There's no good reason to not have these options default to the
installation path of tools/watchfrr.sh. Doing so allows us to ditch
watchfrr_options from daemons/daemons.conf completely.
Fixes: #3652
Signed-off-by: David Lamparter <equinox@diac24.net>
If we wait forever for all daemons to come up, we can hang the entire
boot process, especially on init.d based systems.
Signed-off-by: David Lamparter <equinox@diac24.net>
- some target_CFLAGS that needed to include AM_CFLAGS didn't do so
- libyang/sysrepo/sqlite3/confd CFLAGS + LIBS weren't used at all
- consistently use $(FOO_CFLAGS) instead of @FOO_CFLAGS@
- 2 dependencies were missing for clippy
Signed-off-by: David Lamparter <equinox@diac24.net>
When we make a call to (re)start some daemon(s), we can immediately try
connecting to its VTY socket after the script completes. If the daemon
started correctly, this will always succeed since the start script only
returns after daemon startup is complete.
Among other things, this reduces the delay to "startup complete"
notification at initial watchfrr start.
Signed-off-by: David Lamparter <equinox@diac24.net>
We were waiting for timers to expire even when we already know the
status of all daemons. This delays startup for no good reason.
Signed-off-by: David Lamparter <equinox@diac24.net>
This cleans up watchfrr to be more "normal" like the other daemons in
terms of what it does in main(), i.e. using the full frr_*() call set.
Also, this changes the startup behaviour on watchfrr to stay attached on
the daemon's parent process until startup is really complete. This
should allow removing the "watchfrr.started" hack at some point.
Signed-off-by: David Lamparter <equinox@diac24.net>
The stderr output is not being displayed as part of watchfrr invocation
in system startup. Specifically if the user has not properly sent
1 or more daemons to monitor. If the end-user is using tools/frr
this stderr is dropped( and systemd appears to drop stderr too? )
Modify the two stderr calls in this situation and use the zlog system.
Now I can clearly see an error message that tells me what has gone wrong.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
[DL: fixed typo]
Since we're now building through one large Makefile, we can easily put
things with their daemons and crossreference nicely.
Signed-off-by: David Lamparter <equinox@diac24.net>
watchfrr needs to handle a SIGCHLD also when it calls a global restart
command. Before this patch, it would lead to the following behavior:
15:44:28: zebra state -> down : unexpected read error: Connection reset by peer
15:44:33: Forked background command [pid 6392]: /usr/sbin/frr.init watchrestart all
15:44:53: Warning: restart all child process 6392 still running after 20 seconds, sending signal 15
15:44:53: waitpid returned status for an unknown child process 6392
15:44:53: background (unknown) process 6392 terminated due to signal 15
15:45:13: Warning: restart all child process 6392 still running after 40 seconds, sending signal 9
15:45:33: Warning: restart all child process 6392 still running after 60 seconds, sending signal 9
15:45:53: Warning: restart all child process 6392 still running after 80 seconds, sending signal 9
15:46:13: Warning: restart all child process 6392 still running after 100 seconds, sending signal 9
15:46:33: Warning: restart all child process 6392 still running after 120 seconds, sending signal 9
15:46:53: Warning: restart all child process 6392 still running after 140 seconds, sending signal 9
This is obviously incorrect and can be fixed by comparing the pid to
the global restart object as well.
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
* Use the correct license header
* Stop headers from including themselves
* Use uniform relative include conventions
* Ensure that sources include what they use
* Turn off clang-format around struct array blocks
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
The following types are nonstandard:
- u_char
- u_short
- u_int
- u_long
- u_int8_t
- u_int16_t
- u_int32_t
Replace them with the C99 standard types:
- uint8_t
- unsigned short
- unsigned int
- unsigned long
- uint8_t
- uint16_t
- uint32_t
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
If a daemon is restarting, crashed, or otherwise in the process of
reconnecting to watchfrr and a user issues "write memory" or "write
file" the resulting config will not include the configuration of that
daemon. This is problematic because this output will overwrite the
previous config, potentially causing unintentional loss of configuration
stored only in the config file based upon timing.
This patch remedies that by making watchfrr check that all daemons are
up before attempting a configuration write, and updating vtysh so that
its failsafe respects this condition as well.
Note that this issue only manifests when using integrated config.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
The default timeout of 10 seconds is too quick of a timeout
given some long running cli commands. Modify watchfrr
to have a 90s timeout value instead.
Signed-off-by: Brian Rak <brianrak@gameservers.com>
Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com>
This fixes the following
cel-redxp-10# show debugging
Debugging Information for zebra:
Zebra debugging status:
Debugging Information for bgpd:
BGP debugging status:
Debugging Information for watchfrr:
% Command incomplete.
% Command incomplete.
cel-redxp-10#
Remove -R, -a, -A, -e and -z options. Also remove blocker in the code
that refuses to start if --dry is given together with -k / -s / -r.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
This leaves what were previously modes 0 (monitor-only) and 3 (restart
daemons individually, but restart everything if zebra is restarted).
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
The FSF's address changed, and we had a mixture of comment styles for
the GPL file header. (The style with * at the beginning won out with
580 to 141 in existing files.)
Note: I've intentionally left intact other "variations" of the copyright
header, e.g. whether it says "Zebra", "Quagga", "FRR", or nothing.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Pass pointer to pointer instead of assigning by return value. See
previous commit message.
To ensure that the behavior stays functionally correct, any assignments
with the result of a thread_add* function have been transformed to set
the pointer to null before passing it. These can be removed wherever the
pointer is known to already be null.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
The way thread.c is written, a caller who wishes to be able to cancel a
thread or avoid scheduling it twice must keep a reference to the thread.
Typically this is done with a long lived pointer whose value is checked
for null in order to know if the thread is currently scheduled. The
check-and-schedule idiom is so common that several wrapper macros in
thread.h existed solely to provide it.
This patch removes those macros and adds a new parameter to all
thread_add_* functions which is a pointer to the struct thread * to
store the result of a scheduling call. If the value passed is non-null,
the thread will only be scheduled if the value is null. This helps with
consistency.
A Coccinelle spatch has been used to transform code of the form:
if (t == NULL)
t = thread_add_* (...)
to the form
thread_add_* (..., &t)
The THREAD_ON macros have also been transformed to the underlying
thread.c calls.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Centralise read_config/daemonize/dryrun/pidfile/vty_serv into libfrr.
This also makes multi-instance pid/config handling available as part of
the library. It's only wired up in ospfd, but the code is in lib/.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Start centralising startup & option parsing into the library.
FRR_DAEMON_INFO is a bit weird, but it will become useful later (e.g.
for killing the ZLOG_* enum, and having the daemon name available)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
The library libzebra that is installed with FRR will
conflict with Quagga. So let's rename it to libfrr.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
before this patch, watchfrr was using an inconsistent style that matched
neither GNU nor Linux kernel nor BSD style. Reindent to Linux kernel
style, which should be pretty close to pimd & ldpd.
indent (2.2.11) options used:
-nbad -bap -nbc -bbo -hnl -br -brs -c33 -cd33 -ncdb -ce -ci4
-cli0 -d0 -di1 -nfc1 -i8 -ip0 -l80 -lp -npcs -nprs -npsl -sai
-saf -saw -ncs -nsc -sob -nfca -cp33 -ss -ts8 -il1
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
