bgpd: Enable `enforce-first-as` by default

It's been for a while disabled by default, but this seems reasonable to flip it.

We had `bgp enforce-first-as` as a global BGP knob to enable/disable this
behavior globally, later we introduced `enforce-first-as` per neighbor, with disabled
by default. Now let's enable this by default by bringing a global `bgp enforce-first-as`
command back.

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

diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index 5d6ae589facec5756628c981894e9221c6965182..ae2d586de59a9e657f2d5a0d049b8d834318f352 100644 (file)
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -122,6 +122,10 @@ FRR_CFG_DEFAULT_BOOL(BGP_SOFT_VERSION_CAPABILITY,
        { .val_bool = true, .match_profile = "datacenter", },
        { .val_bool = false },
 );
+FRR_CFG_DEFAULT_BOOL(BGP_ENFORCE_FIRST_AS,
+       { .val_bool = false, .match_version = "< 9.1", },
+       { .val_bool = true },
+);
 
 DEFINE_HOOK(bgp_inst_config_write,
                (struct bgp *bgp, struct vty *vty),
@@ -615,6 +619,8 @@ int bgp_get_vty(struct bgp **bgp, as_t *as, const char *name,
                if (DFLT_BGP_SOFT_VERSION_CAPABILITY)
                        SET_FLAG((*bgp)->flags,
                                 BGP_FLAG_SOFT_VERSION_CAPABILITY);
+               if (DFLT_BGP_ENFORCE_FIRST_AS)
+                       SET_FLAG((*bgp)->flags, BGP_FLAG_ENFORCE_FIRST_AS);
 
                ret = BGP_SUCCESS;
        }
@@ -2828,6 +2834,23 @@ DEFUN(no_bgp_ebgp_requires_policy, no_bgp_ebgp_requires_policy_cmd,
        return CMD_SUCCESS;
 }
 
+DEFPY(bgp_enforce_first_as,
+      bgp_enforce_first_as_cmd,
+      "[no] bgp enforce-first-as",
+      NO_STR
+      BGP_STR
+      "Enforce the first AS for EBGP routes\n")
+{
+       VTY_DECLVAR_CONTEXT(bgp, bgp);
+
+       if (no)
+               UNSET_FLAG(bgp->flags, BGP_FLAG_ENFORCE_FIRST_AS);
+       else
+               SET_FLAG(bgp->flags, BGP_FLAG_ENFORCE_FIRST_AS);
+
+       return CMD_SUCCESS;
+}
+
 DEFPY(bgp_lu_uses_explicit_null, bgp_lu_uses_explicit_null_cmd,
       "[no] bgp labeled-unicast <explicit-null|ipv4-explicit-null|ipv6-explicit-null>$value",
       NO_STR BGP_STR
@@ -17973,8 +17996,13 @@ static void bgp_config_write_peer_global(struct vty *vty, struct bgp *bgp,
                        addr);
 
        /* enforce-first-as */
-       if (peergroup_flag_check(peer, PEER_FLAG_ENFORCE_FIRST_AS))
-               vty_out(vty, " neighbor %s enforce-first-as\n", addr);
+       if (CHECK_FLAG(bgp->flags, BGP_FLAG_ENFORCE_FIRST_AS)) {
+               if (!peergroup_flag_check(peer, PEER_FLAG_ENFORCE_FIRST_AS))
+                       vty_out(vty, " no neighbor %s enforce-first-as\n", addr);
+       } else {
+               if (peergroup_flag_check(peer, PEER_FLAG_ENFORCE_FIRST_AS))
+                       vty_out(vty, " neighbor %s enforce-first-as\n", addr);
+       }
 
        /* update-source */
        if (peergroup_flag_check(peer, PEER_FLAG_UPDATE_SOURCE)) {
@@ -18599,6 +18627,15 @@ int bgp_config_write(struct vty *vty)
                                        ? ""
                                        : "no ");
 
+               /* bgp enforce-first-as */
+               if (!!CHECK_FLAG(bgp->flags, BGP_FLAG_ENFORCE_FIRST_AS) !=
+                   SAVE_BGP_ENFORCE_FIRST_AS)
+                       vty_out(vty, " %sbgp enforce-first-as\n",
+                               CHECK_FLAG(bgp->flags,
+                                          BGP_FLAG_ENFORCE_FIRST_AS)
+                                       ? ""
+                                       : "no ");
+
                if (!!CHECK_FLAG(bgp->flags, BGP_FLAG_LU_IPV4_EXPLICIT_NULL) &&
                    !!CHECK_FLAG(bgp->flags, BGP_FLAG_LU_IPV6_EXPLICIT_NULL))
                        vty_out(vty, " bgp labeled-unicast explicit-null\n");
@@ -19594,6 +19631,9 @@ void bgp_vty_init(void)
        install_element(BGP_NODE, &bgp_ebgp_requires_policy_cmd);
        install_element(BGP_NODE, &no_bgp_ebgp_requires_policy_cmd);
 
+       /* bgp enforce-first-as */
+       install_element(BGP_NODE, &bgp_enforce_first_as_cmd);
+
        /* bgp labeled-unicast explicit-null */
        install_element(BGP_NODE, &bgp_lu_uses_explicit_null_cmd);
 

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 0a01d719687f8a5017801206f6e03a5e39e7fcf0..6ca0b06450161822af8062e65a6a52a6804275d1 100644 (file)
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -1919,6 +1919,9 @@ struct peer *peer_create(union sockunion *su, const char *conf_if,
                }
        }
 
+       if (CHECK_FLAG(bgp->flags, BGP_FLAG_ENFORCE_FIRST_AS))
+               SET_FLAG(peer->flags, PEER_FLAG_ENFORCE_FIRST_AS);
+
        /* auto shutdown if configured */
        if (bgp->autoshutdown)
                peer_flag_set(peer, PEER_FLAG_SHUTDOWN);

diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index 42e4c167f6b469fe38f6c5110efc61819e873b0f..bfaa2e5d7230ac3b193a1c329acd86fc824f096f 100644 (file)
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -516,6 +516,7 @@ struct bgp {
 /* For BGP-LU, force IPv6 local prefixes to use ipv6-explicit-null label */
 #define BGP_FLAG_LU_IPV6_EXPLICIT_NULL (1ULL << 34)
 #define BGP_FLAG_SOFT_VERSION_CAPABILITY (1ULL << 35)
+#define BGP_FLAG_ENFORCE_FIRST_AS (1ULL << 36)
 
        /* BGP default address-families.
         * New peers inherit enabled afi/safis from bgp instance.

diff --git a/doc/user/bgp.rst b/doc/user/bgp.rst
index 43572be07e404ede574284b9a18d4fd137a992f9..f7203a599aff832b3624d5b714c0c1a2399e98d8 100644 (file)
--- a/doc/user/bgp.rst
+++ b/doc/user/bgp.rst
@@ -527,6 +527,27 @@ Reject routes with AS_SET or AS_CONFED_SET types
 
    This command enables rejection of incoming and outgoing routes having AS_SET or AS_CONFED_SET type.
 
+Enforce first AS
+----------------
+
+.. clicmd:: bgp enforce-first-as
+
+   To configure a router to deny an update received from an external BGP (eBGP)
+   peer that does not list its autonomous system number at the beginning of
+   the `AS_PATH` in the incoming update, use the ``bgp enforce-first-as`` command
+   in router configuration mode.
+
+   In order to exclude an arbitrary neighbor from this enforcement, use the
+   command ``no neighbor NAME enforce-first-as``. And vice-versa if a global
+   enforcement is disabled, you can override this behavior per neighbor too.
+
+   Default: enabled.
+
+.. note::
+
+   If you have a peering to RS (Route-Server), most likely you MUST disable the
+   first AS enforcement.
+
 Suppress duplicate updates
 --------------------------
 
@@ -1526,7 +1547,10 @@ Configuring Peers
    Discard updates received from the specified (eBGP) peer if the AS_PATH
    attribute does not contain the PEER's ASN as the first AS_PATH segment.
 
-   Default: disabled.
+   You can enable or disable this enforcement globally too using
+   ``bgp enforce-first-as`` command.
+
+   Default: enabled.
 
 .. clicmd:: neighbor PEER extended-optional-parameters
 

diff --git a/tests/bgpd/test_peer_attr.c b/tests/bgpd/test_peer_attr.c
index bc6eba9069055bc44ae24a20d0bf6169470ee41a..dd154e34297f94a0d614c6661d219a32d79072d6 100644 (file)
--- a/tests/bgpd/test_peer_attr.c
+++ b/tests/bgpd/test_peer_attr.c
@@ -296,11 +296,6 @@ static struct test_peer_attr test_peer_attrs[] = {
                .u.flag = PEER_FLAG_DONT_CAPABILITY,
                .type = PEER_AT_GLOBAL_FLAG,
        },
-       {
-               .cmd = "enforce-first-as",
-               .u.flag = PEER_FLAG_ENFORCE_FIRST_AS,
-               .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 eb57618434960a17e2a61014447f62e26456eb56..bd8b06e2f0526204cd235556f00e5331bcade6a1 100644 (file)
--- a/tests/bgpd/test_peer_attr.py
+++ b/tests/bgpd/test_peer_attr.py
@@ -15,7 +15,6 @@ 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\\enforce-first-as")
 TestFlag.okfail("peer\\local-as")
 TestFlag.okfail("peer\\local-as 1 no-prepend")
 TestFlag.okfail("peer\\local-as 1 no-prepend replace-as")

diff --git a/yang/frr-bgp-neighbor.yang b/yang/frr-bgp-neighbor.yang
index 5a4c37974f40565c46ef12639a2a2dced0da5557..b199ab94694bda8e3bc9850fab6b06a06afe7e2d 100644 (file)
--- a/yang/frr-bgp-neighbor.yang
+++ b/yang/frr-bgp-neighbor.yang
@@ -76,7 +76,7 @@ submodule frr-bgp-neighbor {
 
     leaf enforce-first-as {
       type boolean;
-      default "false";
+      default "true";
       description
         "When set to 'true' it will enforce the first AS for EBGP routes.";
     }