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
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.
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
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.
Krishnasamy R [Tue, 21 Jan 2025 09:06:53 +0000 (01:06 -0800)]
bgpd: Fix for local interface MAC cache issue in 'bgp mac hash' table
Issue:
During FRR restart, we fail to add some of the local interface's MAC
to the 'bgp mac hash'. Not having local MAC in the hash table can cause
lookup issues while receiving EVPN RT-2.
Currently, we have code to add local MAC(bgp_mac_add_mac_entry) while handling
interface add/up events in BGP(bgp_ifp_up/bgp_ifp_create). But the code
'bgp_mac_add_mac_entry' in bgp_ifp_create is not getting invoked as it
is placed under a specific check(vrf->bgp link check).
Fix:
We can skip this check 'vrf->bgp link existence' as the tenant VRF might
not have BGP instance but still we want to cache the tenant VRF local
MACs. So keeping this check in bgp_ifp_create inline with bgp_ifp_up.
Louis Scalbert [Thu, 9 Jan 2025 17:28:53 +0000 (18:28 +0100)]
bgpd: fix crash in displaying json orf prefix-list
bgpd crashes when there is several entries in the prefix-list. No
backtrace is provided because the issue was catched from a code review.
Fixes: 856ca177c4 ("Added json formating support to show-...-neighbors-... bgp commands.") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit 8ccf60921b85893d301186a0f8156fb702da379f)
Louis Scalbert [Thu, 9 Jan 2025 17:24:39 +0000 (18:24 +0100)]
bgpd: fix bgp orf prefix-list json prefix
0x<address>FX was displayed instead of the prefix.
Fixes: b219dda129 ("lib: Convert usage of strings to %pFX and %pRN") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit b7e843d7e8afe57d3815dbb44e30307654e73711)
Jonathan Voss [Fri, 3 Jan 2025 03:19:30 +0000 (03:19 +0000)]
tools: Add missing rpki keyword to vrf in frr-reload
When reloading the following configuration:
```
vrf red
rpki
rpki cache tcp 172.65.0.2 8282 preference 1
exit
exit-vrf
```
frr-reload.py does not properly enter the `rpki` context
within a `vrf`. Because of this, it fails to apply RPKI
configurations.
bgpd: Validate only affected RPKI prefixes instead of a full RIB
Before this fix, if rpki_sync_socket_rtr socket returns EAGAIN, then ALL routes
in the RIB are revalidated which takes lots of CPU and some unnecessary traffic,
e.g. if using BMP servers. With a full feed it would waste 50-80Mbps.
Instead we should try to drain an existing pipe (another end), and revalidate
only affected prefixes.
Philippe Guibert [Wed, 18 Dec 2024 15:53:48 +0000 (16:53 +0100)]
bgpd: fix memory leak when reconfiguring a route distinguisher
A memory leak happens when reconfiguring an already configured route
distinguisher on an L3VPN BGP instance. Fix this by freeing the previous
route distinguisher.
Donatas Abraitis [Tue, 10 Dec 2024 14:28:26 +0000 (16:28 +0200)]
lib: Take ge/le into consideration when checking the prefix with the prefix-list
Without the fix:
```
show ip prefix-list test_1 10.20.30.96/27 first-match
<no result>
show ip prefix-list test_2 192.168.1.2/32 first-match
<no result>
```
With the fix:
```
ip prefix-list test_1 seq 10 permit 10.20.30.64/26 le 27
!
end
donatas# show ip prefix-list test_1 10.20.30.96/27
seq 10 permit 10.20.30.64/26 le 27 (hit count: 1, refcount: 0)
donatas# show ip prefix-list test_1 10.20.30.64/27
seq 10 permit 10.20.30.64/26 le 27 (hit count: 2, refcount: 0)
donatas# show ip prefix-list test_1 10.20.30.64/28
donatas# show ip prefix-list test_1 10.20.30.126/26
seq 10 permit 10.20.30.64/26 le 27 (hit count: 3, refcount: 0)
donatas# show ip prefix-list test_1 10.20.30.126/30
donatas#
```
Rajasekar Raja [Tue, 10 Dec 2024 21:45:02 +0000 (13:45 -0800)]
bgpd: Fix bgp core with a possible Intf delete
Although trigger unknown, based on the backtrace in one of the internal
testing, we do see some delete in the Intf where we can have the peer
ifp pointer null and we try to dereference it while trying to install
the route leading to a crash
Skip updating the ifindex in such cases and since the nexthop is not
properly updated, BGP skips sending it to zebra.
BackTrace:
0 0x00007faef05e7ebc in ?? () from /lib/x86_64-linux-gnu/libc.so.6
1 0x00007faef0598fb2 in raise () from /lib/x86_64-linux-gnu/libc.so.6
2 0x00007faef09900dc in core_handler (signo=11, siginfo=0x7ffdde8cb4b0, context=<optimized out>) at lib/sigevent.c:274
3 <signal handler called>
4 0x00005560aad4b7d8 in update_ipv6nh_for_route_install (api_nh=0x7ffdde8cbe94, is_evpn=false, best_pi=0x5560b21187d0, pi=0x5560b21187d0, ifindex=0, nexthop=0x5560b03cb0dc,
nh_bgp=0x5560ace04df0, nh_othervrf=0) at bgpd/bgp_zebra.c:1273
5 bgp_zebra_announce_actual (dest=dest@entry=0x5560afcfa950, info=0x5560b21187d0, bgp=0x5560ace04df0) at bgpd/bgp_zebra.c:1521
6 0x00005560aad4bc85 in bgp_handle_route_announcements_to_zebra (e=<optimized out>) at bgpd/bgp_zebra.c:1896
7 0x00007faef09a1c0d in thread_call (thread=thread@entry=0x7ffdde8d7580) at lib/thread.c:2008
8 0x00007faef095a598 in frr_run (master=0x5560ac7e5190) at lib/libfrr.c:1223
9 0x00005560aac65db6 in main (argc=<optimized out>, argv=<optimized out>) at bgpd/bgp_main.c:557
(gdb) f 4
4 0x00005560aad4b7d8 in update_ipv6nh_for_route_install (api_nh=0x7ffdde8cbe94, is_evpn=false, best_pi=0x5560b21187d0, pi=0x5560b21187d0, ifindex=0, nexthop=0x5560b03cb0dc,
nh_bgp=0x5560ace04df0, nh_othervrf=0) at bgpd/bgp_zebra.c:1273
1273 in bgpd/bgp_zebra.c
(gdb) p pi->peer->ifp
$26 = (struct interface *) 0x0
Mark Stapp [Wed, 30 Oct 2024 15:02:17 +0000 (11:02 -0400)]
zebra: separate zebra ZAPI server open and accept
Separate zebra's ZAPI server socket handling into two phases:
an early phase that opens the socket, and a later phase that
starts listening for client connections.
Philippe Guibert [Tue, 26 Nov 2024 13:19:34 +0000 (14:19 +0100)]
bgpd: fix use single whitespace when displaying flowspec entries
There is an extra space in the 'Displayed' line of show bgp command,
that should not be present.
Fix this by being consistent with the output of the other address
families.
Fixes: ("a1baf9e84f71") bgpd: Use single whitespace when displaying show bgp summary Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
(cherry picked from commit 561debab5430ac85e8e42e839312d213479a767a)
Donatas Abraitis [Sun, 17 Nov 2024 09:25:41 +0000 (11:25 +0200)]
bgpd: Validate both nexthop information (NEXTHOP and NLRI)
If we receive an IPv6 prefix e.g.: 2001:db8:100::/64 with nextop: 0.0.0.0, and
mp_nexthop: fc00::2, we should not treat this with an invalid nexthop because
of 0.0.0.0. We MUST check for MP_REACH attribute also and decide later if we
have at least one a valid nexthop.
Rajasekar Raja [Mon, 21 Oct 2024 17:53:27 +0000 (10:53 -0700)]
bgpd: Fix for match source-protocol in route-map for redistribute cmd
A redistribute cmd can have a route-map attached to it and adding the
match source-protocol to that route-map means BGP to filter which
protocol routes to accept among the bunch of routes zebra is sending.
bgpd: fix addressing information of non established outgoing sessions
When trying to connect to a BGP peer that does not respons, the 'show
bgp neighbors' command does not give any indication on the local and
remote addresses used:
> # show bgp neighbors
> BGP neighbor is 192.0.2.150, remote AS 65500, local AS 65500, internal link
> Local Role: undefined
> Remote Role: undefined
> BGP version 4, remote router ID 0.0.0.0, local router ID 192.0.2.1
> BGP state = Connect
> [..]
> Connections established 0; dropped 0
> Last reset 00:00:04, Waiting for peer OPEN (n/a)
> Internal BGP neighbor may be up to 255 hops away.
> BGP Connect Retry Timer in Seconds: 120
> Next connect timer due in 117 seconds
> Read thread: off Write thread: off FD used: 27
The addressing information (address and port) are only available
when TCP session is established, whereas this information is present
at the system level:
Add the display for outgoing BGP session, as the information in
the getsockname() API provides information for connected streams.
When getpeername() API does not give any information, use the peer
configuration (destination port is encoded in peer->port).
> # show bgp neighbors
> BGP neighbor is 192.0.2.150, remote AS 65500, local AS 65500, internal link
> Local Role: undefined
> Remote Role: undefined
> BGP version 4, remote router ID 0.0.0.0, local router ID 192.0.2.1
> BGP state = Connect
> [..]
> Connections established 0; dropped 0
> Last reset 00:00:16, Waiting for peer OPEN (n/a)
> Local host: 192.0.2.1, Local port: 46084
> Foreign host: 192.0.2.150, Foreign port: 179
bgpd: remove useless control checks about TCP connection
When attempting to get the src and destination addresses of a given
connection, the API may return the NULL pointer, but further code
in bgp_zebra_nexthop_set() already does a check about the given
pointer.
Relaxing the error code for all the returned adressing.
Fixes: 1ff9a340588a ("bgpd: bgpd-fsm-fix.patch") Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
(cherry picked from commit ba7130309954fbe8d58854339ca43259149e603a)
bgpd: Set LLGR stale routes for all the paths including addpath
Without this patch we set only the first path for the route (if multiple exist)
as LLGR stale and stop doing that for the rest of the paths, which is wrong.
Donatas Abraitis [Thu, 31 Oct 2024 08:47:48 +0000 (10:47 +0200)]
zebra: Add missing new line for help string
```
-A, --asic-offload FRR is interacting with an asic underneath the linux kernel
--v6-with-v4-nexthops Underlying dataplane supports v6 routes with v4 nexthops -s, --nl-bufsize Set netlink receive buffer size
```
Fixes: 1f5611c06d1c243b42279748788f0627793ead9c ("zebra: Allow zebra cli to accept v6 routes with v4 nexthops") Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
(cherry picked from commit 25ae643996d338b8230fb15a9064843fe85de224)
Louis Scalbert [Fri, 25 Oct 2024 15:54:07 +0000 (17:54 +0200)]
bgpd: fix display of local label in show bgp
Fix the display of the local label in show bgp.
> r1# show bgp ipv4 labeled-unicast 172.16.2.2/32
> BGP routing table entry for 172.16.2.2/32, version 2
> Local label: 16 <---- MISSING
> Paths: (1 available, best #1, table default, vrf (null))
> Advertised to non peer-group peers:
> 192.168.1.2
> 65501
> 192.168.1.2 from 192.168.1.2 (172.16.2.2)
> Origin IGP, metric 0, valid, external, best (First path received)
> Remote label: 3
> Last update: Fri Oct 25 17:55:45 2024
Fixes: 67f67ba481 ("bgpd: Drop label_ntop/label_pton functions") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit e7b3276ace65d59edb4d614158d4f2959f12f868)
pimd: allow resolving bsr via directly connected secondary address
This only matters to single hop nodes that are adjacent to the bsr. More common
with IPv6 where LL address is used in PIM as the primary address. If the BSR IP
happens to be an address on the same interface, the receiving pim router
rejects the BSR address because it expects the BSR IP to resolve via the LL address
even if we have a connected route for the same BSR IP subnet. Effectively, we want to
allow rpf to be resolved via secondary IPs with connected routes on the same interface,
and not limit them to primary addresses.
Louis Scalbert [Tue, 22 Oct 2024 16:08:13 +0000 (18:08 +0200)]
bgpd: fix uninitialized labels
Fix uninitialized labels that cause multiple valgrind issues.
> ==3729602== Use of uninitialised value of size 8
> ==3729602== at 0x492B493: hash_get (hash.c:140)
> ==3729602== by 0x2629D2: bgp_labels_intern (bgp_label.c:98)
> ==3729602== by 0x2E6C92: bgp_adj_out_set_subgroup (bgp_updgrp_adv.c:622)
> ==3729602== by 0x2A6810: subgroup_process_announce_selected (bgp_route.c:3340)
> ==3729602== by 0x2E5FF6: group_announce_route_walkcb (bgp_updgrp_adv.c:260)
> ==3729602== by 0x2E3E28: update_group_walkcb (bgp_updgrp.c:1759)
> ==3729602== by 0x492B9A0: hash_walk (hash.c:270)
> ==3729602== by 0x2E498C: update_group_af_walk (bgp_updgrp.c:2090)
> ==3729602== by 0x2E7C0D: group_announce_route (bgp_updgrp_adv.c:1119)
> ==3729602== by 0x2A796E: bgp_process_main_one (bgp_route.c:3865)
> ==3729602== by 0x2A808A: bgp_process_wq (bgp_route.c:3991)
> ==3729602== by 0x49CC7CF: work_queue_run (workqueue.c:282)
> ==3729602== by 0x49BBF25: event_call (event.c:2019)
> ==3729602== by 0x49413CA: frr_run (libfrr.c:1238)
> ==3729602== by 0x1FD1D3: main (bgp_main.c:555)
> ==2604268== Use of uninitialised value of size 8
> ==2604268== at 0x4943016: hash_get (hash.c:159)
> ==2604268== by 0x26EFC1: bgp_labels_intern (bgp_label.c:97)
> ==2604268== by 0x28077B: leak_update (bgp_mplsvpn.c:1298)
> ==2604268== by 0x2824A3: vpn_leak_from_vrf_update (bgp_mplsvpn.c:1932)
> ==2604268== by 0x2C281C: bgp_static_update (bgp_route.c:6974)
> ==2604268== by 0x2C366F: bgp_static_set (bgp_route.c:7263)
> ==2604268== by 0x2C435B: bgp_network_magic (bgp_route.c:7556)
> ==2604268== by 0x2ACF09: bgp_network (bgp_route_clippy.c:86)
> ==2604268== by 0x4914EE7: cmd_execute_command_real (command.c:1003)
> ==2604268== by 0x4915060: cmd_execute_command (command.c:1062)
> ==2604268== by 0x4915610: cmd_execute (command.c:1228)
> ==2604268== by 0x49E7C32: vty_command (vty.c:625)
> ==2604268== by 0x49E9B56: vty_execute (vty.c:1388)
> ==2604268== by 0x49EC331: vtysh_read (vty.c:2400)
> ==2604268== by 0x49E06F1: event_call (event.c:2001)
> ==2604268== by 0x495AB8B: frr_run (libfrr.c:1238)
> ==2604268== by 0x200C4B: main (bgp_main.c:555)
Fixes: ddb5b4880b ("bgpd: vpn-vrf route leaking") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Enke Chen [Sun, 20 Oct 2024 19:25:46 +0000 (12:25 -0700)]
bgpd: allow value 0 in aigp-metric setting
The value of 0 is accepted from peers, and can also be set by the
route-map "set aigp-metric igp-metric". For coonsistency, it should
be allowed in "set aigp-metric <value>" as well.
Enke Chen [Wed, 16 Oct 2024 18:15:28 +0000 (11:15 -0700)]
bgpd: fix several issues in sourcing AIGP attribute
Fix several issues in sourcing AIGP attribute:
1) AIGP should not be set as default for a redistributed route or a
static network. It should be set by config instead.
2) AIGP sourced by "set aigp-metric igp-metric" in a route-map does
not set the correct value for a redistributed route.
3) When redistribute a connected route like loopback, the AGIP (with
value 0) is sourced by "set aigp-metric igp-metric", but the
attribute is not propagated as the attribute flag is not set.