diff options
26 files changed, 540 insertions, 9 deletions
diff --git a/bgpd/bgp_advertise.h b/bgpd/bgp_advertise.h index f2cf78efd..745a0dffc 100644 --- a/bgpd/bgp_advertise.h +++ b/bgpd/bgp_advertise.h @@ -83,6 +83,9 @@ struct bgp_adj_out { /* Advertisement information. */ struct bgp_advertise *adv; + + /* Attribute hash */ + uint32_t attr_hash; }; RB_HEAD(bgp_adj_out_rb, bgp_adj_out); diff --git a/bgpd/bgp_nb.c b/bgpd/bgp_nb.c index f098332b2..08ec64242 100644 --- a/bgpd/bgp_nb.c +++ b/bgpd/bgp_nb.c @@ -352,6 +352,13 @@ const struct frr_yang_module_info frr_bgp_info = { } }, { + .xpath = "/frr-routing:routing/control-plane-protocols/control-plane-protocol/frr-bgp:bgp/global/suppress-duplicates", + .cbs = { + .cli_show = cli_show_router_bgp_suppress_duplicates, + .modify = bgp_global_suppress_duplicates_modify, + } + }, + { .xpath = "/frr-routing:routing/control-plane-protocols/control-plane-protocol/frr-bgp:bgp/global/show-hostname", .cbs = { .cli_show = cli_show_router_bgp_show_hostname, diff --git a/bgpd/bgp_nb.h b/bgpd/bgp_nb.h index f608d4f8c..9c81c2457 100644 --- a/bgpd/bgp_nb.h +++ b/bgpd/bgp_nb.h @@ -127,6 +127,7 @@ int bgp_global_fast_external_failover_modify(struct nb_cb_modify_args *args); int bgp_global_local_pref_modify(struct nb_cb_modify_args *args); int bgp_global_default_shutdown_modify(struct nb_cb_modify_args *args); int bgp_global_ebgp_requires_policy_modify(struct nb_cb_modify_args *args); +int bgp_global_suppress_duplicates_modify(struct nb_cb_modify_args *args); int bgp_global_show_hostname_modify(struct nb_cb_modify_args *args); int bgp_global_show_nexthop_hostname_modify(struct nb_cb_modify_args *args); int bgp_global_import_check_modify(struct nb_cb_modify_args *args); @@ -3639,6 +3640,9 @@ void cli_show_router_bgp_route_selection(struct vty *vty, void cli_show_router_bgp_ebgp_requires_policy(struct vty *vty, struct lyd_node *dnode, bool show_defaults); +void cli_show_router_bgp_suppress_duplicates(struct vty *vty, + struct lyd_node *dnode, + bool show_defaults); void cli_show_router_bgp_default_shutdown(struct vty *vty, struct lyd_node *dnode, bool show_defaults); diff --git a/bgpd/bgp_nb_config.c b/bgpd/bgp_nb_config.c index ec3b0c13a..bee963329 100644 --- a/bgpd/bgp_nb_config.c +++ b/bgpd/bgp_nb_config.c @@ -1599,6 +1599,27 @@ int bgp_global_ebgp_requires_policy_modify(struct nb_cb_modify_args *args) /* * XPath: + * /frr-routing:routing/control-plane-protocols/control-plane-protocol/frr-bgp:bgp/global/suppress-duplicates + */ +int bgp_global_suppress_duplicates_modify(struct nb_cb_modify_args *args) +{ + if (args->event != NB_EV_APPLY) + return NB_OK; + + struct bgp *bgp; + + bgp = nb_running_get_entry(args->dnode, NULL, true); + + if (yang_dnode_get_bool(args->dnode, NULL)) + SET_FLAG(bgp->flags, BGP_FLAG_SUPPRESS_DUPLICATES); + else + UNSET_FLAG(bgp->flags, BGP_FLAG_SUPPRESS_DUPLICATES); + + return NB_OK; +} + +/* + * XPath: * /frr-routing:routing/control-plane-protocols/control-plane-protocol/frr-bgp:bgp/global/show-hostname */ int bgp_global_show_hostname_modify(struct nb_cb_modify_args *args) diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index dfc64c043..b2b9e04bc 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -444,6 +444,13 @@ int bgp_generate_updgrp_packets(struct thread *thread) * yet. */ if (!next_pkt || !next_pkt->buffer) { + /* Make sure we supress BGP UPDATES + * for normal processing later again. + */ + if (!paf->t_announce_route) + UNSET_FLAG(paf->subgroup->sflags, + SUBGRP_STATUS_FORCE_UPDATES); + if (CHECK_FLAG(peer->cap, PEER_CAP_RESTART_RCV)) { if (!(PAF_SUBGRP(paf))->t_coalesce @@ -2116,6 +2123,11 @@ static int bgp_route_refresh_receive(struct peer *peer, bgp_size_t size) peer->orf_plist[afi][safi]; } + /* Avoid supressing duplicate routes later + * when processing in subgroup_announce_table(). + */ + SET_FLAG(paf->subgroup->sflags, SUBGRP_STATUS_FORCE_UPDATES); + /* If the peer is configured for default-originate clear the * SUBGRP_STATUS_DEFAULT_ORIGINATE flag so that we will * re-advertise the diff --git a/bgpd/bgp_updgrp.c b/bgpd/bgp_updgrp.c index 2788a8ea4..059e05ef7 100644 --- a/bgpd/bgp_updgrp.c +++ b/bgpd/bgp_updgrp.c @@ -1387,6 +1387,11 @@ static int updgrp_policy_update_walkcb(struct update_group *updgrp, void *arg) } UPDGRP_FOREACH_SUBGRP (updgrp, subgrp) { + /* Avoid supressing duplicate routes later + * when processing in subgroup_announce_table(). + */ + SET_FLAG(subgrp->sflags, SUBGRP_STATUS_FORCE_UPDATES); + if (changed) { if (bgp_debug_update(NULL, NULL, updgrp, 0)) zlog_debug( diff --git a/bgpd/bgp_updgrp.h b/bgpd/bgp_updgrp.h index 9cad78c26..7261933dc 100644 --- a/bgpd/bgp_updgrp.h +++ b/bgpd/bgp_updgrp.h @@ -252,20 +252,14 @@ struct update_subgroup { uint64_t id; uint16_t sflags; +#define SUBGRP_STATUS_DEFAULT_ORIGINATE (1 << 0) +#define SUBGRP_STATUS_FORCE_UPDATES (1 << 1) - /* Subgroup flags, see below */ uint16_t flags; +#define SUBGRP_FLAG_NEEDS_REFRESH (1 << 0) }; /* - * We need to do an outbound refresh to get this subgroup into a - * consistent state. - */ -#define SUBGRP_FLAG_NEEDS_REFRESH (1 << 0) - -#define SUBGRP_STATUS_DEFAULT_ORIGINATE (1 << 0) - -/* * Add the given value to the specified counter on a subgroup and its * parent structures. */ diff --git a/bgpd/bgp_updgrp_adv.c b/bgpd/bgp_updgrp_adv.c index da9e1f28a..b1ff9ac25 100644 --- a/bgpd/bgp_updgrp_adv.c +++ b/bgpd/bgp_updgrp_adv.c @@ -464,6 +464,7 @@ void bgp_adj_out_set_subgroup(struct bgp_dest *dest, struct peer *adv_peer; struct peer_af *paf; struct bgp *bgp; + uint32_t attr_hash = attrhash_key_make(attr); peer = SUBGRP_PEER(subgrp); afi = SUBGRP_AFI(subgrp); @@ -487,6 +488,26 @@ void bgp_adj_out_set_subgroup(struct bgp_dest *dest, return; } + /* Check if we are sending the same route. This is needed to + * avoid duplicate UPDATES. For instance, filtering communities + * at egress, neighbors will see duplicate UPDATES despite + * the route wasn't changed actually. + * Do not suppress BGP UPDATES for route-refresh. + */ + if (CHECK_FLAG(bgp->flags, BGP_FLAG_SUPPRESS_DUPLICATES) + && !CHECK_FLAG(subgrp->sflags, SUBGRP_STATUS_FORCE_UPDATES) + && adj->attr_hash == attr_hash) { + if (BGP_DEBUG(update, UPDATE_OUT)) { + char attr_str[BUFSIZ] = {0}; + + bgp_dump_attr(attr, attr_str, sizeof(attr_str)); + + zlog_debug("%s suppress UPDATE w/ attr: %s", peer->host, + attr_str); + } + return; + } + if (adj->adv) bgp_advertise_clean_subgroup(subgrp, adj); adj->adv = bgp_advertise_new(); @@ -502,6 +523,7 @@ void bgp_adj_out_set_subgroup(struct bgp_dest *dest, else adv->baa = baa_new(); adv->adj = adj; + adj->attr_hash = attr_hash; /* Add new advertisement to advertisement attribute list. */ bgp_advertise_add(adv->baa, adv); diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index cfac0bbb5..4cdd4d2e6 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -119,6 +119,10 @@ FRR_CFG_DEFAULT_BOOL(BGP_EBGP_REQUIRES_POLICY, { .val_bool = false, .match_version = "< 7.4", }, { .val_bool = true }, ) +FRR_CFG_DEFAULT_BOOL(BGP_SUPPRESS_DUPLICATES, + { .val_bool = false, .match_version = "< 7.6", }, + { .val_bool = true }, +) DEFINE_HOOK(bgp_inst_config_write, (struct bgp *bgp, struct vty *vty), @@ -487,6 +491,8 @@ int bgp_get_vty(struct bgp **bgp, as_t *as, const char *name, SET_FLAG((*bgp)->flags, BGP_FLAG_DETERMINISTIC_MED); if (DFLT_BGP_EBGP_REQUIRES_POLICY) SET_FLAG((*bgp)->flags, BGP_FLAG_EBGP_REQUIRES_POLICY); + if (DFLT_BGP_SUPPRESS_DUPLICATES) + SET_FLAG((*bgp)->flags, BGP_FLAG_SUPPRESS_DUPLICATES); ret = BGP_SUCCESS; } @@ -876,6 +882,7 @@ static int bgp_peer_clear(struct peer *peer, afi_t afi, safi_t safi, struct listnode **nnode, enum bgp_clear_type stype) { int ret = 0; + struct peer_af *paf; /* if afi/.safi not specified, spin thru all of them */ if ((afi == AFI_UNSPEC) && (safi == SAFI_UNSPEC)) { @@ -883,6 +890,11 @@ static int bgp_peer_clear(struct peer *peer, afi_t afi, safi_t safi, safi_t tmp_safi; FOREACH_AFI_SAFI (tmp_afi, tmp_safi) { + paf = peer_af_find(peer, tmp_afi, tmp_safi); + if (paf && paf->subgroup) + SET_FLAG(paf->subgroup->sflags, + SUBGRP_STATUS_FORCE_UPDATES); + if (!peer->afc[tmp_afi][tmp_safi]) continue; @@ -901,6 +913,11 @@ static int bgp_peer_clear(struct peer *peer, afi_t afi, safi_t safi, if (!peer->afc[afi][tmp_safi]) continue; + paf = peer_af_find(peer, afi, tmp_safi); + if (paf && paf->subgroup) + SET_FLAG(paf->subgroup->sflags, + SUBGRP_STATUS_FORCE_UPDATES); + if (stype == BGP_CLEAR_SOFT_NONE) ret = peer_clear(peer, nnode); else @@ -912,6 +929,11 @@ static int bgp_peer_clear(struct peer *peer, afi_t afi, safi_t safi, if (!peer->afc[afi][safi]) return 1; + paf = peer_af_find(peer, afi, safi); + if (paf && paf->subgroup) + SET_FLAG(paf->subgroup->sflags, + SUBGRP_STATUS_FORCE_UPDATES); + if (stype == BGP_CLEAR_SOFT_NONE) ret = peer_clear(peer, nnode); else @@ -2555,6 +2577,37 @@ DEFUN_YANG(no_bgp_always_compare_med, return nb_cli_apply_changes(vty, NULL); } +DEFUN_YANG(bgp_suppress_duplicates, + bgp_suppress_duplicates_cmd, + "bgp suppress-duplicates", + "BGP specific commands\n" + "Suppress duplicate updates if the route actually not changed\n") +{ + nb_cli_enqueue_change(vty, "./global/suppress-duplicates", + NB_OP_MODIFY, "true"); + return nb_cli_apply_changes(vty, NULL); +} + +DEFUN_YANG(no_bgp_suppress_duplicates, + no_bgp_suppress_duplicates_cmd, + "no bgp suppress-duplicates", + NO_STR + "BGP specific commands\n" + "Suppress duplicate updates if the route actually not changed\n") +{ + nb_cli_enqueue_change(vty, "./global/suppress-duplicates", + NB_OP_MODIFY, "false"); + return nb_cli_apply_changes(vty, NULL); +} + +void cli_show_router_bgp_suppress_duplicates(struct vty *vty, + struct lyd_node *dnode, + bool show_defaults) +{ + if (yang_dnode_get_bool(dnode, NULL) != SAVE_BGP_SUPPRESS_DUPLICATES) + vty_out(vty, " bgp suppress-duplicates\n"); +} + DEFUN_YANG(bgp_ebgp_requires_policy, bgp_ebgp_requires_policy_cmd, "bgp ebgp-requires-policy", @@ -17031,6 +17084,16 @@ int bgp_config_write(struct vty *vty) if (bgp->reject_as_sets) vty_out(vty, " bgp reject-as-sets\n"); + /* Suppress duplicate updates if the route actually not changed + */ + if (!!CHECK_FLAG(bgp->flags, BGP_FLAG_SUPPRESS_DUPLICATES) + != SAVE_BGP_SUPPRESS_DUPLICATES) + vty_out(vty, " %sbgp suppress-duplicates\n", + CHECK_FLAG(bgp->flags, + BGP_FLAG_SUPPRESS_DUPLICATES) + ? "" + : "no "); + /* BGP default ipv4-unicast. */ if (CHECK_FLAG(bgp->flags, BGP_FLAG_NO_DEFAULT_IPV4)) vty_out(vty, " no bgp default ipv4-unicast\n"); @@ -17614,6 +17677,10 @@ void bgp_vty_init(void) install_element(BGP_NODE, &bgp_ebgp_requires_policy_cmd); install_element(BGP_NODE, &no_bgp_ebgp_requires_policy_cmd); + /* bgp suppress-duplicates */ + install_element(BGP_NODE, &bgp_suppress_duplicates_cmd); + install_element(BGP_NODE, &no_bgp_suppress_duplicates_cmd); + /* bgp reject-as-sets */ install_element(BGP_NODE, &bgp_reject_as_sets_cmd); install_element(BGP_NODE, &no_bgp_reject_as_sets_cmd); diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 775be7980..16210bed1 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -464,6 +464,7 @@ struct bgp { /* This flag is set if the instance is in administrative shutdown */ #define BGP_FLAG_SHUTDOWN (1 << 27) #define BGP_FLAG_SUPPRESS_FIB_PENDING (1 << 28) +#define BGP_FLAG_SUPPRESS_DUPLICATES (1 << 29) enum global_mode GLOBAL_GR_FSM[BGP_GLOBAL_GR_MODE] [BGP_GLOBAL_GR_EVENT_CMD]; diff --git a/doc/user/bgp.rst b/doc/user/bgp.rst index e0e16aaee..7549cec3e 100644 --- a/doc/user/bgp.rst +++ b/doc/user/bgp.rst @@ -454,6 +454,17 @@ Reject routes with AS_SET or AS_CONFED_SET types This command enables rejection of incoming and outgoing routes having AS_SET or AS_CONFED_SET type. +Suppress duplicate updates +-------------------------- + +.. index:: [no] bgp suppress-duplicates +.. clicmd:: [no] bgp suppress-duplicates + + For example, BGP routers can generate multiple identical announcements with + empty community attributes if stripped at egress. This is an undesired behavior. + Suppress duplicate updates if the route actually not changed. + Default: enabled. + Disable checking if nexthop is connected on EBGP sessions --------------------------------------------------------- diff --git a/tests/topotests/bgp_community_change_update/__init__.py b/tests/topotests/bgp_community_change_update/__init__.py new file mode 100644 index 000000000..e69de29bb --- /dev/null +++ b/tests/topotests/bgp_community_change_update/__init__.py diff --git a/tests/topotests/bgp_community_change_update/c1/bgpd.conf b/tests/topotests/bgp_community_change_update/c1/bgpd.conf new file mode 100644 index 000000000..24cf9dffb --- /dev/null +++ b/tests/topotests/bgp_community_change_update/c1/bgpd.conf @@ -0,0 +1,11 @@ +! +debug bgp updates +! +router bgp 65001 + no bgp ebgp-requires-policy + neighbor 10.0.1.2 remote-as external + neighbor 10.0.1.2 timers 3 10 + address-family ipv4 unicast + redistribute connected + exit-address-family +! diff --git a/tests/topotests/bgp_community_change_update/c1/zebra.conf b/tests/topotests/bgp_community_change_update/c1/zebra.conf new file mode 100644 index 000000000..e3dbbc051 --- /dev/null +++ b/tests/topotests/bgp_community_change_update/c1/zebra.conf @@ -0,0 +1,6 @@ +! +interface c1-eth0 + ip address 10.0.1.1/30 +! +ip forwarding +! diff --git a/tests/topotests/bgp_community_change_update/test_bgp_community_change_update.py b/tests/topotests/bgp_community_change_update/test_bgp_community_change_update.py new file mode 100644 index 000000000..5fc431026 --- /dev/null +++ b/tests/topotests/bgp_community_change_update/test_bgp_community_change_update.py @@ -0,0 +1,225 @@ +#!/usr/bin/env python + +# Copyright (c) 2020 by +# Donatas Abraitis <donatas.abraitis@gmail.com> +# +# 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. +# + +""" +Reference: https://www.cmand.org/communityexploration + + --y2-- + / | \ + c1 ---- x1 ---- y1 | z1 + \ | / + --y3-- + +1. z1 announces 192.168.255.254/32 to y2, y3. +2. y2 and y3 tags this prefix at ingress with appropriate +communities 65004:2 (y2) and 65004:3 (y3). +3. x1 filters all communities at the egress to c1. +4. Shutdown the link between y1 and y2. +5. y1 will generate a BGP UPDATE message regarding the next-hop change. +6. x1 will generate a BGP UPDATE message regarding community change. + +To avoid sending duplicate BGP UPDATE messages we should make sure +we send only actual route updates. In this example, x1 will skip +BGP UPDATE to c1 because the actual route is the same +(filtered communities - nothing changes). +""" + +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, TopoRouter, get_topogen +from lib.topolog import logger +from mininet.topo import Topo + +from lib.common_config import step +from time import sleep + + +class TemplateTopo(Topo): + def build(self, *_args, **_opts): + tgen = get_topogen(self) + + tgen.add_router("z1") + tgen.add_router("y1") + tgen.add_router("y2") + tgen.add_router("y3") + tgen.add_router("x1") + tgen.add_router("c1") + + # 10.0.1.0/30 + switch = tgen.add_switch("s1") + switch.add_link(tgen.gears["c1"]) + switch.add_link(tgen.gears["x1"]) + + # 10.0.2.0/30 + switch = tgen.add_switch("s2") + switch.add_link(tgen.gears["x1"]) + switch.add_link(tgen.gears["y1"]) + + # 10.0.3.0/30 + switch = tgen.add_switch("s3") + switch.add_link(tgen.gears["y1"]) + switch.add_link(tgen.gears["y2"]) + + # 10.0.4.0/30 + switch = tgen.add_switch("s4") + switch.add_link(tgen.gears["y1"]) + switch.add_link(tgen.gears["y3"]) + + # 10.0.5.0/30 + switch = tgen.add_switch("s5") + switch.add_link(tgen.gears["y2"]) + switch.add_link(tgen.gears["y3"]) + + # 10.0.6.0/30 + switch = tgen.add_switch("s6") + switch.add_link(tgen.gears["y2"]) + switch.add_link(tgen.gears["z1"]) + + # 10.0.7.0/30 + switch = tgen.add_switch("s7") + switch.add_link(tgen.gears["y3"]) + switch.add_link(tgen.gears["z1"]) + + +def setup_module(mod): + tgen = Topogen(TemplateTopo, 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_bgp_community_update_path_change(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + def _bgp_converge_initial(): + output = json.loads( + tgen.gears["c1"].vtysh_cmd("show ip bgp neighbor 10.0.1.2 json") + ) + expected = { + "10.0.1.2": { + "bgpState": "Established", + "addressFamilyInfo": {"ipv4Unicast": {"acceptedPrefixCounter": 8}}, + } + } + return topotest.json_cmp(output, expected) + + step("Check if an initial topology is converged") + test_func = functools.partial(_bgp_converge_initial) + success, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result is None, "Failed to see bgp convergence in c1" + + step("Disable link between y1 and y2") + tgen.gears["y1"].run("ip link set dev y1-eth1 down") + + def _bgp_converge_link_disabled(): + output = json.loads(tgen.gears["y1"].vtysh_cmd("show ip bgp nei 10.0.3.2 json")) + expected = {"10.0.3.2": {"bgpState": "Active"}} + return topotest.json_cmp(output, expected) + + step("Check if a topology is converged after a link down between y1 and y2") + test_func = functools.partial(_bgp_converge_link_disabled) + success, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result is None, "Failed to see bgp convergence in y1" + + def _bgp_check_for_duplicate_updates(): + duplicate = False + i = 0 + while i < 5: + if ( + len( + tgen.gears["c1"].run( + 'grep "10.0.1.2 rcvd 192.168.255.254/32 IPv4 unicast...duplicate ignored" bgpd.log' + ) + ) + > 0 + ): + duplicate = True + i += 1 + sleep(0.5) + return duplicate + + step("Check if we see duplicate BGP UPDATE message in c1 (suppress-duplicates)") + assert ( + _bgp_check_for_duplicate_updates() == False + ), "Seen duplicate BGP UPDATE message in c1 from x1" + + step("Disable bgp suppress-duplicates at x1") + tgen.gears["x1"].run( + "vtysh -c 'conf' -c 'router bgp' -c 'no bgp suppress-duplicates'" + ) + + step("Enable link between y1 and y2") + tgen.gears["y1"].run("ip link set dev y1-eth1 up") + + def _bgp_converge_link_enabled(): + output = json.loads(tgen.gears["y1"].vtysh_cmd("show ip bgp nei 10.0.3.2 json")) + expected = { + "10.0.3.2": { + "bgpState": "Established", + "addressFamilyInfo": { + "ipv4Unicast": {"acceptedPrefixCounter": 5, "sentPrefixCounter": 4} + }, + } + } + return topotest.json_cmp(output, expected) + + step("Check if a topology is converged after a link up between y1 and y2") + test_func = functools.partial(_bgp_converge_link_enabled) + success, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result is None, "Failed to see bgp convergence in y1" + + step( + "Check if we see duplicate BGP UPDATE message in c1 (no bgp suppress-duplicates)" + ) + assert ( + _bgp_check_for_duplicate_updates() == True + ), "Didn't see duplicate BGP UPDATE message in c1 from x1" + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args)) diff --git a/tests/topotests/bgp_community_change_update/x1/bgpd.conf b/tests/topotests/bgp_community_change_update/x1/bgpd.conf new file mode 100644 index 000000000..8d7bcb948 --- /dev/null +++ b/tests/topotests/bgp_community_change_update/x1/bgpd.conf @@ -0,0 +1,20 @@ +! +debug bgp updates +! +router bgp 65002 + no bgp ebgp-requires-policy + neighbor 10.0.1.1 remote-as external + neighbor 10.0.1.1 timers 3 10 + neighbor 10.0.2.2 remote-as external + neighbor 10.0.2.2 timers 3 10 + address-family ipv4 unicast + redistribute connected + neighbor 10.0.1.1 route-map c1 out + exit-address-family +! +bgp community-list standard c1 seq 1 permit 65004:2 +bgp community-list standard c1 seq 2 permit 65004:3 +! +route-map c1 permit 10 + set comm-list c1 delete +! diff --git a/tests/topotests/bgp_community_change_update/x1/zebra.conf b/tests/topotests/bgp_community_change_update/x1/zebra.conf new file mode 100644 index 000000000..8b7c03ffb --- /dev/null +++ b/tests/topotests/bgp_community_change_update/x1/zebra.conf @@ -0,0 +1,9 @@ +! +interface x1-eth0 + ip address 10.0.1.2/30 +! +interface x1-eth1 + ip address 10.0.2.1/30 +! +ip forwarding +! diff --git a/tests/topotests/bgp_community_change_update/y1/bgpd.conf b/tests/topotests/bgp_community_change_update/y1/bgpd.conf new file mode 100644 index 000000000..a01008583 --- /dev/null +++ b/tests/topotests/bgp_community_change_update/y1/bgpd.conf @@ -0,0 +1,12 @@ +router bgp 65003 + no bgp ebgp-requires-policy + neighbor 10.0.2.1 remote-as external + neighbor 10.0.2.1 timers 3 10 + neighbor 10.0.3.2 remote-as internal + neighbor 10.0.3.2 timers 3 10 + neighbor 10.0.4.2 remote-as internal + neighbor 10.0.4.2 timers 3 10 + address-family ipv4 unicast + redistribute connected + exit-address-family +! diff --git a/tests/topotests/bgp_community_change_update/y1/zebra.conf b/tests/topotests/bgp_community_change_update/y1/zebra.conf new file mode 100644 index 000000000..4096f2a2e --- /dev/null +++ b/tests/topotests/bgp_community_change_update/y1/zebra.conf @@ -0,0 +1,12 @@ +! +interface y1-eth0 + ip address 10.0.2.2/30 +! +interface y1-eth1 + ip address 10.0.3.1/30 +! +interface y1-eth2 + ip address 10.0.4.1/30 +! +ip forwarding +! diff --git a/tests/topotests/bgp_community_change_update/y2/bgpd.conf b/tests/topotests/bgp_community_change_update/y2/bgpd.conf new file mode 100644 index 000000000..27f1e81f7 --- /dev/null +++ b/tests/topotests/bgp_community_change_update/y2/bgpd.conf @@ -0,0 +1,18 @@ +router bgp 65003 + no bgp ebgp-requires-policy + neighbor 10.0.3.1 remote-as internal + neighbor 10.0.3.1 timers 3 10 + neighbor 10.0.5.2 remote-as internal + neighbor 10.0.5.2 timers 3 10 + neighbor 10.0.6.2 remote-as external + neighbor 10.0.6.2 timers 3 10 + address-family ipv4 unicast + redistribute connected + neighbor 10.0.3.1 route-reflector-client + neighbor 10.0.5.2 route-reflector-client + neighbor 10.0.6.2 route-map z1 in + exit-address-family +! +route-map z1 permit 10 + set community 65004:2 +! diff --git a/tests/topotests/bgp_community_change_update/y2/zebra.conf b/tests/topotests/bgp_community_change_update/y2/zebra.conf new file mode 100644 index 000000000..7e9458a45 --- /dev/null +++ b/tests/topotests/bgp_community_change_update/y2/zebra.conf @@ -0,0 +1,12 @@ +! +interface y2-eth0 + ip address 10.0.3.2/30 +! +interface y2-eth1 + ip address 10.0.5.1/30 +! +interface y2-eth2 + ip address 10.0.6.1/30 +! +ip forwarding +! diff --git a/tests/topotests/bgp_community_change_update/y3/bgpd.conf b/tests/topotests/bgp_community_change_update/y3/bgpd.conf new file mode 100644 index 000000000..8e5728492 --- /dev/null +++ b/tests/topotests/bgp_community_change_update/y3/bgpd.conf @@ -0,0 +1,18 @@ +router bgp 65003 + no bgp ebgp-requires-policy + neighbor 10.0.4.1 remote-as internal + neighbor 10.0.4.1 timers 3 10 + neighbor 10.0.5.1 remote-as internal + neighbor 10.0.5.1 timers 3 10 + neighbor 10.0.7.2 remote-as external + neighbor 10.0.7.2 timers 3 10 + address-family ipv4 unicast + redistribute connected + neighbor 10.0.4.1 route-reflector-client + neighbor 10.0.5.1 route-reflector-client + neighbor 10.0.7.2 route-map z1 in + exit-address-family +! +route-map z1 permit 10 + set community 65004:3 +! diff --git a/tests/topotests/bgp_community_change_update/y3/zebra.conf b/tests/topotests/bgp_community_change_update/y3/zebra.conf new file mode 100644 index 000000000..93dd87dbd --- /dev/null +++ b/tests/topotests/bgp_community_change_update/y3/zebra.conf @@ -0,0 +1,12 @@ +! +interface y3-eth0 + ip address 10.0.4.2/30 +! +interface y3-eth1 + ip address 10.0.5.2/30 +! +interface y3-eth2 + ip address 10.0.7.1/30 +! +ip forwarding +! diff --git a/tests/topotests/bgp_community_change_update/z1/bgpd.conf b/tests/topotests/bgp_community_change_update/z1/bgpd.conf new file mode 100644 index 000000000..2f470f18b --- /dev/null +++ b/tests/topotests/bgp_community_change_update/z1/bgpd.conf @@ -0,0 +1,10 @@ +router bgp 65004 + no bgp ebgp-requires-policy + neighbor 10.0.6.1 remote-as external + neighbor 10.0.6.1 timers 3 10 + neighbor 10.0.7.1 remote-as external + neighbor 10.0.7.1 timers 3 10 + address-family ipv4 unicast + redistribute connected + exit-address-family +! diff --git a/tests/topotests/bgp_community_change_update/z1/zebra.conf b/tests/topotests/bgp_community_change_update/z1/zebra.conf new file mode 100644 index 000000000..7f6bbb66b --- /dev/null +++ b/tests/topotests/bgp_community_change_update/z1/zebra.conf @@ -0,0 +1,12 @@ +! +interface lo + ip address 192.168.255.254/32 +! +interface z1-eth0 + ip address 10.0.6.2/30 +! +interface z1-eth1 + ip address 10.0.7.2/30 +! +ip forwarding +! diff --git a/yang/frr-bgp-common.yang b/yang/frr-bgp-common.yang index cb1a6a8f5..1840e3728 100644 --- a/yang/frr-bgp-common.yang +++ b/yang/frr-bgp-common.yang @@ -358,6 +358,13 @@ submodule frr-bgp-common { "Apply administrative shutdown to newly configured peers."; } + leaf suppress-duplicates { + type boolean; + default "true"; + description + "Suppress duplicate updates if the route actually not changed."; + } + leaf ebgp-requires-policy { type boolean; default "true"; |