]> git.puffer.fish Git - mirror/frr.git/commitdiff
tests: fix and adjust topotest/bgp_aigp 17110/head
authorEnke Chen <enchen@paloaltonetworks.com>
Tue, 15 Oct 2024 01:47:59 +0000 (18:47 -0700)
committerMergify <37929162+mergify[bot]@users.noreply.github.com>
Tue, 15 Oct 2024 14:25:09 +0000 (14:25 +0000)
Fix and adjust the topotest post the fix for route selection with
AIGP.

When there are multiple IGP domains (OSPF in this case), the nexthop
for a BGP route with the AIGP attribute must be resolved in its own
IGP domain.

The changes in r2/bgpd.conf and r3/bgpd.conf are needed as incorrect
IGP metrics are received from NHT for the recursive nexthops. Once
the issue is resolved, the changes can be reverted.

Signed-off-by: Enke Chen <enchen@paloaltonetworks.com>
(cherry picked from commit 1ee7e63a6c432662ef3a9a2bd0e1c41298bdf196)

12 files changed:
tests/topotests/bgp_aigp/r1/ospfd.conf
tests/topotests/bgp_aigp/r2/bgpd.conf
tests/topotests/bgp_aigp/r2/ospfd.conf
tests/topotests/bgp_aigp/r3/bgpd.conf
tests/topotests/bgp_aigp/r3/ospfd.conf
tests/topotests/bgp_aigp/r4/bgpd.conf
tests/topotests/bgp_aigp/r4/ospfd.conf
tests/topotests/bgp_aigp/r5/bgpd.conf
tests/topotests/bgp_aigp/r5/ospfd.conf
tests/topotests/bgp_aigp/r6/bgpd.conf
tests/topotests/bgp_aigp/r6/ospfd.conf
tests/topotests/bgp_aigp/test_bgp_aigp.py

index 38aa11d0362a643aefdd3e3f6b056628f776a743..098bf57b0365815772ad4a8a90c4e3009cdcb157 100644 (file)
@@ -1,6 +1,6 @@
 !
 interface lo
- ip ospf cost 10
+ ip ospf passive
 !
 interface r1-eth0
  ip ospf dead-interval 4
index 4db4687536429e85ce0ef524a20c6b919ed32435..4539016f916b4060a3ed96d510a00969eb67e65e 100644 (file)
@@ -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.1 remote-as internal
  neighbor 10.0.0.1 timers 1 3
  neighbor 10.0.0.1 timers connect 1
@@ -11,5 +12,6 @@ router bgp 65001
  neighbor 192.168.24.4 aigp
  address-family ipv4
   redistribute connected
+  neighbor 10.0.0.1 next-hop-self force
  exit-address-family
 !
index ed31941f65c0bae086e9f7bfd22c4e295c8e2259..106a46251d2ea618e926b00c20547d5c6819cbe3 100644 (file)
@@ -1,6 +1,6 @@
 !
 interface lo
- ip ospf cost 10
+ ip ospf passive
 !
 interface r2-eth0
  ip ospf dead-interval 4
index 5ab712eaba65fd600c9bf3956e1a6feadb24e8c5..bdaa5cf55d1e2b769ed1b523d534972012fe190a 100644 (file)
@@ -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.1 remote-as internal
  neighbor 10.0.0.1 timers 1 3
  neighbor 10.0.0.1 timers connect 1
@@ -11,5 +12,6 @@ router bgp 65001
  neighbor 192.168.35.5 aigp
  address-family ipv4
   redistribute connected
+  neighbor 10.0.0.1 next-hop-self force
  exit-address-family
 !
index f971ad6f897518ad46aefe1831821ea328e8aabb..9ede3a1fab64f98f022f8528a89cf231b66aae0a 100644 (file)
@@ -1,6 +1,6 @@
 !
 interface lo
- ip ospf cost 10
+ ip ospf passive
 !
 interface r3-eth0
  ip ospf dead-interval 4
index aa88bac91385f301eda70a695738f9447b9b516f..e12c45e0bb06c78d18aa43aa67b6d3d259bcd0e4 100644 (file)
@@ -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
+!
index c9e6796f6e74d1267dd992d3e37976bd128088cf..237b5e18abe095dec99e2528c990cf98ca5b95c6 100644 (file)
@@ -1,6 +1,6 @@
 !
 interface lo
