Merge pull request #15351 from louis-6wind/fix-leak-vrf-interface

bgpd: update route leak after vrf interface changes

3 files changed, 159 insertions, 88 deletions

diff --git a/tests/topotests/bgp_vrf_route_leak_basic/r1/bgpd.conf b/tests/topotests/bgp_vrf_route_leak_basic/r1/bgpd.conf
index f52f56b0e0..397f7938d2 100644
--- a/tests/topotests/bgp_vrf_route_leak_basic/r1/bgpd.conf
+++ b/tests/topotests/bgp_vrf_route_leak_basic/r1/bgpd.conf
@@ -12,7 +12,7 @@ router bgp 99 vrf DONNA
   address-family ipv4 unicast
     redistribute connected
     import vrf EVA
-    import vrf NOTEXISTING
+    import vrf ZITA
     import vrf default
   !
 !
@@ -21,10 +21,10 @@ router bgp 99 vrf EVA
   address-family ipv4 unicast
     redistribute connected
     import vrf DONNA
-    import vrf NOTEXISTING
+    import vrf ZITA
   !
 !
-router bgp 99 vrf NOTEXISTING
+router bgp 99 vrf ZITA
   no bgp ebgp-requires-policy
   no bgp network import-check
   address-family ipv4 unicast
diff --git a/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py b/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py
index ef813e9541..013ddfece9 100644
--- a/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py
+++ b/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py
@@ -24,7 +24,7 @@ sys.path.append(os.path.join(CWD, "../"))
 from lib import topotest
 from lib.topogen import Topogen, TopoRouter, get_topogen
 from lib.topolog import logger
-
+from lib.checkping import check_ping
 
 pytestmark = [pytest.mark.bgpd]
 
