From 4777c8376a118629e4916059a8b4f86aa519db6c Mon Sep 17 00:00:00 2001 From: Pooja Jagadeesh Doijode Date: Fri, 18 Aug 2023 10:02:09 -0700 Subject: [PATCH] bgpd: set ifindex only v6 nexthops and nexthops that match peer's LL For v4 nexthops, ifindex was being set. Modified the check to set ifindex only for v6 nexthops. Also modified the check to set ifindex only if the v6 nexthop matches peer's LL address. Signed-off-by: Pooja Jagadeesh Doijode --- bgpd/bgp_nht.c | 11 ++++- .../bgp_blackhole_community/r4/bgpd.conf | 7 +++ .../test_bgp_blackhole_community.py | 23 +++++++++- .../test_bgp_vpn_5549_route_map.py | 46 +++++++++++++++++++ 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:] -- 2.39.5