Ondrej Zajicek [Tue, 3 Jan 2023 16:01:54 +0000 (17:01 +0100)]
Nest: Fix leaking internal attributes in RIP and Babel
During backporting attribute changes from 3.0-branch, some internal
attributes (RIP iface and Babel seqno) leaked to 'show route all' output.
Allow protocols to hide specific attributes with GA_HIDDEN value.
Ondrej Zajicek [Sun, 1 Jan 2023 19:10:23 +0000 (20:10 +0100)]
Nest: Fix several issues with pflags
There were some confusion about validity and usage of pflags, which
caused incorrect usage after some flags from (now removed) protocol-
specific area were moved to pflags.
We state that pflags:
- Are secondary data used by protocol-specific hooks
- Can be changed on an existing route (in contrast to copy-on-write
for primary data)
- Are irrelevant for propagation (not propagated when changed)
- Are specific to a routing table (not propagated by pipe)
The patch did these fixes:
- Do not compare pflags in rte_same(), as they may keep cached values
like BGP_REF_STALE, causing spurious propagation.
- Initialize pflags to zero in rte_get_temp(), avoid initialization in
protocol code, fixing at least two forgotten initializations (krt
and one case in babel).
The seqno request retransmission handling was tracking the destination
that a forwarded request was being sent to and always retransmitting to
that same destination. This is unnecessary because we only need to
retransmit requests we originate ourselves, not those we forward on
behalf of others; in fact retransmitting on behalf of others can lead to
exponential multiplication of requests, which would be bad.
So rework the seqno request tracking so that instead of storing the
destination of a request, we just track whether it was a request that we
forwarded on behalf of another node, or if it was a request we originated
ourselves. Forwarded requests are not retransmitted, they are only used
for duplicate suppression, and for triggering an update when satisfied.
If we end up originating a request that we previously forwarded, we
"upgrade" the old request and restart the retransmit counter.
One complication with this is that requests sent in response to unfeasible
updates (section 3.8.2.2 of the RFC) have to be sent as unicast to a
particular peer. However, we don't really need to retransmit those as
there's no starvation when sending such a request; so we just change
such requests to be one-off unicast requests that are not subject to
retransmission or duplicate suppression. This is the same behaviour as
babeld has for such requests.
Ondrej Zajicek [Sat, 10 Dec 2022 16:32:42 +0000 (17:32 +0100)]
CLI: Fix for long-lived sessions during high loads
When there is a continuos stream of CLI commands, cli_get_command()
always returns 1 (there is a new command). Anyway, the socket receive
buffer was reset only when there was no command at all, leading to a
strange behavior: after a while, the CLI receive buffer came to its end,
then read() was called with zero size buffer, it returned 0 which was
interpreted as EOF.
The patch fixes that by resetting the buffer position after each command
and moving remaining data at the beginning of buffer.
Thanks to Maria Matejka for examining the bug and for the original bugfix.
Ondrej Zajicek [Fri, 9 Dec 2022 21:43:27 +0000 (22:43 +0100)]
Doc: Document issue with import tables
The import table does not work reliably together with re-evaluation of
routes due to recursive next hops or flowspec validation. We will at
least document that here, as import tables are completely redesigned and
this issue is fixed in BIRD 3.x branch.
Netlink support was added to FreeBSD recently. It is not as full-featured
as its Linux counterpart yet, however the added subset is enough to make
a routing daemon work. Specifically, it supports multiple tables,
multipath, nexthops and nexthops groups. No MPLS support yet.
The attached change adds 'bsd-netlink’ sysconf target, allowing to build
both netlink & rtsock versions on FreeBSD.
Ondrej Zajicek [Fri, 9 Dec 2022 04:51:30 +0000 (05:51 +0100)]
BGP: Improve handling of hold and keepalive timers
The effective keepalive time now scales relative to the negotiated
hold time, to maintain proportion between the keepalive time and the
hold time. This avoids issues when both keepalive and hold times
were configured, the hold time was negotiated to a smaller value,
but the keepalive time stayed the same.
Add new options 'min hold time' and 'min keepalive time', which reject
session attempts with too small hold time.
Improve validation of config options an their documentation.
Thanks to Alexander Zubkov and Sergei Goriunov for suggestions.
Ondrej Zajicek [Tue, 6 Dec 2022 18:40:02 +0000 (19:40 +0100)]
Nest: Avoid spurious announcements triggered by filtered routes
When filtered routes (enabled by 'import keep filtered' option) are
updated, they trigger announcements by rte_announce(). For regular
channels (e.g. type RA_OPTIMAL or RA_ANY) such announcement is just
ignored, but in case of RA_ACCEPTED (BGP peer with 'secondary' option)
it just reannounces the old (and still valid) best route.
The patch ensures that such no-change is ignored even for these channels.
Ondrej Zajicek [Wed, 9 Nov 2022 20:09:16 +0000 (21:09 +0100)]
Conf: Free stored old config before parsing new one
BIRD keeps a previous (old) configuration for the purpose of undo. The
existing code frees it after a new configuration is successfully parsed
during reconfiguration. That causes memory usage spikes as there are
temporarily three configurations (old, current, and new). The patch
changes it to free the old one before parsing the new one (as user
already requested a new config). The disadvantage is that undo is
not available after failed reconfiguration.
Maria Matejka [Tue, 1 Nov 2022 17:40:56 +0000 (18:40 +0100)]
Memory pages are not munmapped, instead we just madvise()
Memory unmapping causes slow address space fragmentation, leading in
extreme cases to failing to allocate pages at all. Removing this problem
by keeping all the pages allocated to us, yet calling madvise() to let
kernel dispose of them.
This adds a little complexity and overhead as we have to keep the
pointers to the free pages, therefore to hold e.g. 1 GB of 4K pages with
8B pointers, we have to store 2 MB of data.
Ondrej Zajicek [Tue, 18 Oct 2022 01:58:19 +0000 (03:58 +0200)]
Filter: Fix handling of variables in anonymous filters
Define scope for anonymous filters, and also explicitly distinguish block
scopes and function/filter scopes instead of using anonymous / named
distinction.
Anonymous filters forgot to push scope, so variables for them were in
fact defined in the top scope and therefore they shared a frame. This got
broken after rework of variables, which assumed that there is a named
scope for every function/filter.
Ondrej Zajicek [Wed, 12 Oct 2022 15:57:26 +0000 (17:57 +0200)]
Netlink: Parse onlink flag even on direct routes
While onlink flag is meaningful only with explicit next hops, it can be
defined also on direct routes. Parse it also in this case to avoid
periodic updates of the same route.
Ondrej Zajicek [Mon, 10 Oct 2022 03:06:19 +0000 (05:06 +0200)]
BGP: Add option 'next hop prefer global'
Add BGP channel option 'next hop prefer global' that modifies BGP
recursive next hop resolution to use global next hop IPv6 address instead
of link-local next hop IPv6 address for immediate next hop of received
routes.
Ondrej Zajicek [Mon, 3 Oct 2022 16:53:21 +0000 (18:53 +0200)]
Nest: Add channel config flag to distinguish new or copy
It is useful to distinguish whehter channel config returned from
channel_config_get() was allocated new, or existing from template.
Caller may want to initialize new ones.
Ondrej Zajicek [Thu, 8 Sep 2022 17:41:02 +0000 (19:41 +0200)]
BGP: Do not assume that all channels are struct bgp_channel
In principle, the channel list is a list of parent struct proto and can
contain general structures of type struct channel, That is useful e.g.
for adding MPLS channels to BGP.
Alexander Zubkov [Wed, 21 Sep 2022 15:07:11 +0000 (17:07 +0200)]
KRT: Fix setting default preference
Changes in commit eb937358 broke setting of channel preference for alien
routes learned during scan. The preference was set only for async routes.
Move common attribute processing part of functions krt_learn_async() and
krt_learn_async() to a separate function to have only one place for such
changes.
Maria Matejka [Fri, 22 Oct 2021 17:43:55 +0000 (19:43 +0200)]
Better profylaction recursive route loops
In some specific configurations, it was possible to send BIRD into an
infinite loop of recursive next hop resolution. This was caused by route
priority inversion.
To prevent priority inversions affecting other next hops, we simply
refuse to resolve any next hop if the best route for the matching prefix
is recursive or any other route with the same preference is recursive.
Next hop resolution doesn't change route priority, therefore it is
perfectly OK to resolve BGP next hops e.g. by an OSPF route, yet if the
same (or covering) prefix is also announced by iBGP, by retraction of
the OSPF route we would get a possible priority inversion.
Ondrej Zajicek [Tue, 26 Jul 2022 16:45:20 +0000 (18:45 +0200)]
Netlink: Restrict route replace for IPv6
Seems like the previous patch was too optimistic, as route replace is
still broken even in Linux 4.19 LTS (but fixed in Linux 5.10 LTS) for:
ip route add 2001:db8::/32 via fe80::1 dev eth0
ip route replace 2001:db8::/32 dev eth0
It ends with two routes instead of just the second.
The issue is limited to direct and special type (e.g. unreachable)
routes, the patch restricts route replace for cases when the new route
is a regular route (with a next hop address).
Ondrej Zajicek [Sun, 24 Jul 2022 22:11:40 +0000 (00:11 +0200)]
Netlink: Simplify handling of IPv6 ECMP routes
When IPv6 ECMP support first appeared in Linux kernel, it used different
API than IPv4 ECMP. Individual next hops were updated and announced
separately, instead of using RTA_MULTIPATH as in IPv4. This has several
drawbacks and requires complex code to merge received notifications to
one multipath route.
When Linux came with IPv6 RTA_MULTIPATH support, the initial versions
were somewhat buggy, so we kept using the old API for updates (splitting
multipath routes to sequences of route updates), while accepting both
old-style routes and RTA_MULTIPATH routes in scans / notifications.
As IPv6 RTA_MULTIPATH support is here for a long time, this patch fully
switches Netlink to the IPv6 RTA_MULTIPATH API and removes old complex
code for handling individual next hop announces.
The required Linux version is at least 4.11 for reliable operation.
Ondrej Zajicek [Sun, 24 Jul 2022 00:15:20 +0000 (02:15 +0200)]
KRT: Scan routing tables separetely on linux to avoid congestion
Remove compile-time sysdep option CONFIG_ALL_TABLES_AT_ONCE, replace it
with runtime ability to run either separate table scans or shared scan.
On Linux, use separate table scans by default when the netlink socket
option NETLINK_GET_STRICT_CHK is available, but retreat to shared scan
when it fails.
Running separate table scans has advantages where some routing tables are
managed independently, e.g. when multiple routing daemons are running on
the same machine, as kernel routing table modification performance is
significantly reduced when the table is modified while it is being
scanned.
Thanks Daniel Gröber for the original patch and Toke Høiland-Jørgensen
for suggestions.
Maria Matejka [Fri, 22 Jul 2022 13:37:21 +0000 (15:37 +0200)]
Revert "Export table: Delay freeing of old stored route."
This reverts commit cee0cd148c9b71bf47d007c850193b5fbf9486c1.
This change is not needed in version 2 and the surrounding code has
disappeared mostly in version 3.
Implement BGP roles as described in RFC 9234. It is a mechanism for
route leak prevention and automatic route filtering based on common BGP
topology relationships. It defines role capability (controlled by 'local
role' option) and OTC route attribute, which is used for automatic route
filtering and leak detection.
Filter: Improve handling of stack frames in filter bytecode
When f_line is done, we have to pop the stack frame. The old code just
removed nominal number of args/vars. Change it to use stored ventry value
modified by number of returned values. This allows to allocate variables
on a stack frame during execution of f_lines instead of just at start.
But we need to know the number of returned values for a f_line. It is 1
for term, 0 for cmd. Store that to f_line during linearization.
When a new variable used the same name as an existing symbol in an outer
scope, then offset number was defined based on a scope of the existing
symbol ($3) instead of a scope of the new symbol (sym_). That can lead
to two variables sharing the same memory slot.
Direct recursion almost worked, just crashed on function signature check.
Split function parsing such that function signature is saved before
function body is processed. Recursive calls are marked so they can be
avoided during f_same() and similar code walking.
Also, include tower of hanoi solver as a test case.
Filter: Ensure that all expressions declared return type
All instructions with a return value (i.e. expressions, ones with
non-zero outval, third argument in INST()) should declare their return
type. Check that automatically by M4 macros.
Set outval of FI_RETURN to 0. The instruction adds one value to stack,
but syntactically it is a statement, not an expression.
Add fake return type declaration to FI_CALL, otherwise the automatic
check would fail builds.
Pass instructions of function call arguments as vararg arguments to
FI_CALL instruction constructor and move necessary magic from parser
code to interpreter / instruction code.
Maria Matejka [Mon, 27 Jun 2022 17:04:22 +0000 (19:04 +0200)]
Preexport callback now takes the channel instead of protocol as argument
Passing protocol to preexport was in fact a historical relic from the
old times when channels weren't a thing. Refactoring that to match
current extensibility needs.
Maria Matejka [Tue, 7 Jun 2022 08:35:48 +0000 (10:35 +0200)]
IPv4 flowspec literals should reject IPv6 prefices in a well-behaved way
When writing flow4 { dst 2001:db8::dead:beef/128; }, BIRD crashed on an
not-well-debuggable segfault as it tried to copy the whole 128-bit
prefix into an IPv4-sized memory.
Ondrej Zajicek [Sun, 5 Jun 2022 02:03:43 +0000 (04:03 +0200)]
Babel: Do not try to remove multicast seqno request objects from neighbour list
The Babel seqno request code keeps track of which seqno requests are
outstanding for a neighbour by putting them onto a per-neighbour list. When
reusing a seqno request, it will try to remove this node, but if the seqno
request in question was a multicast request with no neighbour attached this
will result in a crash because it tries to remove a list node that wasn't
added to any list.
Fix this by making the list remove conditional. Also fix neighbor removal
which were changing seqno requests to multicast ones instead of removing
them.
Fixes: ebd5751cdeb4 ("Babel: Seqno requests are properly decoupled from
neighbors when the underlying interface disappears").
Based on the patch from Toke Høiland-Jørgensen <toke@toke.dk>,
bug reported by Stefan Haller <stefan.haller@stha.de>, thanks.
Ondrej Zajicek [Sun, 15 May 2022 13:05:13 +0000 (15:05 +0200)]
BGP: Improve tx performance during feed/flush
The prefix hash table in BGP used the same hash function as the rtable.
When a batch of routes are exported during feed/flush to the BGP, they
all have similar hash values, so they are all crowded in a few slots in
the BGP prefix table (which is much smaller - around the size of the
batch - and uses higher bits from hash values), making it much slower due
to excessive collisions. Use a different hash function to avoid this.
Also, increase the batch size to fill 4k BGP packets and increase minimum
BGP bucket and prefix hash sizes to avoid back and forth resizing during
flushes.
This leads to order of magnitude faster flushes (on my test data).
When shutting down a Babel instance we send a wildcard retraction to make
sure all peers can quickly switch to other route origins. Add another small
optimisation borrowed from babeld: sending a Hello message (along with the
retraction) with a very low interval.
This will cause neighbours to modify their expiry timers for the node's
state to quickly time it out, thus conserving resources in the network.
Maria Matejka [Wed, 13 Apr 2022 15:05:12 +0000 (17:05 +0200)]
RIP: fixed the EA_RIP_FROM attribute
The interface pointer was improperly converted to u32 and back. Fixing
this by explicitly allocating an adata structure for it. It's not so
memory efficient, we'll optimize this later.
Maria Matejka [Wed, 13 Apr 2022 09:28:15 +0000 (11:28 +0200)]
Fixed a munmap abort bug
When BIRD was munmapping too many pages, it sometimes aborted, saying
that munmap failed with "Not enough memory" as the address space was
getting more and more fragmented.
There is a workaround in place, simply keeping that page for future use,
yet it has never been compiled in because I somehow forgot to include
errno.h. And because I also thought that somebody may have ENOMEM not
defined (why?!), there was a check which quietly omitted that
workaround.
Anyway, ENOMEM is POSIX. It's an utter nonsense to check for its
existence. If it doesn't exist, something is broken.
Add BFD protocol option 'strict bind' to use separate listening socket
for each BFD interface bound to its address instead of using shared
listening sockets.
Maria Matejka [Tue, 5 Apr 2022 13:09:56 +0000 (15:09 +0200)]
BGP uses lp_save / lp_restore instead of linpool flushing
It is too cryptic to flush tmp_linpool in these cases and we don't want
anybody in the future to break this code by adding an allocation
somewhere which should persist over that flush.