Passing stack value to thread_add_* causes thread->ref to become an
invalid pointer when the value goes out of scope
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
The CLI changes now make it impossible for numbers
outside the range specified in the cli to make it to
this code. No need to check for it again.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
We have several pieces of code like this in FRR:
for (afi = AFI_IP; afi < AFI_MAX; afi++)
for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++)
bgp_distance_table[afi][safi] = bgp_table_init (afi, safi);
We were creating a lot of useless garbage in the code because of this
gap. Fix this.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
To avoid blocking zebra when it's acting as a proxy for an external
label manager.
Besides:
Fix get chunk reconnection. Socket was still being destroyed on failure,
so next attempt would never work.
Filter out unwanted messages in lm sync sock.
Until LDE client sends ZEBRA_LABEL_MANAGER_CONNECT message, zserv
doesn't know which kind of client it is, so it might enqueue unwanted
messages like interface add, interface up, etc. Changes in this commit
discard those messages in the client side in case they arrive before the
expected response.
Change function name for zclient_connect in label manager to avoid
confusion with zclient one.
Signed-off-by: ßingen <bingen@voltanet.io>
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>
When scheduling a thread, the scheduling function returns a pointer to
the struct thread that was placed on one of the scheduling queues in the
associated thread master. This pointer is used to check whether or not
the thread is scheduled, and is passed to thread_cancel() should the
daemon need to cancel that particular task.
The thread_fetch() function is called to retrieve the next thread to
execute. However, when it returns, the aforementioned pointer is not
updated. As a result, in order for the above use cases to work, every
thread handler function must set the associated pointer to NULL. This is
bug prone, and moreover, not thread safe.
This patch changes the thread scheduling functions to return void. If
the caller needs a reference to the scheduled thread, it must pass in a
pointer to store the pointer to the thread struct in. Subsequent calls
to thread_cancel(), thread_cancel_event() or thread_fetch() will result
in that pointer being nulled before return. These operations occur
within the thread_master critical sections.
Overall this should avoid bugs introduced by thread handler funcs
forgetting to null the associated pointer, double-scheduling caused by
overwriting pointers to currently scheduled threads without performing a
nullity check, and the introduction of true kernel threads causing race
conditions within the userspace threading world.
Also removes the return value for thread_execute since it always returns
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>
Cyclic graphs ftw
Also remove graph pretty printer from permutations.c 'cause it's not
really needed anymore
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Cyclic graphs ftw
Also remove graph pretty printer from permutations.c 'cause it's not
really needed anymore
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Rename HAVE_POLL to HAVE_POLL_CALL, when compiling with
snmp and poll enabled this was causing issues.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This change adds three fields to thread_master and associated code to
use them. The fields are:
* long selectpoll_timeout
This is a millisecond value that, if nonzero, will override the
internally calculated timeout for select()/poll(). -1 indicates
nonblocking while a positive value indicates the desired timeout in
milliseconds.
* bool spin
This indicates whether a call to thread_fetch() should result in a loop
until work is available. By default this is set to true, in order to
keep the default behavior. In this case a return value of NULL indicates
that a fatal signal was received in select() or poll(). If it is set to
false, thread_fetch() will return immediately. NULL is then an
acceptable return value if there is no work to be done.
* bool handle_signals
This indicates whether or not the pthread that owns the thread master
is responsible for handling signals (since this is an MT-unsafe
operation, it is best to have just the root thread do it). It is set to
true by default. Non-root pthreads should set this to false.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Adds infrastructure for keeping track of pthreads.
The general idea is to maintain a daemon-wide table of all pthreads,
running or not. A pthread is associated with its own thread master that
can be used with existing thread.c code, which provides user-space
timers, an event loop, non-blocking I/O callbacks and other facilities.
Each frr_pthread has a unique identifier that can be used to fetch it
from the table. This is to allow naming threads using a macro, for
example:
#define WRITE_THREAD 0
#define READ_THREAD 1
#define WORK_THREAD 2
The idea here is to be relatively flexible with regard to how daemons
manage their collection of pthreads; the implementation could get away
with just some #define'd constants, or keep a dynamically allocated data
structure that provides organization, searching, prioritizing, etc.
Overall this interface should provide a way to maintain the familiar
thread.c userspace threading model while progressively introducing
pthreads.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Fixes a few insufficient critical sections. Adds back locking for
thread_cancel(), since while thread_cancel() is only safe to call from
the pthread which owns the thread master due to races involving
thread_fetch() modifying thread master's ready queue, we still need
mutual exclusion here for all of the other public thread.c functions to
maintain their MT-safety.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
This change introduces synchronization mechanisms to thread.c in order
to allow safe concurrent use.
Thread.c should now be threadstafe with respect to:
* struct thread
* struct thread_master
Calls into thread.c for operations upon data of this type should not
require external synchronization.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Remove the UNDEFINED_NODE as that it's implementation breaks
our ability in BGP to figure out where we are by allowing
default: in the switch statement.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Implement BGP Prefix-SID IETF draft to be able to signal a labeled-unicast
prefix with a label index (segment ID). This makes it easier to deploy
global MPLS labels with BGP, even without other aspects of Segment Routing
implemented.
This patch implements the handling of the BGP-Prefix-SID Label Index
attribute. When received from a peer and the index is acceptable, the local
label is picked up from the SRGB and is programmed as the incoming label as
well as advertised to peers. If the index is not acceptable, no local label
is assigned. The outgoing label will always be the one advertised by the
downstream neighbor.
Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Implement support for negotiating IPv4 or IPv6 labeled-unicast address
family, exchanging prefixes and installing them in the routing table, as
well as interactions with Zebra for FEC registration. This is the
implementation of RFC 3107.
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Implement support for activating the labeled-unicast address family in
BGP and relevant configuration for this address family.
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Internal and IANA definitions for labeled-unicast SAFI. Note that this SAFI
is specific to BGP and maps to the corresponding unicast SAFI in Zebra.
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Support install of labeled-unicast routes by a client. This would be
BGP, in order to install routes corresponding to AFI/SAFI 1/4 (IPv4)
or 2/4 (IPv6). Convert labeled-unicast routes into label forwarding
entries (i.e., transit LSPs) when there is a static label binding.
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Implement interface that allows a client to register a FEC for obtaining
a label binding (in-label). Update client whenever the label binding is
updated and cleanup when client goes away.
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Preface with line identifying which daemon it applies to.
[Also fixes a missed "plugin" -> "module" replace.]
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
lib -> Add a bit of documentation about what units we are in.
zebra -> Fix failure case to be a bit better.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This is a prepatory commit for future improvements.
Add a change to the zapi to pass the interface speed up.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
On BSD systems, the getgrouplist() function returns 0 if successful and
-1 on error.
Linux in the other hand returns *ngroups (the number of groups of which
user is a member) on success and -1 on error.
Given this difference, the most portable way to use getgrouplist()
is use its return value only for checking if it succeeded or not.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
The fact that I originally wrote this in Linux Kernel style and then
reindented it to GNU makes me want to gouge my eyes out every time I
look at it. Restore original indentation.
[This patch is whitespace-only.]
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
the original version of this code already used _Atomic and atomic_*().
Restore this functionality for future multithreading.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Just adding -pthread to gcc options changes libc's behaviour, e.g.
making malloc() use proper locking. This means a SEGV inside malloc()
(e.g. because malloc bookkeeping structures have been damaged by writing
to a broken pointer) can lead to a lockup by the following chain:
- random_function()
- malloc()
--- SEGV
- core_handler()
- zlog_backtrace_sigsafe()
- backtrace()
- malloc()
This will hang forever waiting for the malloc() lock to be released.
Another failure mode is dynamic linking with lazy binding (-z lazy,
default). Since backtrace() is seldomly used, this means the call to
backtrace() in the core handler can in fact result in the dynamic linker
trying to resolve the "backtrace" symbol, which can also deadlock.
Add several safeguards to prevent any of this from happening.
(Unfortunately, these are not theoretical issues - I found them by
running into them headfirst.)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
This is very useful to check whether a command disappeared from a
specific daemon (by comparing against an earlier output of
"grammar find-ambiguous printall nodescan")
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
The vtysh preprocessing stuff doesn't like the first argument to
install_element() being something other than a _NODE constant, and the
comment hack wasn't cutting it... just expand this.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
These have copies in vtysh that do the node-switch locally and are
listed in extract.pl's ignore list. The ignore list however is
redundant since DEFUN_NOSH does the same thing...
ldpd is a bit hacky, but Renato is reworking this anyway.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Only the parent process should handle the SIGHUP signal, but we need
to make sure that this signal is ignored in the child processes so a
command like "killall -SIGHUP ldpd" won't kill ldpd.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
This adds a "-M" option to each daemon, to load dynamic modules at
startup. Modules are by default located in /usr/lib/frr/modules (lib64
if appropriate). Unloading or loading at runtime is not supported at
this point to keep things simple.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Label Manager allows to share MPLS label space among different
daemons. Each daemon can request a chunk of consecutive labels and
release it if it doesn't need them anymore. Label Manager stores the
daemon protocol and instance to identify the owner client. It uses them
to perform garbage collection, releasing all label chunks from a client
when it gets disconnected or reconnected.
Additionally, every client can request that the chunk is never garbage
collected. In that case client has the responsibility to release
non-used labels.
Zebra can host the label manager itself (if no -l param is provided) or
connect to an external one using zserv/zclient (providing its address
with -l param).
Client code is in lib/zclient.c, but currently only LDP is using it.
TODO: Allow for custom ranges requests, i.e., specify the start label
besides the chunk.
TODO: Release labels from LDP.
Signed-off-by: Bingen Eguzkitza <bingen@voltanet.io>
The following changes do not apply on master because the code has
changed:
- "vtysh: fix completion"
reverts commit 09e61a383f.
- "Revert "lib: Fix tab completions memleak, memory stats corruption""
reverts commit 4dcee34bd6.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
