| Age | Commit message (Collapse) | Author |
|---|
|
Signed-off-by: Christian Hopps <chopps@labn.net>
|
|
Configure hash table cleanup with specific free functions for `zrouter.filter_hash`, `zrouter.qdisc_hash`, and `zrouter.class_hash`.
This ensures proper memory cleanup, addressing memory leaks.
The ASan leak log for reference:
```
***********************************************************************************
Address Sanitizer Error detected in tc_basic.test_tc_basic/r1.asan.zebra.15495
=================================================================
==15495==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 176 byte(s) in 1 object(s) allocated from:
#0 0x7fd5660ffd28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
#1 0x7fd565afe238 in qcalloc lib/memory.c:105
#2 0x5564521c6c9e in tc_filter_alloc_intern zebra/zebra_tc.c:389
#3 0x7fd565ac49e8 in hash_get lib/hash.c:147
#4 0x5564521c7c74 in zebra_tc_filter_add zebra/zebra_tc.c:409
#5 0x55645210755a in zread_tc_filter zebra/zapi_msg.c:3428
#6 0x5564521127c1 in zserv_handle_commands zebra/zapi_msg.c:4004
#7 0x5564522208b2 in zserv_process_messages zebra/zserv.c:520
#8 0x7fd565b9e034 in event_call lib/event.c:1974
#9 0x7fd565ae142b in frr_run lib/libfrr.c:1214
#10 0x5564520c14b1 in main zebra/main.c:492
#11 0x7fd564ec2c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
Direct leak of 40 byte(s) in 1 object(s) allocated from:
#0 0x7fd5660ffd28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
#1 0x7fd565afe238 in qcalloc lib/memory.c:105
#2 0x5564521c6c6e in tc_class_alloc_intern zebra/zebra_tc.c:239
#3 0x7fd565ac49e8 in hash_get lib/hash.c:147
#4 0x5564521c784f in zebra_tc_class_add zebra/zebra_tc.c:293
#5 0x556452107ce5 in zread_tc_class zebra/zapi_msg.c:3315
#6 0x5564521127c1 in zserv_handle_commands zebra/zapi_msg.c:4004
#7 0x5564522208b2 in zserv_process_messages zebra/zserv.c:520
#8 0x7fd565b9e034 in event_call lib/event.c:1974
#9 0x7fd565ae142b in frr_run lib/libfrr.c:1214
#10 0x5564520c14b1 in main zebra/main.c:492
#11 0x7fd564ec2c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
Direct leak of 12 byte(s) in 1 object(s) allocated from:
#0 0x7fd5660ffd28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
#1 0x7fd565afe238 in qcalloc lib/memory.c:105
#2 0x5564521c6c3e in tc_qdisc_alloc_intern zebra/zebra_tc.c:128
#3 0x7fd565ac49e8 in hash_get lib/hash.c:147
#4 0x5564521c753b in zebra_tc_qdisc_install zebra/zebra_tc.c:184
#5 0x556452108203 in zread_tc_qdisc zebra/zapi_msg.c:3286
#6 0x5564521127c1 in zserv_handle_commands zebra/zapi_msg.c:4004
#7 0x5564522208b2 in zserv_process_messages zebra/zserv.c:520
#8 0x7fd565b9e034 in event_call lib/event.c:1974
#9 0x7fd565ae142b in frr_run lib/libfrr.c:1214
#10 0x5564520c14b1 in main zebra/main.c:492
#11 0x7fd564ec2c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
SUMMARY: AddressSanitizer: 228 byte(s) leaked in 3 allocation(s).
***********************************************************************************
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
|
|
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
The weight scale value might be useful to have it
change it's behavior at a later time or controlled
by something depending on how FRR is compiled/ran.
Let's start that process
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
add --v6-with-v4-nexthop cli to zebra to allow operator to
specify that this functionality is allowed.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
Since we are moving some code handling out of the dataplane
and into zebra proper, lets move the protodown r bit as well.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
Add a hash_clean_and_free() function as well as convert
the code to use it. This function also takes a double
pointer to the hash to set it NULL. Also it cleanly
does nothing if the pointer is NULL( as a bunch of
code tested for ).
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
Done with a combination of regex'ing and banging my head against a wall.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
|
|
Volta submitted notification changes for the dplane that had a
special use case for their system. Volta is no more, the code
is not being actively developed and from talking with ex-Volta
employees there is no current plans to even maintain this code.
Wrap the special handling of nexthops that their asic-dataplane
did in a bit of code to isolate it and allow for future removal,
as that I do not actually believe anyone else is using this code.
Add a CPP_NOTICE several years into the future that will tell us
to remove the code. If someone starts using it then they will
have to notice this variable to set it and hopefully they will
see my CPP_NOTICE to come talk to us. If this is being used then
we can just remove this wrapper.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
This allows Zebra to manage QDISC, TCLASS, TFILTER in kernel and do cleaning
jobs when it starts up.
Signed-off-by: Siger Yang <siger.yang@outlook.com>
|
|
These functions essentially do the same thing. Combine them
for the goodness of mankind.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
Fixup both memory leaks as well as use after free's in nhg's
on shutdown.
This approach is effectively just iterating through all the
hash items and directly just freeing the memory instead
of handling ref counts or cross references.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
Instead of having global allow_delete move it to
where it belongs in the zrouter data structure.
Additionally show this data in `show zebra`
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
Currently specific local neighbors (attached to SVIs) are maintatined
in an EVPN specific database. There is a need to maintain L3 neighbors
for other purposes including MAC resolution for PBR nexthops.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Cleanup compile and fix crash
Signed-off-by: Anuradha Karuppiah <anuradhak@nvidia.com>
|
|
Allow end operator to set how long a nexthop-group is kept around
in the system after it is no-longer being used.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
The recently-added hashtable of nlsock objects needs to be
thread-safe: it's accessed from the main and dplane pthreads.
Add a mutex for it, use wrapper apis when accessing it. Add
a per-OS init/terminate api so we can do init that's not
per-vrf or per-namespace.
Signed-off-by: Mark Stapp <mstapp@nvidia.com>
|
|
On startup we create a thread timer event to do a rib sweep
of the system. On shutdown we never stopped this timer and
as such we have a situation where a thread event could be run
on shutdown after the data for it has been freed. Here is the
crash I am seeing:
(gdb) bt
(gdb)
Save the thread data in zebra_router and stop the thread so we don't
accidently do work on shutdown we don't mean to. In this case
it happened in our topotests with some severe system load.
Essentially we happened to kill the zebra daemon just as the
graceful_restart timer popped here.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
this reverts commit dd9538c5f36f9d6110, which tried to clear
the LSP workqueue late during shutdown.
Signed-off-by: Mark Stapp <mstapp@nvidia.com>
|
|
Free the LSP workqueue later during shutdown, so that zebra
has enough time to clean up and uninstall any LSPs.
Signed-off-by: Mark Stapp <mstapp@nvidia.com>
|
|
Recent code broke upon compiling with --enable-dev-build
and --enable-werror. Fix.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
Signed-off-by: Donald Lee <dlqs@gmx.com>
|
|
Rework RA handling for vrf-lite scenarios.
Before we were using a single FD descriptor for polling
across multiple zvrf's. This would cause us to hit this
assert() in some bgp unnumbered and vrrp configs:
```
/*
* What happens if we have a thread already
* created for this event?
*/
if (thread_array[fd])
assert(!"Thread already scheduled for file descriptor");
```
We were scheduling a thread_read on the same FD for every zvrf.
With vrf-lite, RAs and ARPs are not vrf-bound, so we can just use one
rtadv instance to manage them for all VRFs. We will choose the default
VRF for this.
This patch removes the rtadv_sock altogether for zrouter and moves the
functionality this represented to the default VRF. All RAs will be
handled in the default VRF under vrf-lite configs with only one poll
thread started for it.
This patch also extends how we track subscribed interfaces (s or msec)
to use an actual sorted list by interface names rather than just a
counter. With multiple daemons turning interfaces/on/off these counters
can get very wrong during ifup/down events. Making them a sorted list
prevents this from happening by preventing duplicates.
With netns-vrf's nothing should change other than the interface list.
Signed-off-by: Stephen Worley <sworley@nvidia.com>
|
|
This one also needed a bit of shuffling around, but MTYPE_RE is the only
one left used across file boundaries now.
Signed-off-by: David Lamparter <equinox@diac24.net>
|
|
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>
|
|
Add a command that allows FRR to know it's being used with
an underlying asic offload, from the linux kernel perspective.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
Some linux kernels are starting to support the idea of knowledge
about the underlying asic. Add a boolean that we can set/unset
to track whether or not we think the router has this functionality
available.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
When given a route_table this allows the corresponding kernel table
ID to be determined. The table_id value is set upon table creation
to the table_id of the VRF, unless the table was created with a
specific ID.
Signed-off-by: Duncan Eastoe <duncan.eastoe@att.com>
|
|
This commit implements:
RIB operational list create/destroy.
Walk over RIB tables using keys.
The first RIB table will be IPV4/unicast (table-id 254)
will be fetched.
Create a new api to fetch RIB table based on
afi-safi and table id as the keys.
remove mandatory true statement from the leaf which
is part of the list key.
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
|
|
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
|
Free unhashable (duplicate NHEs from the kernel) via ID table
cleanup. Since the NHE ID hash table contains extra entries,
that's the one we need to be calling zebra_nhg_hash_free()
on, otherwise we will never free the unhashable NHEs.
This was found via a memleak:
==1478713== HEAP SUMMARY:
==1478713== in use at exit: 10,267 bytes in 46 blocks
==1478713== total heap usage: 76,810 allocs, 76,764 frees, 3,901,237 bytes allocated
==1478713==
==1478713== 208 (88 direct, 120 indirect) bytes in 1 blocks are definitely lost in loss record 35 of 41
==1478713== at 0x483BB1A: calloc (vg_replace_malloc.c:762)
==1478713== by 0x48E35E8: qcalloc (memory.c:110)
==1478713== by 0x451CCB: zebra_nhg_alloc (zebra_nhg.c:369)
==1478713== by 0x453DE3: zebra_nhg_copy (zebra_nhg.c:379)
==1478713== by 0x452670: nhg_ctx_process_new (zebra_nhg.c:1143)
==1478713== by 0x4523A8: nhg_ctx_process (zebra_nhg.c:1234)
==1478713== by 0x452A2D: zebra_nhg_kernel_find (zebra_nhg.c:1294)
==1478713== by 0x4326E0: netlink_nexthop_change (rt_netlink.c:2433)
==1478713== by 0x427320: netlink_parse_info (kernel_netlink.c:945)
==1478713== by 0x432DAD: netlink_nexthop_read (rt_netlink.c:2488)
==1478713== by 0x41B600: interface_list (if_netlink.c:1486)
==1478713== by 0x457275: zebra_ns_enable (zebra_ns.c:127)
Repro with:
ip next add id 1 blackhole
ip next add id 2 blackhole
valgrind /usr/lib/frr/zebra
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
|
|
Replace the existing list of nexthops (via a nexthop_group
struct) in the route_entry with a direct pointer to zebra's
new shared group (from zebra_nhg.h). This allows more
direct access to that shared group and the info it carries.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
|
|
The code for when a new vrf is created to properly handle
router advertisement for it is messed up in several ways:
1) Generation of the zrouter data structure should set the rtadv
socket to -1 so that we don't accidently close someone elses
open file descriptor
2) When you created a new zvrf instance *after* bootup we are XCALLOC'ing
the data structure so the zvrf->fd was 0. The shutdown code was looking
for the >= 0 to know if the fd existed (since fd 0 is valid!)
This sequence of events would cause zebra to consume 100% of the
cpu:
Run zebra by itself ( no other programs )
ip link add vrf1 type vrf table 1003
ip link del vrf vrf1
vtysh -c "configure" -c "no interface vrf1"
This commit fixes this issue.
Fixes: #5376
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
|
Add a private header file for functions that are internal/special
case like how we do it for `lib/nexthop_group_private.h`.
Remove a bunch of functions from the header file only being used
statically and add some comments for those remaining to indicate
better what their use is.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
|
|
This code was from a strategies we elected not to use and
can safely be removed.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
|
|
On restart, if we failed to remove any nexthop objects due
to a kill -9 or such event, sweep them if we aren't using them.
Add a proto field to handle this and remove the is_kernel bool.
Add a dupicate flag that indicates this nexthop group is only
present in our ID hashtable. It is a dupicate nexthop we received
from the kernel, therefore we cannot hash on it.
Make the idcounter globally accessible so that kernel updates
increment it as soon as we receive them, not when we handle them.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
|
|
Add functionality to uninstall nexthops we created on shutdown.
To account for this, I added in a function for zebra_router
cleanup in a shutdown event.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
|
|
Add the functionality to parse new nexthop group messages
from the kernel and insert them into the appropriate hash
tables. Parsing is done at startup between interface and
interface address lookup. Add functionality to parse
changes to nexthops we already have. Add functionality
to parse delete nexthop messages from the kernel and
remove them from our table.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
|
|
The messages we get from the kernel come with ids only
for groups, so lets index with those as well. Also adding
a helper function for lookup and get with the two different
tables.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
|
|
This commit does nothing more than just create a hash structure
that we will use to track nexthop groups.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
|
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
|
|
The multicast mode enum was a global static in zebra_rib.c
it does not belong there, it belongs in zebra_router, moving.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
|
zebra Other tables
|
|
The multipath_num variable is a property of zebra_router,
so move it there.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
|
With the previous commit, the zebra_router_score_proto function
became unnecessary, so let us remove it.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
|
This command is broken and has been broken since the introduction
of vrf's. Since no-one has complained it is safe to assume that
there is no call for this specialized linux command. Remove
from the system with extreme prejudice.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
|
In zebra terminate path, the node was attempted to remove
twice from the RB_TREE table. This lead to a crash during
zebra shutdown zebra_router_free_table already calls RB_REMOVE
to remove a node from rb tree table.
siginfo=0x7fffd9134a30, context=<optimized out>) at lib/sigevent.c:249
rbt=<optimized out>, t=<optimized out>) at lib/openbsd-tree.c:226
t=0x56296965ff50 <zebra_router_table_head_RB_INFO>) at lib/openbsd-tree.c:383
rbt=rbt@entry=0x562969669bd0 <zrouter+16>, elm=elm@entry=0x56296afcf810)
at lib/openbsd-tree.c:393
(elm=0x56296afcf810, head=0x562969669bd0 <zrouter+16>) at zebra/zebra_router.h:46
Singned-off-by: Chirag Shah <chirag@cumulusnetworks.com>
|
|
When shutting down, the individual vrf's own the shutdown of the table
and subsuquent removal from the routes from the kernel.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
|
When a vrf is deleted we need to tell the zebra_router that we have
finished using the tables we are keeping track of. This will allow
us to properly cleanup the data structures associated with them.
This fixes this valgrind error found:
==8579== Invalid read of size 8
==8579== at 0x430034: zvrf_id (zebra_vrf.h:167)
==8579== by 0x432366: rib_process (zebra_rib.c:1580)
==8579== by 0x432366: process_subq (zebra_rib.c:2092)
==8579== by 0x432366: meta_queue_process (zebra_rib.c:2188)
==8579== by 0x48C99FE: work_queue_run (workqueue.c:291)
==8579== by 0x48C3788: thread_call (thread.c:1607)
==8579== by 0x48A2E9E: frr_run (libfrr.c:1011)
==8579== by 0x41316A: main (main.c:473)
==8579== Address 0x5aeb750 is 0 bytes inside a block of size 4,424 free'd
==8579== at 0x4839A0C: free (vg_replace_malloc.c:540)
==8579== by 0x438914: zebra_vrf_delete (zebra_vrf.c:279)
==8579== by 0x48C4225: vrf_delete (vrf.c:243)
==8579== by 0x48C4225: vrf_delete (vrf.c:217)
==8579== by 0x4151CE: netlink_vrf_change (if_netlink.c:364)
==8579== by 0x416810: netlink_link_change (if_netlink.c:1189)
==8579== by 0x41C1FC: netlink_parse_info (kernel_netlink.c:904)
==8579== by 0x41C2D3: kernel_read (kernel_netlink.c:389)
==8579== by 0x48C3788: thread_call (thread.c:1607)
==8579== by 0x48A2E9E: frr_run (libfrr.c:1011)
==8579== by 0x41316A: main (main.c:473)
==8579== Block was alloc'd at
==8579== at 0x483AB1A: calloc (vg_replace_malloc.c:762)
==8579== by 0x48A6030: qcalloc (memory.c:110)
==8579== by 0x4389EF: zebra_vrf_alloc (zebra_vrf.c:382)
==8579== by 0x438A42: zebra_vrf_new (zebra_vrf.c:93)
==8579== by 0x48C40AD: vrf_get (vrf.c:209)
==8579== by 0x415144: netlink_vrf_change (if_netlink.c:319)
==8579== by 0x415E90: netlink_interface (if_netlink.c:653)
==8579== by 0x41C1FC: netlink_parse_info (kernel_netlink.c:904)
==8579== by 0x4163E8: interface_lookup_netlink (if_netlink.c:760)
==8579== by 0x42BB37: zebra_ns_enable (zebra_ns.c:130)
==8579== by 0x42BC5E: zebra_ns_init (zebra_ns.c:208)
==8579== by 0x4130F4: main (main.c:401)
This can be found by: `ip link del <VRF DEVICE NAME>` then `ip link add <NAME> type vrf table X` again and
then attempting to use the vrf.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
|
