Chirag Shah [Mon, 3 Feb 2025 20:00:41 +0000 (12:00 -0800)]
bgpd: fix route-distinguisher in vrf leak json cmd
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.
Chirag Shah [Fri, 31 Jan 2025 01:26:46 +0000 (17:26 -0800)]
zebra: evpn svd hash avoid double free
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
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.
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.
Donald Sharp [Fri, 31 Jan 2025 23:53:30 +0000 (18:53 -0500)]
bgpd: With suppress-fib-pending ensure withdrawal is sent
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.
Mark Stapp [Fri, 31 Jan 2025 18:13:48 +0000 (13:13 -0500)]
libs: return from change_caps if no caps
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.
Donald Sharp [Fri, 31 Jan 2025 17:38:20 +0000 (12:38 -0500)]
zebra: Ensure dplane does not send work back to master at wrong time
When looping through the dplane providers, the worklist was
being populated with items from the last provider and then
the event system was checked to see if we should stop processing.
If the event system says `yes` then the dplane code would stop
and send the worklist to the master zebra pthread for collection.
This obviously skipped the next dplane provider on the list
which is double plus not good.
Nobuhiro MIKI [Wed, 29 Jan 2025 04:31:53 +0000 (04:31 +0000)]
tools: Fix frr-reload for ebgp-multihop TTL reconfiguration.
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
Donald Sharp [Mon, 27 Jan 2025 15:34:31 +0000 (10:34 -0500)]
tests: Add a test that shows the v6 recursive nexthop problem
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.
David Lamparter [Tue, 28 Jan 2025 15:37:52 +0000 (16:37 +0100)]
lib: fix use after free in `clear event cpu`
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>
Donatas Abraitis [Tue, 28 Jan 2025 15:11:58 +0000 (17:11 +0200)]
bgpd: Do not ignore auto generated VRF instances when deleting
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.
David Lamparter [Mon, 27 Jan 2025 17:18:24 +0000 (18:18 +0100)]
staticd: fix botched staticd YANG for dst-src
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'`.
Philippe Guibert [Wed, 22 Jan 2025 14:35:06 +0000 (15:35 +0100)]
bgpd: fix missing braces when dumping json vpn advertised-routes
The json output of advertised-routes is incorrect, as there is a missing
brace with route-distinguisher:
observed with the bgp_vpnv4_noretain test:
> "bgpTableVersion":0,"bgpLocalRouterId":"192.0.2.1","defaultLocPrf":100,"localAS":65500,
> "advertisedRoutes": "192.0.2.1:1":{"rd":"192.0.2.1:1","10.101.0.0/24":{"prefix":"10.101.0.0/24",
Philippe Guibert [Fri, 24 Jan 2025 12:22:03 +0000 (13:22 +0100)]
bgpd: fix add json attribute to reflect suppressed path
When aggregate is used, the suppressed information is not displayed in
the json attributes of a given path. To illustrate, the dump of the
192.168.2.1/32 path in the bgp_aggregate_address_topo1 topotest:
> # show bgp ipv4
> [..]
> s> 192.168.2.1/32 10.0.0.2 0 65001 i
>
> # show bgp ipv4 detail
> [..]
> BGP routing table entry for 192.168.2.1/32, version 17
> Paths: (1 available, best #1, table default, vrf (null), Advertisements suppressed by an aggregate.)
> Not advertised to any peer
> 65001 <---- missing suppressed flag
> 10.0.0.2 from 10.0.0.2 (10.254.254.3)
> Origin IGP, valid, external, best (First path received)
> Last update: Fri Jan 24 13:11:41 2025
>
> # show bgp ipv4 detail json
> [..]
> ,"192.168.2.1/32": [{"aspath":{"string":"65001","segments":[{"type":"as-sequence","list":[65001]}],"length":1},"origin":"IGP","valid":true,"version":17,
> "bestpath":{"overall":true,"selectionReason":"First path received"}, <---- missing suppressed flag
> "lastUpdate":{"epoch":1737720700,"string":"Fri Jan 24 13:11:40 2025\n"},
> "nexthops":[{"ip":"10.0.0.2","afi":"ipv4","metric":0,"accessible":true,"used":true}],
> "peer":{"peerId":"10.0.0.2","routerId":"10.254.254.3","type":"external"}}]
Fix this by adding the json information.
> # show bgp ipv4 detail
> [..]
> BGP routing table entry for 192.168.2.1/32, version 17
> Paths: (1 available, best #1, table default, vrf (null), Advertisements suppressed by an aggregate.)
> Not advertised to any peer
> 65001, (suppressed)
> 10.0.0.2 from 10.0.0.2 (10.254.254.3)
> Origin IGP, valid, external, best (First path received)
> Last update: Fri Jan 24 13:11:41 2025
>
> # show bgp ipv4 detail json
> [..]
> ,"192.168.2.1/32": [{"aspath":{"string":"65001","segments":[{"type":"as-sequence","list":[65001]}],"length":1},"suppressed":true,"origin":"IGP","valid":true,"version":17,
> "bestpath":{"overall":true,"selectionReason":"First path received"},
> "lastUpdate":{"epoch":1737720991,"string":"Fri Jan 24 13:16:31 2025"},
> "nexthops":[{"ip":"10.0.0.2","afi":"ipv4","metric":0,"accessible":true,"used":true}],"peer":{"peerId":"10.0.0.2","routerId":"10.254.254.3","type":"external"}}]
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
TL;DR; Handling BGP AddPath capability is not trivial (possible) dynamically.
When the sender is AddPath-capable and sends NLRIs encoded with AddPath ID,
and at the same time the receiver sends AddPath capability "disable-addpath-rx"
(flag update) via dynamic capabilities, both peers are out of sync about the
AddPath state. The receiver thinks already he's not AddPath-capable anymore,
hence it tries to parse NLRIs as non-AddPath, while they are actually encoded
as AddPath.
AddPath capability itself does not provide (in RFC) any mechanism on backward
compatible way to handle NLRIs if they come mixed (AddPath + non-AddPath).
This explains why we have failures in our CI periodically.
Donald Sharp [Fri, 24 Jan 2025 20:04:13 +0000 (15:04 -0500)]
bgpd: Optimize evaluate paths for a peer going down
Currently when a directly connected peer is going down
BGP gets a call back for nexthop tracking in addition
the interface down events. On the interface down
event BGP goes through and sets up a per peer Q that
holds all the bgp path info's associated with that peer
and then it goes and processes this in the future. In
the meantime zebra is also at work and sends a nexthop
removal event to BGP as well. This triggers a complete
walk of all path info's associated with the bnc( which
happens to be all the path info's already scheduled
for removal here shortly). This evaluate paths
is not an inexpensive operation in addition the work
for handling this is already being done via the
peer down queue. Let's optimize the bnc handling
of evaluate paths and check to see if the peer is
still up to actually do the work here.
Acee Lindem [Wed, 22 Jan 2025 20:41:47 +0000 (20:41 +0000)]
ospfd: Prune duplicate next-hops when installing into zebra
Duplicate next-hops are maintained for OSPF inter-area and AS
external routes in the OSPF routing table as long as they
correspond to LSAs for different adverting routers. The
intra-area route computation will not result in duplicate
next-hops.
Acee Lindem [Wed, 22 Jan 2025 20:32:50 +0000 (20:32 +0000)]
tests: OSPF topotest for next-hop pruning
OSPF topotest to test OSPF next-hop pruning on installation
into zebra routing table. Also fix multicast_pim_dr_nondr_test
topotest which had a duplicate OSPF route in the results.
Martin Winter [Thu, 23 Jan 2025 13:13:51 +0000 (14:13 +0100)]
redhat: Specify minimum libyang version requirement
Version requirement from a BuildRequire get dropped and don't get
reflected in Require's for the package. Specify it both ways for
Libyang as we require >= 2.1.128
Signed-off-by: Martin Winter <mwinter@opensourcerouting.org>
zebra: Return error if v6 prefix is passed to show ip route
Return error if IPv6 address or prefix is passed as an argument
to "show ip route" command.
UT:
r1# show ip route 2::3/128
% Cannot specify IPv6 address/prefix for IPv4 table
r1#
r1# show ip route 2::3
% Cannot specify IPv6 address/prefix for IPv4 table
r1#
Philippe Guibert [Wed, 22 Jan 2025 16:18:03 +0000 (17:18 +0100)]
topotests: isis_srv6_topo1, fix wrong srte path
There is no connectivity by using the proposed srv6 path.
> From Carmine:
> This seg6-route tells rt1 to steer packets destined to fc00:0:9::1 over this path: rt1->rt2->-rt6.
> This path is not correct. Since we are installing this seg6-route on rt1,
> it means that a packet matching this seg6-route has already reached rt1.
> So rt1 should not be part of the path.
> The correct path should be rt2->rt6.
Fix this by changing the proposed seg6 route. Also, the ping test should
be swapped, because invalidating the RT1 locator does not have any
impacts on the built SRv6 path.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
wumu.zsl [Mon, 20 Jan 2025 13:48:38 +0000 (13:48 +0000)]
bfdd: add bfdname in bfd_key
sbfd will use bfdname for key hash, We introduced a bfd-name for every sbfd session, normal BFD sessions can leave it as NULL.
A unique bfd-name can be used to identify a sbfd session quickly. This is quite useful in our Srv6 deployment for path protection case.
For example, if use the sbfd session to protect the SRv6 path A-B-D, we would assign the name 'path-a-b-d' or 'a-b-d' to the session.
projects / matthieu / frr.git / log