bgpd: Fix aggregate-address summary-only matching-MED-only

Before it worked only when configured initially via CLI. Later, when we
receive a new route, that should match a decent MED, we just skip it, because
MED mismatch is not recalculated.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
This commit is contained in:
Donatas Abraitis 2022-09-07 11:32:21 +03:00
parent 451cebeec2
commit f66624f5c0
9 changed files with 223 additions and 22 deletions

View File

@ -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--;

View File

@ -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
!

View File

@ -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
!

View File

@ -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
!

View File

@ -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
!

View File

@ -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
!

View File

@ -0,0 +1,6 @@
!
interface r3-eth0
ip address 192.168.2.1/24
!
ip forwarding
!

View File

@ -0,0 +1,148 @@
#!/usr/bin/env python
#
# Copyright (c) 2022 by
# Donatas Abraitis <donatas@opensourcerouting.org>
#
# 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))