From f784007d677ee20e4dbd20366928652be2a0b23c Mon Sep 17 00:00:00 2001 From: Ryoga Saito Date: Sun, 4 Dec 2022 23:51:45 +0900 Subject: [PATCH 1/2] bgpd: Stop overriding nexthop when BGP unnumberred When we use vrf-to-vrf export, the nexthop has already not been overridden when the peer is BGP unnumberred. However, when we use normal export, the nexthop will be oberridden. This behavior will make the VPN routes invalid in VPN RIB. This PR stops overriding nexthop even if we use normal export. Signed-off-by: Ryoga Saito --- bgpd/bgp_mplsvpn.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index 18cb90763c..ace75976a9 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -1505,7 +1505,8 @@ void vpn_leak_from_vrf_update(struct bgp *to_bgp, /* to */ } else { if (!CHECK_FLAG(from_bgp->af_flags[afi][SAFI_UNICAST], BGP_CONFIG_VRF_TO_VRF_EXPORT)) { - if (afi == AFI_IP) { + if (afi == AFI_IP && + !BGP_ATTR_NEXTHOP_AFI_IP6(path_vrf->attr)) { /* * For ipv4, copy to multiprotocol * nexthop field From 2b0efccd057752a14e88c71f142a5b8bc0ad93a8 Mon Sep 17 00:00:00 2001 From: Ryoga Saito Date: Wed, 7 Dec 2022 02:27:16 +0900 Subject: [PATCH 2/2] tests: Add topotest bgp_vrf_leaking_5549_routes To verify previous changes, this PR introduces topotest to verify whether imported routes learnt from BGP unnumbered peers will be active on VPN RIB and other VRF RIB. Signed-off-by: Ryoga Saito --- .../bgp_vrf_leaking_5549_routes/ce1/bgpd.conf | 23 ++++ .../ce1/zebra.conf | 13 ++ .../bgp_vrf_leaking_5549_routes/pe1/bgpd.conf | 41 ++++++ .../pe1/results/default_ipv4_vpn.json | 32 +++++ .../pe1/results/vrf10_ipv4_unicast.json | 32 +++++ .../pe1/results/vrf20_ipv4_unicast.json | 34 +++++ .../pe1/zebra.conf | 10 ++ .../test_bgp_vrf_leaking.py | 121 ++++++++++++++++++ 8 files changed, 306 insertions(+) create mode 100644 tests/topotests/bgp_vrf_leaking_5549_routes/ce1/bgpd.conf create mode 100644 tests/topotests/bgp_vrf_leaking_5549_routes/ce1/zebra.conf create mode 100644 tests/topotests/bgp_vrf_leaking_5549_routes/pe1/bgpd.conf create mode 100644 tests/topotests/bgp_vrf_leaking_5549_routes/pe1/results/default_ipv4_vpn.json create mode 100644 tests/topotests/bgp_vrf_leaking_5549_routes/pe1/results/vrf10_ipv4_unicast.json create mode 100644 tests/topotests/bgp_vrf_leaking_5549_routes/pe1/results/vrf20_ipv4_unicast.json create mode 100644 tests/topotests/bgp_vrf_leaking_5549_routes/pe1/zebra.conf create mode 100755 tests/topotests/bgp_vrf_leaking_5549_routes/test_bgp_vrf_leaking.py diff --git a/tests/topotests/bgp_vrf_leaking_5549_routes/ce1/bgpd.conf b/tests/topotests/bgp_vrf_leaking_5549_routes/ce1/bgpd.conf new file mode 100644 index 0000000000..66493f0fea --- /dev/null +++ b/tests/topotests/bgp_vrf_leaking_5549_routes/ce1/bgpd.conf @@ -0,0 +1,23 @@ +frr defaults traditional +! +hostname ce1 +password zebra +! +log stdout notifications +log monitor notifications +log commands +! +router bgp 65002 + bgp router-id 192.0.2.2 + no bgp ebgp-requires-policy + no bgp default ipv4-unicast + neighbor eth0 interface + neighbor eth0 remote-as external + neighbor eth0 timers connect 1 + ! + address-family ipv4 unicast + neighbor eth0 activate + redistribute connected + exit-address-family + ! +! diff --git a/tests/topotests/bgp_vrf_leaking_5549_routes/ce1/zebra.conf b/tests/topotests/bgp_vrf_leaking_5549_routes/ce1/zebra.conf new file mode 100644 index 0000000000..a163295844 --- /dev/null +++ b/tests/topotests/bgp_vrf_leaking_5549_routes/ce1/zebra.conf @@ -0,0 +1,13 @@ +log file zebra.log +! +hostname ce1 +! +interface lo + ip address 172.16.0.1/32 +! +interface eth0 + ipv6 nd ra-interval 1 + no ipv6 nd suppress-ra +! +line vty +! diff --git a/tests/topotests/bgp_vrf_leaking_5549_routes/pe1/bgpd.conf b/tests/topotests/bgp_vrf_leaking_5549_routes/pe1/bgpd.conf new file mode 100644 index 0000000000..c5c99270e7 --- /dev/null +++ b/tests/topotests/bgp_vrf_leaking_5549_routes/pe1/bgpd.conf @@ -0,0 +1,41 @@ +frr defaults traditional +! +hostname pe1 +password zebra +! +log stdout notifications +log monitor notifications +log commands +! +router bgp 65001 + bgp router-id 192.0.2.1 + ! +! +router bgp 65001 vrf vrf10 + bgp router-id 192.0.2.1 + no bgp ebgp-requires-policy + no bgp default ipv4-unicast + neighbor eth0 interface + neighbor eth0 remote-as external + neighbor eth0 timers connect 1 + ! + address-family ipv4 unicast + neighbor eth0 activate + rd vpn export 65001:10 + rt vpn both 0:10 + import vpn + export vpn + exit-address-family + ! +! +router bgp 65001 vrf vrf20 + bgp router-id 192.0.2.1 + ! + address-family ipv4 unicast + rd vpn export 65001:20 + rt vpn both 0:10 + import vpn + export vpn + exit-address-family + ! +! diff --git a/tests/topotests/bgp_vrf_leaking_5549_routes/pe1/results/default_ipv4_vpn.json b/tests/topotests/bgp_vrf_leaking_5549_routes/pe1/results/default_ipv4_vpn.json new file mode 100644 index 0000000000..9516016fc2 --- /dev/null +++ b/tests/topotests/bgp_vrf_leaking_5549_routes/pe1/results/default_ipv4_vpn.json @@ -0,0 +1,32 @@ +{ + "vrfName": "default", + "routerId": "192.0.2.1", + "localAS": 65001, + "routes": { + "routeDistinguishers": { + "65001:10": { + "172.16.0.1/32": [ + { + "valid": true, + "bestpath": true, + "pathFrom": "external", + "prefix": "172.16.0.1", + "prefixLen": 32, + "network": "172.16.0.1\/32", + "path": "65002", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "vrf10", + "nexthops": [ + { + "hostname": "pe1", + "afi": "ipv6", + "used": true + } + ] + } + ] + } + } + } +} diff --git a/tests/topotests/bgp_vrf_leaking_5549_routes/pe1/results/vrf10_ipv4_unicast.json b/tests/topotests/bgp_vrf_leaking_5549_routes/pe1/results/vrf10_ipv4_unicast.json new file mode 100644 index 0000000000..768bffbe9d --- /dev/null +++ b/tests/topotests/bgp_vrf_leaking_5549_routes/pe1/results/vrf10_ipv4_unicast.json @@ -0,0 +1,32 @@ +{ + "vrfName": "vrf10", + "routerId": "192.0.2.1", + "localAS": 65001, + "routes": { + "172.16.0.1/32": [ + { + "valid": true, + "bestpath": true, + "pathFrom": "external", + "prefix": "172.16.0.1", + "prefixLen": 32, + "network": "172.16.0.1\/32", + "path": "65002", + "origin": "incomplete", + "nexthops": [ + { + "hostname": "ce1", + "afi": "ipv6", + "scope": "global", + "used": true + }, + { + "hostname": "ce1", + "afi": "ipv6", + "scope": "link-local" + } + ] + } + ] + } +} diff --git a/tests/topotests/bgp_vrf_leaking_5549_routes/pe1/results/vrf20_ipv4_unicast.json b/tests/topotests/bgp_vrf_leaking_5549_routes/pe1/results/vrf20_ipv4_unicast.json new file mode 100644 index 0000000000..1e93715270 --- /dev/null +++ b/tests/topotests/bgp_vrf_leaking_5549_routes/pe1/results/vrf20_ipv4_unicast.json @@ -0,0 +1,34 @@ +{ + "vrfName": "vrf20", + "routerId": "192.0.2.1", + "localAS": 65001, + "routes": { + "172.16.0.1/32": [ + { + "valid": true, + "bestpath": true, + "pathFrom": "external", + "prefix": "172.16.0.1", + "prefixLen": 32, + "network": "172.16.0.1\/32", + "path": "65002", + "origin": "incomplete", + "announceNexthopSelf": true, + "nhVrfName": "vrf10", + "nexthops": [ + { + "hostname": "pe1", + "afi": "ipv6", + "scope": "global", + "used": true + }, + { + "hostname": "pe1", + "afi": "ipv6", + "scope": "link-local" + } + ] + } + ] + } +} diff --git a/tests/topotests/bgp_vrf_leaking_5549_routes/pe1/zebra.conf b/tests/topotests/bgp_vrf_leaking_5549_routes/pe1/zebra.conf new file mode 100644 index 0000000000..d40041ab3c --- /dev/null +++ b/tests/topotests/bgp_vrf_leaking_5549_routes/pe1/zebra.conf @@ -0,0 +1,10 @@ +log file zebra.log +! +hostname pe1 +! +interface eth0 vrf vrf10 + ipv6 nd ra-interval 1 + no ipv6 nd suppress-ra +! +line vty +! diff --git a/tests/topotests/bgp_vrf_leaking_5549_routes/test_bgp_vrf_leaking.py b/tests/topotests/bgp_vrf_leaking_5549_routes/test_bgp_vrf_leaking.py new file mode 100755 index 0000000000..dd27ad3ed1 --- /dev/null +++ b/tests/topotests/bgp_vrf_leaking_5549_routes/test_bgp_vrf_leaking.py @@ -0,0 +1,121 @@ +#!/usr/bin/env python + +# Copyright (c) 2022, LINE Corporation +# Authored by Ryoga Saito +# +# Permission to use, copy, modify, and/or distribute this software +# for any purpose with or without fee is hereby granted, provided +# that the above copyright notice and this permission notice appear +# in all copies. +# +# THE SOFTWARE IS PROVIDED "AS IS" AND NETDEF DISCLAIMS ALL WARRANTIES +# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF +# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL NETDEF BE LIABLE FOR +# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY +# DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, +# WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS +# ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE +# OF THIS SOFTWARE. +# + +import os +import re +import sys +import json +import functools +import pytest + +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join(CWD, "../")) + +# pylint: disable=C0413 +# Import topogen and topotest helpers +from lib import topotest +from lib.topogen import Topogen, TopoRouter, get_topogen +from lib.topolog import logger +from lib.common_config import required_linux_kernel_version + +pytestmark = [pytest.mark.bgpd] + + +def build_topo(tgen): + tgen.add_router("pe1") + tgen.add_router("ce1") + + tgen.add_link(tgen.gears["pe1"], tgen.gears["ce1"], "eth0", "eth0") + + +def setup_module(mod): + tgen = Topogen(build_topo, mod.__name__) + tgen.start_topology() + + for rname, router in tgen.routers().items(): + router.load_config(TopoRouter.RD_ZEBRA, + os.path.join(CWD, '{}/zebra.conf'.format(rname))) + router.load_config(TopoRouter.RD_BGP, + os.path.join(CWD, '{}/bgpd.conf'.format(rname))) + + tgen.gears["pe1"].run("ip link add vrf10 type vrf table 10") + tgen.gears["pe1"].run("ip link set vrf10 up") + tgen.gears["pe1"].run("ip link add vrf20 type vrf table 20") + tgen.gears["pe1"].run("ip link set vrf20 up") + tgen.gears["pe1"].run("ip link set eth0 master vrf10") + + tgen.start_router() + + +def teardown_module(mod): + tgen = get_topogen() + tgen.stop_topology() + + +def open_json_file(path): + try: + with open(path, "r") as f: + return json.load(f) + except IOError: + assert False, "Could not read file {}".format(path) + + +def check_vrf10_rib(output): + expected = open_json_file("%s/pe1/results/vrf10_ipv4_unicast.json" % CWD) + actual = json.loads(output) + return topotest.json_cmp(actual, expected) + + +def check_default_vpn_rib(output): + expected = open_json_file("%s/pe1/results/default_ipv4_vpn.json" % CWD) + actual = json.loads(output) + return topotest.json_cmp(actual, expected) + + +def check_vrf20_rib(output): + expected = open_json_file("%s/pe1/results/vrf20_ipv4_unicast.json" % CWD) + actual = json.loads(output) + return topotest.json_cmp(actual, expected) + + +def check(name, command, checker): + tgen = get_topogen() + router = tgen.gears[name] + + def _check(): + try: + return checker(router.vtysh_cmd(command)) + except: + return False + + logger.info('[+] check {} "{}"'.format(name, command)) + _, result = topotest.run_and_expect(_check, None, count=10, wait=0.5) + assert result is None, "Failed" + + +def test_rib(): + check("pe1", "show bgp vrf vrf10 ipv4 unicast json", check_vrf10_rib) + check("pe1", "show bgp ipv4 vpn json", check_default_vpn_rib) + check("pe1", "show bgp vrf vrf20 ipv4 unicast json", check_vrf20_rib) + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args))