Enke Chen [Sun, 20 Oct 2024 19:25:46 +0000 (12:25 -0700)]
bgpd: allow value 0 in aigp-metric setting
The value of 0 is accepted from peers, and can also be set by the
route-map "set aigp-metric igp-metric". For coonsistency, it should
be allowed in "set aigp-metric <value>" as well.
Donald Sharp [Tue, 8 Oct 2024 01:46:33 +0000 (21:46 -0400)]
lib: Correctly handle ppoll pfds.events == 0
The frrevent system is spitting out this message in bgpd:
20:40:15 mem1-roc-f2-b1-r5-t2-d4 bgpd[13166]: [XETTR-D5MR0][EC 100663316] Attempting to process an I/O event but for fd: 214(8) no thread to handle this!
This is because as each io event is processed, it is possible that a
.events is set to 0. This can leave a situation where we ask
ppoll to handle anything that happens on a fd with a .events of 0,
in this situation ppoll can return POLLERR, which indicates that
something bad has happened on the fd.
Let's set the ppoll fds.fd value to -1 when there are no more
events to be processed. ppoll specifically calls out that
it will just skip this particular one.
Shbinging [Wed, 16 Oct 2024 05:28:02 +0000 (05:28 +0000)]
ospfd: update ospf_asbr_status when using no_area_nssa command
In the processing of nssa, if the number of areas that need to be translated is greater than 0, then abr will be regarded as asbr, and it will be marked (0x3) in the flag of router lsa. When a certain area is set from nssa to a normal area, the areas that need to be translated may be reduced. The asbr should be re-interpreted as abr when the translated area is 0.
Enke Chen [Wed, 16 Oct 2024 18:15:28 +0000 (11:15 -0700)]
bgpd: fix several issues in sourcing AIGP attribute
Fix several issues in sourcing AIGP attribute:
1) AIGP should not be set as default for a redistributed route or a
static network. It should be set by config instead.
2) AIGP sourced by "set aigp-metric igp-metric" in a route-map does
not set the correct value for a redistributed route.
3) When redistribute a connected route like loopback, the AGIP (with
value 0) is sourced by "set aigp-metric igp-metric", but the
attribute is not propagated as the attribute flag is not set.
David Lamparter [Wed, 16 Oct 2024 10:50:50 +0000 (12:50 +0200)]
vtysh: make clang-SA happy about reusing stdin
While the logic here is perfectly fine, clang-SA doesn't understand that
the fopen() and fclose() match up with each other. Just use a separate
variable to make clang-SA happy.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Wed, 16 Oct 2024 10:46:35 +0000 (12:46 +0200)]
vtysh: remove duplicate nonblocking handling
non-blocking retries are already handled in `vtysh_client_receive()`.
And by the point we're back in `vtysh_client_run()`, errno may have been
overwritten by the close() call in vtysh_client_receive().
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Wed, 16 Oct 2024 10:40:01 +0000 (12:40 +0200)]
ldpd: free previous config if it wasn't applied
If a create-config command is received, but the config is never applied,
the config will be leaked on the next create-config command. This
should theoretically never happen, but let's fix it anyway.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Wed, 16 Oct 2024 10:45:35 +0000 (12:45 +0200)]
pimd: mark rest-of-packet ignored in C-RP parse
The `buf` pointer is being updated as the parse goes along. It's not
used after the last update, but I'd rather keep this in for consistency.
Just make a note of it being unused.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Wed, 16 Oct 2024 10:43:52 +0000 (12:43 +0200)]
pimd: initialize prefix value in Auto-RP
clang-SA complains that it's only partially initialized (because it's
used with IPv4 only). The code later calls some AF-generic code,
prompting clang-SA to complain that the non-IPv4 parts are used without
being set. While this shouldn't happen, just initialize it fully.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Wed, 16 Oct 2024 10:42:30 +0000 (12:42 +0200)]
lib: fix invalid use of errno in PTM
errno is only valid if there was an actual error. A zero return value
isn't an error, it's either EOF or an empty datagram depending on
context. Fix the logic.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Wed, 16 Oct 2024 10:41:06 +0000 (12:41 +0200)]
lib: make clang-SA not choke on defun_lex.l
The flex-generated code is disabled for clang-SA builds already, but
that means that function prototypes are missing too. Just add dummy
function prototypes so clang-SA can process the file.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Wed, 16 Oct 2024 10:24:44 +0000 (12:24 +0200)]
*: clang-SA switch-enum initializer workarounds
In these cases the value assigned by the switch block is used directly
rather than returned. Mark the initial/default value as used so
clang-SA doesn't complain about it.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Wed, 16 Oct 2024 10:23:23 +0000 (12:23 +0200)]
*: clang-SA friendly switch-enum-return-string
clang-19's SA complains about unused initializers for this kind of
"switch (enum) { return string }" kind of code. Use direct string
return values to avoid the issue.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Donald Sharp [Wed, 20 Mar 2024 19:32:30 +0000 (15:32 -0400)]
zebra: Attempt to explain the rnh tracking code better
I got asked today what was going on in the rnh code. I
had to take time off of what I was doing and rewrap my
head around this code, since it's been a long time.
As that this question may come up again in the future
I am trying to document this better so that someone
coming behind us will be able to read this and get
a better idea of what the algorithm is attempting
to do.
Donald Sharp [Fri, 11 Oct 2024 18:01:10 +0000 (14:01 -0400)]
*: Fix up improper handling of nexthops for nexthop tracking
Currently FRR needs to send a uint16_t value for the number
of nexthops as well it needs the ability to properly decode
all of this. Find and handle all the places that this happens.
bgpd: Re-announce the routes if the underlay IGP metric changes
If the underlay IGP metric changes, we SHOULD re-announce the routes with the
correct bpi->extra->igpmetric set.
Without this patch if the IGP link cost (metric) changes, we never notice this
and the peers do not have the updated metrics, which in turn causes incorrect
best path selections on remote peers.
David Lamparter [Tue, 15 Oct 2024 11:28:41 +0000 (13:28 +0200)]
tools/gcc-plugins: don't crash on array parameters
Need to have arrays as a stop condition in this type normalization
function, like pointers and function pointers. Actual arrays as
argument types are extremely rare to see because C has this
array-decay-to-pointer thing, but it can happen.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Shbinging [Tue, 15 Oct 2024 07:26:50 +0000 (07:26 +0000)]
ospfd:fix the bug that the empty area was not free after the command was executed
When we use the no area X.X.X.X range A.B.C.D/M command, if the area no longer has an interface to which it belongs, then the area should be deleted from the LSDB. This processing logic is consistent with instructions such as no network area and no area authentication.
Enke Chen [Tue, 15 Oct 2024 01:47:59 +0000 (18:47 -0700)]
tests: fix and adjust topotest/bgp_aigp
Fix and adjust the topotest post the fix for route selection with
AIGP.
When there are multiple IGP domains (OSPF in this case), the nexthop
for a BGP route with the AIGP attribute must be resolved in its own
IGP domain.
The changes in r2/bgpd.conf and r3/bgpd.conf are needed as incorrect
IGP metrics are received from NHT for the recursive nexthops. Once
the issue is resolved, the changes can be reverted.
When local ESI is flapped
torm-11:# ip neigh show 45.0.0.51
45.0.0.51 dev vlan1000 lladdr aa:aa:aa:00:00:01 REACHABLE proto zebra
Before fix:
(The imported route remained in tenant-vrf)
torm-11:# ip route show vrf vrf1 45.0.0.51
45.0.0.51 nhid 257 proto bgp metric 20
After fix:
torm-11# ip route show vrf vrf1 45.0.0.51
torm-11#
trace:
2024/10/11 18:19:29 BGP: [JMP3T-178G8] route [2]:[0]:[48]:[00:02:00:00:00:08]:[32]:[21.1.0.5]
is matched on local esi 03:00:00:00:77:01:04:00:00:0e, uninstall from VRF tenant1 route table
Donald Sharp [Mon, 14 Oct 2024 15:25:32 +0000 (11:25 -0400)]
zebra: Prevent a kernel route from being there when a connected should
There exists a series of events where a kernel route is learned
first( that happens to be exactly what a connected route should be )
and FRR ends up with both a kernel route and a connected route,
leaving us in a very strange spot. This code change just mirrors
the existing code of if there is a connected route drop the kernel
route. Here we just do the reverse, if we have a kernel route
already and a connected should be created, remove the kernel and
keep the connected.
Louis Scalbert [Fri, 11 Oct 2024 04:59:16 +0000 (06:59 +0200)]
bgpd, tests: don't send local nexthop from rr client
AS 65000 | AS 65001
|
RR |
| |
R1 --- | --- R2
|
When r1 peer is an iBGP route reflector client of rr and r2 peer is a
eBGP neighbor of rr, and all three routers shares the same network, r2
receives announcements coming from r1 with a IPv6 link-local nexthop
from rr. This is incorrect as r2 should send traffic to r1 without
involving rr.
Do not send an IPv6 link-local nexthop if the originating peer is a
route-reflector client.
Louis Scalbert [Fri, 11 Oct 2024 04:57:14 +0000 (06:57 +0200)]
bgpd: rename reflect in subgroup_announce_check
In subgroup_announce_check(), the variable reflect is misleading, as it
suggests a relation to route reflection. However, it actually refers to
the scenario where an iBGP peer announces a route to another iBGP peer.
Rename reflect to ibgp_to_ibgp.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Donald Sharp [Fri, 11 Oct 2024 13:33:35 +0000 (09:33 -0400)]
fpm: Allow max fpm message size to float based on ecmp
Currently the max message size is 4k. With a 256 way
ecmp FRR is seeing message sizes that are in the
6k size. There is desire to allow this to increase as
well to 512. Since the multipath size directly effects
how big the message may be when sending the routes ecmp
let's give a bit of headroom for this value when compiling
FRR at greater sizes. Additionally since we know not everyone
is using such large ecmp, allow them to build as appropriate
for their use cases.
Donald Sharp [Fri, 11 Oct 2024 00:08:32 +0000 (20:08 -0400)]
zebra: Slow down fpm_process_queue
When the fpm_process_queue has run out of space
but has written to the fpm output buffer, schedule
it to wake up immediately, as that the write will go out
pretty much immediately, since it was scheduled first.
If the fpm_process_queue has not written to the output
buffer then delay the processing by 10 milliseconds to
allow a possibly backed up write processing to have a
chance to complete it's work.
Donald Sharp [Thu, 10 Oct 2024 20:00:08 +0000 (16:00 -0400)]
zebra: Only notify dplane work pthread when needed
The fpm_nl_process function was getting the count
of the total number of ctx's processed. This leads
to after having processed 1 context to always signal
the dataplane that there is work to do. Change the
code to only notify the dplane worker when a context
was actually added to the outgoing context queue.