Donald Sharp [Wed, 27 Apr 2022 12:16:50 +0000 (08:16 -0400)]
tests: Correctly align variable type in tests
New compilers are noticing that the tests are compiling with
a pointer for the bgpd_privs variable while the bgp library
that is being linked against is not a pointer. Since
these tests had the declaration just to make the compiler
happy, let's actually align the variable type to make the
compiler even happier.
Donald Sharp [Wed, 27 Apr 2022 11:57:41 +0000 (07:57 -0400)]
bgpd: Ensure pkt_afi and pkt_safi are not used uninited in some cases
The compiler is, rightly, pointing out that in some cases it is
possible that the pkt_afi and pkt_safi values are not properly
set and could result in a use before initialized. I do not
actually belive that this is possible, but let's make the compiler
happy.
Carl Baldwin [Wed, 29 Dec 2021 21:32:33 +0000 (14:32 -0700)]
bgpd: ensure that the node gets unlocked in all cases
The logic to unlock dest if iteration completed without iterating the
entire node was flawed. Specifically, if iteration terminated due to
`gr_deferred == 0` then the node would not get unlocked.
This change takes into account the fact that dest will be NULL only in
the case when the entire table was iterated and all nodes were already
unlocked. In any other case, it needs to be unlocked.
David Lamparter [Tue, 26 Apr 2022 12:52:53 +0000 (14:52 +0200)]
zebra, pimd: add AF param on NEXTHOP_LOOKUP_MRIB
By changing this API call to use a `struct ipaddr`, which encodes the
type of IP address with it. (And rename/remove the `IPV4` from the
command name.)
Also add a comment explaining that this function call is going to be
obsolete in the long run since pimd needs to move to proper MRIB NHT.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Mon, 25 Apr 2022 11:53:17 +0000 (13:53 +0200)]
pimd: fix reversed logic for "debug ... detail"
The normal flag was enabling detail messages, but what we want is the
detail flag enabling normal messages.
Remove the _ONLY macro while at it, it's only used for config print &
that seems like a place where making the difference explicitly visible
is helpful regardless.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Mon, 25 Apr 2022 12:07:41 +0000 (14:07 +0200)]
build: fix new gcc 11.2 warnings
Some recent improvement in GCC triggers 2 new warnings, and they're
actual bugs (reading beyond end of prefix_ipv6 by accessing it as
prefix, which is larger.) Luckily it's only in sharpd.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
```
donatas-laptop(config-router-af)# do show ip bgp 10.10.10.10/32
BGP routing table entry for 10.10.10.10/32, version 13
Paths: (1 available, best #1, table default)
Advertised to non peer-group peers:
192.168.10.65
65000 1 2 3 123
192.168.10.65 from 192.168.10.65 (10.10.10.11)
Origin IGP, metric 0, valid, external, best (First path received)
Last update: Mon Apr 25 10:39:50 2022
```
After:
```
donatas-laptop(config-router-af)# do show ip bgp 10.10.10.10/32
BGP routing table entry for 10.10.10.10/32, version 15
Paths: (1 available, best #1, table default)
Advertised to non peer-group peers:
192.168.10.65
65000 65010 2 3 123
192.168.10.65 from 192.168.10.65 (10.10.10.11)
Origin IGP, metric 0, valid, external, best (First path received)
Last update: Mon Apr 25 10:40:16 2022
```
bgpd: take SRv6 func-bits into account when generating SIDs
Prior to this fix, the magic number (16) was used regardless of what value
the user specified for func-bits. With this fix, user-specified values are
handled appropriately.
Sai Gomathi N [Fri, 8 Apr 2022 06:01:34 +0000 (23:01 -0700)]
pim6d: Modifying the mld group watermark command name
Modifying the "mld_group_watermark_cmd" to "ipv6_mld_group_watermark_cmd"
and "igmp_group_watermark_cmd" to "ip_igmp_group_watermark_cmd" for consistency.
Signed-off-by: Sai Gomathi N <nsaigomathi@vmware.com>
Modified BGP to pay more attention the prefix returned from
zebra to ensure that a LPM wasn't accidently causing BGP
import checks to think it had a match when it did not.
This unfortunately removed the check to handle the route
removal.
This sequence of config and events would leave BGP in a bad state:
ip route 100.100.100.0/24 Null0
router bgp 32932
bgp network import-check
address-family ipv4 uni
network 100.100.100.0/24
Then if you removed the static route the import check would
still think the route existed:
donatas-pc(config)# ip route 100.100.100.0/24 Null0
donatas-pc(config)# do sh ip bgp import-check-table
Current BGP import check cache:
100.100.100.0 valid [IGP metric 0], #paths 1
blackhole
Last update: Sat Apr 23 22:51:34 2022
donatas-pc(config)# do sh ip nht
100.100.100.0
resolved via static
is directly connected, Null0
Client list: bgp(fd 17)
donatas-pc(config)# do sh ip bgp neighbors 192.168.10.123 advertised-routes | include 100.100.100.0
*> 100.100.100.0/24 0.0.0.0 0 32768 i
donatas-pc(config)# no ip route 100.100.100.0/24 Null0
donatas-pc(config)# do sh ip nht
100.100.100.0
resolved via kernel
via 192.168.10.1, enp3s0
Client list: bgp(fd 17)
donatas-pc(config)# do sh ip bgp import-check-table
Current BGP import check cache:
100.100.100.0 valid [IGP metric 0], #paths 1
blackhole
Last update: Sat Apr 23 22:51:34 2022
donatas-pc(config)# do sh ip bgp neighbors 192.168.10.123 advertised-routes | include 100.100.100.0
*> 100.100.100.0/24 0.0.0.0 0 32768 i
donatas-pc(config)#
Fix this by moving the code to handle the prefix check to the
evaluation function and mark the bnc as not matching and actually
evaluate the bnc.
David Lamparter [Sun, 24 Apr 2022 15:17:30 +0000 (17:17 +0200)]
staticd: redo VRF_DEFAULT NHT registers on connect
Other VRFs get VRF_ADD notifications from zebra which triggers
static_fixup_vrf_ids, but since the default VRF is implicit we need to
make that same call on connect.
This should fix problems with staticd being started before (or
concurrent with and thus racing) zebra.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Sun, 24 Apr 2022 15:10:52 +0000 (17:10 +0200)]
staticd: merge NHT register & update, reorganize
nh_update is only called in two places and both precede a matching
follow-up nht_register call. Fold the update into register, and make
register do the right thing™ for all cases (i.e. update refcounts as
needed, and retry zebra NHT registration if it failed before).
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
With the simple BFD configuration -
(active mode, single hop, without other parameters)
```
!
bfd
peer 11.11.11.11
exit
!
```
The interface with 11.11.11.0/24 is a *virtual* interface,
which can be deleted.
After BFD FSM is created and session is ok, do these things:
1) delete this interface
2) create this interface
3) set same ip address in this interface
Now, everything seems completely restored because all configuration
is same. But bad thing happens, BFD session hang on "down" status -
```
root# show bfd peer 11.11.11.11
BFD Peer:
peer 11.11.11.11 vrf default
ID: 638815827
Remote ID: 0
Active mode
Status: down
Downtime: 3 second(s)
Diagnostics: path down <- caused by destroyed interface
Remote diagnostics: ok
```
With the interface creating, `bfdd_sessions_enable_interface()`
wrongly compares added interface with the created, even key of
this `bfd_session` isn't binded with any interface. So this
`bfd_session` will hang on "down" status for ever.
So skip the compare in this case (no interface in key) to wake up
this `bfd_session`.
Chirag Shah [Fri, 8 Apr 2022 19:59:53 +0000 (12:59 -0700)]
tools: frr-reload fix bgp nbr delete
When a bgp neighbor removed from associated to peer-group,
the neighbor is fully deleted, subsequent deletion of any
configuration related to the neighbor leads to failure
in frr-reload.
Fix: In frr-reload lines to delete check if any neighbor with
peer-group removal line is present, if so then remove any
further config deletion associated the neighbor needs to removed
from the lines to delete.
Chirag Shah [Sat, 4 Dec 2021 21:27:29 +0000 (13:27 -0800)]
tools: fix bgp instances deletion in frr-reload
BGPd does not allow default instance deletion
in presence of bgp vrf instance;
frr-reload script fails if delete list contains
default instance followed by vrf instance.
Fix:
frr-reload scans lines_to_delete to look for
'router bgp' and 'router bgp vrf ...' line.
If both are present switch the order to delete
bgp vrf instance(s) than default instance at the end.
Testing Done:
Before:
INFO: Loading Config object from file /etc/frr/frr.conf
INFO: Loading Config object from vtysh show running
INFO: Failed to execute no router bgp 40201 <-- Failed to delete
INFO: Failed to execute no router bgp
INFO: Failed to execute no router
ERROR: "no router" we failed to remove this command
ERROR: % Cannot delete default BGP instance. Dependent VRF instances exist
INFO: Executed "no router bgp 40201 vrf bgp-test" <-- vrf instance deleted
INFO: Loading Config object from vtysh show running
After:
order of deletion switched
INFO: Loading Config object from file /etc/frr/frr.conf
INFO: Loading Config object from vtysh show running
INFO: Executed "no router bgp 40201 vrf bgp-test"
INFO: Executed "no router bgp 40201"
INFO: Loading Config object from vtysh show running
vrrpd: Fix display of 'Master Advertisement interval'
VRRP as per RFC 5798 'Master Advertisement interval' field refers to the
advertisement interval, we received the last time we got an Advertisement
from a peer who wasn't us, who was in the master state.
This could be clarified by making the field name 'Master Advertisement
interval (rx)',and when we're in the Master state, we put (stale) after the interval.
That commit aim is to fix an invalid isis access to sptree when
lpd is stopping. isisd is running.
lpd and isisd are running. isis is L1 type configured.
isis_ldp_rlfa_handle_client_close function try to clear
uninitialized spftree.
Expected behavior: isisd not crashing and running.
isis_ldp_rlfa_handle_client_close not trying to clear spftree
that are not initializes due tio the configuration.
Fix: test the configured area's type avoiding to deleted
an unconfigured sptree. function isis_rlfa_handle_client_close
will be aligned on spftree_area_del function
Donald Sharp [Thu, 21 Apr 2022 18:13:01 +0000 (14:13 -0400)]
lib: Ensure an empty string does not get printed for host/domain
End operator is showing:
!
frr version 8.0.1
frr defaults traditional
hostname test.example.com
domainname
domainname should not be printed in this case at all. I do not
see any mechanism in current code that this could happen, but
what do I know? Put some extra stupid insurance in place
to prevent bad config from being generated.
pimd: Set new to true in igmp_get_source_by_addr api
5421bf8 commit forgot to set the parameter "new" to true
when a new source is created, have fixed it.
igmp_get_source_by_addr api is currently setting the parameter
"new" to false always. This is not right. The caller apis are using
this field to decide and based on that take actions to create timers, etc.
Its need to be set to true when a new source is created.
isisd: enable BFD messaging debug when isis bfd is used
In addition to turning on isis bfd debugging traces, the internal
bfd messaging debug is also enabled. Reversely, when isis bfd traces
are off, the internal messaging debug traces are off too.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
zebra: set ZEBRA_IFC_DOWN on connected routes for inactive interfaces
If you are in a situation where you have multiple addresses on an
interface, zebra creates one connected route for them.
The issue is that the rib entry is not created if addresses were
added before the interface was running.
We add the address to a running interface in a typical flow.
Therefore, we handle the route & rib creation within a single ADD event.
In the opposite case, we create the route entries without activating them.
These are considered to be active since ZEBRA_IFC_DOWN is not set.
On the following interface UP, we ignore the same ADDR_ADD as it overlaps
with the existing prefixes -> rib is never created.
The minimal reproducible setup:
-----------------------------------------
ip link add name dummy0 type dummy
ip addr flush dev dummy0
ip link set dummy0 down
ip addr add 192.168.1.7/24 dev dummy0
ip addr add 192.168.1.8/24 dev dummy0
ip link set dummy0 up
vtysh -c 'show ip route' | grep dummy0
Donald Sharp [Tue, 19 Apr 2022 18:48:29 +0000 (14:48 -0400)]
lib: Prevent crash after shutdown request
Recent commit e92508a741e03b8721ccb3424cbebe4d5476e9d changed
the prefix_master->str to a RB tree. This introduced a condition
whnere on shutdown the prefix list was removed from the master list
and then operated on by passing around a name. Which was then used
to lookup the prefix list again when we operated on the code.
This change to a RB Tree first deleted the item from the RB tree
first thus introducing this crash
Crash:
(gdb) bt
index=0x556c07d59650, pentry=0x556c07d29380) at lib/routemap.c:2397
arg=0x7ffdbf84bc60) at lib/hash.c:267
event=RMAP_EVENT_PLIST_DELETED) at lib/routemap.c:2489
Grab the first item on the list, clean it and then remove it.
packaging: Set default permissions for /var/log/frr to 0755
At the moment we set /var/log/frr permissions to 0750 (frr:frr), but the log
file is 0640 (root:adm) (unless logrotated) and that doesn't allow adm group
to even open the directory.
projects / mirror / frr.git / log