bgpd: Fix update message error handling for multiple same attributes
As per RFC7606 section 3g,
g. If the MP_REACH_NLRI attribute or the MP_UNREACH_NLRI [RFC4760]
attribute appears more than once in the UPDATE message, then a
NOTIFICATION message MUST be sent with the Error Subcode
"Malformed Attribute List". If any other attribute (whether
recognized or unrecognized) appears more than once in an UPDATE
message, then all the occurrences of the attribute other than the
first one SHALL be discarded and the UPDATE message will continue
to be processed.
However, notification is sent out currently for all the cases.
Fix:
For cases other than MP_REACH_NLRI & MP_UNREACH_NLRI, handling has been updated
to discard the occurrences other than the first one and proceed with further parsing.
Again, the handling is relaxed only for the EBGP case.
Also, since in case of error, the attribute is discarded &
stream pointer is being adjusted accordingly based on length,
the total attribute length sanity check case has been moved up in the function
to be checked before this case.
Signed-off-by: Samanvitha B Bhargav <bsamanvitha@vmware.com>
bgpd: Fix update message error handling for total attribute length
As per RFC7606 section 4,
when the total attribute length value is in conflict with the
enclosed attribute length, treat-as-withdraw approach must be followed.
However, notification is being sent out for this case currently,
that leads to session reset.
Fix:
The handling has been updated to conform to treat-as-withdraw
approach only for EBGP case. For IBGP, since we are not following
treat-as-withdraw approach for any of the error handling cases,
the existing behavior is retained for the IBGP.
Signed-off-by: Samanvitha B Bhargav <bsamanvitha@vmware.com>
Donald Sharp [Fri, 11 Aug 2023 16:40:40 +0000 (12:40 -0400)]
doc: Prohibit usage of `system()` calls in FRR
See the documentation update, but system() calls and
it's ilk block the processing of SIGINT and they are
not properly handled as a result leading to shutdown
issues where one or more daemons never stop.
Donald Sharp [Fri, 11 Aug 2023 15:17:29 +0000 (11:17 -0400)]
zebra: Use the re->metric instead of 0 for zebra_rmap_obj
The zebra_rmap_obj was storing the re->metric and allowing
matches against it, but in most cases it was just using 0.
Use the Route entries metric instead. This should fix
some bugs where a match metric never worked.
Donald Sharp [Fri, 11 Aug 2023 15:15:06 +0000 (11:15 -0400)]
zebra: Remove instance from zebra_rmap_obj data structure
In all cases the instance is derived from the re pointer
and since the re pointer is already stored, let's just
remove it from the game and cut to the chase.
Donald Sharp [Fri, 11 Aug 2023 15:06:04 +0000 (11:06 -0400)]
zebra: Remove vrf_id from passed around object
The nexthop that is stored already knows it's nexthop and
in all cases the vrf id is derived from the nexthop->vrf_id
let's just cut to the chase and not do this.
Donald Sharp [Fri, 11 Aug 2023 14:18:41 +0000 (10:18 -0400)]
zebra: import table match against interface name could fail
If an import table route-map is trying to match against
a particular interface, The code is matching against
the actual vrf the route entry is in -vs- the vrf
the nexthop entry is in. Let's modify the code
to actually allow the import table entry to match
against the nexthops vrf.
Not working:
ip import-table 91
ip import-table 93 route-map FOO
no service integrated-vtysh-config
!
debug zebra events
!
interface green
ip address 192.168.4.3/24
exit
!
route-map FOO permit 10
match interface green
exit
eva# show ip route
Codes: K - kernel route, C - connected, S - static, R - RIP,
O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP,
T - Table, v - VNC, V - VNC-Direct, A - Babel, D - SHARP,
F - PBR, f - OpenFabric,
> - selected route, * - FIB route, q - queued, r - rejected, b - backup
t - trapped, o - offload failure
K>* 0.0.0.0/0 [0/100] via 192.168.119.1, enp13s0, 1d10h07m
T[91]>* 1.2.3.5/32 [15/0] via 192.168.119.1, enp13s0, 00:00:05
K>* 169.254.0.0/16 [0/1000] is directly connected, virbr0 linkdown, 1d16h34m
C>* 192.168.44.0/24 is directly connected, virbr1, 01:30:51
C>* 192.168.45.0/24 is directly connected, virbr2, 01:30:51
C>* 192.168.119.0/24 is directly connected, enp13s0, 1d16h34m
C>* 192.168.122.0/24 is directly connected, virbr0 linkdown, 01:30:51
eva# show ip route table 91
Codes: K - kernel route, C - connected, S - static, R - RIP,
O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP,
T - Table, v - VNC, V - VNC-Direct, A - Babel, D - SHARP,
F - PBR, f - OpenFabric,
> - selected route, * - FIB route, q - queued, r - rejected, b - backup
t - trapped, o - offload failure
VRF default table 91:
K>* 1.2.3.5/32 [0/0] via 192.168.119.1, enp13s0, 00:00:15
eva# show ip route table 93
Codes: K - kernel route, C - connected, S - static, R - RIP,
O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP,
T - Table, v - VNC, V - VNC-Direct, A - Babel, D - SHARP,
F - PBR, f - OpenFabric,
> - selected route, * - FIB route, q - queued, r - rejected, b - backup
t - trapped, o - offload failure
VRF default table 93:
K * 1.2.3.4/32 [0/0] via 192.168.4.5, green (vrf green), 00:03:05
Working:
eva# show ip route
Codes: K - kernel route, C - connected, S - static, R - RIP,
O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP,
T - Table, v - VNC, V - VNC-Direct, A - Babel, D - SHARP,
F - PBR, f - OpenFabric,
> - selected route, * - FIB route, q - queued, r - rejected, b - backup
t - trapped, o - offload failure
K>* 0.0.0.0/0 [0/100] via 192.168.119.1, enp13s0, 00:03:09
T[93]>* 1.2.3.4/32 [15/0] via 192.168.4.5, green (vrf green), 00:02:21
T[91]>* 1.2.3.5/32 [15/0] via 192.168.119.1, enp13s0, 00:02:26
K>* 169.254.0.0/16 [0/1000] is directly connected, virbr0, 00:03:09
C>* 192.168.44.0/24 is directly connected, virbr1, 00:03:09
C>* 192.168.45.0/24 is directly connected, virbr2, 00:03:09
C>* 192.168.119.0/24 is directly connected, enp13s0, 00:03:09
C>* 192.168.122.0/24 is directly connected, virbr0, 00:03:09
eva# show ip route table 91
Codes: K - kernel route, C - connected, S - static, R - RIP,
O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP,
T - Table, v - VNC, V - VNC-Direct, A - Babel, D - SHARP,
F - PBR, f - OpenFabric,
> - selected route, * - FIB route, q - queued, r - rejected, b - backup
t - trapped, o - offload failure
VRF default table 91:
K * 1.2.3.5/32 [0/0] via 192.168.119.1, enp13s0, 00:03:12
eva# show ip route table 93
Codes: K - kernel route, C - connected, S - static, R - RIP,
O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP,
T - Table, v - VNC, V - VNC-Direct, A - Babel, D - SHARP,
F - PBR, f - OpenFabric,
> - selected route, * - FIB route, q - queued, r - rejected, b - backup
t - trapped, o - offload failure
VRF default table 93:
K * 1.2.3.4/32 [0/0] via 192.168.4.5, green (vrf green), 00:03:14
Donald Sharp [Fri, 11 Aug 2023 14:12:06 +0000 (10:12 -0400)]
zebra: Rename `struct nh_rmap_obj` to `struct zebra_rmap_obj`
This structure is really the generic route map object for
handling routemaps in zebra. Let's name it appropriately.
Future commits will consolidate the data to using the
struct route_entry as part of this data instead of copying
bits and bobs of it. This will allow future work to
set/control the route_entry more directly.
There is no test that checks for the mpls interface
configuration.
The new test checks that mpls configuration per
interface works when value is enabled or disabled.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
The yang NB API does not handle the mpls configuration
on its leaf.
Add an mpls leaf to stick to the mpls configuration.
- true or false to mean if config
- not defined, means no config.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Thu, 13 Jul 2023 07:42:55 +0000 (09:42 +0200)]
zebra: fix 'no mpls' command by using 'mpls disable' instead
The 'no mpls' command wrongly assumes the user wants to disable
the mpls handling on the interface whereas this is just a config
knob that should mean 'I don't care with mpls'.
Fix this by adding a 'disable' option to the mpls command.
Fixes: 39ffa8e8e856 ("zebra: Add a `mpls enable` interface node command") Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
G. Paul Ziemba [Mon, 31 Jul 2023 04:33:10 +0000 (21:33 -0700)]
pbrd: use flags to indicate active fields
Before now, PBRD used non-zero values to imply that a rule's
match or action field was active. This approach was getting
cumbersome for fields where 0 is a valid active value and
various field-specific magic values had to be used.
This commit changes PBRD to use a flag bit per field to
indicate that the field is active.
G. Paul Ziemba [Mon, 31 Jul 2023 02:14:01 +0000 (19:14 -0700)]
pbrd: add explicit 'family' field for rules
In the netlink-mediated kernel dataplane, each rule is stored
in either an IPv4-specific database or an IPv6-specific database.
PBRD opportunistically gleans each rule's address family value
from its source or destination IP address match value (if either
exists), or from its nexthop or nexthop-group (if it exists).
The 'family' value is particularly needed for netlink during
incremental rule deletion when none of the above fields remain set.
Before now, this address family has been encoded by occult means
in the (possibly otherwise unset) source/destination IP match
fields in ZAPI and zebra.
This commit documents the reasons for maintaining the 'family'
field in the PBRD rule structure, adds a 'family' field in the
common lib/pbr.h rule structure, and carries it explicitly in ZAPI.
Valerian_He [Tue, 8 Aug 2023 10:47:29 +0000 (10:47 +0000)]
bgpd: bgp_path_info_extra memory optimization
Even if some of the attributes in bgp_path_info_extra are
not used, their memory is still allocated every time. It
cause a waste of memory.
This commit code deletes all unnecessary attributes and
changes the optional attributes to pointer storage. Memory
will only be allocated when they are actually used. After
optimization, extra info related memory is reduced by about
half(~400B -> ~200B).
Donald Sharp [Mon, 7 Aug 2023 19:57:29 +0000 (15:57 -0400)]
ospfd: Ensure listnode returns are usable
Coverity is complaining that listnode can return a NULL
value and thus FRR could derefence the returned value.
Since this is not crashing we know that this is not happening
in the wild. Let's make this an assert or check that it is
legal to use the value.
A route-map applied on incoming BGP updates is not able
to replace an unwanted as segments by another one.
unwanted as segment are based on an AS path access-list.
The below configuration illustrates the case:
router bgp 65001
address-family ipv4 unicast
neighbor 192.168.1.2 route-map rule_2 in
exit-address-family
bgp as-path access-list RULE permit ^65
route-map rule_2 permit 10
set as-path replace as-path-access-list RULE 6000
```
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)
```
After:
```
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
6000 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)
```
Donald Sharp [Fri, 4 Aug 2023 17:05:42 +0000 (13:05 -0400)]
tests: babel_topo1 Another no such command
babel_topo1.test_babel_topo1/r3/babeld.log:2023/08/04 12:46:55 BABELD: [SHWNK-NWT5S][EC 100663304] No such command on config line 17: redistirbute ipv6 connected
Donald Sharp [Fri, 4 Aug 2023 17:04:48 +0000 (13:04 -0400)]
tests: config_timing calls non-existent command
./config_timing.test_config_timing/r1/zebra.log:2023/08/04 12:34:29 ZEBRA: [SHWNK-NWT5S][EC 100663304] No such command on config line 7: exit-route-map
./config_timing.test_config_timing/r1/zebra.log:2023/08/04 12:34:29 ZEBRA: [SHWNK-NWT5S][EC 100663304] No such command on config line 10: exit-route-map
Donald Sharp [Fri, 4 Aug 2023 17:03:44 +0000 (13:03 -0400)]
tests: bfd_ospf_topo1 there is no passive interface command
./bfd_ospf_topo1.test_bfd_ospf_topo1/rt3/ospfd.log:2023/08/04 12:46:58 OSPF: [SHWNK-NWT5S][EC 100663304] No such command on config line 28: passive interface lo
./bfd_ospf_topo1.test_bfd_ospf_topo1/rt5/ospfd.log:2023/08/04 12:46:59 OSPF: [SHWNK-NWT5S][EC 100663304] No such command on config line 27: passive interface lo
./bfd_ospf_topo1.test_bfd_ospf_topo1/rt1/ospfd.log:2023/08/04 12:46:56 OSPF: [SHWNK-NWT5S][EC 100663304] No such command on config line 30: passive interface lo
./bfd_ospf_topo1.test_bfd_ospf_topo1/rt4/ospfd.log:2023/08/04 12:47:00 OSPF: [SHWNK-NWT5S][EC 100663304] No such command on config line 27: passive interface lo
./bfd_ospf_topo1.test_bfd_ospf_topo1/rt2/ospfd.log:2023/08/04 12:46:57 OSPF: [SHWNK-NWT5S][EC 100663304] No such command on config line 28: passive interface lo
Donald Sharp [Fri, 4 Aug 2023 16:56:11 +0000 (12:56 -0400)]
tests: bgp_vpnv4_noretain test turns on isis but never uses it
The test was reading in the bgp config for the isis config and
clearly the test is working without this. So let's remove
from the test the usage of isisd
Igor Ryzhov [Fri, 4 Aug 2023 15:24:51 +0000 (18:24 +0300)]
staticd: fix comparison of nexthop-vrf
When displaying the configuration, the order of nexthop-vrf is wrong,
because the default VRF is not displayed, but still compared as the word
"default". Therefore it is placed in the middle of the list instead of
always being the first one.
Before the fix:
```
ip route 1.1.1.0/24 2.2.2.2 nexthop-vrf ccc
ip route 1.1.1.0/24 2.2.2.2
ip route 1.1.1.0/24 2.2.2.2 nexthop-vrf eee
```
After the fix:
```
ip route 1.1.1.0/24 2.2.2.2
ip route 1.1.1.0/24 2.2.2.2 nexthop-vrf ccc
ip route 1.1.1.0/24 2.2.2.2 nexthop-vrf eee
```
Donald Sharp [Thu, 3 Aug 2023 12:14:08 +0000 (08:14 -0400)]
bgpd: Allow bgp to specify if it will allow v6 routing with v4 nexthops
Add a `--v6-with-v4-nexthop` cli to bgp to allow it to peer with
neighbors in the configuration where the interface has no v6 addresses
at all and there is a v4 address that is usable as a v4 address
embedded in a v6 address.
Donald Sharp [Thu, 3 Aug 2023 01:15:02 +0000 (21:15 -0400)]
bgpd: Do not allow a peer to come up on v6 if we have no ability to route
Modify bgp to not allow a v6 peer to come up if the v6 afi is negotiated
and the outgoing interface has no v6 address as well as zebra does
not support the v6 with v4 nexthop capabilities that some dataplanes
allow.
bgpd: Fix session reset issue caused by malformed core attributes
RCA:
On encountering any attribute error for core attributes in update message,
the error handling is set to 'treat as withdraw' and
further parsing of the remaining attributes is skipped.
But the stream pointer is not being correctly adjusted to
point to the next NLRI field skipping the rest of the attributes.
This leads to incorrect parsing of the NLRI field,
which causes BGP session to reset.
Fix:
The stream pointer offset is rightly adjusted to point to the NLRI field correctly
when the malformed attribute is encountered and remaining attribute parsing is skipped.
Signed-off-by: Samanvitha B Bhargav <bsamanvitha@vmware.com>