bgpd: Allow `network XXX` to work with bgp suppress-fib-pending

When bgp is using `bgp suppress-fib-pending` and the end
operator is using network statements, bgp was not sending
the network'ed prefix'es to it's peers.  Fix this.

Also update the test cases for bgp_suppress_fib to test
this new corner case( I am sure that there are going to
be others that will need to be added ).

Fixes: #12112
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
(cherry picked from commit 4801fc4670020406fc609dedabc7482d88e3b656)

diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h
index a8ec2dc907cae45d7995dad56ed87efdf95d34ec..003a6cb799cf9bbb24552da8554fc6bd232bb490 100644 (file)
--- a/bgpd/bgp_route.h
+++ b/bgpd/bgp_route.h
@@ -595,19 +595,35 @@ static inline bool bgp_check_advertise(struct bgp *bgp, struct bgp_dest *dest)
  */
 static inline bool bgp_check_withdrawal(struct bgp *bgp, struct bgp_dest *dest)
 {
-       struct bgp_path_info *pi;
+       struct bgp_path_info *pi, *selected = NULL;
 
        if (!BGP_SUPPRESS_FIB_ENABLED(bgp))
                return false;
 
        for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next) {
-               if (CHECK_FLAG(pi->flags, BGP_PATH_SELECTED))
+               if (CHECK_FLAG(pi->flags, BGP_PATH_SELECTED)) {
+                       selected = pi;
                        continue;
+               }
 
                if (pi->sub_type != BGP_ROUTE_NORMAL)
                        return true;
        }
 
+       /*
+        * pi is selected and bgp is dealing with a static route
+        * ( ie a network statement of some sort ).  FIB installed
+        * is irrelevant
+        *
+        * I am not sure what the above for loop is wanted in this
+        * manner at this point.  But I do know that if I have
+        * a static route that is selected and it's the one
+        * being checked for should I withdrawal we do not
+        * want to withdraw the route on installation :)
+        */
+       if (selected && selected->sub_type == BGP_ROUTE_STATIC)
+               return false;
+
        if (CHECK_FLAG(dest->flags, BGP_NODE_FIB_INSTALLED))
                return false;
 

diff --git a/tests/topotests/bgp_suppress_fib/r1/bgp_ipv4_allowas.json b/tests/topotests/bgp_suppress_fib/r1/bgp_ipv4_allowas.json
index bc4d0f4796bc6539bea02807b16d4ae4fa82e255..1a5ede276d8e3e111dbcd2c7480520386b95593b 100644 (file)
--- a/tests/topotests/bgp_suppress_fib/r1/bgp_ipv4_allowas.json
+++ b/tests/topotests/bgp_suppress_fib/r1/bgp_ipv4_allowas.json
@@ -32,7 +32,7 @@
       ],
       "peer":{
         "peerId":"10.0.0.2",
-        "routerId":"10.0.0.9",
+        "routerId":"60.0.0.1",
         "type":"external"
       }
     }

diff --git a/tests/topotests/bgp_suppress_fib/r2/bgp_ipv4_allowas.json b/tests/topotests/bgp_suppress_fib/r2/bgp_ipv4_allowas.json
index 16561ce837f97c65014a3a87e8be145423e36c61..4a35abfd6fd57a83069e41198d930f748ed0e929 100644 (file)
--- a/tests/topotests/bgp_suppress_fib/r2/bgp_ipv4_allowas.json
+++ b/tests/topotests/bgp_suppress_fib/r2/bgp_ipv4_allowas.json
@@ -61,7 +61,7 @@
       ],
       "peer":{
         "peerId":"0.0.0.0",
-        "routerId":"10.0.0.9"
+        "routerId":"60.0.0.1"
       }
     }
   ]

