Fix issue with missing AF cap (e.g. IPv4 unicast when no capabilities
are announced).
Add Linpool save/restore action similar to bgp_create_update().
Based on patch from Michal Zagorski <mzagorsk@akamai.com> co-authored
with Pawel Maslanka <pmaslank@akamai.com>. Thanks!
When an OPEN message without capability options was parsed, the remote
role field was not initialized with the proper (non-zero) default value,
so it was interpreted as if 'provider' was announced.
Thanks to Mikhail Grishin for the bugreport.
The BMP protocol needs OPEN messages of established BGP sessions to
construct appropriate Peer Up messages. Instead of saving them internally
we use OPEN messages stored in BGP instances. This allows BMP instances
to be restarted or enabled later.
Because of this change, we can simplify BMP data structures. No need to
keep track of BGP sessions when we are not started. We have to iterate
over all (established) BGP sessions when the BMP session is established.
This is just a scaffolding now, but some kind of iteration would be
necessary anyway.
Also, the commit cleans up handling of msg/msg_length arguments to be
body/body_length consistently in both rx/tx and peer_up/peer_down calls.
For whatever reason, parser allocated a symbol for every parsed keyword
in each scope. That wasted time and memory. The effect is worsened with
recent changes allowing local scopes, so keywords often promote soft
scopes (with no symbols) to real scopes.
Do not allocate a symbol for a keyword. Take care of keywords that could
be promoted to symbols (kw_sym) and do it explicitly.
Hooks called from BGP to BMP should not log warning when BMP is not
connected, that is not an error (and we do not want to flood logs with
a ton of messages).
Blocked sk_send() should not log warning, that is expected situation.
Error during sk_send() is handled in error hook anyway.
Replace broken TCP connection management with a simple state machine.
Handle failed attempts properly with a timeout, detect and handle TCP
connection close and try to reconnect after that. Remove useless
'station_connected' flag.
Keep open messages saved even after the BMP session establishment,
so they can be used after BMP session flaps.
Use proper log messages for session events.
Use local variable to refence relevant instance instead of using global
instance ptr. Also, use 'p' variable instead of 'bmp' so we can use
common macros like TRACE().
Most error handling code was was for cases that cannot happen,
or they would be code bugs (and should use ASSERT()). Keep error
handling for just for I/O errors, like in rest of BIRD.
Initial implementation of a basic subset of the BMP (BGP Monitoring
Protocol, RFC 7854) from Akamai team. Submitted for further review
and improvement.
Missing translation from BGP attribute ID to eattr ID in bgp_unset_attr()
broke automatic removal of bgp_med during export to EBGP peers.
Thanks to Edward Sun for the bugreport.
Even though the free bind option is primarily meant to alleviate problems
with addresses assigned too late, it's also possible to use BIRD with AnyIP
configuration, assigning whole ranges to the machine. Therefore free bind
allows also to create an outbound connection from specific address even though
such address is not assigned.
The babel protocol normally sends all its messages as multicast packets,
but the protocol specification allows most messages to be sent as either
unicast or multicast, and the two can be mixed freely. In particular, the
babeld implementation can be configured to unicast updates to all peers
instead of sending them as unicast.
Daniel discovered that this can cause problems with the packet counter
checks in the MAC extension due to packet reordering. This happens on WiFi
networks where clients have power save enabled (which is quite common in
infrastructure networks): in this case, the access point will buffer all
multicast traffic and only send it out along with its beacons, leading to a
maximum buffering in default Linux-based access point configuration of up
to 200 ms.
This means that a Babel sender that mixes unicast and multicast messages
can have the unicast messages overtake the multicast messages because of
this buffering; when authentication is enabled, this causes the receiver to
discard the multicast message when it does arrive because it now has a
packet counter value less than the unicast message that arrived before it.
Daniel observed that this happens frequently enough that Babel ceases to
work entirely when runner over a WiFi network.
The issue has been described in draft-ietf-babel-mac-relaxed, which is
currently pending RFC publication. That also describes two mitigation
mechanisms: Keeping separate PC counters for unicast and multicast, and
using a reorder window for PC values. This patch implements the former as
that is the simplest, and resolves the particular issue seen on WiFi.
Thanks to Daniel Gröber for the bugreport.
Minor changes from committer.
The patch implements an IPv4 via IPv6 extension (RFC 9229) to the Babel
routing protocol (RFC 8966) that allows annoncing routes to an IPv4
prefix with an IPv6 next hop, which makes it possible for IPv4 traffic
to flow through interfaces that have not been assigned an IPv4 address.
The implementation is compatible with the current Babeld version.
Thanks to Toke Høiland-Jørgensen for early review on this work.
Minor changes from committer.
When creating a new babel_source object we initialise the seqno to 0. The
caller will update the source object with the right metric and seqno value,
for both newly created and old source objects. However if we initialise the
source object seqno to 0 that may actually turn out to be a valid (higher)
seqno than the one in the routing table, because of seqno wrapping. In this
case the source metric will not be set properly, which breaks feasibility
tracking for subsequent updates.
To fix this, add a new initial_seqno argument to babel_get_source() which
is used when allocating a new object, and set that to the seqno value of
the update we're sending.
Thanks to Juliusz Chroboczek for the bugreport.
Juliusz noticed there were a couple of places we were doing straight
inequality comparisons of seqnos in Babel. This is wrong because seqnos can
wrap: so we need to use the modulo-64k comparison function for these cases
as well.
Introduce a strict-inequality version of the modulo-comparison for this
purpose.
For active sessions, ignore received packets with zero local id and
mismatched remote id. That forces a session timeout instead of an
immediate session restart. It makes BFD sessions more resilient to
packet spoofing.
Thanks to André Grüneberg for the suggestion.
Protocols receive if_notify() announcements that are filtered according
to their VRF setting, but during reconfiguration, they access iface_list
directly and forgot to check VRF setting here, which leads to all
interfaces be addedd.
Fix this issue for Babel, OSPF, RAdv and RIP protocols.
Thanks to Marcel Menzel for the bugreport.
Some of these new BGP role keywords use generic names that collides with
user-defined symbols. Allow them to be redefined. Also remove duplicit
keyword definition for 'prefer'.
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.
Thanks to Nigel Kukard for the bugreport.
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).
- Improve documentation about pflags
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.
Minor changes from committer.
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.
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.
- When next hop is reset to local IP, we should remove BGP label stack,
as it is related to original next hop
- BGP next hop or immediate next hop from one VRF should not be passed
to another VRF, as they are different IP namespaces
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.
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.
Minor changes done by commiter.
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.
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.
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: ebd5751cde ("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.
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.
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.
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.
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.
Saving and restoring linpool state is safer.
A recent change in Babel causes ifaces to disappear after
reconfiguration. The patch fixes that.
Thanks to Johannes Kimmel for an insightful bugreport.
Implement flowspec validation procedure as described in RFC 8955 sec. 6
and RFC 9117. The Validation procedure enforces that only routers in the
forwarding path for a network can originate flowspec rules for that
network.
The patch adds new mechanism for tracking inter-table dependencies, which
is necessary as the flowspec validation depends on IP routes, and flowspec
rules must be revalidated when best IP routes change.
The validation procedure is disabled by default and requires that
relevant IP table uses trie, as it uses interval queries for subnets.
Attach a prefix trie to IP/VPN/ROA tables. Use it for net_route() and
net_roa_check(). This leads to 3-5x speedups for IPv4 and 5-10x
speedup for IPv6 of these calls.
TODO:
- Rebuild the trie during rt_prune_table()
- Better way to avoid trie_add_prefix() in net_get() for existing tables
- Make it configurable (?)
One of previous commits added error logging of invalid routes. This
also inadvertently caused error logging of route loops, which should
be ignored silently. Fix that.
Most error messages in attribute processing are in rx/decode step and
these use L_REMOTE log class. But there are few that are in tx/export
step and these should use L_ERR log class.
Use tx-specific macro (REJECT()) in tx/export code and rename field
err_withdraw to err_reject in struct bgp_export_state to ensure that
appropriate error reporting macros are called in proper contexts.
RFC 6810 and RFC 8210 specify that the "Max Length" value MUST NOT be
less than the Prefix Length element (underflow). On the other side,
overflow of the Max Length element also is possible, it being an 8-bit
unsigned integer allows for values larger than 32 or 128. This also
implicitly ensures there is no overflow of "Length" value.
When a PDU is received where the Max Length field is corrputed, the RTR
client (BIRD) should immediately terminate the session, flush all data
learned from that cache, and log an error for the operator.
Minor changes done by commiter.
