> topo: STEP 8: 'Check if we can see 172.16.1.2/32 after R4 (dynamic peer) was killed'
R4 neighbor is deleted on R2 because it fails to re-connect:
> 14:33:40.128048 BGP: [HKWM3-ZC5QP] 192.168.3.1 fd -1 went from Established to Clearing
> 14:33:40.128154 BGP: [MJ1TJ-HEE3V] 192.168.3.1(r4) graceful restart timer expired
> 14:33:40.128158 BGP: [ZTA2J-YRKGY] 192.168.3.1(r4) graceful restart stalepath timer stopped
> 14:33:40.128162 BGP: [H917J-25EWN] 192.168.3.1(r4) Long-lived stale timer (IPv4 Unicast) started for 20 sec
> 14:33:40.128168 BGP: [H5X66-NXP9S] 192.168.3.1(r4) Long-lived set stale community (LLGR_STALE) for: 172.16.1.2/32
> 14:33:40.128220 BGP: [H5X66-NXP9S] 192.168.3.1(r4) Long-lived set stale community (LLGR_STALE) for: 192.168.3.0/24
> [...]
> 14:33:41.138869 BGP: [RGGAC-RJ6WG] 192.168.3.1 [Event] Connect failed 111(Connection refused)
> 14:33:41.138906 BGP: [ZWCSR-M7FG9] 192.168.3.1 [FSM] TCP_connection_open_failed (Connect->Active), fd 23
> 14:33:41.138912 BGP: [JA9RP-HSD1K] 192.168.3.1 (dynamic neighbor) deleted (bgp_connect_fail)
> 14:33:41.139126 BGP: [P98A2-2RDFE] 192.168.3.1(r4) graceful restart stalepath timer stopped
af8496af08 ("bgpd: Do not delete BGP dynamic peers if graceful restart
kicks in") forgot to modify bgp_connect_fail()
Do not delete the peer in bgp_connect_fail() if Non-Stop-Forwarding is
in progress.
Fixes: af8496af08 ("bgpd: Do not delete BGP dynamic peers if graceful restart kicks in") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit e446308d76b4ac8f26149db982bf5ed5ac93d5f5)
Igor Ryzhov [Mon, 1 Apr 2024 21:56:17 +0000 (00:56 +0300)]
isisd: fix ip/ipv6 reachability tlvs
Don't allocate subtlvs container if there's nothing to add to it. If the
container is allocated, the "sub-TLVs presence" bit is set in the TLVs
even if there's no actual sub-TLVs, what violates the RFC.
David Lamparter [Mon, 12 Dec 2022 16:50:59 +0000 (17:50 +0100)]
pimd: fix order of operations for evaluating join
join_desired looks at whether up->channel_oil is empty. up->channel_oil
is updated from pim_forward_stop(), calling pim_channel_del_oif(). But
that was being called *after* updating join_desired, so join_desired saw
a non-empty OIL. Pull up the pim_forward_stop() call to before updating
join_desired.
David Lamparter [Mon, 17 Apr 2023 09:47:08 +0000 (11:47 +0200)]
pimd: fix null register before aging out reg-stop
It looks like the code was trying to do this with the null_register
parameter on pim_upstream_start_register_stop_timer(), but that didn't
quite work right. Restructure a bit to get it right.
Direct leak of 592 byte(s) in 2 object(s) allocated from:
#0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
#2 0xfeb98ae76138 in srv6_locator_chunk_alloc lib/srv6.c:138
#3 0xb7f3c8508fa0 in ensure_vrf_tovpn_sid_per_vrf bgpd/bgp_mplsvpn.c:831
#4 0xb7f3c8509494 in ensure_vrf_tovpn_sid bgpd/bgp_mplsvpn.c:866
#5 0xb7f3c85028a8 in vpn_leak_postchange bgpd/bgp_mplsvpn.h:289
#6 0xb7f3c851a7c0 in vpn_leak_postchange_all bgpd/bgp_mplsvpn.c:3769
#7 0xb7f3c86f6ef0 in bgp_zebra_process_srv6_locator_chunk bgpd/bgp_zebra.c:3378
#8 0xfeb98afa6e14 in zclient_read lib/zclient.c:4608
#9 0xfeb98af3d684 in event_call lib/event.c:2011
#10 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
#11 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
#12 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
#13 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
#14 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)
Direct leak of 32 byte(s) in 2 object(s) allocated from:
#0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
#2 0xb7f3c8508fd8 in ensure_vrf_tovpn_sid_per_vrf bgpd/bgp_mplsvpn.c:832
#3 0xb7f3c8509494 in ensure_vrf_tovpn_sid bgpd/bgp_mplsvpn.c:866
#4 0xb7f3c85028a8 in vpn_leak_postchange bgpd/bgp_mplsvpn.h:289
#5 0xb7f3c851a7c0 in vpn_leak_postchange_all bgpd/bgp_mplsvpn.c:3769
#6 0xb7f3c86f6ef0 in bgp_zebra_process_srv6_locator_chunk bgpd/bgp_zebra.c:3378
#7 0xfeb98afa6e14 in zclient_read lib/zclient.c:4608
#8 0xfeb98af3d684 in event_call lib/event.c:2011
#9 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
#10 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
#11 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
#12 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
#13 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)
Direct leak of 32 byte(s) in 2 object(s) allocated from:
#0 0xfeb98b28a4b4 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0xfeb98ae572f8 in qcalloc lib/memory.c:105
#2 0xb7f3c8506520 in vpn_leak_zebra_vrf_sid_update_per_vrf bgpd/bgp_mplsvpn.c:439
#3 0xb7f3c85068d8 in vpn_leak_zebra_vrf_sid_update bgpd/bgp_mplsvpn.c:459
#4 0xb7f3c86f6aec in bgp_ifp_create bgpd/bgp_zebra.c:3345
#5 0xfeb98adfd3f8 in hook_call_if_real lib/if.c:48
#6 0xfeb98adfe750 in if_new_via_zapi lib/if.c:181
#7 0xfeb98af98084 in zclient_interface_add lib/zclient.c:2592
#8 0xfeb98afa6d24 in zclient_read lib/zclient.c:4606
#9 0xfeb98af3d684 in event_call lib/event.c:2011
#10 0xfeb98ae2788c in frr_run lib/libfrr.c:1217
#11 0xb7f3c83cbf0c in main bgpd/bgp_main.c:545
#12 0xfeb98a8973f8 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
#13 0xfeb98a8974c8 in __libc_start_main_impl ../csu/libc-start.c:392
#14 0xb7f3c83c832c in _start (/usr/lib/frr/bgpd+0x2d832c)
SUMMARY: AddressSanitizer: 656 byte(s) leaked in 6 allocation(s).
Chirag Shah [Fri, 3 May 2024 03:58:50 +0000 (20:58 -0700)]
zebra: fix EVPN svd based remote nh neigh del
In the context of SVD (Single VxLAN Device) for L3VNI,
the remote VTEP's nexthop is programmed neighbor entry against
SVD along with neighbor entry against SVI.
However, when L3VNI is removed or the VRF is disabled, all SVI
based remote nexthop neighbors are uninstalled and deleted.
The SVD based neigh entries remains in Zebra and the Kernel.
Subsequently, when reconfiguring L3VNI and relearning the same nexthop,
the neighbor entry is not programmed is because it is not removed
from Zebra SVD neighbor hash table, leading to the failure to
reprogram the entry.
With this fix, the SVD nexthop neigh entry is uninstalled
and deleted from Zebra and Kernel.
borderleaf:# ip neigh show 2.2.2.2
2.2.2.2 dev vlan2560_l3 lladdr 00:01:00:00:1d:09 extern_learn NOARP proto zebra
2.2.2.2 dev vxlan99 lladdr 00:01:00:00:1d:09 extern_learn NOARP proto zebra
With the fix:
Zebra log shows both enties SVD (vxlan99) and SVI (vlan2560_l3)
neighbor entries are deleted.
2024/05/03 18:41:33.527125 ZEBRA: [NH6N7-54CD1] Tx RTM_DELNEIGH family
ipv4 IF vxlan99(16) Neigh 2.2.2.2 MAC null flags 0x10 state 0x0
ext_flags 0x0
2024/05/03 18:41:33.527128 ZEBRA: [NH6N7-54CD1] Tx RTM_DELNEIGH family
ipv4 IF vlan2560_l3(18) Neigh 2.2.2.2 MAC null flags 0x10 state 0x0
ext_flags 0x0
bgpd: Apply NOOP when doing negative commands for GR operations
E.g.:
```
% The Graceful Restart command used is not valid at this moment.
zsh: exit 1 vtysh -c configure -c 'router bgp' -c 'no neighbor 127.0.0.1 graceful-restart
1
```
This does not make sense frr-reload to fail.
Instead, just ignore such requests if they are just NOOP.
When BGP receives an SRV6_LOCATOR_ADD message from zebra, it calls the
`bgp_zebra_process_srv6_locator_add()` function to process the message.
`bgp_zebra_process_srv6_locator_add()` decodes the message first, and
then if the pointer to the default BGP instance is NULL (i.e. the
default BGP instance is not configured yet), it returns early without
doing anything and without using the decoded message information.
This commit fixes the order of the operations executed by
`bgp_zebra_process_srv6_locator_add()`. We first ensure that the default
BGP instance is ready and we return early if it is not. Then, we decode
the message and do something with the information contained in it.
David Lamparter [Thu, 25 Apr 2024 10:18:08 +0000 (12:18 +0200)]
ospf6d: force recalculate on interface_up
interface_up also handles changes to the interface type, i.e. broadcast
to ptp to ptmp. Connected routes for these are different and must be
readvertised, which is done in ospf6_interface_recalculate_cost() - but
only if the cost changed. Use the force variant here.
David Lamparter [Thu, 25 Apr 2024 10:16:35 +0000 (12:16 +0200)]
ospf6d: fix loopback/ptp/ptmp conn. route checks
The code emitting connected routes was checking against the interface
state (which can also be lo/ptp/ptmp) rather than the interface type.
This was causing wrong IA prefixes for connected routes getting put up
out if the interface was down intermittently.
David Lamparter [Thu, 25 Apr 2024 09:45:19 +0000 (11:45 +0200)]
ospf6d: fix DEFUN formatting wrecked by clang
clang-format doesn't understand `DEFUN` and formats it rather ugly.
Standard approach was to skip these in clang-format, which hasn't
happened here sadly.
Chirag Shah [Fri, 26 Apr 2024 01:52:31 +0000 (18:52 -0700)]
tools: Handle seq num for BGP as-path in frr-reload.py
If frr.conf has bgp as-path access-list clause without sequence number
then upon performing frr-rleoad, the running config clause with sequence
number will always be deleted and the new ones without sequence will
be re-added.
This could lead to blackholing until the config gets reapplied.
pimd crashs while unconfigure of rp ka timer as we are trying to access
a yand dnode(suppress timer) which does not exist at the moment.
User just configured rp keepalive timer and not suppress timer,
the yang dnode would not be present. Instead of directly accessing
yang_dnode_get_unit16, first check the yang node exist using
the xpath.
Before:
------
tor-11(config)# no ip pim rp keep-alive-timer 3000
vtysh: error reading from pimd: Success (0)Warning: closing connection to pimd because of an I/O error!
Broadcast message from root@tor-11 (somewhere) (Mon Apr 22 17:29:12 2024):
cumulus-core: Running cl-support for core files "pimd.25467.1713806952.core"
After:
-----
tor-11(config)# no ip pim rp keep-alive-timer 3000
tor-11(config)#
Signed-off-by: Donald Sharp <sharpd@nvidia.com> Signed-off-by: Vijayalaxmi Basavaraj <vbasavaraj@nvidia.com>
(cherry picked from commit 0c4b7c1f1f359c1d7647fea34f62378af8b640a5)
ad151f66aa ("bgpd: Refactor bgp_static_set/bgp_static_set_safi") merged
bgp_static_set_safi into bgp_static_set but inadvertently omitted the
handling of prd_pretty.
Donald Sharp [Fri, 19 Apr 2024 16:13:32 +0000 (12:13 -0400)]
lib, zebra: Check for not being a blackhole route
In zebra_interface_nhg_reinstall zebra is checking that the
nhg is a singleton and not a blackhole nhg. This was originally
done with checking that the nexthop is a NEXTHOP_TYPE_IFINDEX,
NEXTHOP_TYPE_IPV4_IFINDEX and NEXTHOP_TYPE_IPV6_IFINDEX. This
was excluding NEXTHOP_TYPE_IPV4 and NEXTHOP_TYPE_IPV6. These
were both possible to be received and maintained from the upper
level protocol for when a route is being recursively resolved.
If we have gotten to this point in zebra_interface_nhg_reinstall
the nexthop group has already been installed at least once
and we *know* that it is actually a valid nexthop. What the
test is really trying to do is ensure that we are not reinstalling
a blackhole nexthop group( Which is not possible to even be
here by the way, but safety first! ). So let's change
to test for that instead.
vtysh: Fix `show route-map` command when calling via `do`
Before:
```
ton(config)# do sh route-map
% [ZEBRA] Unknown command: show route-map
% [BGP] Unknown command: show route-map
% [ISIS] Unknown command: show route-map
```
Fixes: 570fdc55fd65899cd4376193e833594af6bca093 ("lib,vtysh: fix show route map JSON output") Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
(cherry picked from commit 9b8f18a66cd53c506255c19829e5aada822e4ac8)
Igor Ryzhov [Tue, 16 Apr 2024 19:10:45 +0000 (22:10 +0300)]
lib, zebra: fix exit commands
If a command is not marked as `YANG`-converted, the current command
batching buffer is flushed before executing the command. We shouldn't
flush the buffer when executing an `exit` command. It should only be
flushed if the next command is not `YANG`-converted, which is checked by
the command itself, not the previous `exit`.
Chirag Shah [Tue, 9 Apr 2024 00:14:48 +0000 (17:14 -0700)]
tools: frr-reload strip interface vrf ctx line
if frr.conf file contains 'interface x vrf <name> config
it causes protocol (like ospf) neighbor session flap,
as it deletes interface base config line ('interface x') from
running config and readds with 'interface x vrf <name>'
line from frr.conf.
This deletion and readdition of lines leads to neighborship
flaps.
This issue is by product of (PR-10411 | https://github.com/FRRouting/frr/pull/10411)
(commit id: 788a036fdb)
where running config for interface config no loger displays associated
vrf line.
Donald Sharp [Thu, 11 Apr 2024 14:46:46 +0000 (10:46 -0400)]
bgpd: Fix display when using `missing-as-worst`
The usage of the `bgp bestpath med missing-as-worst` command
was being accepted and applied during bestpath, but during output
of the routes affected by this it would not give any indication
that this was happening or what med value was being used.
Major highlights:
- Introduce local host routes
- Require libyang 2.1.128
- Add suport to configure a log file per daemon
- BGP BMP Loc-RIB (RFC9069) support
- eBGP-OAD (One Administrative Domain) support
- BGP RPKI VRF support
- BGP SNMP traps for BGP4-MIBV2
- Management (mgmtd) daemon "replace" operation support
- BGP dynamic capabilities for addpath, fqdn, orf capabilities
- SRv6 encapsulation source address feature
- OSPFv3 Point-To-Multipoint mode
- bgpd
* Add clear bgp capabilities command to resend some dynamic capabilities link
* bgpd: Add debug bgp updates detail command link
* Add debug bgp updates <in|out> <X.Y.Z.W> prefix-list <NAME> command link
* Add neighbor capability fqdn command link
* Add redistribute table-direct support link
* Fix match ip address ... + match evpn ... commands for EVPN
* Remove aggregated (summary-only) suppressed routes from EVPN
- mgmtd
* Implement full XPATH 1.0 predicate functionality
* Output staticd configuration from mgmtd
- ospfd
* Fix crash in OSPF TE parsing
- ospf6d
* Advertise local addresses with la bit
* Set loopback interface cost to 0
* Let the user override interface cost for a loopback
- pathd
* Add dynamic candidate path metric [computed] keyword link
* Add no msd command in the pcc context
* Add no pcep command
- vtysh
* Send interface commands to mgmtd
- watchfrr
* Extend the ignore option to the daemon being killed
- zebrad
* Add mpls label dynamic-block command link
* Add JSON support to show debugging label-table link
* Add zebra to mgmtd oper-state
* Allow longer prefix matches for the next hops
* Push all configured IP addresses when the interface comes up
* Remove static ARP entries on interface-down events
* Support to listen teamd netlink message as bond type
* Fix crash when macvlan link-interface is in another netns
Breaking Changes:
- Enable enforce-first-as by default for BGP
- noprefixroute flag for interface prefixes with NetworkManager
- Deprecate confd
Donald Sharp [Thu, 28 Mar 2024 16:27:38 +0000 (12:27 -0400)]
bgpd: Arrange peer notification to after zebra announce
Currently BGP attempts to send route change information
to it's peers *before* the route is installed into zebra.
This creates a bug in suppress-fib-pending in the following
scenario:
a) bgp suppress-fib-pending and bgp has a route with
2 way ecmp.
b) bgp receives a route withdraw from peer 1. BGP
will send the route to zebra and mark the route as
FIB_INSTALL_PENDING.
c) bgp receives a route withdraw from peer 2. BGP
will see the route has the FIB_INSTALL_PENDING and
not send the withdrawal of the route to the peer.
bgp will then send the route deletion to zebra and
clean up the bgp_path_info's.
At this point BGP is stuck where it has not sent
a route withdrawal to downstream peers.
Let's modify the code in bgp_process_main_one to
send the route notification to zebra first before
attempting to announce the route. The route withdrawal
will remove the FIB_INSTALL_PENDING flag from the dest
and this will allow group_announce_route to believe
it can send the route withdrawal.
For the master branch this is ok because the recent
backpressure commits are in place and nothing is going
to change from an ordering perspective in that regards.
Ostensibly this fix is also for operators of Sonic and
will be backported to the 8.5 branch as well. This will
change the order of the send to peers to be after the
zebra installation but sonic users are using suppress-fib-pending
anyways so updates won't go out until rib ack has been
received anyways.
Donald Sharp [Thu, 28 Mar 2024 16:25:05 +0000 (12:25 -0400)]
bgpd: Note when receiving but not understanding a route notification
When BGP has been asked to wait for FIB installation, on route
removal a return call is likely to not have the dest since BGP
will have cleaned up the node, entirely. Let's just note that
the prefix cannot be found if debugs are turned on and move on.
Donatas Abraitis [Wed, 27 Mar 2024 17:08:38 +0000 (19:08 +0200)]
bgpd: Prevent from one more CVE triggering this place
If we receive an attribute that is handled by bgp_attr_malformed(), use
treat-as-withdraw behavior for unknown (or missing to add - if new) attributes.
Donatas Abraitis [Wed, 27 Mar 2024 16:42:56 +0000 (18:42 +0200)]
bgpd: Fix error handling when receiving BGP Prefix SID attribute
Without this patch, we always set the BGP Prefix SID attribute flag without
checking if it's malformed or not. RFC8669 says that this attribute MUST be discarded.
Also, this fixes the bgpd crash when a malformed Prefix SID attribute is received,
with malformed transitive flags and/or TLVs.
Piotr Suchy [Thu, 28 Mar 2024 11:55:35 +0000 (12:55 +0100)]
vtysh, zebra: Fix malformed json output for multiple vrfs in command 'show ip route vrf all json'
Command 'show ip route vrf <vrf_name> json' returns a valid json object,
however if instead of <vrf_name> we specify 'all', we get an invalid json
object, like:
Philippe Guibert [Fri, 29 Mar 2024 07:35:34 +0000 (08:35 +0100)]
bgpd: fix srv6 memory leak detection
The asan memory leak has been detected:
> Direct leak of 16 byte(s) in 1 object(s) allocated from:
> #0 0x7f9066dadd28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
> #1 0x7f9066779b5d in qcalloc lib/memory.c:105
> #2 0x556d6ca527c2 in vpn_leak_zebra_vrf_sid_update_per_af bgpd/bgp_mplsvpn.c:389
> #3 0x556d6ca530e1 in vpn_leak_zebra_vrf_sid_update bgpd/bgp_mplsvpn.c:451
> #4 0x556d6ca64b3b in vpn_leak_postchange bgpd/bgp_mplsvpn.h:311
> #5 0x556d6ca64b3b in vpn_leak_postchange_all bgpd/bgp_mplsvpn.c:3751
> #6 0x556d6cb9f116 in bgp_zebra_process_srv6_locator_chunk bgpd/bgp_zebra.c:3337
> #7 0x7f906685a6b6 in zclient_read lib/zclient.c:4490
> #8 0x7f9066826a32 in event_call lib/event.c:2011
> #9 0x7f906675c444 in frr_run lib/libfrr.c:1217
> #10 0x556d6c980d52 in main bgpd/bgp_main.c:545
> #11 0x7f9065784c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
Fix this by freeing the previous memory chunk.
Fixes: b72c9e14756f ("bgpd: cli for SRv6 SID alloc to redirect to vrf (step4)") Fixes: 527588aa78b2 ("bgpd: add support for per-VRF SRv6 SID") Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
(cherry picked from commit eea8a8ae248ed726449878c7a74705d779469fda)
Tomi Salminen [Wed, 13 Dec 2023 09:37:21 +0000 (11:37 +0200)]
zebra: Fix crash on macvlan link down/up
Whenever a link up change was detected on a macvlan device where
the linked device wasn't visible in the namespace zebra was
running in, the linked zebra interface was NULL. This was already
handled in the event of a link down, but was ommitted from the
upside. Added the same null check to the up-side.
Christian Hopps [Wed, 20 Mar 2024 19:20:18 +0000 (19:20 +0000)]
grpc: fix grpc for various failures
lib: don't define a `fallthrough` in c++ to avoid conflict with protobuf c++
check: add link libs required by some versions of grpc++ or it's dependent
linked libs
tests: don't fail the test due to known at exit memleaks Signed-off-by: Christian Hopps <chopps@labn.net>
(cherry picked from commit 043a4183c2f10e6117695dec7a0373c1b0a63808)
Donatas Abraitis [Fri, 15 Mar 2024 11:49:06 +0000 (13:49 +0200)]
bgpd: Update default-originate route-map actual map structure
If using with `bgp listen range ... peer-group x`, default_rmap[afi][safi] is not
updated, and after the hard-reset in other side, this is flushed and never updated
again without restarting the sender BGP daemon.
Split zebra's vrf_terminate() into disable() and delete() stages.
The former enqueues all events for the dplane thread.
Memory freeing is performed in the second stage.
Donald Sharp [Sat, 2 Mar 2024 14:50:38 +0000 (09:50 -0500)]
bgpd: Ensure community data is freed in some cases.
Customer has this valgrind trace:
Direct leak of 2829120 byte(s) in 70728 object(s) allocated from:
0 in community_new ../bgpd/bgp_community.c:39
1 in community_uniq_sort ../bgpd/bgp_community.c:170
2 in route_set_community ../bgpd/bgp_routemap.c:2342
3 in route_map_apply_ext ../lib/routemap.c:2673
4 in subgroup_announce_check ../bgpd/bgp_route.c:2367
5 in subgroup_process_announce_selected ../bgpd/bgp_route.c:2914
6 in group_announce_route_walkcb ../bgpd/bgp_updgrp_adv.c:199
7 in hash_walk ../lib/hash.c:285
8 in update_group_af_walk ../bgpd/bgp_updgrp.c:2061
9 in group_announce_route ../bgpd/bgp_updgrp_adv.c:1059
10 in bgp_process_main_one ../bgpd/bgp_route.c:3221
11 in bgp_process_wq ../bgpd/bgp_route.c:3221
12 in work_queue_run ../lib/workqueue.c:282
The above leak detected by valgrind was from a screenshot so I copied it
by hand. Any mistakes in line numbers are purely from my transcription.
Additionally this is against a slightly modified 8.5.1 version of FRR.
Code inspection of 8.5.1 -vs- latest master shows the same problem
exists. Code should be able to be followed from there to here.
What is happening:
There is a route-map being applied that modifes the outgoing community
to a peer. This is saved in the attr copy created in
subgroup_process_announce_selected. This community pointer is not
interned. So the community->refcount is still 0. Normally when
a prefix is announced, the attr and the prefix are placed on a
adjency out structure where the attribute is interned. This will
cause the community to be saved in the community hash list as well.
In a non-normal operation when the decision to send is aborted after
the route-map application, the attribute is just dropped and the
pointer to the community is just dropped too, leading to situations
where the memory is leaked. The usage of bgp suppress-fib would
would be a case where the community is caused to be leaked.
Additionally the previous commit where an unsuppress-map is used
to modify the outgoing attribute but since unsuppress-map was
not considered part of outgoing policy the attribute would be dropped as
well. This pointer drop also extends to any dynamically allocated
memory saved by the attribute pointer that was not interned yet as well.
So let's modify the return case where the decision is made to
not send the prefix to the peer to always just flush the attribute
to ensure memory is not leaked.