- ip ospf cost 10
+ ip ospf passive
 !
 interface r4-eth1
  ip ospf dead-interval 4
index 4fde262053cee49ff081f03bfcd942c0d4059c79..3d1f5e85722bc8ce4805c764c0e05f19d26ec1b0 100644 (file)
@@ -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
+!
index b853c74102a2146fda1e3ad1edefaf4c63ec1e8b..65a213df1721276e5465b6d9d6d88e51fbc8c698 100644 (file)
@@ -1,6 +1,6 @@
 !
 interface lo
- ip ospf cost 10
+ ip ospf passive
 !
 interface r5-eth1
  ip ospf dead-interval 4
index 2faae7720c90e34b2608506b69493463fe59d74e..2d5f7a89baf4d789a3e6b717c4278efe946fcd20 100644 (file)
@@ -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
+!
index 46b293317866593a8ae13725287c3374e41edbf7..89cbefa8952216c1db9b59a5349d3641c9af322b 100644 (file)
@@ -1,6 +1,6 @@
 !
 interface lo
- ip ospf cost 10
+ ip ospf passive
 !
 interface r6-eth0
  ip ospf dead-interval 4
index 655e9ad1848508af0f97581f48539c2b3753bb98..108b19c4a5c456eaa4b7f9267bd3acd3c5fdc009 100644 (file)
@@ -12,9 +12,9 @@ r7 sets aigp-metric for 10.0.0.71/32 to 71, and 72 for 10.0.0.72/32.
 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
@@ -109,15 +109,29 @@ def test_bgp_aigp():
         expected = {
             "paths": [
                 {
-                    "aigpMetric": 111,
+                    "aigpMetric": 81,
                     "valid": True,
-                    "nexthops": [{"hostname": "r3", "accessible": True}],
+                    "nexthops": [
+                        {
+                            "ip": "10.0.0.3",
+                            "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": "10.0.0.2",
+                            "hostname": "r2",
+                            "metric": 10,
+                            "accessible": True,
+                        }
+                    ],
                 },
             ]
         }
@@ -141,30 +155,58 @@ def test_bgp_aigp():
                 "10.0.0.71/32": {
                     "paths": [
                         {
-                            "aigpMetric": 111,
-                            "bestpath": {"selectionReason": "AIGP"},
+                            "aigpMetric": 81,
                             "valid": True,
-                            "nexthops": [{"hostname": "r3", "accessible": True}],
+                            "nexthops": [
+                                {
+                                    "ip": "10.0.0.3",
+                                    "hostname": "r3",
+                                    "metric": 30,
+                                    "accessible": True,
+                                }
+                            ],
                         },
                         {
-                            "aigpMetric": 131,
+                            "aigpMetric": 91,
                             "valid": True,
-                            "nexthops": [{"hostname": "r2", "accessible": True}],
+                            "bestpath": {"selectionReason": "AIGP"},
+                            "nexthops": [
+                                {
+                                    "ip": "10.0.0.2",
+                                    "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}],
+                            "nexthops": [
+                                {
+                                    "ip": "10.0.0.3",
+                                    "hostname": "r3",
+                                    "metric": 30,
+                                    "accessible": True,
+                                }
+                            ],
                         },
                         {
-                            "aigpMetric": 132,
+                            "aigpMetric": 92,
                             "valid": True,
-                            "nexthops": [{"hostname": "r2", "accessible": True}],
+                            "bestpath": {"selectionReason": "AIGP"},
+                            "nexthops": [
+                                {
+                                    "ip": "10.0.0.2",
+                                    "hostname": "r2",
+                                    "metric": 10,
+                                    "accessible": True,
+                                }
+                            ],
                         },
                     ],
                 },
@@ -196,17 +238,17 @@ def test_bgp_aigp():
     _, 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)
+    # r1, check if AIGP is considered in best-path selection (lowest wins: aigp + nexthop-metric)
     test_func = functools.partial(_bgp_check_aigp_metric_bestpath)
     _, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
     assert result is None, "AIGP attribute is not considered in best-path selection"