Merge pull request #12132 from FRRouting/mergify/bp/stable/8.3/pr-12113

bgpd: Allow `network XXX` to work with bgp suppress-fib-pending (backport #12113)
FRRouting · Oct 14, 2022 · b9cb689 · b9cb689
2 parents 98dc6d6 + 02e6ede
commit b9cb689
Show file tree

Hide file tree

Showing 7 changed files with 59 additions and 7 deletions.
diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h
@@ -597,19 +597,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
@@ -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
@@ -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
@@ -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
diff --git 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
@@ -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
@@ -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"