diff --git a/bgpd/bgp_conditional_adv.c b/bgpd/bgp_conditional_adv.c index 82eb8a815e..8e2f6ff501 100644 --- a/bgpd/bgp_conditional_adv.c +++ b/bgpd/bgp_conditional_adv.c @@ -95,6 +95,9 @@ static void bgp_conditional_adv_routes(struct peer *peer, afi_t afi, if (!subgrp) return; + subgrp->pscount = 0; + SET_FLAG(subgrp->sflags, SUBGRP_STATUS_TABLE_REPARSING); + if (BGP_DEBUG(update, UPDATE_OUT)) zlog_debug("%s: %s routes to/from %s for %s", __func__, update_type == ADVERTISE ? "Advertise" : "Withdraw", @@ -162,6 +165,7 @@ static void bgp_conditional_adv_routes(struct peer *peer, afi_t afi, } } } + UNSET_FLAG(subgrp->sflags, SUBGRP_STATUS_TABLE_REPARSING); } /* Handler of conditional advertisement timer event. diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 309699fdf4..4956c7238d 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -1866,6 +1866,17 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi, piattr = bgp_path_info_mpath_count(pi) ? bgp_path_info_mpath_attr(pi) : pi->attr; + if (CHECK_FLAG(peer->af_flags[afi][safi], PEER_FLAG_MAX_PREFIX_OUT) && + peer->pmax_out[afi][safi] != 0 && + subgrp->pscount >= peer->pmax_out[afi][safi]) { + if (BGP_DEBUG(update, UPDATE_OUT) || + BGP_DEBUG(update, UPDATE_PREFIX)) { + zlog_debug("%s reached maximum prefix to be send (%u)", + peer->host, peer->pmax_out[afi][safi]); + } + return false; + } + #ifdef ENABLE_BGP_VNC if (((afi == AFI_IP) || (afi == AFI_IP6)) && (safi == SAFI_MPLS_VPN) && ((pi->type == ZEBRA_ROUTE_BGP_DIRECT) diff --git a/bgpd/bgp_updgrp.c b/bgpd/bgp_updgrp.c index dd3309dad9..a65be3d128 100644 --- a/bgpd/bgp_updgrp.c +++ b/bgpd/bgp_updgrp.c @@ -306,6 +306,7 @@ static void *updgrp_hash_alloc(void *p) * 14. encoding both global and link-local nexthop? * 15. If peer is configured to be a lonesoul, peer ip address * 16. Local-as should match, if configured. + * 17. maximum-prefix-out * ) */ static unsigned int updgrp_hash_key_make(const void *p) @@ -340,6 +341,7 @@ static unsigned int updgrp_hash_key_make(const void *p) key = jhash_1word(peer->v_routeadv, key); key = jhash_1word(peer->change_local_as, key); key = jhash_1word(peer->max_packet_size, key); + key = jhash_1word(peer->pmax_out[afi][safi], key); if (peer->group) key = jhash_1word(jhash(peer->group->name, @@ -453,6 +455,9 @@ static bool updgrp_hash_cmp(const void *p1, const void *p2) if (pe1->change_local_as != pe2->change_local_as) return false; + if (pe1->pmax_out[afi][safi] != pe2->pmax_out[afi][safi]) + return false; + /* flags like route reflector client */ if ((flags1 & PEER_UPDGRP_AF_FLAGS) != (flags2 & PEER_UPDGRP_AF_FLAGS)) return false; diff --git a/bgpd/bgp_updgrp.h b/bgpd/bgp_updgrp.h index 694636ef3d..0da6dfd19d 100644 --- a/bgpd/bgp_updgrp.h +++ b/bgpd/bgp_updgrp.h @@ -204,6 +204,9 @@ struct update_subgroup { /* send prefix count */ uint32_t scount; + /* send prefix count prior to packet update */ + uint32_t pscount; + /* announcement attribute hash */ struct hash *hash; @@ -248,6 +251,7 @@ struct update_subgroup { uint16_t sflags; #define SUBGRP_STATUS_DEFAULT_ORIGINATE (1 << 0) #define SUBGRP_STATUS_FORCE_UPDATES (1 << 1) +#define SUBGRP_STATUS_TABLE_REPARSING (1 << 2) uint16_t flags; #define SUBGRP_FLAG_NEEDS_REFRESH (1 << 0) diff --git a/bgpd/bgp_updgrp_adv.c b/bgpd/bgp_updgrp_adv.c index c6ddb1c1a7..9faf318716 100644 --- a/bgpd/bgp_updgrp_adv.c +++ b/bgpd/bgp_updgrp_adv.c @@ -481,13 +481,18 @@ void bgp_adj_out_set_subgroup(struct bgp_dest *dest, dest, subgrp, bgp_addpath_id_for_peer(peer, afi, safi, &path->tx_addpath)); - if (!adj) { + if (adj) { + if (CHECK_FLAG(subgrp->sflags, SUBGRP_STATUS_TABLE_REPARSING)) + subgrp->pscount++; + } else { adj = bgp_adj_out_alloc( subgrp, dest, bgp_addpath_id_for_peer(peer, afi, safi, &path->tx_addpath)); if (!adj) return; + + subgrp->pscount++; } /* Check if we are sending the same route. This is needed to @@ -607,6 +612,8 @@ void bgp_adj_out_unset_subgroup(struct bgp_dest *dest, /* Free allocated information. */ adj_free(adj); } + if (!CHECK_FLAG(subgrp->sflags, SUBGRP_STATUS_TABLE_REPARSING)) + subgrp->pscount--; } subgrp->version = MAX(subgrp->version, dest->version); @@ -669,6 +676,9 @@ void subgroup_announce_table(struct update_subgroup *subgrp, PEER_FLAG_DEFAULT_ORIGINATE)) subgroup_default_originate(subgrp, 0); + subgrp->pscount = 0; + SET_FLAG(subgrp->sflags, SUBGRP_STATUS_TABLE_REPARSING); + for (dest = bgp_table_top(table); dest; dest = bgp_route_next(dest)) { const struct prefix *dest_p = bgp_dest_get_prefix(dest); @@ -722,6 +732,7 @@ void subgroup_announce_table(struct update_subgroup *subgrp, } } } + UNSET_FLAG(subgrp->sflags, SUBGRP_STATUS_TABLE_REPARSING); /* * We walked through the whole table -- make sure our version number diff --git a/bgpd/bgp_updgrp_packet.c b/bgpd/bgp_updgrp_packet.c index 9c32c7ed1e..341c7dd78b 100644 --- a/bgpd/bgp_updgrp_packet.c +++ b/bgpd/bgp_updgrp_packet.c @@ -709,21 +709,6 @@ struct bpacket *subgroup_update_packet(struct update_subgroup *subgrp) addpath_tx_id = adj->addpath_tx_id; path = adv->pathi; - /* Check if we need to add a prefix to the packet if - * maximum-prefix-out is set for the peer. - */ - if (CHECK_FLAG(peer->af_flags[afi][safi], - PEER_FLAG_MAX_PREFIX_OUT) - && subgrp->scount >= peer->pmax_out[afi][safi]) { - if (BGP_DEBUG(update, UPDATE_OUT) - || BGP_DEBUG(update, UPDATE_PREFIX)) { - zlog_debug( - "%s reached maximum prefix to be send (%u)", - peer->host, peer->pmax_out[afi][safi]); - } - goto next; - } - space_remaining = STREAM_CONCAT_REMAIN(s, snlri, STREAM_SIZE(s)) - BGP_MAX_PACKET_SIZE_OVERFLOW; space_needed = @@ -876,7 +861,6 @@ struct bpacket *subgroup_update_packet(struct update_subgroup *subgrp) subgrp->scount++; adj->attr = bgp_attr_intern(adv->baa->attr); -next: adv = bgp_advertise_clean_subgroup(subgrp, adj); } diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index e3f1abe748..5eae350102 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -7666,6 +7666,7 @@ DEFUN(neighbor_maximum_prefix_out, "Maximum number of prefixes to be sent to this peer\n" "Maximum no. of prefix limit\n") { + int ret; int idx_peer = 1; int idx_number = 3; struct peer *peer; @@ -7679,20 +7680,21 @@ DEFUN(neighbor_maximum_prefix_out, max = strtoul(argv[idx_number]->arg, NULL, 10); - SET_FLAG(peer->af_flags[afi][safi], PEER_FLAG_MAX_PREFIX_OUT); - peer->pmax_out[afi][safi] = max; + ret = peer_maximum_prefix_out_set(peer, afi, safi, max); - return CMD_SUCCESS; + return bgp_vty_return(vty, ret); } DEFUN(no_neighbor_maximum_prefix_out, no_neighbor_maximum_prefix_out_cmd, - "no neighbor maximum-prefix-out", + "no neighbor maximum-prefix-out [(1-4294967295)]", NO_STR NEIGHBOR_STR NEIGHBOR_ADDR_STR2 - "Maximum number of prefixes to be sent to this peer\n") + "Maximum number of prefixes to be sent to this peer\n" + "Maximum no. of prefix limit\n") { + int ret; int idx_peer = 2; struct peer *peer; afi_t afi = bgp_node_afi(vty); @@ -7702,10 +7704,9 @@ DEFUN(no_neighbor_maximum_prefix_out, if (!peer) return CMD_WARNING_CONFIG_FAILED; - UNSET_FLAG(peer->af_flags[afi][safi], PEER_FLAG_MAX_PREFIX_OUT); - peer->pmax_out[afi][safi] = 0; + ret = peer_maximum_prefix_out_unset(peer, afi, safi); - return CMD_SUCCESS; + return bgp_vty_return(vty, ret); } /* Maximum number of prefix configuration. Prefix count is different diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 72e7a936c6..bc7203fec6 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -2012,6 +2012,10 @@ static void peer_group2peer_config_copy_af(struct peer_group *group, PEER_ATTR_INHERIT(peer, group, pmax_restart[afi][safi]); } + /* maximum-prefix-out */ + if (!CHECK_FLAG(pflags_ovrd, PEER_FLAG_MAX_PREFIX_OUT)) + PEER_ATTR_INHERIT(peer, group, pmax_out[afi][safi]); + /* allowas-in */ if (!CHECK_FLAG(pflags_ovrd, PEER_FLAG_ALLOWAS_IN)) PEER_ATTR_INHERIT(peer, group, allowas_in[afi][safi]); @@ -4220,6 +4224,7 @@ static const struct peer_flag_action peer_af_flag_action_list[] = { {PEER_FLAG_MAX_PREFIX, 0, peer_change_none}, {PEER_FLAG_MAX_PREFIX_WARNING, 0, peer_change_none}, {PEER_FLAG_MAX_PREFIX_FORCE, 0, peer_change_none}, + {PEER_FLAG_MAX_PREFIX_OUT, 0, peer_change_none}, {PEER_FLAG_NEXTHOP_LOCAL_UNCHANGED, 0, peer_change_reset_out}, {PEER_FLAG_FORCE_NEXTHOP_SELF, 1, peer_change_reset_out}, {PEER_FLAG_REMOVE_PRIVATE_AS_ALL, 1, peer_change_reset_out}, @@ -7319,6 +7324,96 @@ int peer_maximum_prefix_unset(struct peer *peer, afi_t afi, safi_t safi) return 0; } +void peer_maximum_prefix_out_refresh_routes(struct peer *peer, afi_t afi, + safi_t safi) +{ + update_group_adjust_peer(peer_af_find(peer, afi, safi)); + + if (peer_established(peer)) + bgp_announce_route(peer, afi, safi, false); +} + +int peer_maximum_prefix_out_set(struct peer *peer, afi_t afi, safi_t safi, + uint32_t max) +{ + struct peer *member; + struct listnode *node, *nnode; + + /* Set flag on peer and peer-group member if any */ + peer_af_flag_set(peer, afi, safi, PEER_FLAG_MAX_PREFIX_OUT); + /* Set configuration on peer. */ + peer->pmax_out[afi][safi] = max; + + /* Check if handling a regular peer. */ + if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + /* Skip peer-group mechanics for regular peers. */ + peer_maximum_prefix_out_refresh_routes(peer, afi, safi); + return 0; + } + + /* + * Set flag and configuration on all peer-group members, unless they + * are explicitely overriding peer-group configuration. + */ + for (ALL_LIST_ELEMENTS(peer->group->peer, node, nnode, member)) { + /* Skip peers with overridden configuration. */ + if (CHECK_FLAG(member->af_flags_override[afi][safi], + PEER_FLAG_MAX_PREFIX_OUT)) + continue; + + /* Set configuration on peer-group member. */ + member->pmax_out[afi][safi] = max; + + peer_maximum_prefix_out_refresh_routes(member, afi, safi); + } + return 0; +} + +int peer_maximum_prefix_out_unset(struct peer *peer, afi_t afi, safi_t safi) +{ + struct peer *member; + struct listnode *node; + /* Inherit configuration from peer-group if peer is member. */ + if (peer_group_active(peer)) { + peer_af_flag_inherit(peer, afi, safi, PEER_FLAG_MAX_PREFIX_OUT); + PEER_ATTR_INHERIT(peer, peer->group, pmax_out[afi][safi]); + + peer_maximum_prefix_out_refresh_routes(peer, afi, safi); + return 0; + } + + /* Remove flag and configuration from peer. */ + peer_af_flag_unset(peer, afi, safi, PEER_FLAG_MAX_PREFIX_OUT); + peer->pmax_out[afi][safi] = 0; + + /* Check if handling a regular peer. */ + if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + /* Skip peer-group mechanics for regular peers. */ + peer_maximum_prefix_out_refresh_routes(peer, afi, safi); + return 0; + } + + /* + * Remove flag and configuration from all peer-group members, unless + * they are explicitely overriding peer-group configuration. + */ + for (ALL_LIST_ELEMENTS_RO(peer->group->peer, node, member)) { + /* Skip peers with overridden configuration. */ + if (CHECK_FLAG(member->af_flags_override[afi][safi], + PEER_FLAG_MAX_PREFIX_OUT)) + continue; + + /* Remove flag and configuration on peer-group member. + */ + UNSET_FLAG(member->af_flags[afi][safi], + PEER_FLAG_MAX_PREFIX_OUT); + member->pmax_out[afi][safi] = 0; + + peer_maximum_prefix_out_refresh_routes(member, afi, safi); + } + return 0; +} + int is_ebgp_multihop_configured(struct peer *peer) { struct peer_group *group; diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 45ccf014f8..ae6427b356 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -2207,6 +2207,13 @@ extern int peer_maximum_prefix_set(struct peer *, afi_t, safi_t, uint32_t, uint8_t, int, uint16_t, bool force); extern int peer_maximum_prefix_unset(struct peer *, afi_t, safi_t); +extern void peer_maximum_prefix_out_refresh_routes(struct peer *peer, afi_t afi, + safi_t safi); +extern int peer_maximum_prefix_out_set(struct peer *peer, afi_t afi, + safi_t safi, uint32_t max); +extern int peer_maximum_prefix_out_unset(struct peer *peer, afi_t afi, + safi_t safi); + extern int peer_clear(struct peer *, struct listnode **); extern int peer_clear_soft(struct peer *, afi_t, safi_t, enum bgp_clear_type); diff --git a/tests/topotests/bgp_maximum_prefix_out/test_bgp_maximum_prefix_out.py b/tests/topotests/bgp_maximum_prefix_out/test_bgp_maximum_prefix_out.py index d45f00f697..696d6d94ce 100644 --- a/tests/topotests/bgp_maximum_prefix_out/test_bgp_maximum_prefix_out.py +++ b/tests/topotests/bgp_maximum_prefix_out/test_bgp_maximum_prefix_out.py @@ -80,22 +80,119 @@ def test_bgp_maximum_prefix_out(): if tgen.routers_have_failure(): pytest.skip(tgen.errors) - router = tgen.gears["r2"] + router1 = tgen.gears["r1"] + router2 = tgen.gears["r2"] - def _bgp_converge(router): + # format (router to configure, command, expected received prefixes on r2) + tests = [ + # test of the initial config + (None, 2), + # modifying the max-prefix-out value + ( + "router bgp\n address-family ipv4\n neighbor 192.168.255.1 maximum-prefix-out 4", + 4, + ), + # removing the max-prefix-out value + ( + "router bgp\n address-family ipv4\n no neighbor 192.168.255.1 maximum-prefix-out", + 6, + ), + # setting a max-prefix-out value + ( + "router bgp\n address-family ipv4\n neighbor 192.168.255.1 maximum-prefix-out 3", + 3, + ), + # setting a max-prefix-out value - higher than the total number of prefix + ( + "router bgp\n address-family ipv4\n neighbor 192.168.255.1 maximum-prefix-out 8", + 6, + ), + # adding a new prefix + ("router bgp\n int lo\n ip address 172.16.255.249/32", 7), + # setting a max-prefix-out value - lower than the total number of prefix + ( + "router bgp\n address-family ipv4\n neighbor 192.168.255.1 maximum-prefix-out 1", + 1, + ), + # adding a new prefix + ("router bgp\n int lo\n ip address 172.16.255.248/32", 1), + # removing the max-prefix-out value + ( + "router bgp\n address-family ipv4\n no neighbor 192.168.255.1 maximum-prefix-out 1", + 8, + ), + # test setting the existing neighbor into a peer-group with a max-prefix-out value + ( + """ + router bgp + neighbor test peer-group + neighbor test remote-as 65002 + neighbor test timers 3 10 + address-family ipv4 + neighbor test maximum-prefix-out 3 + ! + neighbor 192.168.255.1 peer-group test + """, + 3, + ), + # max-prefix-out value of the neighbor must take the precedence + ( + "router bgp\n address-family ipv4\n neighbor 192.168.255.1 maximum-prefix-out 4", + 4, + ), + ( + "router bgp\n address-family ipv4\n no neighbor 192.168.255.1 maximum-prefix-out", + 3, + ), + ( + """ + router bgp + no neighbor 192.168.255.1 peer-group test + neighbor 192.168.255.1 remote-as 65002 + neighbor 192.168.255.1 timers 3 10 + """, + 8, + ), + ( + "router bgp\n address-family ipv4\n neighbor 192.168.255.1 maximum-prefix-out 5", + 5, + ), + # test setting the existing neighbor with a max-pref-out value into a peer-group with a max-pref-out value + ("router bgp\n neighbor 192.168.255.1 peer-group test", 5), + ( + "router bgp\n address-family ipv4\n no neighbor 192.168.255.1 maximum-prefix-out 5", + 3, + ), + ] + + def _bgp_converge(router, nb_prefixes): output = json.loads(router.vtysh_cmd("show ip bgp neighbor 192.168.255.2 json")) expected = { "192.168.255.2": { "bgpState": "Established", - "addressFamilyInfo": {"ipv4Unicast": {"acceptedPrefixCounter": 2}}, + "addressFamilyInfo": { + "ipv4Unicast": {"acceptedPrefixCounter": nb_prefixes} + }, } } return topotest.json_cmp(output, expected) - test_func = functools.partial(_bgp_converge, router) - success, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + for test in tests: + cfg, exp_prfxs = test + if cfg: + cmd = ( + """ + configure terminal + %s + """ + % cfg + ) + router1.vtysh_cmd(cmd) - assert result is None, 'Failed bgp convergence in "{}"'.format(router) + test_func = functools.partial(_bgp_converge, router2, exp_prfxs) + success, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) + + assert result is None, 'Failed bgp convergence in "{}"'.format(router2) if __name__ == "__main__":