diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index f042d0bd95..41563c3dbc 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -7423,31 +7423,21 @@ void bgp_aggregate_toggle_suppressed(struct bgp_aggregate *aggregate, static void bgp_aggregate_med_update(struct bgp_aggregate *aggregate, struct bgp *bgp, const struct prefix *p, afi_t afi, safi_t safi, - struct bgp_path_info *pi, bool is_adding) + struct bgp_path_info *pi) { /* MED matching disabled. */ if (!aggregate->match_med) return; - /* Aggregation with different MED, nothing to do. */ - if (aggregate->med_mismatched) - return; - - /* - * Test the current entry: - * - * is_adding == true: if the new entry doesn't match then we must - * install all suppressed routes. - * - * is_adding == false: if the entry being removed was the last - * unmatching entry then we can suppress all routes. + /* Aggregation with different MED, recheck if we have got equal MEDs + * now. */ - if (!is_adding) { - if (bgp_aggregate_test_all_med(aggregate, bgp, p, afi, safi) - && aggregate->summary_only) - bgp_aggregate_toggle_suppressed(aggregate, bgp, p, afi, - safi, true); - } else + if (aggregate->med_mismatched && + bgp_aggregate_test_all_med(aggregate, bgp, p, afi, safi) && + aggregate->summary_only) + bgp_aggregate_toggle_suppressed(aggregate, bgp, p, afi, safi, + true); + else bgp_aggregate_med_match(aggregate, bgp, pi); /* No mismatches, just quit. */ @@ -7813,7 +7803,7 @@ static void bgp_add_route_to_aggregate(struct bgp *bgp, */ if (aggregate->match_med) bgp_aggregate_med_update(aggregate, bgp, aggr_p, afi, safi, - pinew, true); + pinew); if (aggregate->summary_only && AGGREGATE_MED_VALID(aggregate)) aggr_suppress_path(aggregate, pinew); @@ -7936,8 +7926,7 @@ static void bgp_remove_route_from_aggregate(struct bgp *bgp, afi_t afi, * "unsuppressing" twice. */ if (aggregate->match_med) - bgp_aggregate_med_update(aggregate, bgp, aggr_p, afi, safi, pi, - true); + bgp_aggregate_med_update(aggregate, bgp, aggr_p, afi, safi, pi); if (aggregate->count > 0) aggregate->count--; diff --git a/tests/topotests/bgp_aggregate_address_matching_med/__init__.py b/tests/topotests/bgp_aggregate_address_matching_med/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/topotests/bgp_aggregate_address_matching_med/r1/bgpd.conf b/tests/topotests/bgp_aggregate_address_matching_med/r1/bgpd.conf new file mode 100644 index 0000000000..35597602f7 --- /dev/null +++ b/tests/topotests/bgp_aggregate_address_matching_med/r1/bgpd.conf @@ -0,0 +1,21 @@ +! +router bgp 65001 + no bgp ebgp-requires-policy + neighbor 192.168.1.2 remote-as external + neighbor 192.168.1.2 timers 3 10 + address-family ipv4 unicast + redistribute connected + neighbor 192.168.1.2 route-map r2 out + exit-address-family +! +ip prefix-list p1 seq 5 permit 172.16.255.1/32 +ip prefix-list p1 seq 10 permit 172.16.255.2/32 +ip prefix-list p2 seq 15 permit 172.16.255.3/32 +! +route-map r2 permit 10 + match ip address prefix-list p1 + set metric 300 +route-map r2 permit 20 + match ip address prefix-list p2 + set metric 400 +! diff --git a/tests/topotests/bgp_aggregate_address_matching_med/r1/zebra.conf b/tests/topotests/bgp_aggregate_address_matching_med/r1/zebra.conf new file mode 100644 index 0000000000..685adb3080 --- /dev/null +++ b/tests/topotests/bgp_aggregate_address_matching_med/r1/zebra.conf @@ -0,0 +1,11 @@ +! +interface lo + ip address 172.16.255.1/32 + ip address 172.16.255.2/32 + ip address 172.16.255.3/32 +! +interface r1-eth0 + ip address 192.168.1.1/24 +! +ip forwarding +! diff --git a/tests/topotests/bgp_aggregate_address_matching_med/r2/bgpd.conf b/tests/topotests/bgp_aggregate_address_matching_med/r2/bgpd.conf new file mode 100644 index 0000000000..9bc9a3132f --- /dev/null +++ b/tests/topotests/bgp_aggregate_address_matching_med/r2/bgpd.conf @@ -0,0 +1,11 @@ +! +router bgp 65002 + no bgp ebgp-requires-policy + neighbor 192.168.1.1 remote-as external + neighbor 192.168.1.1 timers 3 10 + neighbor 192.168.2.1 remote-as external + neighbor 192.168.2.1 timers 3 10 + address-family ipv4 unicast + aggregate-address 172.16.255.0/24 summary-only matching-MED-only + exit-address-family +! diff --git a/tests/topotests/bgp_aggregate_address_matching_med/r2/zebra.conf b/tests/topotests/bgp_aggregate_address_matching_med/r2/zebra.conf new file mode 100644 index 0000000000..f229954341 --- /dev/null +++ b/tests/topotests/bgp_aggregate_address_matching_med/r2/zebra.conf @@ -0,0 +1,9 @@ +! +interface r2-eth0 + ip address 192.168.1.2/24 +! +interface r2-eth1 + ip address 192.168.2.2/24 +! +ip forwarding +! diff --git a/tests/topotests/bgp_aggregate_address_matching_med/r3/bgpd.conf b/tests/topotests/bgp_aggregate_address_matching_med/r3/bgpd.conf new file mode 100644 index 0000000000..dfb5ac7a3c --- /dev/null +++ b/tests/topotests/bgp_aggregate_address_matching_med/r3/bgpd.conf @@ -0,0 +1,6 @@ +! +router bgp 65003 + no bgp ebgp-requires-policy + neighbor 192.168.2.2 remote-as external + neighbor 192.168.2.2 timers 3 10 +! diff --git a/tests/topotests/bgp_aggregate_address_matching_med/r3/zebra.conf b/tests/topotests/bgp_aggregate_address_matching_med/r3/zebra.conf new file mode 100644 index 0000000000..11e06d47cc --- /dev/null +++ b/tests/topotests/bgp_aggregate_address_matching_med/r3/zebra.conf @@ -0,0 +1,6 @@ +! +interface r3-eth0 + ip address 192.168.2.1/24 +! +ip forwarding +! diff --git a/tests/topotests/bgp_aggregate_address_matching_med/test_bgp_aggregate_address_matching_med.py b/tests/topotests/bgp_aggregate_address_matching_med/test_bgp_aggregate_address_matching_med.py new file mode 100644 index 0000000000..edf50dc9e0 --- /dev/null +++ b/tests/topotests/bgp_aggregate_address_matching_med/test_bgp_aggregate_address_matching_med.py @@ -0,0 +1,148 @@ +#!/usr/bin/env python + +# +# Copyright (c) 2022 by +# Donatas Abraitis +# +# 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. +# + +""" +Test if aggregate-address command works fine when suppressing summary-only +and using matching-MED-only together. +""" + +import os +import sys +import json +import pytest +import functools +from lib.common_config import ( + step, +) + +# pylint: disable=C0413 +from lib import topotest +from lib.topogen import Topogen, TopoRouter, get_topogen + +pytestmark = [pytest.mark.bgpd] + +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join(CWD, "../")) + + +def build_topo(tgen): + for routern in range(1, 5): + tgen.add_router("r{}".format(routern)) + + switch = tgen.add_switch("s1") + switch.add_link(tgen.gears["r1"]) + switch.add_link(tgen.gears["r2"]) + + switch = tgen.add_switch("s2") + switch.add_link(tgen.gears["r2"]) + switch.add_link(tgen.gears["r3"]) + + +def setup_module(mod): + tgen = Topogen(build_topo, mod.__name__) + tgen.start_topology() + + router_list = tgen.routers() + + for i, (rname, router) in enumerate(router_list.items(), 1): + 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.start_router() + + +def teardown_module(mod): + tgen = get_topogen() + tgen.stop_topology() + + +def test_aggregate_address_matching_med(): + 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(r3.vtysh_cmd("show bgp ipv4 unicast json")) + expected = { + "routes": { + "172.16.255.0/24": None, + "172.16.255.1/32": [{"path": "65002 65001"}], + "172.16.255.2/32": [{"path": "65002 65001"}], + "172.16.255.3/32": [{"path": "65002 65001"}], + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial(_bgp_converge) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) + assert result is None, "Failed to see unsuppressed routes from R2" + + step("Change MED for 172.16.255.3/32 from 400 to 300") + r1.vtysh_cmd( + """ + configure terminal + route-map r2 permit 20 + set metric 300 + """ + ) + + step("Check if 172.16.255.0/24 aggregated route was created and others suppressed") + + def _bgp_aggregated_summary_only_med_match(): + output = json.loads(r3.vtysh_cmd("show bgp ipv4 unicast json")) + expected = { + "routes": { + "172.16.255.0/24": [{"path": "65002"}], + "172.16.255.1/32": None, + "172.16.255.2/32": None, + "172.16.255.3/32": None, + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial(_bgp_aggregated_summary_only_med_match) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) + assert result is None, "Failed to see unsuppressed routes from R2" + + step("Change MED for 172.16.255.3/32 back to 400 from 300") + r1.vtysh_cmd( + """ + configure terminal + route-map r2 permit 20 + set metric 400 + """ + ) + test_func = functools.partial(_bgp_converge) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) + assert result is None, "Failed to see unsuppressed routes from R2" + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args))