Merge pull request #15192 from fdumontet6WIND/capa_nego

bgpd: add [no]neighbor capability fqdn

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 */

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 <A.B.C.D|X:X::X:X|WORD>$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);

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[] = {

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

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 <A.B.C.D|X:X::X:X|WORD> 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.

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",

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 fqdn")
 TestFlag.okfail("peer\\local-as")
 TestFlag.okfail("peer\\local-as 1 no-prepend")
 TestFlag.okfail("peer\\local-as 1 no-prepend replace-as")

tests/topotests/bgp_dont_capability_negotiate/test_bgp_dont_capability_negotiate.py

@@ -122,8 +122,9 @@ def _bgp_check_fqdn(fqdn=None):
     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 _bgp_check_fqdn(fqdn=None):
     _, 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:]