Code is no longer using a global FE "client context", and instead
creates client objects, rename the structure and it's uses to reflect this.
Remove an obfuscating abstraction layer whose existence was entirely
based on using a uintptr_t rather than a pointer to an declared-only struct.
Change multi-duty "params" structure into a single duty callbacks one.
Remove unsupported API code.
Signed-off-by: Christian Hopps <chopps@labn.net>
When daemons transition to mgmtd they should stop reading their split config
files, and let mgmtd do that, otherwise things can get out of sync.
Signed-off-by: Christian Hopps <chopps@labn.net>
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>
This commmit introduces Staticd as a backend client for the MGMTd
framework. All the static commands will be diverted to the MGMT
daemon and will use the transactional model to make changes to the
internal state. Similar mechanism can be used by other daemons to use
the MGMT framework in the future.
This commit includes the following functionalities in the changeset:
1. Diverts all the staticd (config only) commands to MGMTd.
2. Enrolls staticd as a backend client to use the MGMT framework.
3. Modify the staticd NB config handlers so that they can be compiled
into a library and loaded in the MGMTd process context.
Co-authored-by: Pushpasis Sarkar <pushpasis@gmail.com>
Co-authored-by: Abhinay Ramesh <rabhinay@vmware.com>
Co-authored-by: Ujwal P <ujwalp@vmware.com>
Signed-off-by: Yash Ranjan <ranjany@vmware.com>
Prevent a use after free and tell the bfd subsystem
we are shutting down in staticd.
./bfd_topo3.test_bfd_topo3/r4.staticd.asan.2264460:==2264460==ERROR: AddressSanitizer: heap-use-after-free on address 0x61f000004698 at pc 0x7f65d1eb11b2 bp 0x7ffdbface490 sp 0x7ffdbface488
./bfd_topo3.test_bfd_topo3/r4.staticd.asan.2264460-READ of size 4 at 0x61f000004698 thread T0
./bfd_topo3.test_bfd_topo3/r4.staticd.asan.2264460- #0 0x7f65d1eb11b1 in zclient_bfd_command lib/bfd.c:307
./bfd_topo3.test_bfd_topo3/r4.staticd.asan.2264460- #1 0x7f65d1eb20f5 in _bfd_sess_send lib/bfd.c:507
./bfd_topo3.test_bfd_topo3/r4.staticd.asan.2264460- #2 0x7f65d20510aa in thread_call lib/thread.c:1989
./bfd_topo3.test_bfd_topo3/r4.staticd.asan.2264460- #3 0x7f65d2051f0a in _thread_execute lib/thread.c:2081
./bfd_topo3.test_bfd_topo3/r4.staticd.asan.2264460- #4 0x7f65d1eb271b in _bfd_sess_remove lib/bfd.c:544
./bfd_topo3.test_bfd_topo3/r4.staticd.asan.2264460- #5 0x7f65d1eb278d in bfd_sess_free lib/bfd.c:553
./bfd_topo3.test_bfd_topo3/r4.staticd.asan.2264460- #6 0x7f65d1eb5400 in bfd_protocol_integration_finish lib/bfd.c:1029
./bfd_topo3.test_bfd_topo3/r4.staticd.asan.2264460- #7 0x7f65d1f42f77 in hook_call_frr_fini lib/libfrr.c:41
./bfd_topo3.test_bfd_topo3/r4.staticd.asan.2264460- #8 0x7f65d1f494a1 in frr_fini lib/libfrr.c:1199
./bfd_topo3.test_bfd_topo3/r4.staticd.asan.2264460- #9 0x563b7abefd76 in sigint staticd/static_main.c:70
./bfd_topo3.test_bfd_topo3/r4.staticd.asan.2264460- #10 0x7f65d200ef91 in frr_sigevent_process lib/sigevent.c:115
./bfd_topo3.test_bfd_topo3/r4.staticd.asan.2264460- #11 0x7f65d204fac6 in thread_fetch lib/thread.c:1758
./bfd_topo3.test_bfd_topo3/r4.staticd.asan.2264460- #12 0x7f65d1f49377 in frr_run lib/libfrr.c:1184
./bfd_topo3.test_bfd_topo3/r4.staticd.asan.2264460- #13 0x563b7abefed1 in main staticd/static_main.c:160
./bfd_topo3.test_bfd_topo3/r4.staticd.asan.2264460- #14 0x7f65d1b92d09 in __libc_start_main ../csu/libc-start.c:308
./bfd_topo3.test_bfd_topo3/r4.staticd.asan.2264460- #15 0x563b7abefa99 in _start (/usr/lib/frr/staticd+0x15a99)
./bfd_topo3.test_bfd_topo3/r4.staticd.asan.2264460-
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Passing a pre-formatted buffer in these places needs a `"%s"` in front
so it doesn't get formatted twice.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Daemons like staticd already implement nexthop zapi (un)registration.
b7ca809d1c ("lib: BFD automatic source selection") has implemented a
concurrent nexthop (un)registration. Some nexthop could be unregistred
by the bfd whereas they were still needed by the daemon.
Let the deamons deal with nexthop zapi (un)registration.
Fixes: b7ca809d1c ("lib: BFD automatic source selection")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
b7ca809d1c ("lib: BFD automatic source selection") has added a dedicated
zclient socket for nht tracking. Since the bfd lib is used by daemons
that already has a zclient socket, those daemons has now a second
zclient socket. However, zebra does not distinguish the two zclient
sessions. For example, the interfaces are asked a second via
zebra_message_send(zclient, ZEBRA_INTERFACE_ADD, VRF_DEFAULT) in
zclient_start(). As a result, callbacks functions like bgp_ifp_create()
are called a second time, which causes some processing overhead and
might cause bugs.
Re-use the existing zclient socket for nht tracking.
Note that BFD automatic source selection is only currently implemented
in staticd. Other daemons will require to add the following in their
ZEBRA_NEXTHOP_UPDATE callback function:
> if (zclient->bfd_integration)
> bfd_nht_update(&matched, &nhr);
Fixes: b7ca809d1c ("lib: BFD automatic source selection")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
This command helps in troubleshooting static bfd feature.
Add traces upon bfd events.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
When configuring a route with multiple next hops on boot and the
interface is missing the route never shows up (even after interface is
properly learned).
Fix: force route clean up by uninstalling it and putting it back
entirely instead of just updating the missing next hop.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Implement all BFD integration northbound callbacks and integrate BFD
with `staticd` route installation procedure.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
When compiling with -fsanitize=thread. I started getting this error:
staticd/static_zebra.c: In function ‘static_zebra_nht_get_prefix’:
staticd/static_zebra.c:316:1: error: control reaches end of non-void function [-Werror=return-type]
316 | }
| ^
Just to make future efforts still work, let's just make the compiler happy.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Simplify all DEFUNs by moving all logic to the northbound translation
function and transforming all function parameters into a single
structure.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Rather than running selected source files through the preprocessor and a
bunch of perl regex'ing to get the list of all DEFUNs, use the data
collected in frr.xref.
This not only eliminates issues we've been having with preprocessor
failures due to nonexistent header files, but is also much faster.
Where extract.pl would take 5s, this now finishes in 0.2s. And since
this is a non-parallelizable build step towards the end of the build
(dependent on a lot of other things being done already), the speedup is
actually noticeable.
Also files containing CLI no longer need to be listed in `vtysh_scan`
since the .xref data covers everything. `#ifndef VTYSH_EXTRACT_PL`
checks are equally obsolete.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
There are lib debugs being set but never show up in
`show debug` commands because there was no way to show
that they were being used. Add a bit of infrastructure
to allow this and then use it for `debug route-map`
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
There exists some nexthop attributes that are not the unique
part of the nexthop and if you change the static route
with those values then the route is not being updated
in zebra with the new values:
Example of brokenness:
eva# conf
eva(config)# ip route 1.2.3.9/32 192.168.119.1 enp39s0 label 16020
eva(config)# do show ip route 1.2.3.9
Routing entry for 1.2.3.9/32
Known via "static", distance 1, metric 0, best
Last update 00:00:05 ago
* 192.168.119.1, via enp39s0, label 16020, weight 1
eva(config)# ip route 1.2.3.9/32 192.168.119.1 enp39s0 label 16040
eva(config)# do show ip route 1.2.3.9
Routing entry for 1.2.3.9/32
Known via "static", distance 1, metric 0, best
Last update 00:00:12 ago
* 192.168.119.1, via enp39s0, label 16020, weight 1
Fixed behavior:
eva# conf
eva(config)# ip route 1.2.3.10/32 192.168.119.1 enp39s0 label 16020
eva(config)# do show ip route 1.2.3.10
Routing entry for 1.2.3.10/32
Known via "static", distance 1, metric 0, best
Last update 00:00:04 ago
* 192.168.119.1, via enp39s0, label 16020, weight 1
eva(config)# ip route 1.2.3.10/32 192.168.119.1 enp39s0 label 16040
eva(config)# do show ip route 1.2.3.10
Routing entry for 1.2.3.10/32
Known via "static", distance 1, metric 0, best
Last update 00:00:01 ago
* 192.168.119.1, via enp39s0, label 16040, weight 1
I've gone through most of the items in staticd that can change
the nexthop
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Other VRFs get VRF_ADD notifications from zebra which triggers
static_fixup_vrf_ids, but since the default VRF is implicit we need to
make that same call on connect.
This should fix problems with staticd being started before (or
concurrent with and thus racing) zebra.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
nh_update is only called in two places and both precede a matching
follow-up nht_register call. Fold the update into register, and make
register do the right thing™ for all cases (i.e. update refcounts as
needed, and retry zebra NHT registration if it failed before).
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Since this is a free()-type function, clear the caller's pointer to
NULL to aid static analysis and prevent UAF bugs.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Put static_nexthop -> prefix code into a small helper, remove extra
prefix variable, and grab AFI from prefix.
This commit should not result in any functional change.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Currently the nexthop tracking code is only sending to the requestor
what it was requested to match against. When the nexthop tracking
code was simplified to not need an import check and a nexthop check
in b8210849b8 for bgpd. It was not
noticed that a longer prefix could match but it would be seen
as a match because FRR was not sending up both the resolved
route prefix and the route FRR was asked to match against.
This code change causes the nexthop tracking code to pass
back up the matched requested route (so that the calling
protocol can figure out which one it is being told about )
as well as the actual prefix that was matched to.
Fixes: #10766
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Recent commit:
abc246e193
Has broken `make check` with recently new compilers:
/usr/bin/ld: staticd/libstatic.a(static_nb_config.o): warning: relocation against `zebra_ecmp_count' in read-only section `.text'
CCLD tests/bgpd/test_peer_attr
CCLD tests/bgpd/test_packet
/usr/bin/ld: staticd/libstatic.a(static_zebra.o): in function `static_zebra_capabilities':
/home/sharpd/frr5/staticd/static_zebra.c:208: undefined reference to `zebra_ecmp_count'
/usr/bin/ld: staticd/libstatic.a(static_zebra.o): in function `static_zebra_route_add':
/home/sharpd/frr5/staticd/static_zebra.c:418: undefined reference to `zebra_ecmp_count'
/usr/bin/ld: staticd/libstatic.a(static_nb_config.o): in function `static_nexthop_create':
/home/sharpd/frr5/staticd/static_nb_config.c:174: undefined reference to `zebra_ecmp_count'
/usr/bin/ld: /home/sharpd/frr5/staticd/static_nb_config.c:175: undefined reference to `zebra_ecmp_count'
/usr/bin/ld: warning: creating DT_TEXTREL in a PIE
collect2: error: ld returned 1 exit status
make: *** [Makefile:8679: tests/lib/test_grpc] Error 1
make: *** Waiting for unfinished jobs....
Essentially the newly introduced variable zebra_ecmp_count is not available in the
libstatic.a compiled and make check has code that compiles against it.
The fix is to just move the variable to the library.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Zebra sends a nexthop-update message on registeration, which will cause
existing routes to be reconfigured even no changes actually happened.
Don't register the nexthop again if it's already done.
Signed-off-by: Xiao Liang <shaw.leon@gmail.com>
Restrict the number of nexthops for a route to the compiled-in
limit. Be careful with the zapi route struct's array of nexthops
too.
Signed-off-by: Mark Stapp <mstapp@nvidia.com>