@@ -126,11 +126,13 @@ def test_vrf_route_leak_donna():
         "172.16.101.0/24": [
             {
                 "protocol": "bgp",
+                "selected": None,
                 "nexthops": [
                     {
-                        "interfaceIndex": 0,
+                        "fib": None,
                         "interfaceName": "unknown",
                         "vrf": "Unknown",
+                        "active": None,
                     },
                 ],
             },
@@ -196,11 +198,13 @@ def test_vrf_route_leak_eva():
         "172.16.101.0/24": [
             {
                 "protocol": "bgp",
+                "selected": None,
                 "nexthops": [
                     {
-                        "interfaceIndex": 0,
+                        "fib": None,
                         "interfaceName": "unknown",
                         "vrf": "Unknown",
+                        "active": None,
                     },
                 ],
             },
@@ -214,7 +218,7 @@ def test_vrf_route_leak_eva():
     assert result, "BGP VRF EVA check failed:\n{}".format(diff)
 
 
-def test_vrf_route_leak_donna():
+def test_vrf_route_leak_default():
     logger.info("Ensure that routes are leaked back and forth")
     tgen = get_topogen()
     # Don't run this test if we have any failure.
@@ -223,37 +227,31 @@ def test_vrf_route_leak_donna():
 
     r1 = tgen.gears["r1"]
 
-    # Test DONNA VRF.
+    # Test default VRF.
     expect = {
         "10.0.0.0/24": [
             {
-                "protocol": "connected",
-            }
-        ],
-        "10.0.1.0/24": [
-            {
                 "protocol": "bgp",
                 "selected": True,
                 "nexthops": [
                     {
                         "fib": True,
-                        "interfaceName": "EVA",
-                        "vrf": "EVA",
+                        "interfaceName": "DONNA",
+                        "vrf": "DONNA",
                         "active": True,
                     },
                 ],
             },
         ],
-        "10.0.2.0/24": [{"protocol": "connected"}],
-        "10.0.3.0/24": [
+        "10.0.2.0/24": [
             {
                 "protocol": "bgp",
                 "selected": True,
                 "nexthops": [
                     {
                         "fib": True,
-                        "interfaceName": "EVA",
-                        "vrf": "EVA",
+                        "interfaceName": "DONNA",
+                        "vrf": "DONNA",
                         "active": True,
                     },
                 ],
@@ -261,26 +259,73 @@ def test_vrf_route_leak_donna():
         ],
         "10.0.4.0/24": [
             {
+                "protocol": "connected",
+            }
+        ],
+    }
+
+    test_func = partial(topotest.router_json_cmp, r1, "show ip route json", expect)
+    result, diff = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
+    assert result, "BGP VRF default check failed:\n{}".format(diff)
+
+
+def test_ping():
+    "Simple ping tests"
+
+    tgen = get_topogen()
+
+    # Don't run this test if we have any failure.
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    r1 = tgen.gears["r1"]
+
+    logger.info("Ping from default to DONNA")
+    check_ping("r1", "10.0.0.1", True, 10, 0.5, source_addr="10.0.4.1")
+
+
+def test_vrf_route_leak_donna_after_eva_down():
+    logger.info("Ensure that route states change after EVA interface goes down")
+    tgen = get_topogen()
+    # Don't run this test if we have any failure.
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    r1 = tgen.gears["r1"]
+    r1.vtysh_cmd(
+        """
+configure
+interface EVA
+ shutdown
+"""
+    )
+
+    # Test DONNA VRF.
+    expect = {
+        "10.0.1.0/24": [
+            {
                 "protocol": "bgp",
-                "selected": True,
+                "selected": None,
                 "nexthops": [
                     {
-                        "fib": True,
-                        "interfaceName": "lo",
-                        "vrf": "default",
-                        "active": True,
+                        "fib": None,
+                        "interfaceName": "EVA",
+                        "vrf": "EVA",
+                        "active": None,
                     },
                 ],
             },
         ],
-        "172.16.101.0/24": [
+        "10.0.3.0/24": [
             {
                 "protocol": "bgp",
+                "selected": None,
                 "nexthops": [
                     {
-                        "interfaceIndex": 0,
-                        "interfaceName": "unknown",
-                        "vrf": "Unknown",
+                        "fib": None,
+                        "interfaceName": "EVA",
+                        "vrf": "EVA",
+                        "active": None,
                     },
                 ],
             },
@@ -294,135 +339,157 @@ def test_vrf_route_leak_donna():
     assert result, "BGP VRF DONNA check failed:\n{}".format(diff)
 
 
-def test_vrf_route_leak_eva():
-    logger.info("Ensure that routes are leaked back and forth")
+def test_vrf_route_leak_donna_after_eva_up():
+    logger.info("Ensure that route states change after EVA interface goes up")
     tgen = get_topogen()
     # Don't run this test if we have any failure.
     if tgen.routers_have_failure():
         pytest.skip(tgen.errors)
 
     r1 = tgen.gears["r1"]
+    r1.vtysh_cmd(
+        """
+configure
+interface EVA
+ no shutdown
+"""
+    )
 
-    # Test EVA VRF.
+    # Test DONNA VRF.
     expect = {
-        "10.0.0.0/24": [
+        "10.0.1.0/24": [
             {
                 "protocol": "bgp",
                 "selected": True,
                 "nexthops": [
                     {
                         "fib": True,
-                        "interfaceName": "DONNA",
-                        "vrf": "DONNA",
+                        "interfaceName": "EVA",
+                        "vrf": "EVA",
                         "active": True,
                     },
                 ],
             },
         ],
-        "10.0.1.0/24": [
-            {
-                "protocol": "connected",
-            }
-        ],
-        "10.0.2.0/24": [
+        "10.0.3.0/24": [
             {
                 "protocol": "bgp",
                 "selected": True,
                 "nexthops": [
                     {
                         "fib": True,
-                        "interfaceName": "DONNA",
-                        "vrf": "DONNA",
+                        "interfaceName": "EVA",
+                        "vrf": "EVA",
                         "active": True,
                     },
                 ],
             },
         ],
-        "10.0.3.0/24": [
-            {
-                "protocol": "connected",
-            }
-        ],
+    }
+
+    test_func = partial(
+        topotest.router_json_cmp, r1, "show ip route vrf DONNA json", expect
+    )
+    result, diff = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
+    assert result, "BGP VRF DONNA check failed:\n{}".format(diff)
+
+
+def test_vrf_route_leak_donna_add_vrf_zita():
+    logger.info("Add VRF ZITA and ensure that the route from VRF ZITA is updated")
+    tgen = get_topogen()
+    # Don't run this test if we have any failure.
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    r1 = tgen.gears["r1"]
+    r1.cmd("ip link add ZITA type vrf table 1003")
+
+    # Test DONNA VRF.
+    expect = {
         "172.16.101.0/24": [
             {
                 "protocol": "bgp",
+                "selected": None,
                 "nexthops": [
                     {
-                        "interfaceIndex": 0,
-                        "interfaceName": "unknown",
-                        "vrf": "Unknown",
+                        "fib": None,
+                        "interfaceName": "ZITA",
+                        "vrf": "ZITA",
+                        "active": None,
                     },
                 ],
             },
         ],
     }
 
+    test_func = partial(
+        topotest.router_json_cmp, r1, "show ip route vrf DONNA json", expect
+    )
+    result, diff = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
+    assert result, "BGP VRF DONNA check failed:\n{}".format(diff)
+
 
-def test_vrf_route_leak_default():
-    logger.info("Ensure that routes are leaked back and forth")
+def test_vrf_route_leak_donna_set_zita_up():
+    logger.info("Set VRF ZITA up and ensure that the route from VRF ZITA is updated")
     tgen = get_topogen()
     # Don't run this test if we have any failure.
     if tgen.routers_have_failure():
         pytest.skip(tgen.errors)
 
     r1 = tgen.gears["r1"]
+    r1.vtysh_cmd(
+        """
+configure
+interface ZITA
+ no shutdown
+"""
+    )
 
-    # Test default VRF.
+    # Test DONNA VRF.
     expect = {
-        "10.0.0.0/24": [
-            {
-                "protocol": "bgp",
-                "selected": True,
-                "nexthops": [
-                    {
-                        "fib": True,
-                        "interfaceName": "DONNA",
-                        "vrf": "DONNA",
-                        "active": True,
-                    },
-                ],
-            },
-        ],
-        "10.0.2.0/24": [
+        "172.16.101.0/24": [
             {
                 "protocol": "bgp",
                 "selected": True,
                 "nexthops": [
                     {
                         "fib": True,
-                        "interfaceName": "DONNA",
-                        "vrf": "DONNA",
+                        "interfaceName": "ZITA",
+                        "vrf": "ZITA",
                         "active": True,
                     },
                 ],
             },
         ],
-        "10.0.4.0/24": [
-            {
-                "protocol": "connected",
-            }
-        ],
     }
 
-    test_func = partial(topotest.router_json_cmp, r1, "show ip route json", expect)
+    test_func = partial(
+        topotest.router_json_cmp, r1, "show ip route vrf DONNA json", expect
+    )
     result, diff = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
-    assert result, "BGP VRF default check failed:\n{}".format(diff)
+    assert result, "BGP VRF DONNA check failed:\n{}".format(diff)
 
 
-def test_ping():
-    "Simple ping tests"
-
+def test_vrf_route_leak_donna_delete_vrf_zita():
+    logger.info("Delete VRF ZITA and ensure that the route from VRF ZITA is deleted")
     tgen = get_topogen()
-
     # Don't run this test if we have any failure.
     if tgen.routers_have_failure():
         pytest.skip(tgen.errors)
 
     r1 = tgen.gears["r1"]
+    r1.cmd("ip link delete ZITA")
 
-    logger.info("Ping from default to DONNA")
-    output = r1.run("ping -c 4 -w 4 -I 10.0.4.1 10.0.0.1")
-    assert " 0% packet loss" in output, "Ping default->DONNA FAILED"
+    # Test DONNA VRF.
+    expect = {
+        "172.16.101.0/24": None,
+    }
+
+    test_func = partial(
+        topotest.router_json_cmp, r1, "show ip route vrf DONNA json", expect
+    )
+    result, diff = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
+    assert result, "BGP VRF DONNA check failed:\n{}".format(diff)
 
 
 def test_memory_leak():
diff --git a/tests/topotests/lib/checkping.py b/tests/topotests/lib/checkping.py
index aaa6164dd4..5500807fab 100644
--- a/tests/topotests/lib/checkping.py
+++ b/tests/topotests/lib/checkping.py
@@ -8,7 +8,7 @@ from lib.topolog import logger
 from lib import topotest
 
 
-def check_ping(name, dest_addr, expect_connected, count, wait):
+def check_ping(name, dest_addr, expect_connected, count, wait, source_addr=None):
     """
     Assert that ping to dest_addr is expected
     * 'name': the router to set the ping from
@@ -18,9 +18,13 @@ def check_ping(name, dest_addr, expect_connected, count, wait):
     * 'wait': how long ping should wait to receive all replies
     """
 
-    def _check(name, dest_addr, match):
+    def _check(name, dest_addr, source_addr, match):
         tgen = get_topogen()
-        output = tgen.gears[name].run("ping {} -c 1 -w 1".format(dest_addr))
+        cmd = "ping {}".format(dest_addr)
+        if source_addr:
+            cmd += " -I {}".format(source_addr)
+        cmd += " -c 1 -w 1"
+        output = tgen.gears[name].run(cmd)
         logger.info(output)
         if match not in output:
             return "ping fail"
@@ -28,6 +32,6 @@ def check_ping(name, dest_addr, expect_connected, count, wait):
     match = ", {} packet loss".format("0%" if expect_connected else "100%")
     logger.info("[+] check {} {} {}".format(name, dest_addr, match))
     tgen = get_topogen()
-    func = functools.partial(_check, name, dest_addr, match)
+    func = functools.partial(_check, name, dest_addr, source_addr, match)
     success, result = topotest.run_and_expect(func, None, count=count, wait=wait)
     assert result is None, "Failed"