From e146ea53ef5a8f33d9bdb2f79410682fe710e37c Mon Sep 17 00:00:00 2001 From: Francois Dumontet Date: Mon, 22 Jan 2024 11:53:36 +0100 Subject: [PATCH 1/3] bgpd: add [no]neighbor capability fqdn command cisco routers are not dealing fairly whith unsupported capabilities. When a cisco router receive an unsupported capabilities it reset the negociation without notifying the unmatching capability as described in RFC2842. Cisco suggest the use of neighbor x.x.x.x capability fqdn to avoid the use of fqdn in open message. this new command is to remove the use of fqdn capability in the open message with the peer "x.x.x.x". Link: https://www.cisco.com/c/en/us/support/docs/ip/border-gateway-protocol-bgp/116189-problemsolution-technology-00.pdf Signed-off-by: Francois Dumontet --- bgpd/bgp_open.c | 5 +++-- bgpd/bgp_vty.c | 33 +++++++++++++++++++++++++++++++++ bgpd/bgpd.c | 4 ++++ bgpd/bgpd.h | 1 + tests/bgpd/test_peer_attr.c | 7 +++++++ tests/bgpd/test_peer_attr.py | 1 + 6 files changed, 49 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c index 154efdedaf..43a59e2448 100644 --- a/bgpd/bgp_open.c +++ b/bgpd/bgp_open.c @@ -1897,8 +1897,9 @@ uint16_t bgp_open_capability(struct stream *s, struct peer *peer, stream_putc(s, CAPABILITY_CODE_DYNAMIC_LEN); } - /* Hostname capability */ - if (cmd_hostname_get()) { + /* FQDN capability */ + if (CHECK_FLAG(peer->flags, PEER_FLAG_CAPABILITY_FQDN) + && cmd_hostname_get()) { SET_FLAG(peer->cap, PEER_CAP_HOSTNAME_ADV); stream_putc(s, BGP_OPEN_OPT_CAP); rcapp = stream_get_endp(s); /* Ptr to length placeholder */ diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 2a91715536..09f9667a9a 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -5735,6 +5735,30 @@ DEFUN (no_neighbor_dont_capability_negotiate, PEER_FLAG_DONT_CAPABILITY); } +/* neighbor capability fqdn */ +DEFPY (neighbor_capability_fqdn, + neighbor_capability_fqdn_cmd, + "[no$no] neighbor $neighbor capability fqdn", + NO_STR + NEIGHBOR_STR + NEIGHBOR_ADDR_STR2 + "Advertise capability to the peer\n" + "Advertise fqdn capability to the peer\n") +{ + struct peer *peer; + + peer = peer_and_group_lookup_vty(vty, neighbor); + if (!peer) + return CMD_WARNING_CONFIG_FAILED; + + if (no) + return peer_flag_unset_vty(vty, neighbor, + PEER_FLAG_CAPABILITY_FQDN); + else + return peer_flag_set_vty(vty, neighbor, + PEER_FLAG_CAPABILITY_FQDN); +} + /* neighbor capability extended next hop encoding */ DEFUN (neighbor_capability_enhe, neighbor_capability_enhe_cmd, @@ -18189,6 +18213,12 @@ static void bgp_config_write_peer_global(struct vty *vty, struct bgp *bgp, if (peergroup_flag_check(peer, PEER_FLAG_DONT_CAPABILITY)) vty_out(vty, " neighbor %s dont-capability-negotiate\n", addr); + /* capability fqdn */ + if (peergroup_flag_check(peer, PEER_FLAG_CAPABILITY_FQDN)) + vty_out(vty, + " no neighbor %s capability fqdn\n", + addr); + /* override-capability */ if (peergroup_flag_check(peer, PEER_FLAG_OVERRIDE_CAPABILITY)) vty_out(vty, " neighbor %s override-capability\n", addr); @@ -20525,6 +20555,9 @@ void bgp_vty_init(void) install_element(BGP_NODE, &neighbor_dont_capability_negotiate_cmd); install_element(BGP_NODE, &no_neighbor_dont_capability_negotiate_cmd); + /* "neighbor capability fqdn" command. */ + install_element(BGP_NODE, &neighbor_capability_fqdn_cmd); + /* "neighbor ebgp-multihop" commands. */ install_element(BGP_NODE, &neighbor_ebgp_multihop_cmd); install_element(BGP_NODE, &neighbor_ebgp_multihop_ttl_cmd); diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 90ac529f8c..58514566ef 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1535,6 +1535,9 @@ struct peer *peer_new(struct bgp *bgp) if (CHECK_FLAG(bgp->flags, BGP_FLAG_ENFORCE_FIRST_AS)) SET_FLAG(peer->flags, PEER_FLAG_ENFORCE_FIRST_AS); + SET_FLAG(peer->flags_invert, PEER_FLAG_CAPABILITY_FQDN); + SET_FLAG(peer->flags, PEER_FLAG_CAPABILITY_FQDN); + /* Initialize per peer bgp GR FSM */ bgp_peer_gr_init(peer); @@ -4571,6 +4574,7 @@ static const struct peer_flag_action peer_flag_action_list[] = { {PEER_FLAG_AIGP, 0, peer_change_none}, {PEER_FLAG_GRACEFUL_SHUTDOWN, 0, peer_change_none}, {PEER_FLAG_CAPABILITY_SOFT_VERSION, 0, peer_change_none}, + {PEER_FLAG_CAPABILITY_FQDN, 0, peer_change_reset}, {0, 0, 0}}; static const struct peer_flag_action peer_af_flag_action_list[] = { diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 098a4e0f70..202cf7e113 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -1460,6 +1460,7 @@ struct peer { #define PEER_FLAG_AIGP (1ULL << 34) #define PEER_FLAG_GRACEFUL_SHUTDOWN (1ULL << 35) #define PEER_FLAG_CAPABILITY_SOFT_VERSION (1ULL << 36) +#define PEER_FLAG_CAPABILITY_FQDN (1ULL << 37) /* fqdn capability */ /* *GR-Disabled mode means unset PEER_FLAG_GRACEFUL_RESTART diff --git a/tests/bgpd/test_peer_attr.c b/tests/bgpd/test_peer_attr.c index 231ecd2066..12c2f1103a 100644 --- a/tests/bgpd/test_peer_attr.c +++ b/tests/bgpd/test_peer_attr.c @@ -282,6 +282,13 @@ static struct test_peer_attr test_peer_attrs[] = { .u.flag = PEER_FLAG_DONT_CAPABILITY, .type = PEER_AT_GLOBAL_FLAG, }, + { + .cmd = "capability fqdn", + .u.flag = PEER_FLAG_CAPABILITY_FQDN, + .type = PEER_AT_GLOBAL_FLAG, + .o.invert_peer = true, + .o.invert_group = true, + }, { .cmd = "local-as", .peer_cmd = "local-as 1", diff --git a/tests/bgpd/test_peer_attr.py b/tests/bgpd/test_peer_attr.py index bd8b06e2f0..b1f88d2ce1 100644 --- a/tests/bgpd/test_peer_attr.py +++ b/tests/bgpd/test_peer_attr.py @@ -15,6 +15,7 @@ TestFlag.okfail("peer\\capability extended-nexthop") TestFlag.okfail("peer\\description") TestFlag.okfail("peer\\disable-connected-check") TestFlag.okfail("peer\\dont-capability-negotiate") +TestFlag.okfail("peer\\capability fqdn") TestFlag.okfail("peer\\local-as") TestFlag.okfail("peer\\local-as 1 no-prepend") TestFlag.okfail("peer\\local-as 1 no-prepend replace-as") From 220c0635a85d4955883135459954782c5755270c Mon Sep 17 00:00:00 2001 From: Francois Dumontet Date: Mon, 22 Jan 2024 14:19:32 +0100 Subject: [PATCH 2/3] tests: improve topotest bgp_dont_capability_negotiate add some steps for testing of add [no]neighbor capability fqdn command support. Signed-off-by: Francois Dumontet --- .../test_bgp_dont_capability_negotiate.py | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/tests/topotests/bgp_dont_capability_negotiate/test_bgp_dont_capability_negotiate.py b/tests/topotests/bgp_dont_capability_negotiate/test_bgp_dont_capability_negotiate.py index ab71f87a04..9a0f5621dc 100644 --- a/tests/topotests/bgp_dont_capability_negotiate/test_bgp_dont_capability_negotiate.py +++ b/tests/topotests/bgp_dont_capability_negotiate/test_bgp_dont_capability_negotiate.py @@ -122,8 +122,9 @@ def test_bgp_check_fqdn(): step("Wait to converge") test_func = functools.partial(bgp_converge, r1) _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) - assert result is None, "Can't converge with dont-capability-negotiate" + assert result is None, "Can't converge with all capabilities" + step("Make sure FQDN capability is set") test_func = functools.partial(_bgp_check_fqdn, "r2") _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) assert result is None, "FQDN capability enabled, but r1 can't see it" @@ -150,6 +151,48 @@ def test_bgp_check_fqdn(): _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) assert result is None, "FQDN capability disabled, but we still have a hostname" + step("Re-enable sending any capability from r2") + r2.vtysh_cmd( + """ + configure terminal + router bgp 65002 + address-family ipv4 unicast + no neighbor 192.168.1.1 dont-capability-negotiate + end + clear bgp 192.168.1.1 + """ + ) + step("Wait to converge") + tgen = get_topogen() + test_func = functools.partial(bgp_converge, r1) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result is None, "Can't converge with all capabilities re enabled" + + step("Make sure FQDN capability is r2") + test_func = functools.partial(_bgp_check_fqdn, "r2") + _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result is None, "FQDN capability enabled, but r1 can't see it" + + step("Disable sending fqdn capability") + r2.vtysh_cmd( + """ + configure terminal + router bgp 65002 + no neighbor 192.168.1.1 capability fqdn + end + clear bgp 192.168.1.1 + """ + ) + step("Wait to converge") + tgen = get_topogen() + test_func = functools.partial(bgp_converge, r1) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result is None, "Can't converge with no capability fqdn" + + step("Make sure FQDN capability is reset") + test_func = functools.partial(_bgp_check_fqdn) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result is None, "FQDN capability disabled, but we still have a hostname" if __name__ == "__main__": args = ["-s"] + sys.argv[1:] From d034d1954a544ea24875ccee6c09a991dc6d65e7 Mon Sep 17 00:00:00 2001 From: Francois Dumontet Date: Mon, 22 Jan 2024 14:29:48 +0100 Subject: [PATCH 3/3] doc: add neighbor PEER capability fqdn command improve bgp doc Signed-off-by: Francois Dumontet --- doc/user/bgp.rst | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/doc/user/bgp.rst b/doc/user/bgp.rst index 540fa6d407..914e879e31 100644 --- a/doc/user/bgp.rst +++ b/doc/user/bgp.rst @@ -1792,6 +1792,18 @@ Configuring Peers This includes changing graceful-restart (LLGR also) timers, enabling/disabling add-path, and other supported capabilities. +.. clicmd:: neighbor PEER capability fqdn + + Allow BGP to negotiate the FQDN Capability with its peers. + + FQDN Capability defines a new BGP message (CAPABILITY) allowing the + use of peer's name and domain name. + + This capability is activated by default. The ``no neighbor PEER capability + fqdn`` avoid negotiation of that capability. This is useful for peers who + are not supporting this capability or supporting BGP Capabilities + Negotiation RFC 2842. + .. clicmd:: neighbor accept-own Enable handling of self-originated VPN routes containing ``accept-own`` community. @@ -2121,7 +2133,6 @@ Capability Negotiation .. clicmd:: neighbor PEER strict-capability-match - Strictly compares remote capabilities and local capabilities. If capabilities are different, send Unsupported Capability error then reset connection.