Donald Sharp [Mon, 10 Apr 2023 18:04:27 +0000 (14:04 -0400)]
bgpd: Do not allow a `no router bgp XXX` when autoimport is happening
When we have these sequence of events causing a crash in
evpn_type5_test_topo1:
(A) no router bgp vrf RED 100
this schedules for deletion the vrf RED instance
(B) a l3vni change event from zebra
this creates a bgp instance for VRF RED in some cases
additionally it auto imports evpn routes into VRF RED
Please note this is desired behavior to allow for the
auto importation of evpn vrf routes
(C) no router bgp 100
The code was allowing the deletion of the default
instance and causing tests to crash.
Effectively the test in bgp_vty to allow/dissallow
the removal of the default instance was not correct
for the case when (B) happens.
Let's just not allow the command to succeed in this case as that
the test was wrong.
Donald Sharp [Mon, 10 Apr 2023 17:59:48 +0000 (13:59 -0400)]
bgpd: Do not allow l3vni changes when shutting down
When a `no router bgp XXX` is issued and the bgp instance
is in the process of shutting down, do not allow a l3vni
change coming up from zebra to do anything. We can just
safely ignore it at this point in time.
Donald Sharp [Wed, 5 Apr 2023 18:57:05 +0000 (14:57 -0400)]
bgpd: Limit flowspec to no attribute means a implicit withdrawal
All other parsing functions done from bgp_nlri_parse() assume
no attributes == an implicit withdrawal. Let's move
bgp_nlri_parse_flowspec() into the same alignment.
Reported-by: Matteo Memelli <mmemelli@amazon.it> Signed-off-by: Donald Sharp <sharpd@nvidia.com>
(cherry picked from commit cfd04dcb3e689754a72507d086ba3b9709fc5ed8)
Trey Aspelund [Fri, 31 Mar 2023 21:46:21 +0000 (17:46 -0400)]
bgpd: fix ecommunity parsing for AS4
The parser for extended communities was incorrectly disallowing an
operator from configuring "Route Origin" extended communities
(e.g. RD/RT/SoO) with a 4-byte value matching BGP_AS4_MAX (UINT32_MAX)
and allowed the user to overflow UINT32_MAX. This updates the parser to
read the value as a uint64_t so that we can do proper checks on the
upper bounds (> BGP_AS4_MAX || errno).
before:
```
TORC11(config-router-af)# neighbor uplink-1 soo 4294967296:65
TORC11(config-router-af)# do sh run | include soo
neighbor uplink-1 soo 0:65
TORC11(config-router-af)# neighbor uplink-1 soo 4294967295:65
% Malformed SoO extended community
TORC11(config-router-af)#
```
after:
```
TORC11(config-router-af)# neighbor uplink-1 soo 4294967296:65
% Malformed SoO extended community
TORC11(config-router-af)# neighbor uplink-1 soo 4294967295:65
TORC11(config-router-af)# do sh run | include soo
neighbor uplink-1 soo 4294967295:65
TORC11(config-router-af)#
```
Donatas Abraitis [Tue, 28 Mar 2023 13:18:47 +0000 (16:18 +0300)]
bgpd: Do not announce routes immediatelly on filter updates
If we set `bgp route-map delay-timer X`, we should ignore starting to announce
routes immediately, and wait for delay timer to expire (or ignore at all if set
to zero).
ospfd: Fix for vitual-link crash in signal handler
Whenever OSPF virtual-link is created, a virtual interface is
associated with it. Name of the virtual interface is derived by
combining "VLINK" string with the value of vlink_count, which is a global
variable.
Problem:
Consider a scenario where 2 virtual links A and B are created in OSPF with
virtual interfaces VLINK0 and VLINK1 respectively. When virtual-link A is unconfigured
and reconfigured, new interface name derived for it will be VLINK1, which is already
associated with virtual-link B. Due to this, both virtual-links A and B will
point to the same interface, VLINK1.
During FRR restart when signal handler is called, OSPF goes through all the virtual
links and deletes the interface(oi) associated with it. During the deletion of interface
for virtual-link B,it accesses the interface which was deleted already(which was deleted
during deletion of virual-link A) and whose fields were set to NULL. This
leads to OSPF crash.
Fixed it by not decrementing vlink_count during unconfig/deletion for virtual-link.
Donald Sharp [Wed, 22 Mar 2023 22:24:56 +0000 (18:24 -0400)]
pimd: Fix use after free issue for ifp's moving vrfs
We have this valgrind trace:
==1125== Invalid read of size 4
==1125== at 0x170A7D: pim_if_delete (pim_iface.c:203)
==1125== by 0x170C01: pim_if_terminate (pim_iface.c:80)
==1125== by 0x174F34: pim_instance_terminate (pim_instance.c:68)
==1125== by 0x17535B: pim_vrf_terminate (pim_instance.c:260)
==1125== by 0x1941CF: pim_terminate (pimd.c:161)
==1125== by 0x1B476D: pim_sigint (pim_signals.c:44)
==1125== by 0x4910C22: frr_sigevent_process (sigevent.c:133)
==1125== by 0x49220A4: thread_fetch (thread.c:1777)
==1125== by 0x48DC8E2: frr_run (libfrr.c:1222)
==1125== by 0x15E12A: main (pim_main.c:176)
==1125== Address 0x6274d28 is 1,192 bytes inside a block of size 1,752 free'd
==1125== at 0x48369AB: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1125== by 0x174FF1: pim_vrf_delete (pim_instance.c:181)
==1125== by 0x4925480: vrf_delete (vrf.c:264)
==1125== by 0x4925480: vrf_delete (vrf.c:238)
==1125== by 0x49332C7: zclient_vrf_delete (zclient.c:2187)
==1125== by 0x4934319: zclient_read (zclient.c:4003)
==1125== by 0x492249C: thread_call (thread.c:2008)
==1125== by 0x48DC8D7: frr_run (libfrr.c:1223)
==1125== by 0x15E12A: main (pim_main.c:176)
==1125== Block was alloc'd at
==1125== at 0x4837B65: calloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1125== by 0x48E80AF: qcalloc (memory.c:116)
==1125== by 0x1750DA: pim_instance_init (pim_instance.c:90)
==1125== by 0x1750DA: pim_vrf_new (pim_instance.c:161)
==1125== by 0x4924FDC: vrf_get (vrf.c:183)
==1125== by 0x493334C: zclient_vrf_add (zclient.c:2157)
==1125== by 0x4934319: zclient_read (zclient.c:4003)
==1125== by 0x492249C: thread_call (thread.c:2008)
==1125== by 0x48DC8D7: frr_run (libfrr.c:1223)
==1125== by 0x15E12A: main (pim_main.c:176)
and you do this series of events:
a) Create a vrf, put an interface in it
b) Turn on pim on that interface and turn on pim in that vrf
c) Delete the vrf
d) Do anything with the interface, in this case shutdown the system
The move of the interface to a new vrf is leaving the pim_ifp->pim pointer pointing
at the old pim instance, which was just deleted, so the instance pointer was freed.
Let's clean up the pim pointer in the interface pointer as well.
rgirada [Mon, 20 Mar 2023 11:22:26 +0000 (11:22 +0000)]
ospfd: Fixing Summary origination after range configuration
Description:
After area range config, summary lsas are aggerated to configured
route but later it was being flushed instead of the actual summary
lsa. This was seen when prefix-id of the aggregated route is same
as one of the actual summary route.
Here, aggregated summary lsa need to be returned to set the flag
SUMMARY_APPROVE after originating aggregated summary lsa but its not.
Which is being cleaned up as part of unapproved summary cleanup.
Corrected this now.
Issue:
When a netns is deleted, since zebra doesn’t receive interface down/delete
notifications from kernel, it manually deletes the interface without removing
the association between zebra_l3vni and the interface that is being deleted
(i.e it deletes the interface without setting “zl3vni->vxlan_if” to NULL).
Later, during the deletion of netns, when zl3vni_rmac_uninstall() is called to
uninstall the remote RMAC from the kernel, zebra ends up accessing stale
“zl3vni->vxlan_if” pointer, which now points to freed memory.
This was causing heap use-after-free.
Fix:
Before zebra starts deleting the interfaces when it receives netns delete notification,
appropriate functions() are being called to remove the association between evpn structs
and interface and set “zl3vni->vxlan_if” to NULL. This ensures that when
zl3vni_rmac_uninstall() is called during netns deletion, it will bail because
“zl3vni->vxlan_if” is NULL.
Donald Sharp [Mon, 20 Mar 2023 20:07:20 +0000 (16:07 -0400)]
lib: on bfd peer shutdown actually stop event
When deleting a bfd peer during shutdown, let's ensure
that any scheduled events are actually stopped.
==7759== Invalid read of size 4
==7759== at 0x48BF700: _bfd_sess_valid (bfd.c:419)
==7759== by 0x48BF700: _bfd_sess_send (bfd.c:470)
==7759== by 0x492F79C: thread_call (thread.c:2008)
==7759== by 0x48E9BD7: frr_run (libfrr.c:1223)
==7759== by 0x1C739B: main (bgp_main.c:550)
==7759== Address 0xfb687a4 is 4 bytes inside a block of size 272 free'd
==7759== at 0x48369AB: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==7759== by 0x48BFA5A: bfd_sess_free (bfd.c:535)
==7759== by 0x2B7034: bgp_peer_remove_bfd (bgp_bfd.c:339)
==7759== by 0x29FF8A: peer_free (bgpd.c:1160)
==7759== by 0x29FF8A: peer_unlock_with_caller (bgpd.c:1192)
==7759== by 0x2A0506: peer_delete (bgpd.c:2633)
==7759== by 0x208190: bgp_stop (bgp_fsm.c:1639)
==7759== by 0x20C082: bgp_event_update (bgp_fsm.c:2751)
==7759== by 0x492F79C: thread_call (thread.c:2008)
==7759== by 0x48E9BD7: frr_run (libfrr.c:1223)
==7759== by 0x1C739B: main (bgp_main.c:550)
==7759== Block was alloc'd at
==7759== at 0x4837B65: calloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==7759== by 0x48F53AF: qcalloc (memory.c:116)
==7759== by 0x48BF98D: bfd_sess_new (bfd.c:397)
==7759== by 0x2B76DC: bgp_peer_configure_bfd (bgp_bfd.c:298)
==7759== by 0x2B76DC: bgp_peer_configure_bfd (bgp_bfd.c:279)
==7759== by 0x29BA06: peer_group2peer_config_copy (bgpd.c:2803)
==7759== by 0x2A3D96: peer_create_bind_dynamic_neighbor (bgpd.c:4107)
==7759== by 0x2A4195: peer_lookup_dynamic_neighbor (bgpd.c:4239)
==7759== by 0x21AB72: bgp_accept (bgp_network.c:422)
==7759== by 0x492F79C: thread_call (thread.c:2008)
==7759== by 0x48E9BD7: frr_run (libfrr.c:1223)
==7759== by 0x1C739B: main (bgp_main.c:550)
tl;dr -> Effectively, in this test setup we have 300 dynamic bgp
sessions all of which are using bfd. When a peer collision is detected
or we remove the peers, if an event has been scheduled but not actually
executed yet the event event was not actually being stopped, leaving
the bsp pointer on the thread->arg and causing a crash when it is
executed.
Donald Sharp [Fri, 17 Mar 2023 19:40:33 +0000 (15:40 -0400)]
bgpd: Prevent Null pointer deref when outputting data
Crash:
(gdb) bt
0 0x00007fee27de15cb in raise () from /lib/x86_64-linux-gnu/libpthread.so.0
1 0x00007fee280ecd9c in core_handler (signo=11, siginfo=0x7ffe56001bb0, context=<optimized out>) at lib/sigevent.c:264
2 <signal handler called>
3 0x0000555e321c41b2 in prefix_rd2str (prd=0x10, buf=buf@entry=0x7ffe56002080 "27.0.0.R\340\373\062\062^U", size=size@entry=28) at bgpd/bgp_rd.c:168
4 0x0000555e321c431a in printfrr_prd (buf=0x7ffe560021a0, ea=<optimized out>, ptr=<optimized out>) at bgpd/bgp_rd.c:224
5 0x00007fee2812069b in vbprintfrr (cb_in=cb_in@entry=0x7ffe56002330, fmt0=fmt0@entry=0x555e3229a3ad " RD: %pRD\n", ap=ap@entry=0x7ffe560023d8) at lib/printf/vfprintf.c:564
6 0x00007fee28122ef7 in vasnprintfrr (mt=mt@entry=0x7fee281cb5e0 <MTYPE_VTY_OUT_BUF>, out=out@entry=0x7ffe560023f0 " RD: : R\n", outsz=outsz@entry=1024, fmt=fmt@entry=0x555e3229a3ad " RD: %pRD\n", ap=ap@entry=0x7ffe560023d8) at lib/printf/glue.c:103
7 0x00007fee28103504 in vty_out (vty=vty@entry=0x555e33f82d10, format=format@entry=0x555e3229a3ad " RD: %pRD\n") at lib/vty.c:190
8 0x0000555e32185156 in bgp_evpn_es_show_entry_detail (vty=0x555e33f82d10, es=0x555e33c38420, json=<optimized out>) at bgpd/bgp_evpn_mh.c:2655
9 0x0000555e32188fe5 in bgp_evpn_es_show (vty=vty@entry=0x555e33f82d10, uj=false, detail=true) at bgpd/bgp_evpn_mh.c:2721
notice prd=0x10 in #3. This is because in bgp_evpn_mh.c we are sending &es->es_base_frag->prd.
There is one spot in the code where during output the es->es_base_frag is checked for non nullness
Let's just make sure it's right in all the places.
pbr-map map6 seq 1
match src-ip 2000::200:100:100:0/96
match dst-ip 2000::100:100:100:0/96
set nexthop-group group3
Before:
torc-12(config)# pbr-map map6 seq 1
torc-12(config-pbr-map)# no match src-ip 2000::200:100:100:0/96
Cannot mismatch families within match src/dst
After:
torc-12(config)# pbr-map map6 seq 1
torc-12(config-pbr-map)# no match src-ip 2000::200:100:100:0/96
torc-12(config-pbr-map)#
Donatas Abraitis [Fri, 17 Mar 2023 12:48:35 +0000 (14:48 +0200)]
lib: Adjust only `any` flag for prefix-list entries if destroying
Before this patch, if we destroy `any` flag for a prefix-list entry, we always
set destination as 0.0.0.0/0 and/or ::/0.
This means that, if we switch from `ip prefix-list r1-2 seq 5 deny any` to
`ip prefix-list r1-2 seq 5 permit 10.10.10.10/32` we will have
`permit any` eventually, which broke ACLs.
Donald Sharp [Thu, 16 Mar 2023 14:24:25 +0000 (10:24 -0400)]
bgpd: Use interface name instead of pointer value
Log message is borked in a manner that makes it unusable:
bgpd[52]: [VX6SM-8YE5W][EC 33554460] 2000:31:0:53::2: nexthop_set failed, resetting connection - intf 0x561eb9005a30
This a convenience release/tag for house keeping. We currently don't plan to publish
binary packages with this release.
Changelog:
babeld
Fix filtering against all interfaces
bgpd
Avoid double aspath_dup() for confederation when remote-as != as_specified
Convert evpn output to not pretty print json
Evpn route detail json display non prett
Fix deterministic-med check for stale paths
Fix use-after-free crash for evpn
Flowspec overflow issue
Free rfapi callback object always
Increment version number even when no data is sent
Intern default-originate attributes to avoid use-after-free
Pass global asn for confederation peers if not as_specified
Prevent multipathing among evpn and non-evpn paths
lib, bgpd
Add ability to specify that some json output should not be pretty
lib
Fix non-use of option
Free dnode before returning if yang translator model is already loaded
Release memory of yang translation module on error
ospfclient
Bugfix and no implicit register
Remove register "ready" requirement
ospfd
Compare prefix values in host order
Inc. opaque data in `show opaque info detail json` output
Small bugfix for miscounting a[s]brs
pbrd
Fix large tableids displayed as negative
pceplib
Add <time.h> include for time_t
pimd
In_multicast needs host order
Make logs useful for input drops
Try to reinstall mfc when we get nocache
V6 intentionally turned off route-maps. turn it on.
tests
Check if bgp confederation works with as_external (!as_specified)
Improve the ospfapi test (move to square topology)
tools
Fix missing pbrd in rsyslog.d 45-frr.conf file
Frr-reload fix list value not present
vrrpd
Give null when using null ifp to lookup vr
zebrad
Add debug for bridge capability
Fix other table inactive when ip import-table is on
Chirag Shah [Wed, 15 Mar 2023 04:32:40 +0000 (21:32 -0700)]
tools: frr-reload fix list value not present
Check for value present in list before removing
as in certain python3 ValueError traceback is observed.
Traceback (most recent call last):
File "/usr/lib/frr/frr-reload.py",
line 2278, in <module>
(lines_to_add, lines_to_del, restart_frr)
= compare_context_objects(newconf, running)
File "/usr/lib/frr/frr-reload.py",
line 1933, in compare_context_objects
lines_to_add, lines_to_del
File "/usr/lib/frr/frr-reload.py",
line 1549, in ignore_delete_re_add_lines
lines_to_del.remove((ctx_keys, line))
ValueError: list.remove(x): x not in list
Testing Done:
With fix perform frr-relaod on frr.conf config where earlier
traceback was seen.
Signed-off-by: Donald Sharp <sharpd@nvidia.com> Signed-off-by: Chirag Shah <chirag@nvidia.com>
(cherry picked from commit 1543f58b5541c0ddb5e53bb7994136dcb5f836cb)
Donald Sharp [Sun, 12 Mar 2023 00:37:21 +0000 (19:37 -0500)]
pimd: IN_MULTICAST needs host order
New correct behavior:
eva# conf
eva(config)# ip pim rp 192.168.1.224 224.0.0.0/24
No Path to RP address specified: 192.168.1.224
eva(config)# ip pim rp 224.1.2.3 224.0.0.0/24
% Bad RP address specified: 224.1.2.3
eva(config)#
Donald Sharp [Sat, 11 Mar 2023 17:05:44 +0000 (12:05 -0500)]
bgpd: Increment version number even when no data is sent
When an update group decides to not send a prefix
announcement because it has not changed, still increment
the version number. Why? To allow for the situation
where you have say 2 peers in 1 peer group and shortly
after they come up a 3rd peer comes up. It will be
placed into a separate update group and could be
coalesced down, when it finishes updating all data
to it. Now imagine that a single prefix changes at
this point in time as well. Then first 2 peers may
decide to not send the data, since nothing has changed.
While the 3rd peer will and since the versions numbers
never match they will never coalesce. So when the decision
is made to skip, update the version number as well.
Donald Sharp [Thu, 23 Feb 2023 18:29:32 +0000 (13:29 -0500)]
bgpd: Flowspec overflow issue
According to the flowspec RFC 8955 a flowspec nlri is <length, <nlri data>>
Specifying 0 as a length makes BGP get all warm on the inside. Which
in this case is not a good thing at all. Prevent warmth, stay cold
on the inside.
Reported-by: Iggy Frankovic <iggyfran@amazon.com> Signed-off-by: Donald Sharp <sharpd@nvidia.com>
(cherry picked from commit 0b999c886e241c52bd1f7ef0066700e4b618ebb3)
David Lamparter [Wed, 1 Jun 2022 07:54:31 +0000 (09:54 +0200)]
pimd: try to reinstall MFC when we get NOCACHE
Whether due to a pimd bug, some expiry, or someone just deleting MFC
entries, when we're in NOCACHE we *know* there's no MFC entry. Add an
install call to make sure pimd's MFC view aligns with the actual kernel
MFC.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Thu, 10 Mar 2022 12:59:26 +0000 (13:59 +0100)]
pimd: make logs useful for input drops
This path here is pretty far on top of the list of issues that operators
will run into and have to debug when setting up PIM. Make the log
messages actually tell what's going on. Also escalate some from
`debug mroute detail` to `debug mroute`.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Donatas Abraitis [Tue, 21 Feb 2023 21:10:45 +0000 (23:10 +0200)]
bgpd: Pass global ASN for confederation peers if not AS_SPECIFIED
When we specify remote-as as external/internal, we need to set local_as to
bgp->as, instead of bgp->confed_id. Before this patch, (bgp->as != *as) is
always valid for such a case because *as is always 0.
Also, append peer->local_as as CONFED_SEQ to avoid other side withdrawing
the routes due to confederation own AS received and/or malformed as-path.
zyxwvu Shi [Wed, 15 Feb 2023 15:55:00 +0000 (23:55 +0800)]
zebra: Fix other table inactive when ip import-table is on
In `rib_link`, if is_zebra_import_table_enabled returns
true, `rib_queue_add` will not called, resulting in other
table route node never processed. This actually should not
be dependent on whether the route is imported.
In `rib_delnode`, if is_zebra_import_table_enabled returns
true, it will use `rib_unlink` instead of enqueuing the
route node for process. There is no reason that imported
route nodes should not be reprocessed. Long ago, the
behaviour was dependent on whether the route_entry comes
from a table other than main.
Donald Sharp [Thu, 2 Feb 2023 21:28:27 +0000 (16:28 -0500)]
lib: Fix non-use of option
Commit d7c6467ba2f55d1055babbb7fe82716ca3efdc7e added the
ability to specify non pretty printing but unfortunately
forgot to use the option variable to make the whole
thing work.
vivek [Fri, 18 Dec 2020 18:55:40 +0000 (10:55 -0800)]
bgpd: Prevent multipathing among EVPN and non-EVPN paths
Ensure that a multipath set is fully comprised of EVPN paths (i.e.,
paths imported into the VRF from EVPN address-family) or non-EVPN
paths. This is actually a condition that existed already in the code
but was not properly enforced.
This change, as a side effect, eliminates the known trigger condition
for bad or missing RMAC programming in an EVPN deployment, described
in tickets CM-29043 and CM-31222. Routes (actually, paths) in a VRF
routing table that require VXLAN tunneling to the next hop currently
need some special handling in zebra to deal with the nexthop (neigh)
and RMAC programming, and this is implemented for the entire route
(prefix), not per-path. This can lead to the bad or missing RMAC
situation, which is now eliminated by ensuring all paths in the route
are 'similar'.
The longer-term solution in CL 5.x will be to deal with the special
programming by means of explicit communication between bgpd and zebra.
This is already implemented for EVPN-MH via CM-31398. These changes
will be extended to non-MH also and the special code in zebra removed
or refined.
Signed-off-by: Vivek Venkatraman <vivek@nvidia.com> Acked-by: Trey Aspelund <taspelund@nvidia.com> Acked-by: Anuradha Karuppiah <anuradhak@nvidia.com> Acked-by: Chirag Shah <chirag@nvidia.com>
Ticket: CM-29043
Testing Done:
1. Manual testing
2. precommit on both MLX and BCM platforms
3. evpn-smoke - BCM and VX
vivek [Thu, 3 Dec 2020 04:04:19 +0000 (20:04 -0800)]
bgpd: Fix deterministic-med check for stale paths
When performing deterministic MED processing, ensure that the peer
status is not checked when we encounter a stale path. Otherwise, this
path will be skipped from the DMED consideration leading to it potentially
not being installed.
Test scenario: Consider a prefix with 2 (multi)paths. The peer that
announces the path with the winning DMED undergoes a graceful-restart.
Before it comes back up, the other path goes away. Prior to the fix, a
third router that receives both these paths would have ended up not
having any path installed to the prefix after the above events.
projects / mirror / frr.git / log