Merge pull request #16941 from opensourcerouting/fix/issue_16877

bgpd: Relax the same prefix and nexthop to be valid
This commit is contained in:
Russ White 2024-10-08 10:14:30 -04:00 committed by GitHub
commit 02e5a059ee
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 174 additions and 294 deletions

View File

@ -347,11 +347,10 @@ int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop,
&p.u.prefix6)) &p.u.prefix6))
ifindex = pi->peer->connection->su.sin6.sin6_scope_id; ifindex = pi->peer->connection->su.sin6.sin6_scope_id;
if (!is_bgp_static_route && orig_prefix if (!is_bgp_static_route && orig_prefix && prefix_same(&p, orig_prefix) &&
&& prefix_same(&p, orig_prefix)) { CHECK_FLAG(bgp_route->flags, BGP_FLAG_IMPORT_CHECK)) {
if (BGP_DEBUG(nht, NHT)) { if (BGP_DEBUG(nht, NHT)) {
zlog_debug( zlog_debug("%s(%pFX): prefix loops through itself (import-check enabled)",
"%s(%pFX): prefix loops through itself",
__func__, &p); __func__, &p);
} }
return 0; return 0;

View File

@ -831,7 +831,6 @@ def test_bgp_with_loopback_interface(request):
for bgp_neighbor in topo["routers"][routerN]["bgp"]["address_family"]["ipv4"][ for bgp_neighbor in topo["routers"][routerN]["bgp"]["address_family"]["ipv4"][
"unicast" "unicast"
]["neighbor"].keys(): ]["neighbor"].keys():
# Adding ['source_link'] = 'lo' key:value pair # Adding ['source_link'] = 'lo' key:value pair
topo["routers"][routerN]["bgp"]["address_family"]["ipv4"]["unicast"][ topo["routers"][routerN]["bgp"]["address_family"]["ipv4"]["unicast"][
"neighbor" "neighbor"
@ -876,294 +875,6 @@ def test_bgp_with_loopback_interface(request):
write_test_footer(tc_name) write_test_footer(tc_name)
def test_bgp_with_loopback_with_same_subnet_p1(request):
"""
Verify routes not installed in zebra when /32 routes received
with loopback BGP session subnet
"""
tgen = get_topogen()
if BGP_CONVERGENCE is not True:
pytest.skip("skipped because of BGP Convergence failure")
# test case name
tc_name = request.node.name
write_test_header(tc_name)
# Creating configuration from JSON
reset_config_on_routers(tgen)
step("Delete BGP seesion created initially")
input_dict_r1 = {
"r1": {"bgp": {"delete": True}},
"r2": {"bgp": {"delete": True}},
"r3": {"bgp": {"delete": True}},
"r4": {"bgp": {"delete": True}},
}
result = create_router_bgp(tgen, topo, input_dict_r1)
assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result)
step("Create BGP session over loop address")
topo_modify = deepcopy(topo)
for routerN in sorted(topo["routers"].keys()):
for addr_type in ADDR_TYPES:
for bgp_neighbor in topo_modify["routers"][routerN]["bgp"][
"address_family"
][addr_type]["unicast"]["neighbor"].keys():
# Adding ['source_link'] = 'lo' key:value pair
topo_modify["routers"][routerN]["bgp"]["address_family"][addr_type][
"unicast"
]["neighbor"][bgp_neighbor]["dest_link"] = {
"lo": {"source_link": "lo", "ebgp_multihop": 2}
}
result = create_router_bgp(tgen, topo_modify["routers"])
assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result)
step("Disable IPv6 BGP nbr from ipv4 address family")
raw_config = {
"r1": {
"raw_config": [
"router bgp {}".format(topo["routers"]["r1"]["bgp"]["local_as"]),
"address-family ipv4 unicast",
"no neighbor {} activate".format(
topo["routers"]["r2"]["links"]["lo"]["ipv6"].split("/")[0]
),
"no neighbor {} activate".format(
topo["routers"]["r3"]["links"]["lo"]["ipv6"].split("/")[0]
),
]
},
"r2": {
"raw_config": [
"router bgp {}".format(topo["routers"]["r2"]["bgp"]["local_as"]),
"address-family ipv4 unicast",
"no neighbor {} activate".format(
topo["routers"]["r1"]["links"]["lo"]["ipv6"].split("/")[0]
),
"no neighbor {} activate".format(
topo["routers"]["r3"]["links"]["lo"]["ipv6"].split("/")[0]
),
]
},
"r3": {
"raw_config": [
"router bgp {}".format(topo["routers"]["r3"]["bgp"]["local_as"]),
"address-family ipv4 unicast",
"no neighbor {} activate".format(
topo["routers"]["r1"]["links"]["lo"]["ipv6"].split("/")[0]
),
"no neighbor {} activate".format(
topo["routers"]["r2"]["links"]["lo"]["ipv6"].split("/")[0]
),
"no neighbor {} activate".format(
topo["routers"]["r4"]["links"]["lo"]["ipv6"].split("/")[0]
),
]
},
"r4": {
"raw_config": [
"router bgp {}".format(topo["routers"]["r4"]["bgp"]["local_as"]),
"address-family ipv4 unicast",
"no neighbor {} activate".format(
topo["routers"]["r3"]["links"]["lo"]["ipv6"].split("/")[0]
),
]
},
}
step("Configure kernel routes")
result = apply_raw_config(tgen, raw_config)
assert result is True, "Testcase {} : Failed Error: {}".format(tc_name, result)
r1_ipv4_lo = topo["routers"]["r1"]["links"]["lo"]["ipv4"]
r1_ipv6_lo = topo["routers"]["r1"]["links"]["lo"]["ipv6"]
r2_ipv4_lo = topo["routers"]["r2"]["links"]["lo"]["ipv4"]
r2_ipv6_lo = topo["routers"]["r2"]["links"]["lo"]["ipv6"]
r3_ipv4_lo = topo["routers"]["r3"]["links"]["lo"]["ipv4"]
r3_ipv6_lo = topo["routers"]["r3"]["links"]["lo"]["ipv6"]
r4_ipv4_lo = topo["routers"]["r4"]["links"]["lo"]["ipv4"]
r4_ipv6_lo = topo["routers"]["r4"]["links"]["lo"]["ipv6"]
r1_r2 = topo["routers"]["r1"]["links"]["r2"]["ipv6"].split("/")[0]
r2_r1 = topo["routers"]["r2"]["links"]["r1"]["ipv6"].split("/")[0]
r1_r3 = topo["routers"]["r1"]["links"]["r3"]["ipv6"].split("/")[0]
r3_r1 = topo["routers"]["r3"]["links"]["r1"]["ipv6"].split("/")[0]
r2_r3 = topo["routers"]["r2"]["links"]["r3"]["ipv6"].split("/")[0]
r3_r2 = topo["routers"]["r3"]["links"]["r2"]["ipv6"].split("/")[0]
r3_r4 = topo["routers"]["r3"]["links"]["r4"]["ipv6"].split("/")[0]
r4_r3 = topo["routers"]["r4"]["links"]["r3"]["ipv6"].split("/")[0]
r1_r2_ipv4 = topo["routers"]["r1"]["links"]["r2"]["ipv4"].split("/")[0]
r2_r1_ipv4 = topo["routers"]["r2"]["links"]["r1"]["ipv4"].split("/")[0]
r1_r3_ipv4 = topo["routers"]["r1"]["links"]["r3"]["ipv4"].split("/")[0]
r3_r1_ipv4 = topo["routers"]["r3"]["links"]["r1"]["ipv4"].split("/")[0]
r2_r3_ipv4 = topo["routers"]["r2"]["links"]["r3"]["ipv4"].split("/")[0]
r3_r2_ipv4 = topo["routers"]["r3"]["links"]["r2"]["ipv4"].split("/")[0]
r3_r4_ipv4 = topo["routers"]["r3"]["links"]["r4"]["ipv4"].split("/")[0]
r4_r3_ipv4 = topo["routers"]["r4"]["links"]["r3"]["ipv4"].split("/")[0]
r1_r2_intf = topo["routers"]["r1"]["links"]["r2"]["interface"]
r2_r1_intf = topo["routers"]["r2"]["links"]["r1"]["interface"]
r1_r3_intf = topo["routers"]["r1"]["links"]["r3"]["interface"]
r3_r1_intf = topo["routers"]["r3"]["links"]["r1"]["interface"]
r2_r3_intf = topo["routers"]["r2"]["links"]["r3"]["interface"]
r3_r2_intf = topo["routers"]["r3"]["links"]["r2"]["interface"]
r3_r4_intf = topo["routers"]["r3"]["links"]["r4"]["interface"]
r4_r3_intf = topo["routers"]["r4"]["links"]["r3"]["interface"]
ipv4_list = [
("r1", r1_r2_intf, r2_ipv4_loopback),
("r1", r1_r3_intf, r3_ipv4_loopback),
("r2", r2_r1_intf, r1_ipv4_loopback),
("r2", r2_r3_intf, r3_ipv4_loopback),
("r3", r3_r1_intf, r1_ipv4_loopback),
("r3", r3_r2_intf, r2_ipv4_loopback),
("r3", r3_r4_intf, r4_ipv4_loopback),
("r4", r4_r3_intf, r3_ipv4_loopback),
]
ipv6_list = [
("r1", r1_r2_intf, r2_ipv6_loopback, r2_r1),
("r1", r1_r3_intf, r3_ipv6_loopback, r3_r1),
("r2", r2_r1_intf, r1_ipv6_loopback, r1_r2),
("r2", r2_r3_intf, r3_ipv6_loopback, r3_r2),
("r3", r3_r1_intf, r1_ipv6_loopback, r1_r3),
("r3", r3_r2_intf, r2_ipv6_loopback, r2_r3),
("r3", r3_r4_intf, r4_ipv6_loopback, r4_r3),
("r4", r4_r3_intf, r3_ipv6_loopback, r3_r4),
]
for dut, intf, loop_addr in ipv4_list:
result = addKernelRoute(tgen, dut, intf, loop_addr)
assert result is True, "Testcase {}:Failed \n Error: {}".format(tc_name, result)
for dut, intf, loop_addr, next_hop in ipv6_list:
result = addKernelRoute(tgen, dut, intf, loop_addr, next_hop)
assert result is True, "Testcase {}:Failed \n Error: {}".format(tc_name, result)
step("Configure static routes")
input_dict = {
"r1": {
"static_routes": [
{"network": r2_ipv4_loopback, "next_hop": r2_r1_ipv4},
{"network": r3_ipv4_loopback, "next_hop": r3_r1_ipv4},
{"network": r2_ipv6_loopback, "next_hop": r2_r1},
{"network": r3_ipv6_loopback, "next_hop": r3_r1},
]
},
"r2": {
"static_routes": [
{"network": r1_ipv4_loopback, "next_hop": r1_r2_ipv4},
{"network": r3_ipv4_loopback, "next_hop": r3_r2_ipv4},
{"network": r1_ipv6_loopback, "next_hop": r1_r2},
{"network": r3_ipv6_loopback, "next_hop": r3_r2},
]
},
"r3": {
"static_routes": [
{"network": r1_ipv4_loopback, "next_hop": r1_r3_ipv4},
{"network": r2_ipv4_loopback, "next_hop": r2_r3_ipv4},
{"network": r4_ipv4_loopback, "next_hop": r4_r3_ipv4},
{"network": r1_ipv6_loopback, "next_hop": r1_r3},
{"network": r2_ipv6_loopback, "next_hop": r2_r3},
{"network": r4_ipv6_loopback, "next_hop": r4_r3},
]
},
"r4": {
"static_routes": [
{"network": r3_ipv4_loopback, "next_hop": r3_r4_ipv4},
{"network": r3_ipv6_loopback, "next_hop": r3_r4},
]
},
}
result = create_static_routes(tgen, input_dict)
assert result is True, "Testcase {} :Failed \n Error: {}".format(tc_name, result)
step("Verify BGP session convergence")
result = verify_bgp_convergence(tgen, topo_modify)
assert result is True, "Testcase {} :Failed \n Error: {}".format(tc_name, result)
step("Configure redistribute connected on R2 and R4")
input_dict_1 = {
"r2": {
"bgp": {
"address_family": {
"ipv4": {
"unicast": {"redistribute": [{"redist_type": "connected"}]}
},
"ipv6": {
"unicast": {"redistribute": [{"redist_type": "connected"}]}
},
}
}
},
"r4": {
"bgp": {
"address_family": {
"ipv4": {
"unicast": {"redistribute": [{"redist_type": "connected"}]}
},
"ipv6": {
"unicast": {"redistribute": [{"redist_type": "connected"}]}
},
}
}
},
}
result = create_router_bgp(tgen, topo, input_dict_1)
assert result is True, "Testcase {} :Failed \n Error: {}".format(tc_name, result)
step("Verify Ipv4 and Ipv6 network installed in R1 RIB but not in FIB")
input_dict_r1 = {
"r1": {
"static_routes": [
{"network": "1.0.2.17/32"},
{"network": "2001:db8:f::2:17/128"},
]
}
}
dut = "r1"
protocol = "bgp"
for addr_type in ADDR_TYPES:
result = verify_fib_routes(
tgen, addr_type, dut, input_dict_r1, expected=False
) # pylint: disable=E1123
assert result is not True, (
"Testcase {} : Failed \n "
"Expected: Routes should not be present in {} FIB \n "
"Found: {}".format(tc_name, dut, result)
)
step("Verify Ipv4 and Ipv6 network installed in r3 RIB but not in FIB")
input_dict_r3 = {
"r3": {
"static_routes": [
{"network": "1.0.4.17/32"},
{"network": "2001:db8:f::4:17/128"},
]
}
}
dut = "r3"
protocol = "bgp"
for addr_type in ADDR_TYPES:
result = verify_fib_routes(
tgen, addr_type, dut, input_dict_r1, expected=False
) # pylint: disable=E1123
assert result is not True, (
"Testcase {} : Failed \n "
"Expected: Routes should not be present in {} FIB \n "
"Found: {}".format(tc_name, dut, result)
)
write_test_footer(tc_name)
if __name__ == "__main__": if __name__ == "__main__":
args = ["-s"] + sys.argv[1:] args = ["-s"] + sys.argv[1:]
sys.exit(pytest.main(args)) sys.exit(pytest.main(args))

View File

@ -0,0 +1,19 @@
!
int lo
ip address 10.0.0.1/32
!
int r1-eth0
ip address 192.168.1.1/24
!
router bgp 65000
no bgp ebgp-requires-policy
no bgp network import-check
neighbor 10.0.0.2 remote-as internal
neighbor 10.0.0.2 update-source lo
neighbor 10.0.0.2 next-hop-self
neighbor 10.0.0.3 remote-as external
neighbor 10.0.0.3 update-source lo
neighbor 10.0.0.3 next-hop-self
!
ip route 10.0.0.2/32 192.168.1.2
ip route 10.0.0.3/32 192.168.1.3

View File

@ -0,0 +1,20 @@
!
int lo
ip address 10.0.0.2/32
!
int r2-eth0
ip address 192.168.1.2/24
!
router bgp 65000
no bgp ebgp-requires-policy
no bgp network import-check
neighbor 10.0.0.1 remote-as internal
neighbor 10.0.0.1 timers 1 3
neighbor 10.0.0.1 timers connect 1
neighbor 10.0.0.1 update-source lo
neighbor 10.0.0.1 next-hop-self
address-family ipv4 unicast
network 10.0.0.2/32
exit-address-family
!
ip route 10.0.0.1/32 192.168.1.1

View File

@ -0,0 +1,20 @@
!
int lo
ip address 10.0.0.3/32
!
int r3-eth0
ip address 192.168.1.3/24
!
router bgp 65003
no bgp ebgp-requires-policy
no bgp network import-check
neighbor 10.0.0.1 remote-as external
neighbor 10.0.0.1 timers 1 3
neighbor 10.0.0.1 timers connect 1
neighbor 10.0.0.1 update-source lo
neighbor 10.0.0.1 next-hop-self
address-family ipv4 unicast
network 10.0.0.3/32
exit-address-family
!
ip route 10.0.0.1/32 192.168.1.1

View File

@ -0,0 +1,111 @@
#!/usr/bin/env python
# SPDX-License-Identifier: ISC
# Copyright (c) 2024 by
# Donatas Abraitis <donatas@opensourcerouting.org>
#
import os
import sys
import json
import pytest
import functools
CWD = os.path.dirname(os.path.realpath(__file__))
sys.path.append(os.path.join(CWD, "../"))
# pylint: disable=C0413
from lib import topotest
from lib.topogen import Topogen, get_topogen
pytestmark = [pytest.mark.bgpd]
def setup_module(mod):
topodef = {"s1": ("r1", "r2", "r3")}
tgen = Topogen(topodef, mod.__name__)
tgen.start_topology()
router_list = tgen.routers()
for _, (rname, router) in enumerate(router_list.items(), 1):
router.load_frr_config(os.path.join(CWD, "{}/frr.conf".format(rname)))
tgen.start_router()
def teardown_module(mod):
tgen = get_topogen()
tgen.stop_topology()
def test_bgp_self_prefix():
tgen = get_topogen()
if tgen.routers_have_failure():
pytest.skip(tgen.errors)
r1 = tgen.gears["r1"]
r3 = tgen.gears["r3"]
def _bgp_converge():
output = json.loads(r1.vtysh_cmd("show bgp ipv4 unicast json"))
expected = {
"routes": {
"10.0.0.2/32": [
{
"valid": True,
"path": "",
"nexthops": [
{"ip": "10.0.0.2", "hostname": "r2", "afi": "ipv4"}
],
}
],
"10.0.0.3/32": [
{
"valid": True,
"path": "65003",
"nexthops": [
{"ip": "10.0.0.3", "hostname": "r3", "afi": "ipv4"}
],
}
],
}
}
return topotest.json_cmp(output, expected)
test_func = functools.partial(
_bgp_converge,
)
_, result = topotest.run_and_expect(test_func, None, count=30, wait=1)
assert result is None, "Can't converge"
def _bgp_check_received_routes():
output = json.loads(r3.vtysh_cmd("show bgp ipv4 unicast json"))
expected = {
"routes": {
"10.0.0.2/32": [
{
"valid": True,
"bestpath": True,
"nexthops": [
{"ip": "10.0.0.1", "hostname": "r1", "afi": "ipv4"}
],
}
],
}
}
return topotest.json_cmp(output, expected)
test_func = functools.partial(
_bgp_check_received_routes,
)
_, result = topotest.run_and_expect(test_func, None, count=30, wait=1)
assert result is None, "Can't see 10.0.0.2/32"
if __name__ == "__main__":
args = ["-s"] + sys.argv[1:]
sys.exit(pytest.main(args))