From 3d2f4df976518c04e94cfce56807ca0b899e9c72 Mon Sep 17 00:00:00 2001 From: Francois Dumontet Date: Mon, 22 Jan 2024 11:53:36 +0100 Subject: [PATCH] bgpd: add [no]neighbor dont-capability-negotiate hostname 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 dont-capability-negociate capability-type to avoid the use of "capability-type" in open message. Our case is about the hostname capability. this new command is to remove the use of hostname 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 | 65 ++++++++++++++++++------------------ bgpd/bgp_vty.c | 28 ++++++++++++++++ bgpd/bgpd.c | 3 ++ bgpd/bgpd.h | 1 + tests/bgpd/test_peer_attr.c | 5 +++ tests/bgpd/test_peer_attr.py | 1 + 6 files changed, 71 insertions(+), 32 deletions(-) diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c index 154efdedaf35..cd4013ba192c 100644 --- a/bgpd/bgp_open.c +++ b/bgpd/bgp_open.c @@ -1898,45 +1898,46 @@ uint16_t bgp_open_capability(struct stream *s, struct peer *peer, } /* Hostname capability */ - if (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 */ - ext_opt_params ? stream_putw(s, 0) - : stream_putc(s, 0); /* Capability Length */ - stream_putc(s, CAPABILITY_CODE_FQDN); - capp = stream_get_endp(s); - stream_putc(s, 0); /* dummy len for now */ - len = strlen(cmd_hostname_get()); - if (len > BGP_MAX_HOSTNAME) - len = BGP_MAX_HOSTNAME; - - stream_putc(s, len); - stream_put(s, cmd_hostname_get(), len); - if (cmd_domainname_get()) { - len = strlen(cmd_domainname_get()); + if (CHECK_FLAG(peer->flags, PEER_FLAG_CAPABILITY_HOSTNAME)) + if (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 */ + ext_opt_params ? stream_putw(s, 0) + : stream_putc(s, 0); /* Capability Length */ + stream_putc(s, CAPABILITY_CODE_FQDN); + capp = stream_get_endp(s); + stream_putc(s, 0); /* dummy len for now */ + len = strlen(cmd_hostname_get()); if (len > BGP_MAX_HOSTNAME) len = BGP_MAX_HOSTNAME; stream_putc(s, len); - stream_put(s, cmd_domainname_get(), len); - } else - stream_putc(s, 0); /* 0 length */ + stream_put(s, cmd_hostname_get(), len); + if (cmd_domainname_get()) { + len = strlen(cmd_domainname_get()); + if (len > BGP_MAX_HOSTNAME) + len = BGP_MAX_HOSTNAME; - /* Set the lengths straight */ - len = stream_get_endp(s) - rcapp - 1; - ext_opt_params ? stream_putw_at(s, rcapp, len - 1) - : stream_putc_at(s, rcapp, len); + stream_putc(s, len); + stream_put(s, cmd_domainname_get(), len); + } else + stream_putc(s, 0); /* 0 length */ - len = stream_get_endp(s) - capp - 1; - stream_putc_at(s, capp, len); + /* Set the lengths straight */ + len = stream_get_endp(s) - rcapp - 1; + ext_opt_params ? stream_putw_at(s, rcapp, len - 1) + : stream_putc_at(s, rcapp, len); - if (bgp_debug_neighbor_events(peer)) - zlog_debug( - "%s Sending hostname cap with hn = %s, dn = %s", - peer->host, cmd_hostname_get(), - cmd_domainname_get()); - } + len = stream_get_endp(s) - capp - 1; + stream_putc_at(s, capp, len); + + if (bgp_debug_neighbor_events(peer)) + zlog_debug( + "%s Sending hostname cap with hn = %s, dn = %s", + peer->host, cmd_hostname_get(), + cmd_domainname_get()); + } bgp_peer_send_gr_capability(s, peer, ext_opt_params); bgp_peer_send_llgr_capability(s, peer, ext_opt_params); diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 2a917155362b..5e0590a7c4cd 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -5735,6 +5735,25 @@ DEFUN (no_neighbor_dont_capability_negotiate, PEER_FLAG_DONT_CAPABILITY); } +/* neighbor capability hostname */ +DEFPY (neighbor_capability_hostname, + neighbor_capability_hostname_cmd, + "[no$no]neighbor $neighbor capability hostname", + NO_STR + NEIGHBOR_STR + NEIGHBOR_ADDR_STR2 + "Advertise capability to the peer\n" + "Advertise hostname capability to the peer\n") +{ + if (no) + return peer_flag_unset_vty(vty, neighbor, + PEER_FLAG_CAPABILITY_HOSTNAME); + else + return peer_flag_set_vty(vty, neighbor, + PEER_FLAG_CAPABILITY_HOSTNAME); +} + + /* neighbor capability extended next hop encoding */ DEFUN (neighbor_capability_enhe, neighbor_capability_enhe_cmd, @@ -18189,6 +18208,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 hostname*/ + if (peergroup_flag_check(peer, PEER_FLAG_CAPABILITY_HOSTNAME)) + vty_out(vty, + " neighbor %s capability hostname\n", + addr); + /* override-capability */ if (peergroup_flag_check(peer, PEER_FLAG_OVERRIDE_CAPABILITY)) vty_out(vty, " neighbor %s override-capability\n", addr); @@ -20525,6 +20550,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 hostname" command.*/ + install_element(BGP_NODE, &neighbor_capability_hostname_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 90ac529f8cfb..fc676afb7c5f 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1535,6 +1535,8 @@ 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, PEER_FLAG_CAPABILITY_HOSTNAME); + /* Initialize per peer bgp GR FSM */ bgp_peer_gr_init(peer); @@ -4571,6 +4573,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_HOSTNAME, 0, peer_change_none}, {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 4c12872ee9e0..7195b3de2065 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -1454,6 +1454,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_HOSTNAME (1ULL << 37) /* hostname 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 231ecd206625..83f8fa1f707b 100644 --- a/tests/bgpd/test_peer_attr.c +++ b/tests/bgpd/test_peer_attr.c @@ -282,6 +282,11 @@ static struct test_peer_attr test_peer_attrs[] = { .u.flag = PEER_FLAG_DONT_CAPABILITY, .type = PEER_AT_GLOBAL_FLAG, }, + { + .cmd = "capability hostname", + .u.flag = PEER_FLAG_CAPABILITY_HOSTNAME, + .type = PEER_AT_GLOBAL_FLAG, + }, { .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 bd8b06e2f052..e54447e0f02f 100644 --- a/tests/bgpd/test_peer_attr.py +++ b/tests/bgpd/test_peer_attr.py @@ -15,6 +15,7 @@ class TestFlag(frrtest.TestMultiOut): TestFlag.okfail("peer\\description") TestFlag.okfail("peer\\disable-connected-check") TestFlag.okfail("peer\\dont-capability-negotiate") +TestFlag.okfail("peer\\capability hostname") TestFlag.okfail("peer\\local-as") TestFlag.okfail("peer\\local-as 1 no-prepend") TestFlag.okfail("peer\\local-as 1 no-prepend replace-as")