Before this PR, when graceful restart was configured for a neighbor
and when the restart flag was set by the restarting speaker, if
the neighbor was not advertising the GR capability, the initial
paths list was never sent by the restarting speaker to its neighbor
This is a problem when the server is configured with graceful
restart for all its peers without knowing if the peer supports it.
If some of the peers don't support it, they may never receive the
routes from the restarting speaker, leading to an inconsistent
routing state.
The gRPC code paths uses different functions than the BGP code path.
Thus is did not receive the fix for the mac mobility handling.
Fixes: c393f43 ("evpn: fix quadratic evpn mac-mobility handling")
Func capabilitiesFromConfig was always taken under the read lock.
However, when graceful restart is enabled for some families, this
function writes to the neighbor config which creates a data race.
Send Cease/Hard Reset notification for certain scenario when graceful
restart + notification support (RFC8538) are enabled. In this
implementation, we follow the suggestion of RFC8538 and map following
notification subcodes to Hard Reset subcode.
1. BGP_ERROR_SUB_MAXIMUM_NUMBER_OF_PREFIXES_REACHED
In this case, GoBGP is in the resource shortage and not working
properly. Thus, the peer should stop forwarding packet immediately.
2. BGP_ERROR_SUB_ADMINISTRATIVE_SHUTDOWN
This happens when the user uses DisablePeer API. This clearly indicates
user's intention of shutting down the session. Thus, we should send Hard
Reset.
3. BGP_ERROR_SUB_PEER_DECONFIGURED
This happens when the user uses DeletePeer API or StopBgp API or there's
an ASN mismatch found in the Open phase. The former two cases, the user
shows the intention to shutdown the session, so we should Hard Reset.
The latter case is not so obvious, but I think it's ok to do Hard Reset
because it is an unrecoverable error that cannot be solved without
user's involvement.
4. BGP_ERROR_SUB_HARD_RESET
This case currently doesn't exist, but obviously we should send Hard
Reset when someone explicitly specifies it.
The behavior for the remaining subcodes are unchanged. We may want to
expose a knob to adjust the behavior of BGP_ERROR_SUB_ADMINISTRATIVE_RESET
as suggested by RFC8538, but for this initial implementation, we kept it
as is.
Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Currently, even though `ApplyPolicy` is called for determining the
accepted routes after apply policy, the new route with attribute
modifications is not returned. This is problematic for gRPC API users.
Tests are added for all four cases that were described in
https://github.com/osrg/gobgp/issues/2765. This PR makes the behaviour
correct for "Case/Attempt 2" described in the issue.
- Compares IPs using net.IP.Equal instead of using a string comparison
in order to avoid unnecessary allocations.
- Adds peer.routerID to access IP, called by peer.RouterID.
Moves a number of heavily called debug lines behind if checks to avoid
needless allocations of Fields objects and stringification of fields.
For cases where the server is not set to "debug" log level, these fields
were allocated on the heap and then immediately discarded - as well a
number of these were stringifying state / NLRIs regardless of log level.
In servers with significant amounts of routes and BGP peers, this lead
to a large amount of wasted allocations - in our case looking at Go's
memory profiler, 25% of all allocations were from these lines alone.
Fixes a deadlock condition that can happen if StopBgp is called when the
pending connection queue is full. During teardown, StopBgp calls a
mgmtOp on the server goroutine which attempts to stop the goroutine
accepting inbound connections, and waits for it to finish before
continuing.
This connection goroutine can block if the connection queue is full,
which is read by the same goroutine that processes all mgmtOps. This
means that if the queue is full and the goroutine is currently blocked,
then calling StopBgp will lead to a complete deadlock, as the connection
goroutine will never close as it is trying to send to the queue, but the
queue will not be read as the server goroutine is currently waiting for
the connection goroutine to exit.
To correct this, a context has been added that gets passed to the
connection goroutine. This is then checked in a new select statement on
the connection queue which gets cancelled by tcpListener.Close() ensuring
the goroutine exits correctly even if the queue is full.
Additionally change the `Key` to be the listener address, which is hopefully
more useful than a difficult-to-decipher dump of the listener struct.
The previous behavior would result in log lines like the following, even if
nothing went wrong:
```
time="2024-01-31T17:30:25Z" level=warning msg="accept failed" Error="<nil>" Key="&{0x140002e4000 {<nil> 0 0}}" Topic=grpc
```
With this change, the message is only logged if there was an error, and it will look like this:
```
time="2024-01-31T17:40:25Z" level=warning msg="accept failed" Error="lolol just testing" Key="127.0.0.1:59289" Topic=grpc
```
Currently, graceful restart waits for the EoR message for all address
families "enabled" for the peer, but it should only wait for "received"
address families (the address families the peer is capable of handling).
Fixes: #2524
Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
It could be that a peer gets deleted and added back during the
transition from active state to open confirm. In that case, the TCP
connection from the old version of the peer could still be up. This
is a problem if the server is a listener only as the remote peer
would consider the old TCP connection as being valid and it won't
be able to connect until the TCP connection is eventually cleaned
by the Golang GC.
Adds usage of the "prefix-based" TCP MD5 for dynamic
neighbors. Non-dynamic neighbors will continue to use
non-prefix based, which makes it more compatible with
running on older kernels, as only 4.14+ includes the
necessary support.
This change also includes tests of dynamic peers in general.
This method was attempting to read from peer.fsm before acquiring a read
lock, leading to a data race as this struct is written by a different
goroutine in parallel. Commit moves the call to RLock before the first
read from the struct.
Currently GoBGP does not accept UPDATE messages with nexthops pointing
to a loopback address. This disallows multiple GoBGP instances from
running at the same time on 127.0.0.0/8.
This PR proposes removing this constraint when the RouterID of the
current GoBGP instance itself resides within the testing subnet of
127.0.0.0/8.
With a lot of paths (hundreds of thousands) gobgp may oom or
stuck in swapping.
Allow to specify max batch size via grpc and keep unlimited batch
size by default since 21093fbc87
without preallocation on the first run, so it still should not
affect perfomance/allocations with small ammount of paths.
Signed-off-by: Vladislav Grishenko <themiron@yandex-team.ru>
Filtering by peer address worked only for initial state.
Using watch event's filter allows to use it for each event and could be extended by other conditions, e.g. peer group.
peer_address and peer_group were added to API (backward compatible).
Signed-off-by: Rinat Baygildin <bayrinat@yandex-team.ru>
Use Go's standard library context package instead of the
golang.org/x/net/context package. Package context has been available in
the standard library since Go 1.7 and x/net/context.Context is merely an
alias for the standard library type.
Use the TCPMD5Sig type and the corresponding SetsockoptTCPMD5Sig func
added upstream in golang.org/x/sys v0.6.0
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
https://datatracker.ietf.org/doc/html/draft-abraitis-bgp-version-capability
Working example with FRR
```
% ./gobgp neighbor 192.168.10.124
BGP neighbor is 192.168.10.124, remote AS 65001
BGP version 4, remote router ID 200.200.200.202
BGP state = ESTABLISHED, up for 00:01:14
BGP OutQ = 0, Flops = 0
Hold time is 3, keepalive interval is 1 seconds
Configured hold time is 90, keepalive interval is 30 seconds
Neighbor capabilities:
multiprotocol:
ipv6-unicast: advertised
ipv4-unicast: advertised and received
route-refresh: advertised and received
extended-nexthop: advertised
Local: nlri: ipv4-unicast, nexthop: ipv6
UnknownCapability(6): received
UnknownCapability(9): received
graceful-restart: advertised and received
Local: restart time 10 sec
ipv6-unicast
ipv4-unicast
Remote: restart time 120 sec, notification flag set
ipv4-unicast, forward flag set
4-octet-as: advertised and received
add-path: received
Remote:
ipv4-unicast: receive
enhanced-route-refresh: received
long-lived-graceful-restart: advertised and received
Local:
ipv6-unicast, restart time 10 sec
ipv4-unicast, restart time 20 sec
Remote:
ipv4-unicast, restart time 0 sec, forward flag set
fqdn: advertised and received
Local:
name: donatas-pc, domain:
Remote:
name: spine1-debian-11, domain:
software-version: advertised and received
Local:
GoBGP/3.10.0
Remote:
FRRouting/8.5-dev-MyOwnFRRVersion-gdc92f44a4
cisco-route-refresh: received
Message statistics:
```
FRR side:
```
root@spine1-debian-11:~# vtysh -c 'show bgp neighbor 192.168.10.17 json' | \
> jq '."192.168.10.17".neighborCapabilities.softwareVersion.receivedSoftwareVersion'
"GoBGP/3.10.0"
root@spine1-debian-11:~#
```
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Both `gobgpd` and `gobgpd` now build for and run on Windows. This is based on https://github.com/osrg/gobgp/pull/2250/ with less required changes. Tested and confirmed it can peer successfully and receive routes. It still builds on Linux (doesn't change syscalls for non-windows files).
Adding enable_only_binary allows using only binary representation of nlri and attributes on the ListPath call.
For clients who uses only binary representation it helps to significantly reduce
resource consumption by refusing unnecessary conversion.
It is vital while processing a large number of paths, e.g. full-view.
This change doesn't break backward compatibility.
The context used there was a background context, which was not inherited
from the stream. Thus when the client ends the stream (e.g. ^C
`gobgp monitor global rib`), the context never completed, preventing the
stop of most goroutines responsible for forwarding events. The only case
where it completes is when an event gets generated, the transmit channel
was closed, ending the chain of goroutines.
Fix by inheriting the context from the stream, which completes when the
stream ends, properly cleaning up all resources.
Arthur de Kerhor
Tuetuopay
Maxime Peim
Yutaro Hayakawa
wenovus
Rinat Baygildin
FUJITA Tomonori
Britton Payne
Dawn Minion
Christian Persson
Jeremiah Millay
Joshua DeWald
Dawn Minion
Vladislav Grishenko
Kirill Pletnev
Rastislav Szabo
Tobias Klauser
Vladimir Kikhtenko
Donatas Abraitis
Shengjing Zhu
Ben Renninson
toby
Hitoshi Irino