Merge pull request #13090 from pguibert6WIND/explicit_null

bgpd: add cli command to control explicit-null label usage

9 files changed, 293 insertions, 12 deletions

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index a23bffd4ae..b49206adce 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -3070,7 +3070,9 @@ static void bgp_process_evpn_route_injection(struct bgp *bgp, afi_t afi,
  * the IMPLICIT_NULL label. This is pretty specialized: it's only called
  * in a path where we basically _know_ this is a BGP-LU route.
  */
-static bool bgp_lu_need_imp_null(const struct bgp_path_info *new_select)
+static bool bgp_lu_need_null_label(struct bgp *bgp,
+				   const struct bgp_path_info *new_select,
+				   afi_t afi, mpls_label_t *label)
 {
 	/* Certain types get imp null; so do paths where the nexthop is
 	 * not labeled.
@@ -3079,12 +3081,20 @@ static bool bgp_lu_need_imp_null(const struct bgp_path_info *new_select)
 	    || new_select->sub_type == BGP_ROUTE_AGGREGATE
 	    || new_select->sub_type == BGP_ROUTE_REDISTRIBUTE)
 		return true;
-	else if (new_select->extra == NULL ||
-		 !bgp_is_valid_label(&new_select->extra->label[0]))
-		/* TODO -- should be configurable? */
+	else if (new_select->extra &&
+		 bgp_is_valid_label(&new_select->extra->label[0]))
+		return false;
+	if (label == NULL)
 		return true;
+	if (!!CHECK_FLAG(bgp->flags, BGP_FLAG_LU_EXPLICIT_NULL))
+		/* Disable PHP : explicit-null */
+		*label = afi == AFI_IP ? MPLS_LABEL_IPV4_EXPLICIT_NULL
+				       : MPLS_LABEL_IPV6_EXPLICIT_NULL;
 	else
-		return false;
+		/* Enforced PHP popping: implicit-null */
+		*label = MPLS_LABEL_IMPLICIT_NULL;
+
+	return true;
 }
 
 /*
@@ -3113,6 +3123,7 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest,
 	struct bgp_path_info *old_select;
 	struct bgp_path_info_pair old_and_new;
 	int debug = 0;
+	mpls_label_t mpls_label_null;
 
 	if (CHECK_FLAG(bgp->flags, BGP_FLAG_DELETE_IN_PROGRESS)) {
 		if (dest)
@@ -3167,7 +3178,7 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest,
 	 * Right now, since we only deal with per-prefix labels, it is not
 	 * necessary to do this upon changes to best path. Exceptions:
 	 * - label index has changed -> recalculate resulting label
-	 * - path_info sub_type changed -> switch to/from implicit-null
+	 * - path_info sub_type changed -> switch to/from null label value
 	 * - no valid label (due to removed static label binding) -> get new one
 	 */
 	if (bgp->allocate_mpls_labels[afi][safi]) {
@@ -3176,11 +3187,12 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest,
 			    || bgp_label_index_differs(new_select, old_select)
 			    || new_select->sub_type != old_select->sub_type
 			    || !bgp_is_valid_label(&dest->local_label)) {
-				/* Enforced penultimate hop popping:
-				 * implicit-null for local routes, aggregate
-				 * and redistributed routes
+				/* control label imposition for local routes,
+				 * aggregate and redistributed routes
 				 */
-				if (bgp_lu_need_imp_null(new_select)) {
+				mpls_label_null = MPLS_LABEL_IMPLICIT_NULL;
+				if (bgp_lu_need_null_label(bgp, new_select, afi,
+							   &mpls_label_null)) {
 					if (CHECK_FLAG(
 						    dest->flags,
 						    BGP_NODE_REGISTERED_FOR_LABEL)
@@ -3189,8 +3201,7 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest,
 						    BGP_NODE_LABEL_REQUESTED))
 						bgp_unregister_for_label(dest);
 					dest->local_label = mpls_lse_encode(
-						MPLS_LABEL_IMPLICIT_NULL, 0, 0,
-						1);
+						mpls_label_null, 0, 0, 1);
 					bgp_set_valid_label(&dest->local_label);
 				} else
 					bgp_register_for_label(dest,
diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index f7b0caf493..de781d6b1e 100644
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -2820,6 +2820,21 @@ DEFUN(no_bgp_ebgp_requires_policy, no_bgp_ebgp_requires_policy_cmd,
 	return CMD_SUCCESS;
 }
 
+DEFPY(bgp_lu_uses_explicit_null, bgp_lu_uses_explicit_null_cmd,
+      "[no] bgp labeled-unicast explicit-null",
+      NO_STR BGP_STR
+      "BGP Labeled-unicast options\n"
+      "Use explicit-null label values for local prefixes\n")
+{
+	VTY_DECLVAR_CONTEXT(bgp, bgp);
+
+	if (no)
+		UNSET_FLAG(bgp->flags, BGP_FLAG_LU_EXPLICIT_NULL);
+	else
+		SET_FLAG(bgp->flags, BGP_FLAG_LU_EXPLICIT_NULL);
+	return CMD_SUCCESS;
+}
+
 DEFUN(bgp_suppress_duplicates, bgp_suppress_duplicates_cmd,
       "bgp suppress-duplicates",
       BGP_STR
@@ -18235,6 +18250,9 @@ int bgp_config_write(struct vty *vty)
 					? ""
 					: "no ");
 
+		if (!!CHECK_FLAG(bgp->flags, BGP_FLAG_LU_EXPLICIT_NULL))
+			vty_out(vty, " bgp labeled-unicast explicit-null\n");
+
 		/* draft-ietf-idr-deprecate-as-set-confed-set */
 		if (bgp->reject_as_sets)
 			vty_out(vty, " bgp reject-as-sets\n");
@@ -19154,6 +19172,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 labeled-unicast explicit-null */
+	install_element(BGP_NODE, &bgp_lu_uses_explicit_null_cmd);
+
 	/* bgp suppress-duplicates */
 	install_element(BGP_NODE, &bgp_suppress_duplicates_cmd);
 	install_element(BGP_NODE, &no_bgp_suppress_duplicates_cmd);
diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index a08a2870ee..b6491bf799 100644
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -500,6 +500,8 @@ struct bgp {
 #define BGP_FLAG_HARD_ADMIN_RESET (1ULL << 31)
 /* Evaluate the AIGP attribute during the best path selection process */
 #define BGP_FLAG_COMPARE_AIGP (1ULL << 32)
+/* For BGP-LU, force local prefixes to use explicit-null label */
+#define BGP_FLAG_LU_EXPLICIT_NULL (1ULL << 33)
 
 	/* 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 946f0699f2..97d7ce6b75 100644
--- a/doc/user/bgp.rst
+++ b/doc/user/bgp.rst
@@ -2767,6 +2767,17 @@ happened automatically if local-role is set.
    value of his role (by setting local-role on his side). Otherwise, a Role
    Mismatch Notification will be sent.
 
+Labeled unicast
+---------------
+
+*bgpd* supports labeled information, as per :rfc:`3107`.
+
+.. clicmd:: bgp labeled-unicast explicit-null
+
+By default, locally advertised prefixes use the `implicit-null` label to
+encode in the outgoing NLRI. The following command uses the `explicit-null`
+label value for all the BGP instances.
+
 .. _bgp-l3vpn-vrfs:
 
 L3VPN VRFs
diff --git a/tests/topotests/bgp_lu_explicitnull/r1/bgpd.conf b/tests/topotests/bgp_lu_explicitnull/r1/bgpd.conf
new file mode 100644
index 0000000000..a31439c984
--- /dev/null
+++ b/tests/topotests/bgp_lu_explicitnull/r1/bgpd.conf
@@ -0,0 +1,15 @@
+router bgp 65500
+ bgp router-id 192.0.2.1
+ no bgp ebgp-requires-policy
+ bgp labeled-unicast explicit-null
+ neighbor 192.0.2.2 remote-as 65501
+!
+ address-family ipv4 unicast
+  no neighbor 192.0.2.2 activate
+  network 192.168.2.1/32
+ exit-address-family
+ !
+ address-family ipv4 labeled-unicast
+  neighbor 192.0.2.2 activate
+ exit-address-family
+!
diff --git a/tests/topotests/bgp_lu_explicitnull/r1/zebra.conf b/tests/topotests/bgp_lu_explicitnull/r1/zebra.conf
new file mode 100644
index 0000000000..b84574891e
--- /dev/null
+++ b/tests/topotests/bgp_lu_explicitnull/r1/zebra.conf
@@ -0,0 +1,6 @@
+interface r1-eth0
+ ip address 192.0.2.1/24
+!
+interface r1-eth1
+ ip address 192.168.2.1/32
+!
\ No newline at end of file
diff --git a/tests/topotests/bgp_lu_explicitnull/r2/bgpd.conf b/tests/topotests/bgp_lu_explicitnull/r2/bgpd.conf
new file mode 100644
index 0000000000..41c2b9b6fa
--- /dev/null
+++ b/tests/topotests/bgp_lu_explicitnull/r2/bgpd.conf
@@ -0,0 +1,15 @@
+router bgp 65501
+ bgp router-id 192.0.2.2
+ no bgp ebgp-requires-policy
+ bgp labeled-unicast explicit-null
+ neighbor 192.0.2.1 remote-as 65500
+!
+ address-family ipv4 unicast
+  no neighbor 192.0.2.1 activate
+  network 192.168.2.2/32
+ exit-address-family
+ !
+ address-family ipv4 labeled-unicast
+  neighbor 192.0.2.1 activate
+ exit-address-family
+!
diff --git a/tests/topotests/bgp_lu_explicitnull/r2/zebra.conf b/tests/topotests/bgp_lu_explicitnull/r2/zebra.conf
new file mode 100644
index 0000000000..9a639610c1
--- /dev/null
+++ b/tests/topotests/bgp_lu_explicitnull/r2/zebra.conf
@@ -0,0 +1,6 @@
+interface r2-eth0
+ ip address 192.0.2.2/24
+!
+interface r2-eth1
+ ip address 192.168.2.2/32
+!
diff --git a/tests/topotests/bgp_lu_explicitnull/test_bgp_lu_explicitnull.py b/tests/topotests/bgp_lu_explicitnull/test_bgp_lu_explicitnull.py
new file mode 100644
index 0000000000..d53ac68e84
--- /dev/null
+++ b/tests/topotests/bgp_lu_explicitnull/test_bgp_lu_explicitnull.py
@@ -0,0 +1,194 @@
+#!/usr/bin/env python
+# SPDX-License-Identifier: ISC
+#
+# test_bgp_explicitnull.py
+#
+# Part of NetDEF Topology Tests
+#
+# Copyright 2023 by 6WIND S.A.
+#
+
+"""
+test_bgp_lu_explicitnull.py: Test BGP LU label allocation
+"""
+
+import os
+import sys
+import json
+import functools
+import pytest
+
+# Save the Current Working Directory to find configuration files.
+CWD = os.path.dirname(os.path.realpath(__file__))
+sys.path.append(os.path.join(CWD, "../"))
+
+# pylint: disable=C0413
+# Import topogen and topotest helpers
+from lib import topotest
+from lib.topogen import Topogen, TopoRouter, get_topogen
+from lib.topolog import logger
+
+
+pytestmark = [pytest.mark.bgpd]
+
+
+# Basic scenario for BGP-LU. Nodes are directly connected.
+# The 192.168.2.2/32 prefix is advertised from r2 to r1
+# The explicit-null label should be used
+# The 192.168.2.1/32 prefix is advertised from r1 to r2
+# The explicit-null label should be used
+# Traffic from 192.168.2.1 to 192.168.2.2 should use explicit-null label
+#
+#  AS65500    BGP-LU        AS65501
+# +-----+                +-----+
+# |     |.1            .2|     |
+# |  1  +----------------+  2  + 192.168.0.2/32
+# |     |   192.0.2.0/24 |     |
+# +-----+                +-----+
+
+
+def build_topo(tgen):
+    "Build function"
+
+    # Create routers
+    tgen.add_router("r1")
+    tgen.add_router("r2")
+
+    # r1-r2
+    switch = tgen.add_switch("s1")
+    switch.add_link(tgen.gears["r1"])
+    switch.add_link(tgen.gears["r2"])
+
+    # r1
+    switch = tgen.add_switch("s2")
+    switch.add_link(tgen.gears["r1"])
+
+    # r2
+    switch = tgen.add_switch("s3")
+    switch.add_link(tgen.gears["r2"])
+
+
+def setup_module(mod):
+    "Sets up the pytest environment"
+    # This function initiates the topology build with Topogen...
+    tgen = Topogen(build_topo, mod.__name__)
+
+    # Skip if no mpls support
+    if not tgen.hasmpls:
+        logger.info("MPLS is not available, skipping test")
+        pytest.skip("MPLS is not available, skipping")
+        return
+
+    # ... and here it calls Mininet initialization functions.
+    tgen.start_topology()
+
+    # This is a sample of configuration loading.
+    router_list = tgen.routers()
+
+    # Enable mpls input for routers, so we can ping
+    sval = "net.mpls.conf.{}.input"
+    topotest.sysctl_assure(router_list["r2"], sval.format("r2-eth0"), 1)
+    topotest.sysctl_assure(router_list["r1"], sval.format("r1-eth0"), 1)
+
+    # For all registred routers, load the zebra configuration file
+    for rname, router in router_list.items():
+        router.load_config(
+            TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname))
+        )
+        router.load_config(
+            TopoRouter.RD_BGP, os.path.join(CWD, "{}/bgpd.conf".format(rname))
+        )
+
+    # After loading the configurations, this function loads configured daemons.
+    tgen.start_router()
+
+
+def teardown_module(mod):
+    "Teardown the pytest environment"
+    tgen = get_topogen()
+
+    # This function tears down the whole topology.
+    tgen.stop_topology()
+
+
+def check_show_ip_label_prefix_found(router, ipversion, prefix, label):
+    output = json.loads(
+        router.vtysh_cmd("show {} route {} json".format(ipversion, prefix))
+    )
+    expected = {prefix: [{"prefix": prefix, "nexthops": [{"fib": True, "labels": [label]}]}]}
+    ret = topotest.json_cmp(output, expected)
+    if ret is None:
+        return "not good"
+    return None
+
+def test_converge_bgplu():
+    "Wait for protocol convergence"
+
+    tgen = get_topogen()
+    # Don't run this test if we have any failure.
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    # tgen.mininet_cli();
+    r1 = tgen.gears["r1"]
+    r2 = tgen.gears["r2"]
+    # Check r1 gets prefix 192.168.2.2/32
+    test_func = functools.partial(
+        check_show_ip_label_prefix_found,
+        tgen.gears["r1"],
+        "ip",
+        "192.168.2.2/32",
+        "0",
+    )
+    success, result = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
+    assert (
+        success
+    ), "r1, prefix 192.168.2.2/32 from r2 not present"
+
+    # Check r2 gets prefix 192.168.2.1/32
+    test_func = functools.partial(
+        check_show_ip_label_prefix_found,
+        tgen.gears["r2"],
+        "ip",
+        "192.168.2.1/32",
+        "0",
+    )
+    success, result = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
+    assert (
+        success
+    ), "r2, prefix 192.168.2.1/32 from r1 not present"
+
+def test_traffic_connectivity():
+    "Wait for protocol convergence"
+
+    tgen = get_topogen()
+    # Don't run this test if we have any failure.
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    def _check_ping(name, dest_addr, src_addr):
+        tgen = get_topogen()
+        output = tgen.gears[name].run("ping {} -c 1 -w 1 -I {}".format(dest_addr, src_addr))
+        logger.info(output)
+        if " 0% packet loss" not in output:
+            return True
+
+    logger.info("r1, check ping 192.168.2.2 from 192.168.2.1 is OK")
+    tgen = get_topogen()
+    func = functools.partial(_check_ping, "r1", "192.168.2.2", "192.168.2.1")
+    # tgen.mininet_cli()
+    success, result = topotest.run_and_expect(func, None, count=10, wait=0.5)
+    assert result is None, "r1, ping to 192.168.2.2 from 192.168.2.1 fails"
+
+def test_memory_leak():
+    "Run the memory leak test and report results."
+    tgen = get_topogen()
+    if not tgen.is_memleak_enabled():
+        pytest.skip("Memory leak test/report is disabled")
+
+    tgen.report_memory_leaks()
+
+
+if __name__ == "__main__":
+    args = ["-s"] + sys.argv[1:]
+    sys.exit(pytest.main(args))