Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bgpd: Unset advertised capabilities if capability is disabled (backport #15514) #15518

Closed
wants to merge 3 commits into from
Closed

bgpd: Unset advertised capabilities if capability is disabled (backport #15514) #15518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6f3344d
bgpd: Unset advertised capabilities if capability is disabled
ton31337 Mar 9, 2024
1966942
bgpd: Allow dynamically disable graceful-restart/long-lived graceful-…
ton31337 Mar 10, 2024
1818186
tests: Check if capabilities can be disabled via dynamic capabilities
ton31337 Mar 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 91 additions & 50 deletions bgpd/bgp_packet.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1239,7 +1239,6 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi,
/* Encode MP_EXT capability. */
switch (capability_code) {
case CAPABILITY_CODE_SOFT_VERSION:
SET_FLAG(peer->cap, PEER_CAP_SOFT_VERSION_ADV);
stream_putc(s, action);
stream_putc(s, CAPABILITY_CODE_SOFT_VERSION);
cap_len = stream_get_endp(s);
Expand Down Expand Up @@ -1270,6 +1269,9 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi,
: "Removing",
capability, iana_afi2str(pkt_afi),
iana_safi2str(pkt_safi));

COND_FLAG(peer->cap, PEER_CAP_SOFT_VERSION_ADV,
action == CAPABILITY_ACTION_SET);
break;
case CAPABILITY_CODE_MP:
stream_putc(s, action);
Expand All @@ -1289,11 +1291,6 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi,
iana_safi2str(pkt_safi));
break;
case CAPABILITY_CODE_RESTART:
if (!CHECK_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART) &&
!CHECK_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART_HELPER))
return;

SET_FLAG(peer->cap, PEER_CAP_RESTART_ADV);
stream_putc(s, action);
stream_putc(s, CAPABILITY_CODE_RESTART);
cap_len = stream_get_endp(s);
Expand Down Expand Up @@ -1342,13 +1339,10 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi,
capability, iana_afi2str(pkt_afi),
iana_safi2str(pkt_safi));

COND_FLAG(peer->cap, PEER_CAP_RESTART_ADV,
action == CAPABILITY_ACTION_SET);
break;
case CAPABILITY_CODE_LLGR:
if (!CHECK_FLAG(peer->cap, PEER_CAP_RESTART_ADV))
return;

SET_FLAG(peer->cap, PEER_CAP_LLGR_ADV);

stream_putc(s, action);
stream_putc(s, CAPABILITY_CODE_LLGR);
cap_len = stream_get_endp(s);
Expand Down Expand Up @@ -1380,10 +1374,11 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi,
: "Removing",
capability, iana_afi2str(pkt_afi),
iana_safi2str(pkt_safi));

COND_FLAG(peer->cap, PEER_CAP_LLGR_ADV,
action == CAPABILITY_ACTION_SET);
break;
case CAPABILITY_CODE_ADDPATH:
SET_FLAG(peer->cap, PEER_CAP_ADDPATH_ADV);

