Merge pull request #14234 from Pdoijode/pdoijode/frr-bgp-nexthop-find-fix-1

bgpd: set ifindex only for v6 nexthops and nexthops that match peer's LL

4 files changed, 84 insertions, 3 deletions

diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c
index 733dcc72a2..3bf7ac91b3 100644
--- a/bgpd/bgp_nht.c
+++ b/bgpd/bgp_nht.c
@@ -317,11 +317,18 @@ int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop,
 			return 1;
 
 		/*
-		 * If path is learnt from an interface based peer,
+		 * If it's a V6 nexthop, path is learnt from a v6 LL peer,
+		 * and if the NH prefix matches peer's LL address then
 		 * set the ifindex to peer's interface index so that
 		 * correct nexthop can be found in nexthop tree.
+		 *
+		 * NH could be set to different v6 LL address (compared to
+		 * peer's LL) using route-map. In such a scenario, do not set
+		 * the ifindex.
 		 */
-		if (pi->peer->conf_if)
+		if (afi == AFI_IP6 &&
+		    IN6_IS_ADDR_LINKLOCAL(&pi->peer->su.sin6.sin6_addr) &&
+		    IPV6_ADDR_SAME(&pi->peer->su.sin6.sin6_addr, &p.u.prefix6))
 			ifindex = pi->peer->su.sin6.sin6_scope_id;
 
 		if (!is_bgp_static_route && orig_prefix
diff --git a/tests/topotests/bgp_blackhole_community/r4/bgpd.conf b/tests/topotests/bgp_blackhole_community/r4/bgpd.conf
index 0ac963e642..eca12bd3d9 100644
--- a/tests/topotests/bgp_blackhole_community/r4/bgpd.conf
+++ b/tests/topotests/bgp_blackhole_community/r4/bgpd.conf
@@ -4,3 +4,10 @@ router bgp 65002
   no bgp ebgp-requires-policy
   neighbor r4-eth0 interface remote-as internal
 !
+address-family ipv4 unicast
+  neighbor r4-eth0 route-map FOO in
+exit-address-family
+!
+route-map FOO permit 10
+ set ipv6 next-hop local fe80::202:ff:fe00:99
+exit
diff --git a/tests/topotests/bgp_blackhole_community/test_bgp_blackhole_community.py b/tests/topotests/bgp_blackhole_community/test_bgp_blackhole_community.py
index 16191911a9..9f5c0ef924 100644
--- a/tests/topotests/bgp_blackhole_community/test_bgp_blackhole_community.py
+++ b/tests/topotests/bgp_blackhole_community/test_bgp_blackhole_community.py
@@ -107,6 +107,23 @@ def test_bgp_blackhole_community():
 
         return topotest.json_cmp(output, expected)
 
+    def _bgp_verify_nexthop_validity():
+        output = json.loads(tgen.gears["r4"].vtysh_cmd("show bgp nexthop json"))
+
+        expected = {
+            "ipv6": {
+                "fe80::202:ff:fe00:99": {
+                    "valid": True,
+                    "complete": True,
+                    "igpMetric": 0,
+                    "pathCount": 2,
+                    "nexthops": [{"interfaceName": "r4-eth0"}],
+                },
+            }
+        }
+
+        return topotest.json_cmp(output, expected)
+
     test_func = functools.partial(_bgp_converge)
     success, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5)
 
@@ -124,7 +141,6 @@ def test_bgp_blackhole_community():
     )
 
     step("Check if 172.16.255.254/32 is advertised to iBGP peers")
-
     test_func = functools.partial(_bgp_no_advertise_ibgp)
     success, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5)
 
@@ -134,6 +150,11 @@ def test_bgp_blackhole_community():
         tgen.gears["r2"]
     )
 
+    step("Verify if the nexthop set via route-map on r4 is marked valid")
+    test_func = functools.partial(_bgp_verify_nexthop_validity)
+    success, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5)
+    assert result is None, 'Nexthops are not valid "{}"'.format(tgen.gears["r4"])
+
 
 if __name__ == "__main__":
     args = ["-s"] + sys.argv[1:]
diff --git a/tests/topotests/bgp_vpn_5549_route_map/test_bgp_vpn_5549_route_map.py b/tests/topotests/bgp_vpn_5549_route_map/test_bgp_vpn_5549_route_map.py
index 533af19d93..84ef603d6e 100644
--- a/tests/topotests/bgp_vpn_5549_route_map/test_bgp_vpn_5549_route_map.py
+++ b/tests/topotests/bgp_vpn_5549_route_map/test_bgp_vpn_5549_route_map.py
@@ -115,10 +115,56 @@ def test_bgp_vpn_5549():
         }
         return topotest.json_cmp(output, expected)
 
+    def _bgp_verify_v4_nexthop_validity():
+        output = json.loads(tgen.gears["cpe1"].vtysh_cmd("show bgp nexthop json"))
+        expected = {
+            "ipv4": {
+                "192.168.1.2": {
+                    "valid": True,
+                    "complete": True,
+                    "igpMetric": 0,
+                    "pathCount": 0,
+                    "nexthops": [{"interfaceName": "cpe1-eth0"}],
+                },
+            }
+        }
+        return topotest.json_cmp(output, expected)
+
+    def _bgp_verify_v6_global_nexthop_validity():
+        output = json.loads(tgen.gears["pe2"].vtysh_cmd("show bgp nexthop json"))
+        expected = {
+            "ipv6": {
+                "2001:db8::1": {
+                    "valid": True,
+                    "complete": True,
+                    "igpMetric": 0,
+                    "pathCount": 2,
+                    "nexthops": [{"interfaceName": "pe2-eth0"}],
+                },
+                "2001:db8:1::1": {
+                    "valid": True,
+                    "complete": True,
+                    "igpMetric": 20,
+                    "pathCount": 2,
+                    "peer": "2001:db8:1::1",
+                    "nexthops": [{"interfaceName": "pe2-eth0"}],
+                },
+            }
+        }
+        return topotest.json_cmp(output, expected)
+
     test_func = functools.partial(_bgp_vpn_nexthop_changed)
     _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5)
     assert result is None, "Failed overriding IPv6 next-hop for VPN underlay"
 
+    test_func = functools.partial(_bgp_verify_v4_nexthop_validity)
+    _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5)
+    assert result is None, "IPv4 nexthop is invalid"
+
+    test_func = functools.partial(_bgp_verify_v6_global_nexthop_validity)
+    _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5)
+    assert result is None, "IPv6 nexthop is invalid"
+
 
 if __name__ == "__main__":
     args = ["-s"] + sys.argv[1:]