Blocking all signals on non-main threads is not the way to go, at least
the handlers for SIGSEGV, SIGBUS, SIGILL, SIGABRT and SIGFPE need to run
so we get backtraces. Otherwise the process just exits.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Add a new condition var and mutex to serialize pthread startup.
When a new pthread is started, it will wait very early on for the
parent pthread to permit it to run. This ensures that that the
ordering between parent and child is predictable.
Signed-off-by: Mark Stapp <mjs@cisco.com>
In a non-controlled startup, the rcu data structures were
not being created until after logging could happen. This
is bad. Move it so that the rcu data structures are
created first, before logging( HA! ) can happen.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
External libraries can re-enter the FRR code through a hook function. A
crash occurs when logging from this hook function if the library has
initiated a new pthread, as the FRR RCU context is not initialized for
this thread.
Add frr_pthread_non_controlled_startup() function to initialize a valid
RCU context within a FRR pthread context, originating from an external
pthread.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Effectively a massive search and replace of
`struct thread` to `struct event`. Using the
term `thread` gives people the thought that
this event system is a pthread when it is not
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
convert:
frr_with_mutex(..)
to:
frr_with_mutex (..)
To make all our code agree with what clang-format is going to produce
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Creating any threads before we fork() into the background (if `-d` is
given) is an extremely dangerous footgun; the threads are created in
the parent and terminated when that exits.
This is extra dangerous because while testing, you'd often run the
daemon in foreground without `-d`, and everything works as expected.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Back when I put this together in 2015, ISO C11 was still reasonably new
and we couldn't require it just yet. Without ISO C11, there is no
"good" way (only bad hacks) to require a semicolon after a macro that
ends with a function definition. And if you added one anyway, you'd get
"spurious semicolon" warnings on some compilers...
With C11, `_Static_assert()` at the end of a macro will make it so that
the semicolon is properly required, consumed, and not warned about.
Consistently requiring semicolons after "file-level" macros matches
Linux kernel coding style and helps some editors against mis-syntax'ing
these macros.
Signed-off-by: David Lamparter <equinox@diac24.net>
- tracepoint() -> frrtrace()
- tracelog() -> frrtracelog()
- tracepoint_enabled() -> frrtrace_enabled()
Also removes copypasta'd #ifdefs for those LTTng macros, those are
handled in lib/trace.h
Signed-off-by: Quentin Young <qlyoung@nvidia.com>
Block signals in child/additional pthreads; frr daemons generally
expect that only the main thread will handle signals.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
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>
Pthreads were not being deleted from the list after destruction. This
isn't causing any bugs currently but that's just by dumb luck.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
frr_with_mutex(...) { ... } locks and automatically unlocks the listed
mutex(es) when the block is exited. This adds a bit of safety against
forgetting the unlock in error paths & co. and makes the code a slight
bit more readable.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
CC lib/frr_pthread.lo
lib/frr_pthread.c:128:40: error: too many arguments to function call, expected 1, have 3
ret = pthread_setname_np(fpt->thread, fpt->os_name, NULL);
~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~~~
/Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/usr/include/pthread.h:512:1: note: 'pthread_setname_np' declared here
__API_AVAILABLE(macos(10.6), ios(3.2))
Mac OS does have pthread_setname_np, but we can't use it here since it
only accepts a single argument, the thread name, and thus only works for
the current thread.
Signed-off-by: Ruben Kerkhof <ruben@rubenkerkhof.com>
The current invocation of frr_pthread_set_name was causing it reset the os_name.
There is no need for this, we now always create the pthread appropriately
to have both name and os_name. So convert this function to a simple
call through of the pthread call now.
Before(any of these changes):
sharpd@robot ~/frr1> ps -L -p 16895
PID LWP TTY TIME CMD
16895 16895 ? 00:01:39 bgpd
16895 16896 ? 00:00:54
16895 16897 ? 00:00:07 bgpd_ka
After:
sharpd@donna ~/frr1> ps -L -p 1752
PID LWP TTY TIME CMD
1752 1752 ? 00:00:00 bgpd
1752 1753 ? 00:00:00 bgpd_io
1752 1754 ? 00:00:00 bgpd_ka
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
When we start a thread we always call fpt_run and since
the last commit we know os_name is filled with something,
therefore we can just set the name on startup.
This creates this output now for zebra:
sharpd@donna ~/frr2> ps -L -p 25643
PID LWP TTY TIME CMD
25643 25643 ? 00:00:00 zebra
25643 25644 ? 00:00:00 Zebra dplane
25643 25684 ? 00:00:00 zebra_apic
sharpd@donna ~/frr2>
I removed the abstraction to frr_pthread_set_name because
it was snprintf'ing into the same buffer which was the
real bug here( the first character of os_name became null).
In the next commit I'll remove that api because
it is unneeded and was a horrible hack to get
this to work for the one place it was wanted.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
On call of frr_pthread_new, save the os_name if given,
if not given use the name passed in( shortening to fit
in available space ) and finally if the name was not
passed in use the default value.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
OS-level yield is generally a bad and possibly dangerous idea. If the
thread should be suspended, there should always be something to wait on,
or it turns into busy waiting. And if it's "just giving something else
the chance to run" - that's the kernel's job to determine, and the
kernel will do so while considering priorities, cgroups, and whatnot.
Let it do its job.
Signed-off-by: David Lamparter <equinox@diac24.net>
All I can see is an unneccessary complication. If there's some purpose
here it needs to be documented...
Signed-off-by: David Lamparter <equinox@diac24.net>
FreeBSD supports pthread_set_name_np() too. Also, pthread_set_name_np()
returns void. And NetBSD has pthread_setname_np() with an extra arg...
Signed-off-by: David Lamparter <equinox@diac24.net>
Some work on FRR's pthread wrapper.
* Provide a built-in way to synchronize thread startup
* Make utility functions take frr_pthread * instead of its integer ID
* Pass frr_pthread * as pthread start function argument
* Correct some comment styling
* Rename some variables to match naming conventions in the file
* Change parameter ordering in stop function prototype to follow the
convention in the other functions
* Default new frr_pthreads to using a vanilla event loop
For the last point, the original goal when designing the implementation
of pthreads into FRR was to be able to use the thread.c event based
system inside pthreads. This code essentially encapuslates all the
thread.c functionality into an easy to use pthread out of the box.
Creating a new frr_pthread with a null attributes field will cause the
created frr_pthread to run a thread.c event loop. The upshot of this is
that it is now possible to safely run existing functions in a pthread in
roughly 3 lines of code. It also serves as an example / starting point
for others.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
