From 388f9ef0cbdfefb5e36a3719954e8f865bf3ce2a Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 29 Jan 2025 22:24:39 +0200 Subject: [PATCH 1/3] doc: Say that 0.0.0.0 (0) BGP identifier is invalid Signed-off-by: Donatas Abraitis (cherry picked from commit c9a29289548028163a8d1edd55dbf03f27d53484) --- doc/user/bgp.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/user/bgp.rst b/doc/user/bgp.rst index 1493c2fb98..5add30b6f4 100644 --- a/doc/user/bgp.rst +++ b/doc/user/bgp.rst @@ -282,7 +282,9 @@ internal or external. interface and address information. In that case default router ID value is selected as the largest IP Address of the interfaces. When `router zebra` is not enabled *bgpd* can't get interface information so `router-id` is set to - 0.0.0.0. So please set router-id by hand. + 0.0.0.0, which is invalid and BGP session can't be established. + + So please set router-id by manually. .. _bgp-multiple-autonomous-systems: @@ -2938,7 +2940,7 @@ BGP Extended Communities in Route Map ``CO:COLOR`` This is a format to define colors value. ``CO`` part is always 00 (default), - it can be used to support the requirements of Color-Only steering when using + it can be used to support the requirements of Color-Only steering when using a Null Endpoint in the SR-TE Policy as specified in Section 8.8 of [RFC9256]. The below shows in detail what the different combinations of ``CO`` bits can match on to for the purpose of determining what type of SR-TE Policy Tunnel From 46d210ce80e2bbabf3471114d70a9712eb9116f3 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 29 Jan 2025 23:03:06 +0200 Subject: [PATCH 2/3] bgpd: Do not start BGP session if BGP identifier is not set If we have IPv6-only network and no IPv4 addresses at all, then by default 0.0.0.0 is created which is treated as malformed according to RFC 6286. Signed-off-by: Donatas Abraitis (cherry picked from commit 739f2b566a8217acce84d4c21aaf033314f535bb) --- bgpd/bgp_fsm.c | 1 + bgpd/bgp_network.c | 17 ++++++++++++++++- bgpd/bgpd.h | 1 + 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index 1a30cb37f4..c7b7f9e284 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -607,6 +607,7 @@ const char *const peer_down_str[] = { "Admin. shutdown (RTT)", "Suppress Fib Turned On or Off", "Password config change", + "Router ID is missing", }; static void bgp_graceful_restart_timer_off(struct peer_connection *connection, diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c index af5d815d30..3df4aa286e 100644 --- a/bgpd/bgp_network.c +++ b/bgpd/bgp_network.c @@ -571,7 +571,7 @@ static void bgp_accept(struct event *thread) /* Do not try to reconnect if the peer reached maximum * prefixes, restart timer is still running or the peer - * is shutdown. + * is shutdown, or BGP identifier is not set (0.0.0.0). */ if (BGP_PEER_START_SUPPRESSED(peer1)) { if (bgp_debug_neighbor_events(peer1)) { @@ -588,6 +588,14 @@ static void bgp_accept(struct event *thread) return; } + if (peer1->bgp->router_id.s_addr == INADDR_ANY) { + zlog_warn("[Event] Incoming BGP connection rejected from %s due missing BGP identifier, set it with `bgp router-id`", + peer1->host); + peer1->last_reset = PEER_DOWN_ROUTER_ID_ZERO; + close(bgp_sock); + return; + } + if (bgp_debug_neighbor_events(peer1)) zlog_debug("[Event] connection from %s fd %d, active peer status %d fd %d", inet_sutop(&su, buf), bgp_sock, connection1->status, @@ -776,6 +784,13 @@ enum connect_result bgp_connect(struct peer_connection *connection) assert(!CHECK_FLAG(connection->thread_flags, PEER_THREAD_READS_ON)); ifindex_t ifindex = 0; + if (peer->bgp->router_id.s_addr == INADDR_ANY) { + peer->last_reset = PEER_DOWN_ROUTER_ID_ZERO; + zlog_warn("%s: BGP identifier is missing for peer %s, set it with `bgp router-id`", + __func__, peer->host); + return connect_error; + } + if (peer->conf_if && BGP_CONNECTION_SU_UNSPEC(connection)) { if (bgp_debug_neighbor_events(peer)) zlog_debug("Peer address not learnt: Returning from connect"); diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 96a78e6662..ee904391c1 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -1863,6 +1863,7 @@ struct peer { #define PEER_DOWN_RTT_SHUTDOWN 35U /* Automatically shutdown due to RTT */ #define PEER_DOWN_SUPPRESS_FIB_PENDING 36U /* Suppress fib pending changed */ #define PEER_DOWN_PASSWORD_CHANGE 37U /* neighbor password command */ +#define PEER_DOWN_ROUTER_ID_ZERO 38U /* router-id is 0.0.0.0 */ /* * Remember to update peer_down_str in bgp_fsm.c when you add * a new value to the last_reset reason From 3f788da60b9f93919ac4ebe0df82fc4c89c4dcb1 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 29 Jan 2025 23:11:35 +0200 Subject: [PATCH 3/3] tests: Check if the peer stays Idle if router-id is not set Signed-off-by: Donatas Abraitis (cherry picked from commit 48560b5c9b8084e903ebf0ef5987b4e581459608) --- .../bgp_ipv6_ll_peering/r1/bgpd.conf | 3 ++ .../bgp_ipv6_ll_peering/r1/zebra.conf | 3 ++ .../bgp_ipv6_ll_peering/r3/bgpd.conf | 5 ++++ .../bgp_ipv6_ll_peering/r3/zebra.conf | 4 +++ .../test_bgp_ipv6_ll_peering.py | 29 ++++++++++++++++++- 5 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 tests/topotests/bgp_ipv6_ll_peering/r3/bgpd.conf create mode 100644 tests/topotests/bgp_ipv6_ll_peering/r3/zebra.conf diff --git a/tests/topotests/bgp_ipv6_ll_peering/r1/bgpd.conf b/tests/topotests/bgp_ipv6_ll_peering/r1/bgpd.conf index 724cbf84ab..a26efb4c4f 100644 --- a/tests/topotests/bgp_ipv6_ll_peering/r1/bgpd.conf +++ b/tests/topotests/bgp_ipv6_ll_peering/r1/bgpd.conf @@ -4,3 +4,6 @@ router bgp 65001 neighbor fe80:1::2 remote-as external neighbor fe80:1::2 timers 3 10 neighbor fe80:1::2 interface r1-eth0 + neighbor fe80:1::3 remote-as external + neighbor fe80:1::3 timers 3 10 + neighbor fe80:1::3 interface r1-eth1 diff --git a/tests/topotests/bgp_ipv6_ll_peering/r1/zebra.conf b/tests/topotests/bgp_ipv6_ll_peering/r1/zebra.conf index 4e93d4f4e5..f1bbff2e44 100644 --- a/tests/topotests/bgp_ipv6_ll_peering/r1/zebra.conf +++ b/tests/topotests/bgp_ipv6_ll_peering/r1/zebra.conf @@ -2,3 +2,6 @@ interface r1-eth0 ipv6 address fe80:1::1/64 ! +interface r1-eth1 + ipv6 address fe80:1::1/64 +! diff --git a/tests/topotests/bgp_ipv6_ll_peering/r3/bgpd.conf b/tests/topotests/bgp_ipv6_ll_peering/r3/bgpd.conf new file mode 100644 index 0000000000..f1684880b3 --- /dev/null +++ b/tests/topotests/bgp_ipv6_ll_peering/r3/bgpd.conf @@ -0,0 +1,5 @@ +router bgp 65003 + no bgp ebgp-requires-policy + neighbor fe80:1::1 remote-as external + neighbor fe80:1::1 timers 3 10 + neighbor fe80:1::1 interface r3-eth0 diff --git a/tests/topotests/bgp_ipv6_ll_peering/r3/zebra.conf b/tests/topotests/bgp_ipv6_ll_peering/r3/zebra.conf new file mode 100644 index 0000000000..71053cd2c3 --- /dev/null +++ b/tests/topotests/bgp_ipv6_ll_peering/r3/zebra.conf @@ -0,0 +1,4 @@ +! +interface r3-eth0 + ipv6 address fe80:1::3/64 +! diff --git a/tests/topotests/bgp_ipv6_ll_peering/test_bgp_ipv6_ll_peering.py b/tests/topotests/bgp_ipv6_ll_peering/test_bgp_ipv6_ll_peering.py index aaa68ea340..fbd4097605 100644 --- a/tests/topotests/bgp_ipv6_ll_peering/test_bgp_ipv6_ll_peering.py +++ b/tests/topotests/bgp_ipv6_ll_peering/test_bgp_ipv6_ll_peering.py @@ -27,13 +27,17 @@ pytestmark = [pytest.mark.bgpd] def build_topo(tgen): - for routern in range(1, 3): + for routern in range(1, 4): tgen.add_router("r{}".format(routern)) switch = tgen.add_switch("s1") switch.add_link(tgen.gears["r1"]) switch.add_link(tgen.gears["r2"]) + switch = tgen.add_switch("s2") + switch.add_link(tgen.gears["r1"]) + switch.add_link(tgen.gears["r3"]) + def setup_module(mod): tgen = Topogen(build_topo, mod.__name__) @@ -64,6 +68,7 @@ def test_bgp_ipv6_link_local_peering(): pytest.skip(tgen.errors) r1 = tgen.gears["r1"] + r3 = tgen.gears["r3"] def _bgp_converge(): output = json.loads(r1.vtysh_cmd("show bgp summary json")) @@ -82,6 +87,28 @@ def test_bgp_ipv6_link_local_peering(): _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) assert result is None, "Failed to see BGP convergence on R2" + def _bgp_router_id_missing(): + output = json.loads(r3.vtysh_cmd("show bgp summary failed json")) + expected = { + "ipv4Unicast": { + "routerId": "0.0.0.0", + "as": 65003, + "peers": { + "fe80:1::1": { + "connectionsEstablished": 0, + "connectionsDropped": 0, + "peerUptime": "never", + "lastResetDueTo": "Router ID is missing", + } + }, + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial(_bgp_router_id_missing) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result is None, "r3 should stay down due to missing router ID" + if __name__ == "__main__": args = ["-s"] + sys.argv[1:]