If you have undefined behavior compilation checking gcc
starts to complain about a bunch of places that do not
have return's. When most of them actually do and we
have the assert's to prove it. I'm just doing this
to make the compiler happy for me, so I can continue
to do work.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
While here, also make "VPN SAFI clear" test wait for clear result
(tests/topotests/bgp_rfapi_basic_sanity{,_config2})
Original RFAPI code relied on the frr timer system to remember
various allocations that were supposed to be freed at future times
rather than manage a parallel database. However, if bgpd is terminated
before the times expire, those pending allocations are marked as
memory leaks, even though they wouldn't be leaks under normal operation.
This change adds some hash tables to track these outstanding
allocations that are associated with pending timers, and uses
those tables to free the allocations when bgpd exits.
Signed-off-by: G. Paul Ziemba <paulz@labn.net>
su_local and su_remote in the peer can change based upon
if we are initiating the remote connection or receiving it.
As such we need to treat it as a property of the connection.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Fix static-analyser warnings with BGP labels:
> $ scan-build make -j12
> bgpd/bgp_updgrp_packet.c:819:10: warning: Access to field 'extra' results in a dereference of a null pointer (loaded from variable 'path') [core.NullDereference]
> ? &path->extra->labels->label[0]
> ^~~~~~~~~
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
The handling of MPLS labels in BGP faces an issue due to the way labels
are stored in memory. They are stored in bgp_path_info but not in
bgp_adj_in and bgp_adj_out structures. As a consequence, some
configuration changes result in losing labels or even a bgpd crash. For
example, when retrieving routes from the Adj-RIB-in table
("soft-reconfiguration inbound" enabled), labels are missing.
bgp_path_info stores the MPLS labels, as shown below:
> struct bgp_path_info {
> struct bgp_path_info_extra *extra;
> [...]
> struct bgp_path_info_extra {
> mpls_label_t label[BGP_MAX_LABELS];
> uint32_t num_labels;
> [...]
To solve those issues, a solution would be to set label data to the
bgp_adj_in and bgp_adj_out structures in addition to the
bgp_path_info_extra structure. The idea is to reference a common label
pointer in all these three structures. And to store the data in a hash
list in order to save memory.
However, an issue in the code prevents us from setting clean data
without a rework. The extra->num_labels field, which is intended to
indicate the number of labels in extra->label[], is not reliably checked
or set. The code often incorrectly assumes that if the extra pointer is
present, then a label must also be present, leading to direct access to
extra->label[] without verifying extra->num_labels. This assumption
usually works because extra->label[0] is set to MPLS_INVALID_LABEL when
a new bgp_path_info_extra is created, but it is technically incorrect.
Cleanup the label code by setting num_labels each time values are set in
extra->label[] and checking extra->num_labels before accessing the
labels.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Structure size of bgp_path_info_extra when compiled
with vnc is 184 bytes. Reduce this size to 72 bytes
when compiled w/ vnc but not necessarily turned
on vnc.
With 2 full bgp feeds this saves aproximately 100mb
when compiling with vnc and not using vnc.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The rfapi code was not using the zlog_backtrace()
functionality. Let's just convert over to using
the proper functionality that we have built in now.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
This can lead into some garbage outputs, that can't be decoded in utf-8 or so.
This was catched when testing 76b246aa1f.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
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>
This is a first in a series of commits, whose goal is to rename
the thread system in FRR to an event system. There is a continual
problem where people are confusing `struct thread` with a true
pthread. In reality, our entire thread.c is an event system.
In this commit rename the thread.[ch] files to event.[ch].
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
An rfapi timer callback is responsible for the memory in a
context object, even in special-case exit paths. Always
free that object.
Signed-off-by: Mark Stapp <mjs@labn.net>
RD may be built based on an AS number. Like for the AS, the RD
may use the AS notation. The two below examples can illustrate:
RD 1.1:20 stands for an AS4B:NN RD with AS4B=65536 in dot format.
RD 0.1:20 stands for an AS2B:NNNN RD with AS2B=0.1 in dot+ format.
This commit adds the asnotation mode to prefix_rd2str() API so as
to pick up the relevant display.
Two new printfrr extensions are available to display the RD with
the two above display methods.
- The pRDD extension stands for dot asnotation format
- The pRDE extension stands for dot+ asnotation format.
- The pRD extension has been renamed to pRDP extension
The code is changed each time '%pRD' printf extension is called.
Possibly, the asnotation may change the output, then a macro defines
the asnotation mode to use. A side effect of forging the mode to
use is that the string could not be concatenated with other strings
in vty_out and snprintfrr. Those functions have been called multiple
times. When zlog_debug needs to display the RD with some other string,
the prefix_rd2str() old API is used instead of the printf extension.
Some code has been kept untouched:
- code related to running-config. Actually, wherever an RD is displayed,
its configured name should be dumped.
- bgp rfapi code
- bgp evpn multihoming code (partially done), since the logic is
missing to get the asnotation of 'struct bgp_evpn_es'.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
The files converted in this commit either had some random misspelling or
formatting weirdness that made them escape automated replacement, or
have a particularly "weird" licensing setup (e.g. dual-licensed.)
This also marks a bunch of "public domain" files as SPDX License "NONE".
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Use %pI4/%pI6 where possible, otherwise at least atjust stack buffer sizes
for inet_ntop() calls.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Just convert all uses of thread_cancel to THREAD_OFF. Additionally
use THREAD_ARG instead of t->arg to get the arguement. Individual
files should never be accessing thread private data like this.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The `struct ecommunity` structure is using an int for a size value.
Let's switch it over to a uint32_t for size values since a size
value for data can never be negative.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Change thread_cancel to take a ** to an event, NULL-check
before dereferencing, and NULL the caller's pointer. Update
many callers to use the new signature.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
Create appropriate accessor functions for the rn->lock
data. We should be accessing this data through accessor
functions since it is private data to the data structure.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
This is the bulk part extracted from "bgpd: Convert from `struct
bgp_node` to `struct bgp_dest`". It should not result in any functional
change.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Certain extended communities cannot be repeated. An example is the
BGP link bandwidth extended community. Enhance the extended community
add function to ensure uniqueness, if requested.
Note: This commit does not change the lack of uniqueness for any of
the already-supported extended communities. Many of them such as the
BGP route target can obviously be present multiple times. Others like
the Router's MAC should most probably be present only once. The portions
of the code which add these may already be structured such that duplicates
do not arise.
Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Add new function `bgp_node_get_prefix()` and modify
the bgp code base to use it.
This is prep work for the struct bgp_dest rework.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
