From 4338e21aa2feba57ea7004c36362e5d8186340b8 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Sat, 25 Jan 2025 20:28:26 +0200 Subject: [PATCH] Revert "bgpd: Handle Addpath capability using dynamic capabilities" This reverts commit 05cf9d03b345393b8d63ffe9345c42debd8362b6. TL;DR; Handling BGP AddPath capability is not trivial (possible) dynamically. When the sender is AddPath-capable and sends NLRIs encoded with AddPath ID, and at the same time the receiver sends AddPath capability "disable-addpath-rx" (flag update) via dynamic capabilities, both peers are out of sync about the AddPath state. The receiver thinks already he's not AddPath-capable anymore, hence it tries to parse NLRIs as non-AddPath, while they are actually encoded as AddPath. AddPath capability itself does not provide (in RFC) any mechanism on backward compatible way to handle NLRIs if they come mixed (AddPath + non-AddPath). This explains why we have failures in our CI periodically. Signed-off-by: Donatas Abraitis --- bgpd/bgp_addpath.c | 32 +-- bgpd/bgp_addpath.h | 9 +- bgpd/bgp_packet.c | 183 +----------------- bgpd/bgp_vty.c | 33 +--- bgpd/bgpd.c | 67 +++---- .../bgp_dynamic_capability/r2/frr.conf | 4 +- ...test_bgp_dynamic_capability_path_limit.py} | 121 +----------- 7 files changed, 56 insertions(+), 393 deletions(-) rename tests/topotests/bgp_dynamic_capability/{test_bgp_dynamic_capability_addpath.py => test_bgp_dynamic_capability_path_limit.py} (63%) diff --git a/bgpd/bgp_addpath.c b/bgpd/bgp_addpath.c index aada6e555f..030db4b28e 100644 --- a/bgpd/bgp_addpath.c +++ b/bgpd/bgp_addpath.c @@ -10,8 +10,6 @@ #include "bgp_addpath.h" #include "bgp_route.h" -#include "bgp_open.h" -#include "bgp_packet.h" static const struct bgp_addpath_strategy_names strat_names[BGP_ADDPATH_MAX] = { { @@ -361,30 +359,6 @@ void bgp_addpath_type_changed(struct bgp *bgp) } } -int bgp_addpath_capability_action(enum bgp_addpath_strat addpath_type, uint16_t paths) -{ - int action = CAPABILITY_ACTION_UNSET; - - switch (addpath_type) { - case BGP_ADDPATH_ALL: - case BGP_ADDPATH_BEST_PER_AS: - action = CAPABILITY_ACTION_SET; - break; - case BGP_ADDPATH_BEST_SELECTED: - if (paths) - action = CAPABILITY_ACTION_SET; - else - action = CAPABILITY_ACTION_UNSET; - break; - case BGP_ADDPATH_NONE: - case BGP_ADDPATH_MAX: - action = CAPABILITY_ACTION_UNSET; - break; - } - - return action; -} - /* * Change the addpath type assigned to a peer, or peer group. In addition to * adjusting the counts, peer sessions will be reset as needed to make the @@ -398,7 +372,6 @@ void bgp_addpath_set_peer_type(struct peer *peer, afi_t afi, safi_t safi, struct listnode *node, *nnode; struct peer *tmp_peer; struct peer_group *group; - int action = bgp_addpath_capability_action(addpath_type, paths); if (safi == SAFI_LABELED_UNICAST) safi = SAFI_UNICAST; @@ -456,12 +429,9 @@ void bgp_addpath_set_peer_type(struct peer *peer, afi_t afi, safi_t safi, } } } else { - if (!CHECK_FLAG(peer->cap, PEER_CAP_DYNAMIC_RCV) && - !CHECK_FLAG(peer->cap, PEER_CAP_DYNAMIC_ADV)) - peer_change_action(peer, afi, safi, peer_change_reset); + peer_change_action(peer, afi, safi, peer_change_reset); } - bgp_capability_send(peer, afi, safi, CAPABILITY_CODE_ADDPATH, action); } /* diff --git a/bgpd/bgp_addpath.h b/bgpd/bgp_addpath.h index f1ff98ea7a..c136671ea4 100644 --- a/bgpd/bgp_addpath.h +++ b/bgpd/bgp_addpath.h @@ -15,11 +15,7 @@ #include "bgpd/bgp_table.h" #include "lib/json.h" -struct bgp_addpath_capability { - uint16_t afi; - uint8_t safi; - uint8_t flags; -}; +#define BGP_ADDPATH_TX_ID_FOR_DEFAULT_ORIGINATE 1 struct bgp_paths_limit_capability { uint16_t afi; @@ -27,8 +23,6 @@ struct bgp_paths_limit_capability { uint16_t paths_limit; } __attribute__((packed)); -#define BGP_ADDPATH_TX_ID_FOR_DEFAULT_ORIGINATE 1 - void bgp_addpath_init_bgp_data(struct bgp_addpath_bgp_data *d); bool bgp_addpath_is_addpath_used(struct bgp_addpath_bgp_data *d, afi_t afi, @@ -68,5 +62,4 @@ void bgp_addpath_update_ids(struct bgp *bgp, struct bgp_dest *dest, afi_t afi, safi_t safi); void bgp_addpath_type_changed(struct bgp *bgp); -extern int bgp_addpath_capability_action(enum bgp_addpath_strat addpath_type, uint16_t paths); #endif diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index ca2e8de041..0780b4f72d 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -1218,7 +1218,6 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi, uint16_t len; uint32_t gr_restart_time; uint8_t addpath_afi_safi_count = 0; - bool adv_addpath_tx = false; unsigned long number_of_orfs_p; uint8_t number_of_orfs = 0; const char *capability = lookup_msg(capcode_str, capability_code, @@ -1383,87 +1382,6 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi, COND_FLAG(peer->cap, PEER_CAP_LLGR_ADV, action == CAPABILITY_ACTION_SET); break; - case CAPABILITY_CODE_ADDPATH: - FOREACH_AFI_SAFI (afi, safi) { - if (peer->afc[afi][safi]) { - addpath_afi_safi_count++; - - /* Only advertise addpath TX if a feature that - * will use it is - * configured */ - if (peer->addpath_type[afi][safi] != - BGP_ADDPATH_NONE) - adv_addpath_tx = true; - - /* If we have enabled labeled unicast, we MUST check - * against unicast SAFI because addpath IDs are - * allocated under unicast SAFI, the same as the RIB - * is managed in unicast SAFI. - */ - if (safi == SAFI_LABELED_UNICAST) - if (peer->addpath_type[afi][SAFI_UNICAST] != - BGP_ADDPATH_NONE) - adv_addpath_tx = true; - } - } - - stream_putc(s, action); - stream_putc(s, CAPABILITY_CODE_ADDPATH); - stream_putc(s, CAPABILITY_CODE_ADDPATH_LEN * - addpath_afi_safi_count); - - FOREACH_AFI_SAFI (afi, safi) { - if (peer->afc[afi][safi]) { - bool adv_addpath_rx = - !CHECK_FLAG(peer->af_flags[afi][safi], - PEER_FLAG_DISABLE_ADDPATH_RX); - uint8_t flags = 0; - - /* Convert AFI, SAFI to values for packet. */ - bgp_map_afi_safi_int2iana(afi, safi, &pkt_afi, - &pkt_safi); - - stream_putw(s, pkt_afi); - stream_putc(s, pkt_safi); - - if (adv_addpath_rx) { - SET_FLAG(flags, BGP_ADDPATH_RX); - SET_FLAG(peer->af_cap[afi][safi], - PEER_CAP_ADDPATH_AF_RX_ADV); - } else { - UNSET_FLAG(peer->af_cap[afi][safi], - PEER_CAP_ADDPATH_AF_RX_ADV); - } - - if (adv_addpath_tx) { - SET_FLAG(flags, BGP_ADDPATH_TX); - SET_FLAG(peer->af_cap[afi][safi], - PEER_CAP_ADDPATH_AF_TX_ADV); - if (safi == SAFI_LABELED_UNICAST) - SET_FLAG(peer->af_cap[afi] - [SAFI_UNICAST], - PEER_CAP_ADDPATH_AF_TX_ADV); - } else { - UNSET_FLAG(peer->af_cap[afi][safi], - PEER_CAP_ADDPATH_AF_TX_ADV); - } - - stream_putc(s, flags); - } - } - - if (bgp_debug_neighbor_events(peer)) - zlog_debug("%pBP sending CAPABILITY has %s %s for afi/safi: %s/%s", - peer, - action == CAPABILITY_ACTION_SET - ? "Advertising" - : "Removing", - capability, iana_afi2str(pkt_afi), - iana_safi2str(pkt_safi)); - - COND_FLAG(peer->cap, PEER_CAP_ADDPATH_ADV, - action == CAPABILITY_ACTION_SET); - break; case CAPABILITY_CODE_PATHS_LIMIT: FOREACH_AFI_SAFI (afi, safi) { if (!peer->afc[afi][safi]) @@ -1619,6 +1537,7 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi, case CAPABILITY_CODE_REFRESH: case CAPABILITY_CODE_AS4: case CAPABILITY_CODE_DYNAMIC: + case CAPABILITY_CODE_ADDPATH: case CAPABILITY_CODE_ENHANCED_RR: case CAPABILITY_CODE_EXT_MESSAGE: break; @@ -3174,102 +3093,6 @@ static int bgp_route_refresh_receive(struct peer_connection *connection, return BGP_PACKET_NOOP; } -static void bgp_dynamic_capability_addpath(uint8_t *pnt, int action, - struct capability_header *hdr, - struct peer *peer) -{ - uint8_t *data = pnt + 3; - uint8_t *end = data + hdr->length; - size_t len = end - data; - afi_t afi; - safi_t safi; - - if (action == CAPABILITY_ACTION_SET) { - if (len % CAPABILITY_CODE_ADDPATH_LEN) { - flog_warn(EC_BGP_CAPABILITY_INVALID_LENGTH, - "Add Path: Received invalid length %zu, non-multiple of 4", - len); - return; - } - - SET_FLAG(peer->cap, PEER_CAP_ADDPATH_RCV); - - while (data + CAPABILITY_CODE_ADDPATH_LEN <= end) { - afi_t afi; - safi_t safi; - iana_afi_t pkt_afi; - iana_safi_t pkt_safi; - struct bgp_addpath_capability bac; - - memcpy(&bac, data, sizeof(bac)); - pkt_afi = ntohs(bac.afi); - pkt_safi = safi_int2iana(bac.safi); - - /* If any other value (other than 1-3) is received, - * then the capability SHOULD be treated as not - * understood and ignored. - */ - if (!bac.flags || bac.flags > 3) { - flog_warn(EC_BGP_CAPABILITY_INVALID_LENGTH, - "Add Path: Received invalid send/receive value %u in Add Path capability", - bac.flags); - goto ignore; - } - - if (bgp_debug_neighbor_events(peer)) - zlog_debug("%s OPEN has %s capability for afi/safi: %s/%s%s%s", - peer->host, lookup_msg(capcode_str, hdr->code, NULL), - iana_afi2str(pkt_afi), iana_safi2str(pkt_safi), - CHECK_FLAG(bac.flags, BGP_ADDPATH_RX) ? ", receive" : "", - CHECK_FLAG(bac.flags, BGP_ADDPATH_TX) ? ", transmit" - : ""); - - if (bgp_map_afi_safi_iana2int(pkt_afi, pkt_safi, &afi, - &safi)) { - if (bgp_debug_neighbor_events(peer)) - zlog_debug("%s Addr-family %s/%s(afi/safi) not supported. Ignore the Addpath Attribute for this AFI/SAFI", - peer->host, - iana_afi2str(pkt_afi), - iana_safi2str(pkt_safi)); - goto ignore; - } else if (!peer->afc[afi][safi]) { - if (bgp_debug_neighbor_events(peer)) - zlog_debug("%s Addr-family %s/%s(afi/safi) not enabled. Ignore the AddPath capability for this AFI/SAFI", - peer->host, - iana_afi2str(pkt_afi), - iana_safi2str(pkt_safi)); - goto ignore; - } - - if (CHECK_FLAG(bac.flags, BGP_ADDPATH_RX)) - SET_FLAG(peer->af_cap[afi][safi], - PEER_CAP_ADDPATH_AF_RX_RCV); - else - UNSET_FLAG(peer->af_cap[afi][safi], - PEER_CAP_ADDPATH_AF_RX_RCV); - - if (CHECK_FLAG(bac.flags, BGP_ADDPATH_TX)) - SET_FLAG(peer->af_cap[afi][safi], - PEER_CAP_ADDPATH_AF_TX_RCV); - else - UNSET_FLAG(peer->af_cap[afi][safi], - PEER_CAP_ADDPATH_AF_TX_RCV); - -ignore: - data += CAPABILITY_CODE_ADDPATH_LEN; - } - } else { - FOREACH_AFI_SAFI (afi, safi) { - UNSET_FLAG(peer->af_cap[afi][safi], - PEER_CAP_ADDPATH_AF_RX_RCV); - UNSET_FLAG(peer->af_cap[afi][safi], - PEER_CAP_ADDPATH_AF_TX_RCV); - } - - UNSET_FLAG(peer->cap, PEER_CAP_ADDPATH_RCV); - } -} - static void bgp_dynamic_capability_paths_limit(uint8_t *pnt, int action, struct capability_header *hdr, struct peer *peer) @@ -4030,9 +3853,6 @@ static int bgp_capability_msg_parse(struct peer *peer, uint8_t *pnt, case CAPABILITY_CODE_LLGR: bgp_dynamic_capability_llgr(pnt, action, hdr, peer); break; - case CAPABILITY_CODE_ADDPATH: - bgp_dynamic_capability_addpath(pnt, action, hdr, peer); - break; case CAPABILITY_CODE_PATHS_LIMIT: bgp_dynamic_capability_paths_limit(pnt, action, hdr, peer); @@ -4046,6 +3866,7 @@ static int bgp_capability_msg_parse(struct peer *peer, uint8_t *pnt, case CAPABILITY_CODE_REFRESH: case CAPABILITY_CODE_AS4: case CAPABILITY_CODE_DYNAMIC: + case CAPABILITY_CODE_ADDPATH: case CAPABILITY_CODE_ENHANCED_RR: case CAPABILITY_CODE_EXT_MESSAGE: break; diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 2b3e11929b..04b9ce5ea6 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -9206,21 +9206,12 @@ DEFUN(neighbor_disable_addpath_rx, struct peer *peer; afi_t afi = bgp_node_afi(vty); safi_t safi = bgp_node_safi(vty); - int ret; - int action; peer = peer_and_group_lookup_vty(vty, peer_str); if (!peer) return CMD_WARNING_CONFIG_FAILED; - action = bgp_addpath_capability_action(peer->addpath_type[afi][safi], 0); - - ret = peer_af_flag_set_vty(vty, peer_str, afi, safi, - PEER_FLAG_DISABLE_ADDPATH_RX); - - bgp_capability_send(peer, afi, safi, CAPABILITY_CODE_ADDPATH, action); - - return ret; + return peer_af_flag_set_vty(vty, peer_str, afi, safi, PEER_FLAG_DISABLE_ADDPATH_RX); } DEFUN(no_neighbor_disable_addpath_rx, @@ -9235,21 +9226,12 @@ DEFUN(no_neighbor_disable_addpath_rx, struct peer *peer; afi_t afi = bgp_node_afi(vty); safi_t safi = bgp_node_safi(vty); - int ret; - int action; peer = peer_and_group_lookup_vty(vty, peer_str); if (!peer) return CMD_WARNING_CONFIG_FAILED; - action = bgp_addpath_capability_action(peer->addpath_type[afi][safi], 0); - - ret = peer_af_flag_unset_vty(vty, peer_str, afi, safi, - PEER_FLAG_DISABLE_ADDPATH_RX); - - bgp_capability_send(peer, afi, safi, CAPABILITY_CODE_ADDPATH, action); - - return ret; + return peer_af_flag_unset_vty(vty, peer_str, afi, safi, PEER_FLAG_DISABLE_ADDPATH_RX); } DEFUN (neighbor_addpath_tx_all_paths, @@ -9261,15 +9243,12 @@ DEFUN (neighbor_addpath_tx_all_paths, { int idx_peer = 1; struct peer *peer; - afi_t afi = bgp_node_afi(vty); - safi_t safi = bgp_node_safi(vty); peer = peer_and_group_lookup_vty(vty, argv[idx_peer]->arg); if (!peer) return CMD_WARNING_CONFIG_FAILED; - bgp_addpath_set_peer_type(peer, afi, safi, BGP_ADDPATH_ALL, 0); - + bgp_addpath_set_peer_type(peer, bgp_node_afi(vty), bgp_node_safi(vty), BGP_ADDPATH_ALL, 0); return CMD_SUCCESS; } @@ -9289,20 +9268,18 @@ DEFUN (no_neighbor_addpath_tx_all_paths, { int idx_peer = 2; struct peer *peer; - afi_t afi = bgp_node_afi(vty); - safi_t safi = bgp_node_safi(vty); peer = peer_and_group_lookup_vty(vty, argv[idx_peer]->arg); if (!peer) return CMD_WARNING_CONFIG_FAILED; - if (peer->addpath_type[afi][safi] != BGP_ADDPATH_ALL) { + if (peer->addpath_type[bgp_node_afi(vty)][bgp_node_safi(vty)] != BGP_ADDPATH_ALL) { vty_out(vty, "%% Peer not currently configured to transmit all paths."); return CMD_WARNING_CONFIG_FAILED; } - bgp_addpath_set_peer_type(peer, afi, safi, BGP_ADDPATH_NONE, 0); + bgp_addpath_set_peer_type(peer, bgp_node_afi(vty), bgp_node_safi(vty), BGP_ADDPATH_NONE, 0); return CMD_SUCCESS; } diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index d5463f3d0c..6de403b30c 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -4820,39 +4820,40 @@ static const struct peer_flag_action peer_flag_action_list[] = { {0, 0, 0}}; static const struct peer_flag_action peer_af_flag_action_list[] = { - {PEER_FLAG_SEND_COMMUNITY, 1, peer_change_reset_out}, - {PEER_FLAG_SEND_EXT_COMMUNITY, 1, peer_change_reset_out}, - {PEER_FLAG_SEND_LARGE_COMMUNITY, 1, peer_change_reset_out}, - {PEER_FLAG_NEXTHOP_SELF, 1, peer_change_reset_out}, - {PEER_FLAG_REFLECTOR_CLIENT, 1, peer_change_reset}, - {PEER_FLAG_RSERVER_CLIENT, 1, peer_change_reset}, - {PEER_FLAG_SOFT_RECONFIG, 0, peer_change_reset_in}, - {PEER_FLAG_AS_PATH_UNCHANGED, 1, peer_change_reset_out}, - {PEER_FLAG_NEXTHOP_UNCHANGED, 1, peer_change_reset_out}, - {PEER_FLAG_MED_UNCHANGED, 1, peer_change_reset_out}, - {PEER_FLAG_DEFAULT_ORIGINATE, 0, peer_change_none}, - {PEER_FLAG_REMOVE_PRIVATE_AS, 1, peer_change_reset_out}, - {PEER_FLAG_ALLOWAS_IN, 0, peer_change_reset_in}, - {PEER_FLAG_ALLOWAS_IN_ORIGIN, 0, peer_change_reset_in}, - {PEER_FLAG_ORF_PREFIX_SM, 1, peer_change_reset}, - {PEER_FLAG_ORF_PREFIX_RM, 1, peer_change_reset}, - {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}, - {PEER_FLAG_REMOVE_PRIVATE_AS_REPLACE, 1, peer_change_reset_out}, - {PEER_FLAG_AS_OVERRIDE, 1, peer_change_reset_out}, - {PEER_FLAG_REMOVE_PRIVATE_AS_ALL_REPLACE, 1, peer_change_reset_out}, - {PEER_FLAG_WEIGHT, 0, peer_change_reset_in}, - {PEER_FLAG_DISABLE_ADDPATH_RX, 0, peer_change_none}, - {PEER_FLAG_SOO, 0, peer_change_reset}, - {PEER_FLAG_ACCEPT_OWN, 0, peer_change_reset}, - {PEER_FLAG_SEND_EXT_COMMUNITY_RPKI, 1, peer_change_reset_out}, - {PEER_FLAG_ADDPATH_RX_PATHS_LIMIT, 0, peer_change_none}, - {0, 0, 0}}; + { PEER_FLAG_SEND_COMMUNITY, 1, peer_change_reset_out }, + { PEER_FLAG_SEND_EXT_COMMUNITY, 1, peer_change_reset_out }, + { PEER_FLAG_SEND_LARGE_COMMUNITY, 1, peer_change_reset_out }, + { PEER_FLAG_NEXTHOP_SELF, 1, peer_change_reset_out }, + { PEER_FLAG_REFLECTOR_CLIENT, 1, peer_change_reset }, + { PEER_FLAG_RSERVER_CLIENT, 1, peer_change_reset }, + { PEER_FLAG_SOFT_RECONFIG, 0, peer_change_reset_in }, + { PEER_FLAG_AS_PATH_UNCHANGED, 1, peer_change_reset_out }, + { PEER_FLAG_NEXTHOP_UNCHANGED, 1, peer_change_reset_out }, + { PEER_FLAG_MED_UNCHANGED, 1, peer_change_reset_out }, + { PEER_FLAG_DEFAULT_ORIGINATE, 0, peer_change_none }, + { PEER_FLAG_REMOVE_PRIVATE_AS, 1, peer_change_reset_out }, + { PEER_FLAG_ALLOWAS_IN, 0, peer_change_reset_in }, + { PEER_FLAG_ALLOWAS_IN_ORIGIN, 0, peer_change_reset_in }, + { PEER_FLAG_ORF_PREFIX_SM, 1, peer_change_reset }, + { PEER_FLAG_ORF_PREFIX_RM, 1, peer_change_reset }, + { 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 }, + { PEER_FLAG_REMOVE_PRIVATE_AS_REPLACE, 1, peer_change_reset_out }, + { PEER_FLAG_AS_OVERRIDE, 1, peer_change_reset_out }, + { PEER_FLAG_REMOVE_PRIVATE_AS_ALL_REPLACE, 1, peer_change_reset_out }, + { PEER_FLAG_WEIGHT, 0, peer_change_reset_in }, + { PEER_FLAG_DISABLE_ADDPATH_RX, 0, peer_change_reset }, + { PEER_FLAG_SOO, 0, peer_change_reset }, + { PEER_FLAG_ACCEPT_OWN, 0, peer_change_reset }, + { PEER_FLAG_SEND_EXT_COMMUNITY_RPKI, 1, peer_change_reset_out }, + { PEER_FLAG_ADDPATH_RX_PATHS_LIMIT, 0, peer_change_none }, + { 0, 0, 0 } +}; /* Proper action set. */ static int peer_flag_action_set(const struct peer_flag_action *action_list, diff --git a/tests/topotests/bgp_dynamic_capability/r2/frr.conf b/tests/topotests/bgp_dynamic_capability/r2/frr.conf index 621e9381e3..cca07078ea 100644 --- a/tests/topotests/bgp_dynamic_capability/r2/frr.conf +++ b/tests/topotests/bgp_dynamic_capability/r2/frr.conf @@ -18,7 +18,6 @@ router bgp 65002 neighbor 192.168.1.1 timers connect 1 neighbor 192.168.1.1 capability dynamic neighbor 192.168.1.1 capability extended-nexthop - neighbor 192.168.1.1 addpath-rx-paths-limit 20 neighbor 2001:db8::1 remote-as external neighbor 2001:db8::1 timers 1 3 neighbor 2001:db8::1 timers connect 1 @@ -27,6 +26,9 @@ router bgp 65002 ! address-family ipv4 unicast redistribute connected + neighbor 192.168.1.1 addpath-tx-all-paths + neighbor 192.168.1.1 disable-addpath-rx + neighbor 192.168.1.1 addpath-rx-paths-limit 20 exit-address-family ! address-family ipv6 unicast diff --git a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_addpath.py b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_path_limit.py similarity index 63% rename from tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_addpath.py rename to tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_path_limit.py index 91df89b1b5..22e4fe687b 100644 --- a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_addpath.py +++ b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_path_limit.py @@ -9,8 +9,6 @@ Test if Addpath/Paths-Limit capabilities are adjusted dynamically. T1: Enable Addpath/Paths-Limit capabilities and check if they are exchanged dynamically T2: Disable paths limit and check if it's exchanged dynamically -T3: Disable Addpath capability RX and check if it's exchanged dynamically -T4: Disable Addpath capability and check if it's exchanged dynamically """ import os @@ -65,12 +63,12 @@ def test_bgp_addpath_paths_limit(): "dynamic": "advertisedAndReceived", "addPath": { "ipv4Unicast": { - "txAdvertisedAndReceived": False, + "txAdvertisedAndReceived": True, "txAdvertised": True, - "txReceived": False, - "rxAdvertisedAndReceived": True, + "txReceived": True, + "rxAdvertisedAndReceived": False, "rxAdvertised": True, - "rxReceived": True, + "rxReceived": False, } }, "pathsLimit": { @@ -105,7 +103,6 @@ def test_bgp_addpath_paths_limit(): configure terminal router bgp address-family ipv4 unicast - neighbor 192.168.1.1 addpath-tx-all-paths neighbor 192.168.1.1 addpath-rx-paths-limit 21 """ ) @@ -122,9 +119,9 @@ def test_bgp_addpath_paths_limit(): "txAdvertisedAndReceived": True, "txAdvertised": True, "txReceived": True, - "rxAdvertisedAndReceived": True, + "rxAdvertisedAndReceived": False, "rxAdvertised": True, - "rxReceived": True, + "rxReceived": False, } }, "pathsLimit": { @@ -143,7 +140,7 @@ def test_bgp_addpath_paths_limit(): "messageStats": { "notificationsRecv": 0, "notificationsSent": 0, - "capabilityRecv": 2, + "capabilityRecv": 1, }, } } @@ -170,58 +167,6 @@ def test_bgp_addpath_paths_limit(): ) def _disable_paths_limit(): - output = json.loads(r1.vtysh_cmd("show bgp neighbor json")) - expected = { - "192.168.1.2": { - "bgpState": "Established", - "neighborCapabilities": { - "dynamic": "advertisedAndReceived", - "addPath": { - "ipv4Unicast": { - "txAdvertisedAndReceived": True, - "txAdvertised": True, - "txReceived": True, - "rxAdvertisedAndReceived": True, - "rxAdvertised": True, - "rxReceived": True, - } - }, - "pathsLimit": { - "ipv4Unicast": { - "advertisedAndReceived": True, - "advertisedPathsLimit": 10, - "receivedPathsLimit": 0, - } - }, - }, - "messageStats": { - "notificationsRecv": 0, - "notificationsSent": 0, - "capabilityRecv": 3, - }, - } - } - return topotest.json_cmp(output, expected) - - test_func = functools.partial( - _disable_paths_limit, - ) - _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) - assert result is None, "Something went wrong after disabling paths limit" - - ### - # T3: Disable Addpath capability RX and check if it's exchanged dynamically - ### - r2.vtysh_cmd( - """ - configure terminal - router bgp - address-family ipv4 unicast - neighbor 192.168.1.1 disable-addpath-rx - """ - ) - - def _disable_addpath_rx(): output = json.loads(r1.vtysh_cmd("show bgp neighbor json")) expected = { "192.168.1.2": { @@ -249,63 +194,17 @@ def test_bgp_addpath_paths_limit(): "messageStats": { "notificationsRecv": 0, "notificationsSent": 0, - "capabilityRecv": 4, - }, - } - } - return topotest.json_cmp(output, expected) - - test_func = functools.partial( - _disable_addpath_rx, - ) - _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) - assert result is None, "Something went wrong after disabling Addpath RX flags" - - ### - # T4: Disable Addpath capability and check if it's exchanged dynamically - ### - r1.vtysh_cmd( - """ - configure terminal - router bgp - address-family ipv4 unicast - no neighbor 192.168.1.2 addpath-tx-all-paths - """ - ) - - def _disable_addpath(): - output = json.loads(r1.vtysh_cmd("show bgp neighbor json")) - expected = { - "192.168.1.2": { - "bgpState": "Established", - "neighborCapabilities": { - "dynamic": "advertisedAndReceived", - "addPath": { - "ipv4Unicast": { - "txAdvertisedAndReceived": False, - "txAdvertised": False, - "txReceived": True, - "rxAdvertisedAndReceived": False, - "rxAdvertised": True, - "rxReceived": False, - } - }, - }, - "messageStats": { - "notificationsRecv": 0, - "notificationsSent": 0, - "capabilitySent": 1, - "capabilityRecv": 4, + "capabilityRecv": 2, }, } } return topotest.json_cmp(output, expected) test_func = functools.partial( - _disable_addpath, + _disable_paths_limit, ) _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) - assert result is None, "Something went wrong when disabling Addpath capability" + assert result is None, "Something went wrong after disabling paths limit" if __name__ == "__main__": -- 2.39.5