tests: fix and adjust topotest/bgp_aigp

When there are multiple IGP domains (OSPF in this case), the nexthop for a BGP route with the AIGP attribute must be resolved to its own IGP domain. Fix the nexthop setting in topotest/bgp_aigp, and also adjust the test to refelct the correct IGP metric (post the fix for IGP metric calculation for recursive nexthops in NHT). Signed-off-by: Enke Chen <[email protected]>
FRRouting · Sep 27, 2024 · a14ca6f · a14ca6f
1 parent 6b9e141
commit a14ca6f
Show file tree

Hide file tree

Showing 10 changed files with 64 additions and 34 deletions.
diff --git a/tests/topotests/bgp_aigp/r1/ospfd.conf b/tests/topotests/bgp_aigp/r1/ospfd.conf
@@ -1,6 +1,6 @@
 !
 interface lo
- ip ospf cost 10
+ ip ospf passive
 !
 interface r1-eth0
  ip ospf dead-interval 4

diff --git a/tests/topotests/bgp_aigp/r2/ospfd.conf b/tests/topotests/bgp_aigp/r2/ospfd.conf
@@ -1,6 +1,6 @@
 !
 interface lo
- ip ospf cost 10
+ ip ospf passive
 !
 interface r2-eth0
  ip ospf dead-interval 4

diff --git a/tests/topotests/bgp_aigp/r3/ospfd.conf b/tests/topotests/bgp_aigp/r3/ospfd.conf
@@ -1,6 +1,6 @@
 !
 interface lo
- ip ospf cost 10
+ ip ospf passive
 !
 interface r3-eth0
  ip ospf dead-interval 4

diff --git a/tests/topotests/bgp_aigp/r4/bgpd.conf b/tests/topotests/bgp_aigp/r4/bgpd.conf
@@ -1,6 +1,7 @@
 router bgp 65001
  no bgp ebgp-requires-policy
  no bgp network import-check
+ bgp route-reflector allow-outbound-policy
  neighbor 192.168.24.2 remote-as internal
  neighbor 192.168.24.2 timers 1 3
  neighbor 192.168.24.2 timers connect 1
@@ -11,7 +12,18 @@ router bgp 65001
  neighbor 10.0.0.6 timers connect 1
  neighbor 10.0.0.6 update-source lo
  address-family ipv4
-  redistribute connected
-  redistribute ospf
+  redistribute connected route-map connected-to-bgp
+  neighbor 192.168.24.2 route-map set-nexthop out
  exit-address-family
 !
+! Two OSPF domains should be isolated - otherwise the connected routes
+! on r4 would be advertised to r3 (via r4 -> r6 -> r5 -> r3), and can
+! mess up bgp bestpath calculation (igp metrics for the BGP nexthops).
+!
+route-map connected-to-bgp permit 10
+ set community no-advertise
+!
+route-map set-nexthop permit 10
+ set ip next-hop peer-address
+exit
+!
diff --git a/tests/topotests/bgp_aigp/r4/ospfd.conf b/tests/topotests/bgp_aigp/r4/ospfd.conf
@@ -1,6 +1,6 @@
 !
 interface lo
- ip ospf cost 10
+ ip ospf passive
 !
 interface r4-eth1
  ip ospf dead-interval 4

diff --git a/tests/topotests/bgp_aigp/r5/bgpd.conf b/tests/topotests/bgp_aigp/r5/bgpd.conf
@@ -1,6 +1,7 @@
 router bgp 65001
  no bgp ebgp-requires-policy
  no bgp network import-check
+ bgp route-reflector allow-outbound-policy
  neighbor 192.168.35.3 remote-as internal
  neighbor 192.168.35.3 timers 1 3
  neighbor 192.168.35.3 timers connect 1
@@ -11,7 +12,18 @@ router bgp 65001
  neighbor 10.0.0.6 timers connect 1
  neighbor 10.0.0.6 update-source lo
  address-family ipv4
-  redistribute connected
-  redistribute ospf
+  redistribute connected route-map connected-to-bgp
+  neighbor 192.168.35.3 route-map set-nexthop out
  exit-address-family
 !
+! Two OSPF domains should be isolated - otherwise the connected routes
+! on r5 would be advertised to r2 (via r5 -> r6 -> r4 -> r2), and can
+! mess up bgp bestpath calculation (igp metrics for the BGP nexthops).
+!
+route-map connected-to-bgp permit 10
+ set community no-advertise
+!
+route-map set-nexthop permit 10
+ set ip next-hop peer-address
+exit
+!
diff --git a/tests/topotests/bgp_aigp/r5/ospfd.conf b/tests/topotests/bgp_aigp/r5/ospfd.conf
@@ -1,6 +1,6 @@
 !
 interface lo
- ip ospf cost 10
+ ip ospf passive
 !
 interface r5-eth1
  ip ospf dead-interval 4

diff --git a/tests/topotests/bgp_aigp/r6/bgpd.conf b/tests/topotests/bgp_aigp/r6/bgpd.conf
@@ -1,6 +1,7 @@
 router bgp 65001
  no bgp ebgp-requires-policy
  no bgp network import-check
+ bgp route-reflector allow-outbound-policy
  neighbor 10.0.0.4 remote-as internal
  neighbor 10.0.0.4 timers 1 3
  neighbor 10.0.0.4 timers connect 1
