bgpd: Deprecate BGP `internet` community

Quite a few well-known communities from IANA's list do
   not receive special treatment in Cisco IOS XR, and at least one
   community on Cisco IOS XR's special treatment list, internet == 0:0,
   is not formally a well-known community as it is not in [IANA-WKC] (it
   is taken from the Reserved range [0x00000000-0x0000FFFF]).

https://datatracker.ietf.org/doc/html/rfc8642

This is Cisco-specific command which is causing lots of questions when it
comes to debugging and/or configuring it properly, but overall, this behavior
is very odd and it's not clear how it should be treated between different
vendor implementations.

Let's deprecate it and let the operators use 0:0/0 communities as they want.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

diff --git a/bgpd/bgp_clist.c b/bgpd/bgp_clist.c
index 2cbbe0a5a9da43384d931f4480b0d78519b8ba98..1d2ba3bf5845eff39742cd91bff8670c8b5545d9 100644 (file)
--- a/bgpd/bgp_clist.c
+++ b/bgpd/bgp_clist.c
@@ -449,8 +449,12 @@ static char *community_str_get(struct community *com, int i)
        comval = ntohl(comval);
 
        switch (comval) {
+#if CONFDATE > 20230801
+CPP_NOTICE("Deprecate COMMUNITY_INTERNET BGP community")
+#endif
        case COMMUNITY_INTERNET:
                str = XSTRDUP(MTYPE_COMMUNITY_STR, "internet");
+               zlog_warn("`internet` community is deprecated");
                break;
        case COMMUNITY_GSHUT:
                str = XSTRDUP(MTYPE_COMMUNITY_STR, "graceful-shutdown");

diff --git a/bgpd/bgp_community.c b/bgpd/bgp_community.c
index e2bce9cbcef4864e959e44217a1df4e82d09f741..ff6e477355b6026efb5cee9a75ca24f61e10987e 100644 (file)
--- a/bgpd/bgp_community.c
+++ b/bgpd/bgp_community.c
@@ -168,7 +168,6 @@ struct community *community_uniq_sort(struct community *com)
 
    For Well-known communities value, below keyword is used.
 
-   0x0             "internet"
    0xFFFF0000      "graceful-shutdown"
    0xFFFF0001      "accept-own"
    0xFFFF0002      "route-filter-translated-v4"
@@ -229,8 +228,12 @@ static void set_community_string(struct community *com, bool make_json,
                comval = ntohl(comval);
 
                switch (comval) {
+#if CONFDATE > 20230801
+CPP_NOTICE("Deprecate COMMUNITY_INTERNET BGP community")
+#endif
                case COMMUNITY_INTERNET:
                        len += strlen(" internet");
+                       zlog_warn("`internet` community is deprecated");
                        break;
                case COMMUNITY_GSHUT:
                        len += strlen(" graceful-shutdown");
@@ -295,6 +298,9 @@ static void set_community_string(struct community *com, bool make_json,
                        strlcat(str, " ", len);
 
                switch (comval) {
+#if CONFDATE > 20230801
+CPP_NOTICE("Deprecate COMMUNITY_INTERNET BGP community")
+#endif
                case COMMUNITY_INTERNET:
                        strlcat(str, "internet", len);
                        if (make_json) {
@@ -303,6 +309,7 @@ static void set_community_string(struct community *com, bool make_json,
                                json_object_array_add(json_community_list,
                                                      json_string);
                        }
+                       zlog_warn("`internet` community is deprecated");
                        break;
                case COMMUNITY_GSHUT:
                        strlcat(str, "graceful-shutdown", len);
@@ -673,10 +680,14 @@ community_gettoken(const char *buf, enum community_token *token, uint32_t *val)
 
        /* Well known community string check. */
        if (isalpha((unsigned char)*p)) {
+#if CONFDATE > 20230801
+CPP_NOTICE("Deprecate COMMUNITY_INTERNET BGP community")
+#endif
                if (strncmp(p, "internet", strlen("internet")) == 0) {
                        *val = COMMUNITY_INTERNET;
                        *token = community_token_no_export;
                        p += strlen("internet");
+                       zlog_warn("`internet` community is deprecated");
                        return p;
                }
                if (strncmp(p, "graceful-shutdown", strlen("graceful-shutdown"))

diff --git a/bgpd/bgp_community.h b/bgpd/bgp_community.h
index 35909a638dfd0cc33d1de0e2f28c09d0738315ee..e7af362ea845d3c0ff8b1443c8ba3ae0b4b3ad58 100644 (file)
--- a/bgpd/bgp_community.h
+++ b/bgpd/bgp_community.h
@@ -30,6 +30,9 @@ struct community {
 };
 
 /* Well-known communities value.  */
+#if CONFDATE > 20230801
+CPP_NOTICE("Deprecate COMMUNITY_INTERNET BGP community")
+#endif
 #define COMMUNITY_INTERNET                      0x0
 #define COMMUNITY_GSHUT                         0xFFFF0000
 #define COMMUNITY_ACCEPT_OWN                    0xFFFF0001

diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c
index f9def03693c69df861a602db13e03c529da259c2..873884ba54cd8afdc863c2c900da943f9de7b548 100644 (file)
--- a/bgpd/bgp_routemap.c
+++ b/bgpd/bgp_routemap.c
@@ -5870,9 +5870,14 @@ DEFUN_YANG (set_community,
                else
                        first = 1;
 
+#if CONFDATE > 20230801
+CPP_NOTICE("Deprecate COMMUNITY_INTERNET BGP community")
+#endif
                if (strncmp(argv[i]->arg, "internet", strlen(argv[i]->arg))
                    == 0) {
                        buffer_putstr(b, "internet");
+                       vty_out(vty, "%% `internet` community is deprecated\n");
+                       zlog_warn("`internet` community is deprecated");
                        continue;
                }
                if (strncmp(argv[i]->arg, "local-AS", strlen(argv[i]->arg))

diff --git a/doc/user/bgp.rst b/doc/user/bgp.rst
index c5319a8e8830c368eddc0f75ef0778ef22906044..06d5d5857a65e1f4146f02aeedeb053c43079eb6 100644 (file)
--- a/doc/user/bgp.rst
+++ b/doc/user/bgp.rst
@@ -2113,9 +2113,6 @@ is 4 octet long. The following format is used to define the community value.
    ``7675:80`` can be used when AS 7675 wants to pass local policy value 80 to
    neighboring peer.
 
-``internet``
-   ``internet`` represents well-known communities value 0.
-
 ``graceful-shutdown``
    ``graceful-shutdown`` represents well-known communities value
    ``GRACEFUL_SHUTDOWN`` ``0xFFFF0000`` ``65535:0``. :rfc:`8326` implements
@@ -2486,17 +2483,6 @@ community-list.
     match community FILTER
 
 
-The communities value keyword ``internet`` has special meanings in standard
-community lists. In the below example ``internet`` matches all BGP routes even
-if the route does not have communities attribute at all. So community list
-``INTERNET`` is the same as ``FILTER`` in the previous example.
-
-.. code-block:: frr
-
-   bgp community-list standard INTERNET deny 1:1
-   bgp community-list standard INTERNET permit internet
-
-
 The following configuration is an example of communities value deletion.  With
 this configuration the community values ``100:1`` and ``100:2`` are removed
 from BGP updates. For communities value deletion, only ``permit``
@@ -2566,9 +2552,6 @@ Extended Community Lists
    there is no matched entry, deny will be returned. When `extcommunity` is
    empty it matches to any routes.
 
-   A special handling for ``internet`` community is applied. It matches
-   any community.
-
 .. clicmd:: bgp extcommunity-list expanded NAME permit|deny LINE
 
    This command defines a new expanded extcommunity-list. `line` is a string

diff --git a/tests/topotests/bgp_communities_topo1/test_bgp_communities_topo2.py b/tests/topotests/bgp_communities_topo1/test_bgp_communities_topo2.py
index 7db6a51c927808ba6d64a5a886377e5cb99b7e2f..324f53f3a6d18957e1cbbc461b6213b829ab42ae 100644 (file)
--- a/tests/topotests/bgp_communities_topo1/test_bgp_communities_topo2.py
+++ b/tests/topotests/bgp_communities_topo1/test_bgp_communities_topo2.py
@@ -11,7 +11,7 @@
 Following tests are covered to test bgp community functionality:
 1. Verify that BGP well known communities work fine for
    eBGP and iBGP peers.
-   Well known communities tested: no-export, local-AS, internet
+   Well known communities tested: no-export, local-AS
 
 """
 
@@ -140,11 +140,11 @@ def teardown_module(mod):
 #####################################################
 
 
-def test_bgp_no_export_local_as_and_internet_communities_p0(request):
+def test_bgp_no_export_local_as_communities_p0(request):
     """
     Verify that BGP well known communities work fine for
     eBGP and iBGP peers.
-    Well known communities tested: no-export, local-AS, internet
+    Well known communities tested: no-export, local-AS
     """
 
     tc_name = request.node.name
@@ -170,7 +170,7 @@ def test_bgp_no_export_local_as_and_internet_communities_p0(request):
             tc_name, result
         )
 
-    for comm_type in ["no-export", "local-AS", "internet"]:
+    for comm_type in ["no-export", "local-AS"]:
 
         step("Create a route-map on R1 to set community as {}".format(comm_type))
 
@@ -258,45 +258,23 @@ def test_bgp_no_export_local_as_and_internet_communities_p0(request):
                 tc_name, result
             )
 
-            if comm_type == "internet":
-                step(
-                    "Verify that these prefixes, originated on R1, are"
-                    "received on both R2 and R3"
-                )
-
-                result = verify_rib(
-                    tgen,
-                    addr_type,
-                    "r3",
-                    input_dict_4,
-                    next_hop=topo["routers"]["r1"]["links"]["r3"][addr_type].split("/")[
-                        0
-                    ],
-                )
-                assert result is True, "Testcase {} : Failed \n Error: {}".format(
-                    tc_name, result
-                )
-            else:
-                step(
-                    "Verify that these prefixes, originated on R1, are not"
-                    "received on R3 but received on R2"
-                )
-
-                result = verify_rib(
-                    tgen,
-                    addr_type,
-                    "r3",
-                    input_dict_4,
-                    next_hop=topo["routers"]["r1"]["links"]["r3"][addr_type].split("/")[
-                        0
-                    ],
-                    expected=False,
-                )
-                assert result is not True, (
-                    "Testcase {} : Failed \n "
-                    "Expected: Routes are still present in rib of r3 \n "
-                    "Found: {}".format(tc_name, result)
-                )
+        step(
+            "Verify that these prefixes, originated on R1, are not"
+            "received on R3 but received on R2"
+        )
+        result = verify_rib(
+            tgen,
+            addr_type,
+            "r3",
+            input_dict_4,
+            next_hop=topo["routers"]["r1"]["links"]["r3"][addr_type].split("/")[0],
+            expected=False,
+        )
+        assert result is not True, (
+            "Testcase {} : Failed \n "
+            "Expected: Routes are still present in rib of r3 \n "
+            "Found: {}".format(tc_name, result)
+        )
 
         step("Remove route-map from redistribute static on R1")
         input_dict_2 = {