bfdd
Fix malformed session with vrf
Remove redundant nb destroy callbacks
bgpd
Ensure stream received has enough data
Fix bgpd core when unintern attr
Fix the json output of show bgp all json to be in a valid format
Make sure aigp attribute is non-transitive
Using no pretty json output for l2vpn-evpn routes
doc
Add `neighbor aigp` command for bgp
lib
Fix memory leak in in link state
Fix vtysh core when handling questionmark
Link state memory corruption
ospfd
Fix interface param type update
Fix memory leaks w/ `show ip ospf int x json` commands
Ospf opaque lsa stale processing fix and topotests.
Respect loopback's cost that is set and set loopback costs to 0
pim6d
Fix crash in ipv6 pim command
pimd
Pim not sending register packets after changing from non dr to dr
tests
Adjust aigp metric numbers for ibgp setup
tools
Fix list value remove in frr-reload
vtysh
Give actual pam error messages
zebra
Evpn handle del event for dup detected mac
Fix dp_out_queued counter to actually reflect real life
Fix evpn dup detected local mac del event
Reduce creation and fix memory leak of frrscripting pointers
Unlock the route node when sending route notifications
Chirag Shah [Fri, 26 May 2023 20:43:50 +0000 (13:43 -0700)]
ospfd: fix interface param type update
interface link update event needs
to be handle properly in ospf interface
cache.
Example:
When vrf (interface) is created its default type
would be set to BROADCAST because ifp->status
is not set to VRF.
Subsequent link event sets ifp->status to vrf,
ospf interface update need to compare current type
to new default type which would be VRF (OSPF_IFTYPE_LOOPBACK).
Since ospf type param was created in first add event,
ifp vrf link event didn't update ospf type param which
leads to treat vrf as non loopback interface.
Donald Sharp [Tue, 6 Dec 2022 15:23:11 +0000 (10:23 -0500)]
bgpd: Ensure stream received has enough data
BGP_PREFIX_SID_SRV6_L3_SERVICE attributes must not
fully trust the length value specified in the nlri.
Always ensure that the amount of data we need to read
can be fullfilled.
Reported-by: Iggy Frankovic <iggyfran@amazon.com> Signed-off-by: Donald Sharp <sharpd@nvidia.com>
(cherry picked from commit 06431bfa7570f169637ebb5898f0b0cc3b010802)
Chirag Shah [Tue, 6 Jun 2023 04:48:12 +0000 (21:48 -0700)]
tools: fix list value remove in frr-reload
There might be a time element(s) from
temporary list are removed more than once
which leads to valueError in certain python3
version.
commit-id 1543f58b5 did not handle valueError
properly. This caused regression where
prefix-list config leads to delete followed
by add.
The new fix should just pass the exception as
value removal from list_to_add or list_to_del
is best effort.
This allows prefix-list config has no change
then removes the lines from lines_to_del and
lines_to_add properly.
Configure prefix-list in frr.conf and perform
multiple frr-reload. After first reload operatoin
subsequent ones should not result in delete followed
by add of the prefix-list but rather no-op operation.
Donald Sharp [Wed, 31 May 2023 15:40:07 +0000 (11:40 -0400)]
zebra: Unlock the route node when sending route notifications
When using a context to send route notifications to upper
level protocols, the code was using a locking function to
get the route node. There is no need for this to be locked
as such FRR should free it up.
Yuan Yuan [Tue, 30 May 2023 19:20:09 +0000 (19:20 +0000)]
lib: fix vtysh core when handling questionmark
When issue vtysh command with ?, the initial buf size for the
element is 16. Then it would loop through each element in the cmd
output vector. If the required size for printing out the next
element is larger than the current buf size, realloc the buf memory
by doubling the current buf size regardless of the actual size
that's needed. This would cause vtysh core when the doubled size
is not enough for the next element.
Sarita Patra [Fri, 5 May 2023 17:52:33 +0000 (10:52 -0700)]
pim6d: Fix crash in ipv6 pim command
Problem:
Execute the below commands, pim6d core happens.
interface ens193
ip address 69.0.0.2/24
ipv6 address 8000::1/120
ipv6 mld
ipv6 pim
We see crash only if the interface is not configured, and
we are executing PIM/MLD commands.
RootCause:
Interface ens193 is not configured. So, it will have
ifindex = 0 and mroute_vif_index = -1.
Currently, we don't enable MLD on an interface if
mroute_vif_index < 0. So, pim_ifp->MLD = NULL.
In the API pim_if_membership_refresh(), we are accessing
pim_ifp->MLD NULL pointer which leads to crash.
Fix:
Added NULL check before accessing pim_ifp->MLD pointer in
the API pim_if_membership_refresh().
Yuan Yuan [Tue, 30 May 2023 18:53:32 +0000 (18:53 +0000)]
bgpd: fix bgpd core when unintern attr
When the remote peer is neither EBGP nor confed, aspath is the
shadow copy of attr->aspath in bgp_packet_attribute(). Striping
AS4_PATH should not be done on the aspath directly, since
that would lead to bgpd core dump when unintern the attr.
Donald Sharp [Fri, 26 May 2023 11:44:11 +0000 (07:44 -0400)]
vtysh: Give actual pam error messages
Code was was written where the pam error message put out
was the result from a previous call to the pam modules
instead of the current call to the pam module.
Rajasekar Raja [Mon, 22 May 2023 21:14:30 +0000 (14:14 -0700)]
bgpd: Using no pretty json output for l2vpn-Evpn routes
The output of show bgp all json is inconsistent across Address-families
i.e. ipv4/ipv6 is a no pretty format while l2vpn-evpn is in a pretty
format. For huge scale (lots of routes with lots of paths), it is better
to use no_pretty format.
The vrf check should use the carefully adjusted `vrfid`, which is
based on globally/reliable interface. We can't believe the
`bvrf->vrf->vrf_id` because the `/proc/sys/net/ipv4/udp_l3mdev_accept`
maybe is set "1" in VRF-lite backend even with security drawback.
Acee [Tue, 9 May 2023 20:51:03 +0000 (16:51 -0400)]
ospfd: OSPF opaque LSA stale processing fix and topotests.
1. Fix OSPF opaque LSA processing to preserve the stale opaque
LSAs in the Link State Database for 60 seconds consistent with
what is done for other LSA types.
2. Add a topotest that tests for cases where ospfd is restarted
and a stale OSPF opaque LSA exists in the OSPF routing domain
both when the LSA is purged and when the LSA is reoriginagted
with a more recent instance.
Donald Sharp [Tue, 9 May 2023 17:10:35 +0000 (13:10 -0400)]
ospfd: Respect loopback's cost that is set and set loopback costs to 0
When setting an loopback's cost, set the value to 0, unless the operator
has assigned a value for the loopback's cost.
RFC states:
If the state of the interface is Loopback, add a Type 3
link (stub network) as long as this is not an interface
to an unnumbered point-to-point network. The Link ID
should be set to the IP interface address, the Link Data
set to the mask 0xffffffff (indicating a host route),
and the cost set to 0.
FRR is going to allow this to be overridden if the operator specifically
sets a value too.
In function ls_find_subnet(), prefix argument is directly copied into
subnet.key structure to find corresponding subnet in RB Tree. This could leadr
to a memory corruption. Function prefix_copy() must be used instead.
This patch replaces the direct prefix copy by a call to prefix_copy() function
to avoid this memory issue.
When using ls_stream2ted() function to parse Opaque Link State message to local
TED, in case of vertex or subnet deletion, the function return a pointer to the
deleted ls_element instead of NULL. This could lead into a potential pointer
corruption when caller try to access to the deleted ls_element.
This patch ensure that the ls_element pointer return by ls_stream2ted()
function is NULL when the message event is a delete operation for vertex and
subnet. Note that edge deletion was correctly handled.
Sai Gomathi N [Fri, 17 Mar 2023 10:51:16 +0000 (03:51 -0700)]
pimd: PIM not sending register packets after changing from non DR to DR
When the router is non dr for an interface, it installs mroute to drop
the packets from directly connected source. This was done to avoid packets
coming to cpu as nocache hit. Later when it gets change from non-DR to DR,
these entries are not cleared. So the packets are still dropped.
This causes register packets not getting generated.
So cleaning up the mroute entries and channel oil without
upstream reference which was created to drop.
Co-authored-by: Saravanan K <saravanank@vmware.com> Signed-off-by: Sai Gomathi N <nsaigomathi@vmware.com>
(cherry picked from commit 1c883aef96013753f5467ba5e5028dee0f0a82c5)
Chirag Shah [Sat, 22 Oct 2022 23:00:14 +0000 (16:00 -0700)]
zebra:fix evpn dup detected local mac del event
The current local mac delete event send to flag with force
always which breaks the duplicate detected MACs where
it requires to be resynced from bgpd to earlier state.
Chirag Shah [Wed, 1 Dec 2021 04:42:01 +0000 (20:42 -0800)]
zebra: evpn handle del event for dup detected mac
Upon receiving local mobility event for MAC + NEIGH,
both are detected as duplicate upon hitting DAD threshold.
Duplicated detected ( freezed) MAC + NEIGH are not known
to bgpd.
If locally learnt MAC + NEIGH are deleted in kernel,
the MAC is marked as AUTO after sending delete event
to bgpd.
Bgpd only reinstalls best route for MAC_IP route (NEIGH)
but not for MAC event.
This puts a situation where MAC is AUTO state and
associated neigh as remote.
Fix:
DUPLICATE + LOCAL MAC deletion, set MAC delete request
as reinstall from bgpd.
bgpd
- Fix crash due to community aliases size
- Aggregate-address memory leak fix
- Bmp fix peer-up ports byte order
- Check 7 bytes for long-lived graceful-restart capability
- Copy the password from the previous peer on peer_xfer_config()
- Do not allow a `no router bgp xxx` when autoimport is happening
- Do not allow l3vni changes when shutting down
- Do not announce routes immediatelly on filter updates
- Do not call bgp_soft_reconfig_in() twice in a row on policy change
- Evpn-mh esi not active suppress ead-es route
- Fix crash for `show bgp ... neighbor received-routes detail|prefix`
- Fix debug output for route-map names when using a unsuppress-map
- Fix ecommunity parsing for as4
- Fix for ain->attr corruption during path update
- Increase buffer size used for dumping bgp to mrt files
- Limit flowspec to no attribute means a implicit withdrawal
- Prevent null pointer deref when outputting data
lib
- Adjust only `any` flag for prefix-list entries if destroying
- Destroy `any` flag when creating a prefix-list entry with prefix
- Fix clear route-map cmd using defpy
- Fix link state memory leak
- Include clippy generated commands for routemap.c
- On bfd peer shutdown actually stop event
ospfd
- Cleanup some memory leaks on shutdown in ospf_apiserver.c
- Fix for vitual-link crash in signal handler
- Fix ospf_lsa memory leak
- Fix ospf_ti_lfa drop of an entire table
- Fixing summary origination after range configuration
- Free up q_space in early return path
- Log adjacency changes with neighbor ip in addition to neighbor id
pbrd
- Fix mismatching in match src-dst
pim6d
- Fixing mroutes not created after disabling and enabling pimv6.
pimd
- Fix use after free issue for ifp's moving vrfs
- In_multicast needs host order
- Process no-forward bsm packet
tools
- Fix missing remote-as configuration when reload
- Frr-reload fix list value not present
- Make check flag really work for reload
- Set correct directory of vtysh for frr-reload.py
zebrad
- Add link_nsid to zebra interface
- Cleanup ctx leak on shutdown and turn off event
- Evpn mh sync mac install as inactive
- Fix for heap-use-after-free in evpn
- Fix race during shutdown
- Install directly connected route after interface flap
Donald Sharp [Thu, 20 Apr 2023 20:27:20 +0000 (16:27 -0400)]
bgpd: Fix lcom->str string length to correctly cover aliases
If you have a very large number of large communities whose
string length happened to be greater than BUFSIZ FRR's bgpd
would crash. This is because bgpd would write beyond
the end of the string.
Originally the code auto-calculated the string size appropriately
but commit ed0e57e3f079352714c3a3a8a5b0dddf4aadfe1d modified
the string length to be a hard coded BUFSIZ. When a route-map
like this is added:
Modify the code to correctly determine the string length of the communities
and to also double check if the string has an alias and ensure that the
string is still sufficiently large enough. If not auto size it again.
bgpd: Fix for ain->attr corruption during path update
1. Consider a established L2VPN EVPN BGP peer with soft-reconfiguartion
inbound configured
2. When the interface of this directly connected BGP peer is shutdown,
bgp_soft_reconfig_table_update() is called, which memsets the evpn buffer
and calls bgp_update() with received attributes stored in ain table(ain->attr).
In bgp_update(), evpn_overlay attribute in ain->attr (which is an interned
attr) was modified by doing a memcpy
3. Above action causes 2 attributes in the attrhash (which were previously different)
to match!
4. Later during fsm change event of the peer, bgp_adj_in_remove() is called
to clean up the ain->attr. But, because 2 attrs in attrhash match, it causes
BGP to assert in bgp_attr_unintern()
tor-11# bridge -d fdb show | grep 00:65:00:00:00:01
00:65:00:00:00:01 dev hostbond1 vlan 1000 notify master bridge static
tor-12 receives sync MAC route:
Before fix:
----------
tor-12:/# bridge -d fdb show | grep 00:65:00:00:00:01
00:65:00:00:00:01 dev hostbond1 vlan 1000 notify master bridge static
After fix: inactive is set to MAC entry
----------
tor-12:/#bridge -d fdb show | grep 00:65:00:00:00:01
00:65:00:00:00:01 dev hostbond1 vlan 1000 notify inactive master bridge
static
Notice the difference in `inactive` post notify on tor-12
with the fix.
Signed-off-by: Trey Aspelund <taspelund@nvidia.com> Signed-off-by: Chirag Shah <chirag@nvidia.com>
(cherry picked from commit 4a1f91a366bacc8178afcc3d2712e08a3ba3e1ba)
tools: fix missing remote-as configuration when reload
From commit `411d1a2`, `bgp_delete_nbr_remote_as_line()` is added to
remove some specific bgp neighbors. But, when reloading the following
configuration, it will wrongly remove some good ones:
`neighbor 66.66.66.6 remote-as internal`:
lynne [Wed, 23 Jun 2021 17:15:17 +0000 (13:15 -0400)]
ospf6d: missing ECMP NHs in certain topologies
When ospf6 is started up and SPF is run depending on which route is
selected as the parent route we could miss adding a NH. If one
possible parent route has two equal cost paths and the second possible
parent route has only one depending on which one is selected first
determines if we have have one or two NHs.
In the network below when creating a route 2001:db8:3:4::/64 in R2.
When SPF is run there are two possible parent routes R3 and R4.
The problem was if we first created the route to 2001:db8:3:4::/64 with R3
as the parent route all is fine. The code was merging the NH from the parent
route and R3 has 2 NH, one pointing to R1 and one to R5. But if route
2001:db8:3:4::/64 was first created with parent as R4, it has only one NH
pointing to R1, and then later a new vertex was created pointing to R3 the
code would only copy the nhs from the vertex not from the parent route. The
vertex always has just one NH. But the parent route could have more. So
when we would bringup this setup one time we would see one NH for
2001:db8:3:4::/64 and the next time we would see two NHs. The code has been
modified so that it behaves the same when the route is first created, or when
a vertex is created, it selects the NHs from the parent route.
Abhishek N R [Fri, 24 Mar 2023 04:55:06 +0000 (21:55 -0700)]
pim6d: Fixing mroutes not created after disabling and enabling PIMv6.
After doing "no ipv6 pim" and "ipv6 pim" on receiver interface in LHR,
mroutes were not created.
When we enable pim after disabling it, we refresh the membership on pim interface.
First we clear the membership on the interface, then we add it back.
For PIMv6 we were only clearing it, membership was not added back. So mroutes were not created.
Now added code to fetch all the local membership information into PIM.
Initially PIM nbr is down between FRR4----FRR2 from FRR2 side
Cisco is sending BSR packet to FRR4.
Problem Statement:
=================
No shutdown the PIM neighbor on FRR2 towards FRR4.
FRR2, receives BSR packet immediately as the new neighbor
comes up. This BSR packet is having no-forward bit set.
FRR2 is not able to process the BSR packet, and drop the
BSR packet.
Root Cause:
==========
When PIMD comes up, we start BSM timer for 60 seconds.
Here, the value accept_nofwd_bsm is setting to false.
FRR2, when receives no-forward BSR packet, it is getting
accept_nofwd_bsm value as false.
So, it drops, the no-forward BSM packet.
Fix:
===
Set accept_nofwd_bsm as false after first BSM packet received.
zebra: Install directly connected route after interface flap
Issue:
After vlan flap, zebra was not marking the selected/best route as installed.
As a result, when a static route was configured with nexthop as directly
connected interface's(vlan) IP, the static route was not being installed
in the kernel since its nexthop was unresolved. The nexthop was marked
unresolved because zebra failed to mark the best route as installed after
interface flap.
This was happening because, in dplane_route_update_internal() if the old and
new context type, and nexthop group id are the same, then zebra doesn't send
down a route replace request to kernel. But, the installed (ROUTE_ENTRY_INSTALLED)
flag is set when zebra receives a response from kernel. Since the
request to kernel was being skipped for the route entry, installed flag
was not being set
Fix:
In dplane_route_update_internal() if the old and new context type, and
nexthop group id are the same, then before returning, installed flag will
be set on the route-entry if it's not set already.
Xiao Liang [Tue, 21 Feb 2023 06:00:36 +0000 (14:00 +0800)]
zebra: Add link_nsid to zebra interface
Create VRF and interfaces:
ip netns add vrf1
ip link add veth1 index 100 type veth
ip link add link veth1 veth1.200 type vlan id 200
ip link set veth1.200 netns vrf1
ip -n vrf1 link add veth2 index 100 type veth
After reloading zebra, "show interface veth1.200" shows wrong parent
interface:
test# show interface veth1.200
Interface veth1.200 is down
...
Parent interface: veth2
This is because veth1.200 and veth1 are in different netns, and veth2
happens to have the same ifindex as veth1, in the same netns of
veth1.200.
When looking for parent, link-ifindex 100 should be looked up within
link-netns, rather than that of the child interface.
Add link_nsid to zebra interface, so that the <link_nsid, link_ifindex>
pair can uniquely identify the link interface.
Mark Stapp [Tue, 11 Apr 2023 19:58:13 +0000 (15:58 -0400)]
zebra: fix race during shutdown
During shutdown, the main pthread stops the dplane pthread
before exiting. Don't try to clean up any events scheduled
to the dplane pthread at that point - just let the thread
exit and clean up. This is the 8.5 version.
Chirag Shah [Sat, 8 Apr 2023 03:14:25 +0000 (20:14 -0700)]
bgpd:evpn-mh esi not active suppress ead-es route
update_type1_routes_for_evi() is called from
L3VNI/L2VNI up event, if ESI is not UP then
do not advertise EAD-ES Type-1 route.
Just like from multiple places EAD-ES route
origination checks for its oper status.
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)#
```
Currently the process of the `route-map` configuration for `per-vrf-rip`
is wrong.
There are two problems:
1. `ctx->name` for `if_rmap_ctx` is not initialized in `if_rmap_ctx_create()`.
2. The global `if_rmap_ctx_list` is wrongly used for `per-vrf-rip`.
So, two changes for it:
1. Correctly initializes `ctx->name`.
2. Use specific `if_rmap_ctx` for `per-vrf-rip`, not global one.
Note, this related implementation for `route-map` is only for `ripd`.
Before:
```
anlan(config)# route rip vrf vrf1
anlan(config-router)# route-map aa in lan
anlan(config-router)# do show run
!
router rip
route-map aa in lan
exit
!
```
After:
```
anlan(config)# route rip vrf vrf1
anlan(config-router)# route-map aa in lan
anlan(config-router)# do show run
!
router rip vrf vrf1
route-map aa in lan
exit
!
```
rbarroetavena [Thu, 23 Mar 2023 19:19:48 +0000 (16:19 -0300)]
bgpd: Trim long neighbor description with no whitespace
Fix for missing neighbor description in "show bgp summary [wide]"
when its length exceeds 20[64] chars and it doesn't contain
withespaces.
Existing behavior remains if description contains whitespaces
before size limit.
Donatas Abraitis [Thu, 30 Mar 2023 13:50:15 +0000 (16:50 +0300)]
tests: Check received routes count for labeled-unicast with addpath
Test failed time to time, let's try this way:
```
$ for x in $(seq 1 20); do cp test_bgp_labeled_unicast_addpath.py test_$x.py; done
$ sudo pytest -s -n 20
```
Ran 10 times using this pattern, no failure :shrug:
Before this change, we checked advertised routes, and at some point `=` was
missing from the output, but advertised correctly. Receiving router gets as
much routes as expected to receive.
I reversed checking received routes, not advertised.