tests: fix and adjust topotest/bgp_aigp

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, 111 insertions, 35 deletions

diff --git a/tests/topotests/bgp_aigp/r1/ospfd.conf b/tests/topotests/bgp_aigp/r1/ospfd.conf
index 38aa11d036..098bf57b03 100644
--- 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/bgpd.conf b/tests/topotests/bgp_aigp/r2/bgpd.conf
index 4db4687536..4539016f91 100644
--- a/tests/topotests/bgp_aigp/r2/bgpd.conf
+++ b/tests/topotests/bgp_aigp/r2/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.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
 !
diff --git a/tests/topotests/bgp_aigp/r2/ospfd.conf b/tests/topotests/bgp_aigp/r2/ospfd.conf
index ed31941f65..106a46251d 100644
--- 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/bgpd.conf b/tests/topotests/bgp_aigp/r3/bgpd.conf
index 5ab712eaba..bdaa5cf55d 100644
--- a/tests/topotests/bgp_aigp/r3/bgpd.conf
+++ b/tests/topotests/bgp_aigp/r3/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.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
 !
diff --git a/tests/topotests/bgp_aigp/r3/ospfd.conf b/tests/topotests/bgp_aigp/r3/ospfd.conf
index f971ad6f89..9ede3a1fab 100644
--- 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
index aa88bac913..e12c45e0bb 100644
--- 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
index c9e6796f6e..237b5e18ab 100644
--- 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
index 4fde262053..3d1f5e8572 100644
--- 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
index b853c74102..65a213df17 100644
--- 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
index 2faae7720c..2d5f7a89ba 100644
--- 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
index 46b2933178..89cbefa895 100644
--- 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
index 655e9ad184..108b19c4a5 100644
--- a/tests/topotests/bgp_aigp/test_bgp_aigp.py
+++ b/tests/topotests/bgp_aigp/test_bgp_aigp.py
@@ -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"