bgpd/bgpd.c:8975:5: error: "ENABLE_BGP_VNC" is not defined, evaluates to 0 [-Werror=undef]
8975 | #if ENABLE_BGP_VNC
Fixes: FRRouting#18546
Fixes: 1629c05924 ("bgpd: rfapi: track outstanding rib and import timers, free mem at exit")
Cc: G. Paul Ziemba <paulz@labn.net>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Fix bgp_pbr_rule memory leak. Found by code review.
Fixes: 27e376d4e1 ("bgpd: an hash list of pbr iprule is created")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
When attempting to check rpki status and the connection
has been turned off, let's check to see if we are connected
before we ask the rpki subsystem, else we will get a crash
in the rpki library.
Signed-off-by: Donald Sharp <donaldsharp72@gmail.com>
The 'acceptedPrefixCounter' is available in 'show bgp neighbor json', but
there is no equivalent when using the non json output. Add it.
> # show bgp neighbor
> [..]
> Community attribute sent to this neighbor(all)
> 0 accepted prefixes, 1 sent prefixes
Fixes: 856ca177c4 ("Added json formating support to show-...-neighbors-... bgp commands.")
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Issue:
In a scaled setup, (where number of nets > BGP_CLEARING_BATCH_MAX_DESTS
for walk_batch_table_helper), when peer is shutdown, it is seen some
of the paths are not deleted, which are received from that peer.
Fix:
This is due to, in clear_batch_rib_helper, once walk_batch_table_helper
returns after BGP_CLEARING_BATCH_MAX_DESTS is reached, we just break
from inner loop for the afi/safi for loops. So during walk for next
afi/safi that 'ret' state is overwritten with new state. Also the
resume context is overwritten. This causes to lose the start point
for next walk, some nets are skipped forever. So they are not marked
for deletion anymore. To fix this, we immediately return from current
run. This will have resume state to be stored correctly, and next walk
will start from there.
Testing:
32 ecmp paths were received from the shutdown peer
Before fix:
show bgp ipv6 2052:52:1:167::/64
BGP routing table entry for 2052:52:1:167::/64, version 495
Paths: (246 available, best #127, table default)
Not advertised to any peer
<snip>
4200165500 4200165002
2021:21:51:101::2(spine-5) from spine-5(2021:21:51:101::2) (6.0.0.17)
(fe80::202:ff:fe00:55) (prefer-global)
Origin incomplete, valid, external, multipath
Last update: Fri Apr 4 17:25:05 2025
4200165500 4200165002
2021:21:11:116::2(spine-1) from spine-1(2021:21:11:116::2) (0.0.0.0)
(fe80::202:ff:fe00:3d) (prefer-global)<<<<path not deleted
Origin incomplete, valid, external
Last update: Fri Apr 4 17:25:05 2025
4200165500 4200165002
2021:21:11:115::2(spine-1) from spine-1(2021:21:11:115::2) (0.0.0.0)
(fe80::202:ff:fe00:3d) (prefer-global)<<<<path not deleted
Origin incomplete, valid, external
Last update: Fri Apr 4 17:25:05 2025
<snip>
32 paths are supposed to be withdrawn:
root@leaf-1:mgmt:# vtysh -c "show bgp ipv6 2052:52:1:167::/64" | grep "prefer-global" | wc -l
256
root@leaf-1:mgmt# vtysh -c "show bgp ipv6 2052:52:1:167::/64" | grep "prefer-global" | wc -l
246<<should be 224, but showing 246, which is wrong
After fix:
32 paths are supposed to be withdrawn:
root@leaf-1:mgmt:# vtysh -c "show bgp ipv6 2052:52:1:167::/64" | grep "prefer-global" | wc -l
256
root@leaf-1:mgmt:# vtysh -c "show bgp ipv6 2052:52:1:167::/64" | grep "prefer-global" | wc -l
224<<<shows correctly
Signed-off-by: Soumya Roy <souroy@nvidia.com>
Indirect leak of 56 byte(s) in 1 object(s) allocated from:
0 0x7fdaf6cb83b7 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
1 0x7fdaf683a480 in qcalloc lib/memory.c:106
2 0x7fdaf68dd706 in route_table_init_with_delegate lib/table.c:38
3 0x5649b22c05b0 in bgp_table_init bgpd/bgp_table.c:139
4 0x5649b2273da0 in bgp_static_set bgpd/bgp_route.c:7779
5 0x5649b21eba58 in vpnv4_network bgpd/bgp_mplsvpn.c:3244
6 0x7fdaf67b6d61 in cmd_execute_command_real lib/command.c:1003
7 0x7fdaf67b7080 in cmd_execute_command lib/command.c:1062
8 0x7fdaf67b75ac in cmd_execute lib/command.c:1228
9 0x7fdaf68ffb20 in vty_command lib/vty.c:626
10 0x7fdaf6900073 in vty_execute lib/vty.c:1389
11 0x7fdaf6903e24 in vtysh_read lib/vty.c:2408
12 0x7fdaf68f0222 in event_call lib/event.c:2019
13 0x7fdaf681b3c6 in frr_run lib/libfrr.c:1247
14 0x5649b211c903 in main bgpd/bgp_main.c:565
15 0x7fdaf630c249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
Table was being created but never deleted. Let's delete it.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
This commit fixes two types of problems:
a) Avoidance of cleaning up memory when a instance is
hidden, thus causing it never to be freed on shutdown
b) In some instances bgp_create is called 2 times
for some code. We are double allocating memory
and dropping it on the second allocation.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Section 4.1 of RFC8955 defines how the length field of flowspec NLRIs is encoded.
The method use implies a maximum length of 4095 for a single flowspec NLRI.
However, in bgp_flowspec.c, we check the length attribute of the bgp_nlri structure against this maximum value, which actually is the *total* length of all NLRI included in the considered MP_REACH_NLRI path attribute.
Due to this confusion, frr would reject valid announces that contain many flowspec NLRIs, when their cummulative length exceeds 4095, and close the session.
The proposed change removes that check entirely. Indeed, there is no need to check the length field of each invidual NLRI because the method employed make it impossible to encode a length greater than 4095.
Signed-off-by: Stephane Poignant <stephane.poignant@proton.ch>
Issue:
"Processing EVPN MAC interface change on peer" log message is printed
even when the peer didnt have EVPN address family.
Fix:
Process only if the peer is in EVPN address family
Ticket: #17890
Signed-off-by: Rajesh Varatharaj <rvaratharaj@nvidia.com>
If we have `neighbor X bfd` and BFD status is DOWN and/or ADMIN_DOWN, and BGP
session is not yet established, we never allow the session to establish.
Let's fix this regression that was in 10.2.
Fixes: 1fb48f5 ("bgpd: Do not start BGP session if BFD profile is in shutdown state")
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
While here, also make "VPN SAFI clear" test wait for clear result
(tests/topotests/bgp_rfapi_basic_sanity{,_config2})
Original RFAPI code relied on the frr timer system to remember
various allocations that were supposed to be freed at future times
rather than manage a parallel database. However, if bgpd is terminated
before the times expire, those pending allocations are marked as
memory leaks, even though they wouldn't be leaks under normal operation.
This change adds some hash tables to track these outstanding
allocations that are associated with pending timers, and uses
those tables to free the allocations when bgpd exits.
Signed-off-by: G. Paul Ziemba <paulz@labn.net>
Fix this:
==3890443== 92 (48 direct, 44 indirect) bytes in 1 blocks are definitely lost in loss record 68 of 98
==3890443== at 0x484DA83: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==3890443== by 0x49737B3: qcalloc (memory.c:106)
==3890443== by 0x3EA63B: aspath_dup (bgp_aspath.c:703)
==3890443== by 0x2F5438: route_set_aspath_exclude (bgp_routemap.c:2604)
==3890443== by 0x49BC52A: route_map_apply_ext (routemap.c:2708)
==3890443== by 0x2C1069: bgp_input_modifier (bgp_route.c:1925)
==3890443== by 0x2C9F12: bgp_update (bgp_route.c:5205)
==3890443== by 0x2CF281: bgp_nlri_parse_ip (bgp_route.c:7271)
==3890443== by 0x2A28C7: bgp_nlri_parse (bgp_packet.c:338)
==3890443== by 0x2A7F5C: bgp_update_receive (bgp_packet.c:2448)
==3890443== by 0x2ACCA6: bgp_process_packet (bgp_packet.c:4046)
==3890443== by 0x49EB77C: event_call (event.c:2019)
==3890443== by 0x495FAD1: frr_run (libfrr.c:1247)
==3890443== by 0x208D6D: main (bgp_main.c:557)
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
There are some tables not being freed upon shutdown. This
is happening because the table is being locked as dests
are being put on the metaQ. When in shutdown it was clearing
the MetaQ it was not unlocking the table
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Commit: e0ae285eb8
Modified the fsm state machine to attempt to not
clear routes on a peer that was not established.
The peer should be not a peer self. We do not want
to ever clear the peer self.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
RFC8277 says:
The procedures in [RFC3107] for withdrawing the binding of a label
or sequence of labels to a prefix are not specified clearly and correctly.
=> How to Explicitly Withdraw the Binding of a Label to a Prefix
Suppose a BGP speaker has announced, on a given BGP session, the
binding of a given label or sequence of labels to a given prefix.
Suppose it now wishes to withdraw that binding. To do so, it may
send a BGP UPDATE message with an MP_UNREACH_NLRI attribute. The
NLRI field of this attribute is encoded as follows:
0 1 2 3
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| Length | Compatibility |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| Prefix ~
~ |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
Figure 4: NLRI for Withdrawal
Upon transmission, the Compatibility field SHOULD be set to 0x800000.
Upon reception, the value of the Compatibility field MUST be ignored.
[RFC3107] also made it possible to withdraw a binding without
specifying the label explicitly, by setting the Compatibility field
to 0x800000. However, some implementations set it to 0x000000. In
order to ensure backwards compatibility, it is RECOMMENDED by this
document that the Compatibility field be set to 0x800000, but it is
REQUIRED that it be ignored upon reception.
In FRR case where a single label is used per-prefix, we should send 0x800000,
and not 0x000000.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
On receiving side we already did the job correctly, but the peer which initiates
the clear does not retain the other's routes. This commit fixes that.
Fixes: 20170775da ("bgpd: Activate Graceful-Restart when receiving CEASE/HOLDTIME notifications")
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Current behavor of BGP is to have a event per connection. Given
that on startup of BGP with a high number of neighbors you end
up with 2 * # of peers events that are being processed. Additionally
once BGP has selected the connection this still only comes down
to 512 events. This number of events is swamping the event system
and in addition delaying any other work from being done in BGP at
all because the the 512 events are always going to take precedence
over everything else. The other main events are the handling
of the metaQ(1 event), update group events( 1 per update group )
and the zebra batching event. These are being swamped.
Modify the BGP code to have a FIFO of connections. As new data
comes in to read, place the connection on the end of the FIFO.
Have the bgp_process_packet handle up to 100 packets spread
across the individual peers where each peer/connection is limited
to the original quanta. During testing I noticed that withdrawal
events at very very large scale are taking up to 40 seconds to process
so I added a check for yielding to further limit the number of packets
being processed.
This change also allow for BGP to be interactive again on scale
setups on initial convergence. Prior to this change any vtysh
command entered would be delayed by 10's of seconds in my setup
while BGP was doing other work.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Test for existing explicit config as part of validation of
route-target configuration: allow explicit config of generic/
default AS+VNI, for example, instead of rejecting it.
Signed-off-by: Mark Stapp <mjs@cisco.com>
Assuming attr is null, a dereference can happen in the function
make_prefix(). Add the protection over attr before accessing the
variable.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Commit: cc9f21da22
Modified the bgp_fsm code to dissallow the extension
of the hold time when the system is under extremely
heavy load. This was a attempt to remove the return
code but it was too aggressive and messed up this bit
of code.
Put the behavior back that was introduced in:
d0874d195d
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
When exporting a VPN SRv6 route, the path may not be considered valid if
the nexthop is not valid. This is the case when the 'nexthop vpn export'
command is used. The below example illustrates that the VPN path to
2001:1::/64 is not selected, as the expected nexthop to find in vrf10 is
the one configured:
> # show running-config
> router bgp 1 vrf vrf10
> address-family ipv6 unicast
> nexthop vpn export 2001::1
> # show bgp ipv6 vpn
> [..]
> Route Distinguisher: 1:10
> 2001:1::/64 2001::1@4 0 0 65001 i
> UN=2001::1 EC{99:99} label=16 sid=2001:db8:1:1:: sid_structure=[40,24,16,0] type=bgp, subtype=5
The analysis indicates that the 2001::1 nexthop is considered.
> 2025/03/20 21:47:53.751853 BGP: [RD1WY-YE9EC] leak_update: entry: leak-to=VRF default, p=2001:1::/64, type=10, sub_type=0
> 2025/03/20 21:47:53.751855 BGP: [VWNP2-DNMFV] Found existing bnc 2001::1/128(0)(VRF vrf10) flags 0x82 ifindex 0 #paths 2 peer 0x0, resolved prefix UNK prefix
> 2025/03/20 21:47:53.751856 BGP: [VWC2R-4REXZ] leak_update_nexthop_valid: 2001:1::/64 nexthop is not valid (in VRF vrf10)
> 2025/03/20 21:47:53.751857 BGP: [HX87B-ZXWX9] leak_update: ->VRF default: 2001:1::/64: Found route, no change
Actually, to check the nexthop validity, only the source path in the VRF
has the correct nexthop. Fix this by reusing the source path information
instead of the current one.
> 2025/03/20 22:43:51.703521 BGP: [RD1WY-YE9EC] leak_update: entry: leak-to=VRF default, p=2001:1::/64, type=10, sub_type=0
> 2025/03/20 22:43:51.703523 BGP: [VWNP2-DNMFV] Found existing bnc fe80::b812:37ff:fe13:d441/128(0)(VRF vrf10) flags 0x87 ifindex 0 #paths 2 peer 0x0, resolved prefix fe80::/64
> 2025/03/20 22:43:51.703525 BGP: [VWC2R-4REXZ] leak_update_nexthop_valid: 2001:1::/64 nexthop is valid (in VRF vrf10)
> 2025/03/20 22:43:51.703526 BGP: [HX87B-ZXWX9] leak_update: ->VRF default: 2001:1::/64: Found route, no change
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