diff --git a/tests/topotests/bgp_suppress_fib/r2/bgpd.conf b/tests/topotests/bgp_suppress_fib/r2/bgpd.conf
index ebef2012a800c76b2cfee3b984a6b231d9c6de32..010e86aad704a10ef0f86da60b1eaa857282d249 100644 (file)
--- a/tests/topotests/bgp_suppress_fib/r2/bgpd.conf
+++ b/tests/topotests/bgp_suppress_fib/r2/bgpd.conf
@@ -7,3 +7,5 @@ router bgp 2
   bgp suppress-fib-pending
   neighbor 10.0.0.1 remote-as 1
   neighbor 10.0.0.10 remote-as 3
+  address-family ipv4 uni
+     network 60.0.0.0/24
\ No newline at end of file

diff --git a/tests/topotests/bgp_suppress_fib/r2/zebra.conf b/tests/topotests/bgp_suppress_fib/r2/zebra.conf
index 443fffc7032923433b369745bee58c722c7bd09a..6e8bce0450ddaffc16fe07cf941f4e8fb3911180 100644 (file)
--- a/tests/topotests/bgp_suppress_fib/r2/zebra.conf
+++ b/tests/topotests/bgp_suppress_fib/r2/zebra.conf
@@ -1,4 +1,7 @@
 !
+interface lo
+ ip address 60.0.0.1/24
+!
 interface r2-eth0
  ip address 10.0.0.2/30
 !

diff --git a/tests/topotests/bgp_suppress_fib/r3/v4_route3.json b/tests/topotests/bgp_suppress_fib/r3/v4_route3.json
new file mode 100644 (file)
index 0000000..ab8c3aa
--- /dev/null
+++ b/tests/topotests/bgp_suppress_fib/r3/v4_route3.json
@@ -0,0 +1,23 @@
+{
+  "60.0.0.0/24":[
+    {
+      "prefix":"60.0.0.0/24",
+      "protocol":"bgp",
+      "selected":true,
+      "destSelected":true,
+      "distance":20,
+      "metric":0,
+      "installed":true,
+      "table":254,
+      "nexthops":[
+        {
+          "fib":true,
+          "ip":"10.0.0.9",
+          "afi":"ipv4",
+          "interfaceName":"r3-eth0",
+          "active":true
+        }
+      ]
+    }
+  ]
+}

diff --git a/tests/topotests/bgp_suppress_fib/test_bgp_suppress_fib.py b/tests/topotests/bgp_suppress_fib/test_bgp_suppress_fib.py
index 2c87d9d7b0f80b543e5e7372c531336f5d967396..96a294cae3453bc797c62d5678c5cbe3f62fefc8 100644 (file)
--- a/tests/topotests/bgp_suppress_fib/test_bgp_suppress_fib.py
+++ b/tests/topotests/bgp_suppress_fib/test_bgp_suppress_fib.py
@@ -84,8 +84,6 @@ def test_bgp_route():
 
     r3 = tgen.gears["r3"]
 
-    sleep(5)
-
     json_file = "{}/r3/v4_route.json".format(CWD)
     expected = json.loads(open(json_file).read())
 
@@ -95,7 +93,7 @@ def test_bgp_route():
         "show ip route 40.0.0.0 json",
         expected,
     )
-    _, result = topotest.run_and_expect(test_func, None, count=2, wait=0.5)
+    _, result = topotest.run_and_expect(test_func, None, count=20, wait=0.5)
     assertmsg = '"r3" JSON output mismatches'
     assert result is None, assertmsg
 
@@ -112,6 +110,16 @@ def test_bgp_route():
     assertmsg = '"r3" JSON output mismatches'
     assert result is None, assertmsg
 
+    json_file = "{}/r3/v4_route3.json".format(CWD)
+    expected = json.loads(open(json_file).read())
+
+    test_func = partial(
+        topotest.router_json_cmp,
+        r3,
+        "show ip route 10.0.0.3 json",
+        expected,
+        )
+    _, result = topotest.run_and_expect(test_func, None, count=3, wait=0.5)
 
 def test_bgp_better_admin_won():
     "A better Admin distance protocol may come along and knock us out"