FOREACH_AFI_SAFI (afi, safi) {
if (peer->afc[afi][safi]) {
addpath_afi_safi_count++;
Expand Down Expand Up @@ -1461,7 +1456,55 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi,
capability, iana_afi2str(pkt_afi),
iana_safi2str(pkt_safi));

COND_FLAG(peer->cap, PEER_CAP_ADDPATH_ADV,
action == CAPABILITY_ACTION_SET);
break;
<<<<<<< HEAD
=======
case CAPABILITY_CODE_PATHS_LIMIT:
FOREACH_AFI_SAFI (afi, safi) {
if (!peer->afc[afi][safi])
continue;

addpath_afi_safi_count++;
}

stream_putc(s, action);
stream_putc(s, CAPABILITY_CODE_PATHS_LIMIT);
stream_putc(s, CAPABILITY_CODE_PATHS_LIMIT_LEN *
addpath_afi_safi_count);

FOREACH_AFI_SAFI (afi, safi) {
if (!peer->afc[afi][safi])
continue;

bgp_map_afi_safi_int2iana(afi, safi, &pkt_afi,
&pkt_safi);

stream_putw(s, pkt_afi);
stream_putc(s, pkt_safi);
stream_putw(s,
peer->addpath_paths_limit[afi][safi].send);

SET_FLAG(peer->af_cap[afi][safi],
PEER_CAP_PATHS_LIMIT_AF_ADV);

if (bgp_debug_neighbor_events(peer))
zlog_debug("%pBP sending CAPABILITY has %s %s for afi/safi: %s/%s, limit: %u",
peer,
action == CAPABILITY_ACTION_SET
? "Advertising"
: "Removing",
capability, iana_afi2str(pkt_afi),
iana_safi2str(pkt_safi),
peer->addpath_paths_limit[afi][safi]
.send);
}

COND_FLAG(peer->cap, PEER_CAP_PATHS_LIMIT_ADV,
action == CAPABILITY_ACTION_SET);
break;
>>>>>>> 77102e853 (bgpd: Unset advertised capabilities if capability is disabled)
case CAPABILITY_CODE_ORF:
/* Convert AFI, SAFI to values for packet. */
bgp_map_afi_safi_int2iana(afi, safi, &pkt_afi, &pkt_safi);
Expand Down Expand Up @@ -1534,43 +1577,42 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi,
iana_safi2str(pkt_safi));
break;
case CAPABILITY_CODE_FQDN:
if (CHECK_FLAG(peer->flags, PEER_FLAG_CAPABILITY_FQDN) &&
hostname) {
SET_FLAG(peer->cap, PEER_CAP_HOSTNAME_ADV);
stream_putc(s, action);
stream_putc(s, CAPABILITY_CODE_FQDN);
cap_len = stream_get_endp(s);
stream_putc(s, 0); /* Capability Length */

len = strlen(hostname);
stream_putc(s, action);
stream_putc(s, CAPABILITY_CODE_FQDN);
cap_len = stream_get_endp(s);
stream_putc(s, 0); /* Capability Length */

len = strlen(hostname);
if (len > BGP_MAX_HOSTNAME)
len = BGP_MAX_HOSTNAME;

stream_putc(s, len);
stream_put(s, hostname, len);

if (domainname) {
len = strlen(domainname);
if (len > BGP_MAX_HOSTNAME)
len = BGP_MAX_HOSTNAME;

stream_putc(s, len);
stream_put(s, hostname, len);

if (domainname) {
len = strlen(domainname);
if (len > BGP_MAX_HOSTNAME)
len = BGP_MAX_HOSTNAME;
stream_put(s, domainname, len);
} else
stream_putc(s, 0);

stream_putc(s, len);
stream_put(s, domainname, len);
} else
stream_putc(s, 0);
len = stream_get_endp(s) - cap_len - 1;
stream_putc_at(s, cap_len, len);

len = stream_get_endp(s) - cap_len - 1;
stream_putc_at(s, cap_len, len);
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));

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_HOSTNAME_ADV,
action == CAPABILITY_ACTION_SET);
break;
case CAPABILITY_CODE_REFRESH:
case CAPABILITY_CODE_AS4:
Expand All @@ -1580,13 +1622,12 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi,
case CAPABILITY_CODE_EXT_MESSAGE:
break;
case CAPABILITY_CODE_ROLE:
if (peer->local_role != ROLE_UNDEFINED) {
SET_FLAG(peer->cap, PEER_CAP_ROLE_ADV);
stream_putc(s, action);
stream_putc(s, CAPABILITY_CODE_ROLE);
stream_putc(s, CAPABILITY_CODE_ROLE_LEN);
stream_putc(s, peer->local_role);
}
stream_putc(s, action);
stream_putc(s, CAPABILITY_CODE_ROLE);
stream_putc(s, CAPABILITY_CODE_ROLE_LEN);
stream_putc(s, peer->local_role);
COND_FLAG(peer->cap, PEER_CAP_ROLE_ADV,
action == CAPABILITY_ACTION_SET);
break;
default:
break;
Expand Down
11 changes: 11 additions & 0 deletions bgpd/bgp_vty.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3260,6 +3260,8 @@ DEFUN (bgp_graceful_restart_disable,
GR_DISABLE)
{
int ret = BGP_GR_FAILURE;
struct listnode *node, *nnode;
struct peer *peer;

if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART))
zlog_debug(
Expand All @@ -3278,6 +3280,15 @@ DEFUN (bgp_graceful_restart_disable,
vty_out(vty,
"Graceful restart configuration changed, reset all peers to take effect\n");

for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) {
bgp_capability_send(peer, AFI_IP, SAFI_UNICAST,
CAPABILITY_CODE_RESTART,
CAPABILITY_ACTION_UNSET);
bgp_capability_send(peer, AFI_IP, SAFI_UNICAST,
CAPABILITY_CODE_LLGR,
CAPABILITY_ACTION_UNSET);
}

return bgp_vty_return(vty, ret);
}

Expand Down
42 changes: 41 additions & 1 deletion tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_addpath.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def _bgp_check_if_addpath_rx_tx_and_session_not_reset():
step("Disable Addpath capability RX and check if it's exchanged dynamically")

# Clear message stats to check if we receive a notification or not after we
# change the settings fo LLGR.
# disable addpath-rx.
r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats")
r2.vtysh_cmd(
"""
Expand Down Expand Up @@ -174,6 +174,46 @@ def _bgp_check_if_addpath_tx_and_session_not_reset():
_, result = topotest.run_and_expect(test_func, None, count=30, wait=1)
assert result is None, "Session was reset after disabling Addpath RX flags"

# Clear message stats to check if we receive a notification or not after we
# disable Addpath capability.
r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats")
r1.vtysh_cmd(
"""
configure terminal
router bgp
address-family ipv4 unicast
no neighbor 192.168.1.2 addpath-tx-all-paths
"""
)

def _bgp_check_if_addpath_capability_is_absent():
output = json.loads(r1.vtysh_cmd("show bgp neighbor json"))
expected = {
"192.168.1.2": {
"bgpState": "Established",
"neighborCapabilities": {
"dynamic": "advertisedAndReceived",
"addPath": {
"ipv4Unicast": {
"txAdvertisedAndReceived": None,
"txAdvertised": None,
"rxAdvertised": True,
}
},
},
"messageStats": {
"notificationsRecv": 0,
},
}
}
return topotest.json_cmp(output, expected)

test_func = functools.partial(
_bgp_check_if_addpath_capability_is_absent,
)
_, result = topotest.run_and_expect(test_func, None, count=30, wait=1)
assert result is None, "Failed to disable Addpath capability"


if __name__ == "__main__":
args = ["-s"] + sys.argv[1:]
Expand Down
Loading
Loading