When looping through the dplane providers, the worklist was
being populated with items from the last provider and then
the event system was checked to see if we should stop processing.
If the event system says `yes` then the dplane code would stop
and send the worklist to the master zebra pthread for collection.
This obviously skipped the next dplane provider on the list
which is double plus not good.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The SRv6 SID Manager does not allow allocating an SRv6 End/uN function
even though it is already supported by staticd.
Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
If we have IPv6-only network and no IPv4 addresses at all, then by default
0.0.0.0 is created which is treated as malformed according to RFC 6286.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
We were hashing 4 bytes of the address. Even for IPv6 addresses.
Oops.
The reason this was done was to try to make it faster, but made a
complex maze out of everything. Time for a refactor.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
When reading in a nexthop from ZAPI, only set the fields that actually
have meaning. While it shouldn't happen to begin with, we can otherwise
carry padding garbage into the unused leftover union bytes.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
rmap_src wasn't initialized, so for IPv4 the unused 12 bytes would
contain whatever junk is on the stack on function entry. Also move
the IPv4 parse before the IPv6 parse so if it's successful we can be
sure the other bytes haven't been touched.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Doesn't seem to break anything but really poor style to pass potentially
uninitialized data to hash_lookup.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
While the loop is currently exited in all cases after using nexthop, it
is a footgun to have "nh" around to be reused in another iteration of
the loop. This would leave nexthop with partial data from the previous
use. Make it local where needed instead.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
In ebgp-multihop, there is a difference in reload behavior when TTL is
unspecified (meaning default 255) and when 255 is explicitly specified.
For example, when reloading with 'neighbor <neighbor> ebgp-multihop
255' in the config, the following difference is created. This commit
fixes that.
Lines To Delete
===============
router bgp 65001
no neighbor 10.0.0.4 ebgp-multihop
exit
Lines To Add
============
router bgp 65001
neighbor 10.0.0.4 ebgp-multihop 255
exit
The commit 767aaa3a80 is not sufficient and frr-reload needs to be
fixed to handle both unspecified and specified cases.
Signed-off-by: Nobuhiro MIKI <nob@bobuhiro11.net>
Currently FRR does not handle v6 recurisive resolution properly
when the route being recursed through changes and the most
significant bits of the route are not changed.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Freeing any item here means freeing someone's `event->hist`, leaving a
dangling pointer there. Which will immediately be written to because
we're executing in a CLI function under the `vty_read` event, whose
`event->hist` is then updated.
Deallocating `event->hist` anywhere other than shutting down the whole
event loop is a bad idea to begin with, just zero out the stats instead.
Fixes: FRRouting/frr#16419
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
When VRF instance is going to be deleted inside bgp_vrf_disable(), it uses
a helper method that skips auto created VRF instances and that leads to STALE
issue.
When creating a VNI for a particular VRF vrfX with e.g. `advertise-all-vni`,
auto VRF instance is created, and then we do `router bgp ASN vrf vrfX`.
But when we do a reload bgp_vrf_disable() is called, and we miss previously
created auto instance.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
The "static_simple" test has code for testing IPv6 routes, but it wasn't
even being run (duh.) Enable it, and also test IPv6 dst-src routes.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
When a daemon wants to know about its routes, make it possible to have
that work for dst-src routes.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
The staticd YANG conversion completely f*cked up dst-src routes.
Stupidly enough, the correct thing is much simpler as seen by the amount
of deletes in this commit.
This does, unfortunately, involve a rather annoying YANG edge case with
what should reasonably be an optional leaf as part of a list key, which
is not possible. It uses `::/0` as unconditional filler instead, since
that is semantically correct.
The `test_yang_mgmt` topotest needed to be adjusted after this to add
`src-prefix='::/0'`.
Fixes: 88fa5104a0 ("staticd : Configuration northbound implementation")
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
The Linux kernel doesn't support dst-src routes with NHGs as nexthop,
for some (rather dubious) caching reasons.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
The json output of advertised-routes is incorrect, as there is a missing
brace with route-distinguisher:
observed with the bgp_vpnv4_noretain test:
> "bgpTableVersion":0,"bgpLocalRouterId":"192.0.2.1","defaultLocPrf":100,"localAS":65500,
> "advertisedRoutes": "192.0.2.1:1":{"rd":"192.0.2.1:1","10.101.0.0/24":{"prefix":"10.101.0.0/24",
expected:
> "bgpTableVersion":0,"bgpLocalRouterId":"192.0.2.1","defaultLocPrf":100,"localAS":65500,
> "advertisedRoutes": { "192.0.2.1:1":{"rd":"192.0.2.1:1","10.101.0.0/24":{"prefix":"10.101.0.0/24",
> ^
> missing brace
Fix this by adding the missing braces.
Fixes: 4838bac033 ("bgpd: neighbors received-routes/advertised-routes stringify changes")
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Add a test that check that the detailed command of show bgp advertised
neighbors 10.125.0.2 displays the locpref value.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Add a test that checks that the BGP route to 192.168.0.1 has all the
necessary json outputs. This route is chosen because it is a suppressed
route.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
When aggregate is used, the suppressed information is not displayed in
the json attributes of a given path. To illustrate, the dump of the
192.168.2.1/32 path in the bgp_aggregate_address_topo1 topotest:
> # show bgp ipv4
> [..]
> s> 192.168.2.1/32 10.0.0.2 0 65001 i
>
> # show bgp ipv4 detail
> [..]
> BGP routing table entry for 192.168.2.1/32, version 17
> Paths: (1 available, best #1, table default, vrf (null), Advertisements suppressed by an aggregate.)
> Not advertised to any peer
> 65001 <---- missing suppressed flag
> 10.0.0.2 from 10.0.0.2 (10.254.254.3)
> Origin IGP, valid, external, best (First path received)
> Last update: Fri Jan 24 13:11:41 2025
>
> # show bgp ipv4 detail json
> [..]
> ,"192.168.2.1/32": [{"aspath":{"string":"65001","segments":[{"type":"as-sequence","list":[65001]}],"length":1},"origin":"IGP","valid":true,"version":17,
> "bestpath":{"overall":true,"selectionReason":"First path received"}, <---- missing suppressed flag
> "lastUpdate":{"epoch":1737720700,"string":"Fri Jan 24 13:11:40 2025\n"},
> "nexthops":[{"ip":"10.0.0.2","afi":"ipv4","metric":0,"accessible":true,"used":true}],
> "peer":{"peerId":"10.0.0.2","routerId":"10.254.254.3","type":"external"}}]
Fix this by adding the json information.
> # show bgp ipv4 detail
> [..]
> BGP routing table entry for 192.168.2.1/32, version 17
> Paths: (1 available, best #1, table default, vrf (null), Advertisements suppressed by an aggregate.)
> Not advertised to any peer
> 65001, (suppressed)
> 10.0.0.2 from 10.0.0.2 (10.254.254.3)
> Origin IGP, valid, external, best (First path received)
> Last update: Fri Jan 24 13:11:41 2025
>
> # show bgp ipv4 detail json
> [..]
> ,"192.168.2.1/32": [{"aspath":{"string":"65001","segments":[{"type":"as-sequence","list":[65001]}],"length":1},"suppressed":true,"origin":"IGP","valid":true,"version":17,
> "bestpath":{"overall":true,"selectionReason":"First path received"},
> "lastUpdate":{"epoch":1737720991,"string":"Fri Jan 24 13:16:31 2025"},
> "nexthops":[{"ip":"10.0.0.2","afi":"ipv4","metric":0,"accessible":true,"used":true}],"peer":{"peerId":"10.0.0.2","routerId":"10.254.254.3","type":"external"}}]
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
This reverts commit 05cf9d03b3.
TL;DR; Handling BGP AddPath capability is not trivial (possible) dynamically.
When the sender is AddPath-capable and sends NLRIs encoded with AddPath ID,
and at the same time the receiver sends AddPath capability "disable-addpath-rx"
(flag update) via dynamic capabilities, both peers are out of sync about the
AddPath state. The receiver thinks already he's not AddPath-capable anymore,
hence it tries to parse NLRIs as non-AddPath, while they are actually encoded
as AddPath.
AddPath capability itself does not provide (in RFC) any mechanism on backward
compatible way to handle NLRIs if they come mixed (AddPath + non-AddPath).
This explains why we have failures in our CI periodically.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
