For auto configured value RD value comes as NULL,
switching back to original change will ensure to cover
for both auto and user configured RD value in JSON.
tor-11# show bgp vrf blue ipv4 unicast route-leak json
{
"vrf":"blue",
"afiSafi":"ipv4Unicast",
"importFromVrfs":[
"purple"
],
"importRts":"10.10.3.11:6",
"exportToVrfs":[
"purple"
],
"routeDistinguisher":"(null)", <<<<<
"exportRts":"10.10.3.11:10"
}
Signed-off-by: Chirag Shah <chirag@nvidia.com>
Upon zebra shutdown hash_clean_and_free is called
where user free function is passed,
The free function should not call hash_release
which lead to double free of hash bucket.
Fix:
The fix is to avoid calling hash_release from
free function if its called from hash_clean_and_free
path.
10 0x00007f0422b7df1f in free () from /lib/x86_64-linux-gnu/libc.so.6
11 0x00007f0422edd779 in qfree (mt=0x7f0423047ca0 <MTYPE_HASH_BUCKET>,
ptr=0x55fc8bc81980) at ../lib/memory.c:130
12 0x00007f0422eb97e2 in hash_clean (hash=0x55fc8b979a60,
free_func=0x55fc8a529478 <svd_nh_del_terminate>) at
../lib/hash.c:290
13 0x00007f0422eb98a1 in hash_clean_and_free (hash=0x55fc8a675920
<svd_nh_table>, free_func=0x55fc8a529478 <svd_nh_del_terminate>) at
../lib/hash.c:305
14 0x000055fc8a5323a5 in zebra_vxlan_terminate () at
../zebra/zebra_vxlan.c:6099
15 0x000055fc8a4c9227 in zebra_router_terminate () at
../zebra/zebra_router.c:276
16 0x000055fc8a4413b3 in zebra_finalize (dummy=0x7fffb881c1d0) at
../zebra/main.c:269
17 0x00007f0422f44387 in event_call (thread=0x7fffb881c1d0) at
../lib/event.c:2011
18 0x00007f0422ecb6fa in frr_run (master=0x55fc8b733cb0) at
../lib/libfrr.c:1243
19 0x000055fc8a441987 in main (argc=14, argv=0x7fffb881c4a8) at
../zebra/main.c:584
Signed-off-by: Chirag Shah <chirag@nvidia.com>
Add a new test case that re-add the deleted SIDs and verifies that all
SIDs are added back to the RIB.
Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
Add a new test case that deletes a SID and verifies that only this
SID has been removed from the RIB.
Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
When a user wants to delete a specific SRv6 SID, he executes the
`no sid X:X::X:X/M` command.
However, by mistake, in addition to deleting the SID requested by the
user, this command also removes all other SIDs.
This happens because `no sid X:X::X:X/M` triggers a destroy operation
on the wrong xpath `frr-staticd:staticd/segment-routing/srv6`.
This commit fixes the issue by replacing the wrong xpath
`frr-staticd:staticd/segment-routing/srv6` with the correct xpath
`frr-staticd:staticd/segment-routing/srv6/static-sids/sid[sid='%s']`.
This ensures that the `no sid X:X::X:X/M` command only deletes the SID
that was requested by the user.
Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
Just to make it simpler for compiling with a different default value.
No change to its default value.
Signed-off-by: Enke Chen <enchen@paloaltonetworks.com>
When staticd receives a `ZAPI_SRV6_SID_RELEASED` notification from SRv6
SID Manager, it tries to unset the validity flag of `sid`. But since
the `sid` variable is NULL, we get a NULL pointer dereference.
```
=================================================================
==13815==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000060 (pc 0xc14b813d9eac bp 0xffffcb135a40 sp 0xffffcb135a40 T0)
==13815==The signal is caused by a READ memory access.
==13815==Hint: address points to the zero page.
#0 0xc14b813d9eac in static_zebra_srv6_sid_notify staticd/static_zebra.c:1172
#1 0xe44e7aa2c194 in zclient_read lib/zclient.c:4746
#2 0xe44e7a9b69d8 in event_call lib/event.c:1984
#3 0xe44e7a85ac28 in frr_run lib/libfrr.c:1246
#4 0xc14b813ccf98 in main staticd/static_main.c:193
#5 0xe44e7a4773f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
#6 0xe44e7a4774c8 in __libc_start_main_impl ../csu/libc-start.c:392
#7 0xc14b813cc92c in _start (/usr/lib/frr/staticd+0x1c92c)
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV staticd/static_zebra.c:1172 in static_zebra_srv6_sid_notify
==13815==ABORTING
```
This commit fixes the problem by doing a SID lookup first. If the SID
can't be found, we log an error and return. If the SID is found, we go
ahead and unset the validity flag.
Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
Just to make it simpler for compiling with a different default value.
No change to its default value.
Signed-off-by: Enke Chen <enchen@paloaltonetworks.com>
When you have suppress-fib-pending turned on it is possible
to end up in a situation where the prefix is not withdrawn
from downstream peers.
Here is the timing that I believe is happening:
a) have 2 paths to a peer.
b) receive a withdrawal from 1 path, set BGP_NODE_FIB_INSTALL_PENDING
and send the route install to zebra.
c) receive a withdrawal from the other path.
d) At this point we have a dest->flags set BGP_NODE_FIB_INSTALL_PENDING
old_select the path_info going away, new_select is NULL
e) A bit further down we call group_announce_route() which calls
the code to see if we should advertise the path. It sees the
BGP_NODE_FIB_INSTALL_PENDING flag and says, nope.
f) the route is sent to zebra to withdraw, which unsets the
BGP_NODE_FIB_INSTALL_PENDING.
g) This function winds up and deletes the path_info. Dest now
has no path infos.
h) BGP receives the route install(from step b) and unsets the
BGP_NODE_FIB_INSTALL_PENDING flag
i) BGP receives the route removed from zebra (from step f) and
unsets the flag again.
We know if there is no new_select, let's go ahead and just
unset the PENDING flag to allow the withdrawal to go out
at the time when the second withdrawal is received.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
When called without caps/privs, just return from "change_caps"
instead of exiting - it's possible that a process may not need
privs, but a lib (for example) may use the api.
Signed-off-by: Mark Stapp <mjs@cisco.com>
The SRv6 SID Manager does not allow allocating an SRv6 End/uN function
even though it is already supported by staticd.
Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
In ebgp-multihop, there is a difference in reload behavior when TTL is
unspecified (meaning default 255) and when 255 is explicitly specified.
For example, when reloading with 'neighbor <neighbor> ebgp-multihop
255' in the config, the following difference is created. This commit
fixes that.
Lines To Delete
===============
router bgp 65001
no neighbor 10.0.0.4 ebgp-multihop
exit
Lines To Add
============
router bgp 65001
neighbor 10.0.0.4 ebgp-multihop 255
exit
The commit 767aaa3a80 is not sufficient and frr-reload needs to be
fixed to handle both unspecified and specified cases.
Signed-off-by: Nobuhiro MIKI <nob@bobuhiro11.net>
Currently FRR does not handle v6 recurisive resolution properly
when the route being recursed through changes and the most
significant bits of the route are not changed.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Freeing any item here means freeing someone's `event->hist`, leaving a
dangling pointer there. Which will immediately be written to because
we're executing in a CLI function under the `vty_read` event, whose
`event->hist` is then updated.
Deallocating `event->hist` anywhere other than shutting down the whole
event loop is a bad idea to begin with, just zero out the stats instead.
Fixes: FRRouting/frr#16419
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
When VRF instance is going to be deleted inside bgp_vrf_disable(), it uses
a helper method that skips auto created VRF instances and that leads to STALE
issue.
When creating a VNI for a particular VRF vrfX with e.g. `advertise-all-vni`,
auto VRF instance is created, and then we do `router bgp ASN vrf vrfX`.
But when we do a reload bgp_vrf_disable() is called, and we miss previously
created auto instance.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
The "static_simple" test has code for testing IPv6 routes, but it wasn't
even being run (duh.) Enable it, and also test IPv6 dst-src routes.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
When a daemon wants to know about its routes, make it possible to have
that work for dst-src routes.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
The staticd YANG conversion completely f*cked up dst-src routes.
Stupidly enough, the correct thing is much simpler as seen by the amount
of deletes in this commit.
This does, unfortunately, involve a rather annoying YANG edge case with
what should reasonably be an optional leaf as part of a list key, which
is not possible. It uses `::/0` as unconditional filler instead, since
that is semantically correct.
The `test_yang_mgmt` topotest needed to be adjusted after this to add
`src-prefix='::/0'`.
Fixes: 88fa5104a0 ("staticd : Configuration northbound implementation")
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>