From cc75bdf02686cf271bb3781a5787ad7a8a214445 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 8 Nov 2023 15:26:40 +0200 Subject: [PATCH 1/2] bgpd: Set the software version capability received flag only after a validation We shouldn't set it blindly once the packet is received, but first we have to do some sanity checks. Signed-off-by: Donatas Abraitis --- bgpd/bgp_open.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c index 6ee5b5dc5c..b030c455bc 100644 --- a/bgpd/bgp_open.c +++ b/bgpd/bgp_open.c @@ -889,8 +889,6 @@ static int bgp_capability_software_version(struct peer *peer, size_t end = stream_get_getp(s) + hdr->length; uint8_t len; - SET_FLAG(peer->cap, PEER_CAP_SOFT_VERSION_RCV); - len = stream_getc(s); if (stream_get_getp(s) + len > end) { flog_warn( @@ -900,6 +898,8 @@ static int bgp_capability_software_version(struct peer *peer, return -1; } + SET_FLAG(peer->cap, PEER_CAP_SOFT_VERSION_RCV); + if (len > BGP_MAX_SOFT_VERSION) { flog_warn(EC_BGP_CAPABILITY_INVALID_LENGTH, "%s: Received Software Version, but the length is too big, truncating, from peer %s", From b80bc3fd1405e2e5c6406d17bb413c4291434567 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 8 Nov 2023 16:36:07 +0200 Subject: [PATCH 2/2] tests: Check received prefixes before immediately sending dynamic capabilities If we send capabilities immediately, before receiving an UPDATE message, we end up with a notification received from the neighbor. Let's wait until we have the fully converged topology and do the stuff. Tested locally and can't replicate the failure, let's see how happy is the CI this time. Signed-off-by: Donatas Abraitis --- .../test_bgp_dynamic_capability_graceful_restart.py | 10 ++++++++++ .../test_bgp_dynamic_capability_role.py | 10 ++++++++++ .../test_bgp_dynamic_capability_software_version.py | 10 ++++++++++ 3 files changed, 30 insertions(+) diff --git a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_graceful_restart.py b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_graceful_restart.py index db1eb2723b..75c712eb84 100644 --- a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_graceful_restart.py +++ b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_graceful_restart.py @@ -68,6 +68,11 @@ def test_bgp_dynamic_capability_graceful_restart(): "gracefulRestart": "advertisedAndReceived", "longLivedGracefulRestart": "advertisedAndReceived", }, + "addressFamilyInfo": { + "ipv4Unicast": { + "acceptedPrefixCounter": 2, + } + }, "gracefulRestartInfo": { "nBit": True, "timers": { @@ -116,6 +121,11 @@ def test_bgp_dynamic_capability_graceful_restart(): "gracefulRestart": "advertisedAndReceived", "longLivedGracefulRestart": "advertisedAndReceived", }, + "addressFamilyInfo": { + "ipv4Unicast": { + "acceptedPrefixCounter": 2, + } + }, "gracefulRestartInfo": { "nBit": True, "timers": { diff --git a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_role.py b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_role.py index da45110e39..576ef740b2 100644 --- a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_role.py +++ b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_role.py @@ -66,6 +66,11 @@ def test_bgp_dynamic_capability_role(): "neighborCapabilities": { "dynamic": "advertisedAndReceived", }, + "addressFamilyInfo": { + "ipv4Unicast": { + "acceptedPrefixCounter": 2, + } + }, } } return topotest.json_cmp(output, expected) @@ -108,6 +113,11 @@ def test_bgp_dynamic_capability_role(): "dynamic": "advertisedAndReceived", "role": "advertisedAndReceived", }, + "addressFamilyInfo": { + "ipv4Unicast": { + "acceptedPrefixCounter": 2, + } + }, "messageStats": { "notificationsRecv": 0, "capabilityRecv": 1, diff --git a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_software_version.py b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_software_version.py index a653da4655..002d7828c4 100644 --- a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_software_version.py +++ b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_software_version.py @@ -68,6 +68,11 @@ def test_bgp_dynamic_capability_software_version(): "receivedSoftwareVersion": None, }, }, + "addressFamilyInfo": { + "ipv4Unicast": { + "acceptedPrefixCounter": 2, + } + }, } } return topotest.json_cmp(output, expected) @@ -129,6 +134,11 @@ def test_bgp_dynamic_capability_software_version(): "receivedSoftwareVersion": rcv, }, }, + "addressFamilyInfo": { + "ipv4Unicast": { + "acceptedPrefixCounter": 2, + } + }, "messageStats": { "notificationsRecv": 0, "capabilityRecv": 1,