@@ -15,6 +16,11 @@ router bgp 65001
  neighbor 192.168.67.7 timers 1 3
  neighbor 192.168.67.7 timers connect 1
  address-family ipv4
-  redistribute ospf
+  neighbor 10.0.0.4 route-map set-nexthop out
+  neighbor 10.0.0.5 route-map set-nexthop out
  exit-address-family
 !
+route-map set-nexthop permit 10
+ set ip next-hop peer-address
+exit
+!
diff --git a/tests/topotests/bgp_aigp/r6/ospfd.conf b/tests/topotests/bgp_aigp/r6/ospfd.conf
@@ -1,6 +1,6 @@
 !
 interface lo
- ip ospf cost 10
+ ip ospf passive
 !
 interface r6-eth0
  ip ospf dead-interval 4

diff --git a/tests/topotests/bgp_aigp/test_bgp_aigp.py b/tests/topotests/bgp_aigp/test_bgp_aigp.py
@@ -12,9 +12,9 @@
 r6 receives those routes with aigp-metric TLV.
 
 r2 and r3 receives those routes with aigp-metric TLV increased by 20,
-and 30 appropriately.
+and 10 appropriately.
 
-r1 receives routes with aigp-metric TLV 111,131 and 112,132 appropriately.
+r1 receives routes with aigp-metric TLV 81, 91 and 82, 92 respectively.
 """
 
 import os
@@ -108,15 +108,15 @@ def _bgp_converge():
         expected = {
             "paths": [
                 {
-                    "aigpMetric": 111,
+                    "aigpMetric": 81,
                     "valid": True,
-                    "nexthops": [{"hostname": "r3", "accessible": True}],
+                    "nexthops": [{"ip": "192.168.35.5", "hostname": "r3", "metric": 30, "accessible": True}],
                 },
                 {
-                    "aigpMetric": 131,
+                    "aigpMetric": 91,
                     "valid": True,
-                    "bestpath": {"selectionReason": "Neighbor IP"},
-                    "nexthops": [{"hostname": "r2", "accessible": True}],
+                    "bestpath": {"selectionReason": "IGP Metric"},
+                    "nexthops": [{"ip": "192.168.24.4", "hostname": "r2", "metric": 10, "accessible": True}],
                 },
             ]
         }
@@ -140,30 +140,30 @@ def _bgp_check_aigp_metric_bestpath():
                 "10.0.0.71/32": {
                     "paths": [
                         {
-                            "aigpMetric": 111,
-                            "bestpath": {"selectionReason": "AIGP"},
+                            "aigpMetric": 81,
                             "valid": True,
-                            "nexthops": [{"hostname": "r3", "accessible": True}],
+                            "bestpath": {"selectionReason": "AIGP"},
+                            "nexthops": [{"ip": "192.168.35.5", "hostname": "r3", "metric": 30, "accessible": True}],
                         },
                         {
-                            "aigpMetric": 131,
+                            "aigpMetric": 91,
                             "valid": True,
-                            "nexthops": [{"hostname": "r2", "accessible": True}],
+                            "nexthops": [{"ip": "192.168.24.4", "hostname": "r2", "metric": 10, "accessible": True}],
                         },
                     ],
                 },
                 "10.0.0.72/32": {
                     "paths": [
                         {
-                            "aigpMetric": 112,
-                            "bestpath": {"selectionReason": "AIGP"},
+                            "aigpMetric": 82,
                             "valid": True,
-                            "nexthops": [{"hostname": "r3", "accessible": True}],
+                            "bestpath": {"selectionReason": "AIGP"},
+                            "nexthops": [{"ip": "192.168.35.5", "hostname": "r3", "metric": 30, "accessible": True}],
                         },
                         {
-                            "aigpMetric": 132,
+                            "aigpMetric": 92,
                             "valid": True,
-                            "nexthops": [{"hostname": "r2", "accessible": True}],
+                            "nexthops": [{"ip": "192.168.24.4", "hostname": "r2", "metric": 10, "accessible": True}],
                         },
                     ],
                 },
@@ -195,15 +195,15 @@ def _bgp_check_aigp_metric_bestpath():
     _, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
     assert result is None, "aigp-metric for 10.0.0.72/32 is not 72"
 
-    # r2, 10.0.0.71/32 with aigp-metric 101 (71 + 30)
-    test_func = functools.partial(_bgp_check_aigp_metric, r2, "10.0.0.71/32", 101)
+    # r2, 10.0.0.71/32 with aigp-metric 101 (71 + 20)
+    test_func = functools.partial(_bgp_check_aigp_metric, r2, "10.0.0.71/32", 91)
     _, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
-    assert result is None, "aigp-metric for 10.0.0.71/32 is not 101"
+    assert result is None, "aigp-metric for 10.0.0.71/32 is not 91"
 
-    # r3, 10.0.0.72/32 with aigp-metric 92 (72 + 20)
-    test_func = functools.partial(_bgp_check_aigp_metric, r3, "10.0.0.72/32", 92)
+    # r3, 10.0.0.72/32 with aigp-metric 92 (72 + 10)
+    test_func = functools.partial(_bgp_check_aigp_metric, r3, "10.0.0.72/32", 82)
     _, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
-    assert result is None, "aigp-metric for 10.0.0.72/32 is not 92"
+    assert result is None, "aigp-metric for 10.0.0.72/32 is not 82"
 
     # r1, check if AIGP is considered in best-path selection (lowest wins)
     test_func = functools.partial(_bgp_check_aigp_metric_bestpath)