diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index 9d484d901ac1..405375dcc0ac 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -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); @@ -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); @@ -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); @@ -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); @@ -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++; @@ -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); @@ -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: @@ -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; diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 31524e2221b9..b322e4ddde52 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -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( @@ -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); } diff --git a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_addpath.py b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_addpath.py index f83ee2971c7e..5202f51e28d1 100644 --- a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_addpath.py +++ b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_addpath.py @@ -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( """ @@ -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:] diff --git a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_fqdn.py b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_fqdn.py new file mode 100644 index 000000000000..338886d5f422 --- /dev/null +++ b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_fqdn.py @@ -0,0 +1,122 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + +# Copyright (c) 2024 by +# Donatas Abraitis +# + +""" +Test if fqdn capability is exchanged dynamically. +""" + +import os +import re +import sys +import json +import pytest +import functools + +pytestmark = pytest.mark.bgpd + +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.common_config import step + +pytestmark = [pytest.mark.bgpd] + + +def setup_module(mod): + topodef = {"s1": ("r1", "r2")} + tgen = Topogen(topodef, mod.__name__) + tgen.start_topology() + + router_list = tgen.routers() + + for _, (rname, router) in enumerate(router_list.items(), 1): + router.load_frr_config(os.path.join(CWD, "{}/frr.conf".format(rname))) + + tgen.start_router() + + +def teardown_module(mod): + tgen = get_topogen() + tgen.stop_topology() + + +def test_bgp_dynamic_capability_fqdn(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + r2 = tgen.gears["r2"] + + def _bgp_converge(): + output = json.loads(r1.vtysh_cmd("show bgp neighbor json")) + expected = { + "192.168.1.2": { + "bgpState": "Established", + "neighborCapabilities": { + "dynamic": "advertisedAndReceived", + "hostName": { + "advHostName": "r1", + "rcvHostName": "r2", + }, + }, + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_converge, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Can't converge" + + step("Disable fqdn capability and check if it's exchanged dynamically") + + # Clear message stats to check if we receive a notification or not after we + # disable fqdn capability. + r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats") + r1.vtysh_cmd( + """ + configure terminal + router bgp + no neighbor 192.168.1.2 capability fqdn + """ + ) + + def _bgp_check_if_fqdn_capability_is_absent(): + output = json.loads(r1.vtysh_cmd("show bgp neighbor json")) + expected = { + "192.168.1.2": { + "bgpState": "Established", + "neighborCapabilities": { + "dynamic": "advertisedAndReceived", + "hostName": { + "advHostName": None, + "rcvHostName": "r2", + }, + }, + "messageStats": { + "notificationsRecv": 0, + }, + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_check_if_fqdn_capability_is_absent, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Failed to disable fqdn capability" + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args)) diff --git a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_graceful_restart.py b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_graceful_restart.py index b7e2090eee30..4644ef3293df 100644 --- a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_graceful_restart.py +++ b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_graceful_restart.py @@ -159,7 +159,7 @@ def _bgp_check_if_session_not_reset_after_changing_gr_settings(): ) # Clear message stats to check if we receive a notification or not after we - # change the settings fo LLGR. + # disable graceful-restart notification support. r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats") r2.vtysh_cmd( """ @@ -207,6 +207,40 @@ def _bgp_check_if_session_not_reset_after_changing_notification(): result is None ), "Session was reset after changing Graceful-Restart notification support" + # Clear message stats to check if we receive a notification or not after we + # disable GR. + r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats") + r1.vtysh_cmd( + """ + configure terminal + router bgp + bgp graceful-restart-disable + """ + ) + + def _bgp_check_if_gr_llgr_capability_is_absent(): + output = json.loads(r1.vtysh_cmd("show bgp neighbor json")) + expected = { + "192.168.1.2": { + "bgpState": "Established", + "neighborCapabilities": { + "dynamic": "advertisedAndReceived", + "gracefulRestartCapability": "received", + "longLivedGracefulRestart": "received", + }, + "messageStats": { + "notificationsRecv": 0, + }, + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_check_if_gr_llgr_capability_is_absent, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Failed to disable GR/LLGR capabilities" + if __name__ == "__main__": args = ["-s"] + sys.argv[1:] diff --git a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_orf.py b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_orf.py index f1ad74c05c00..ba95bd161489 100644 --- a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_orf.py +++ b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_orf.py @@ -172,6 +172,51 @@ def _bgp_check_if_we_send_correct_prefix(): result is None ), "Only 10.10.10.20/32 SHOULD be advertised due to ORF filtering" + # Clear message stats to check if we receive a notification or not after we + # disable ORF 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 capability orf prefix-list both + """ + ) + + def _bgp_check_if_orf_capability_is_absent(): + output = json.loads(r1.vtysh_cmd("show bgp neighbor json")) + expected = { + "192.168.1.2": { + "bgpState": "Established", + "neighborCapabilities": { + "dynamic": "advertisedAndReceived", + }, + "messageStats": { + "notificationsRecv": 0, + "notificationsSent": 0, + }, + "addressFamilyInfo": { + "ipv4Unicast": { + "acceptedPrefixCounter": 1, + "afDependentCap": { + "orfPrefixList": { + "sendMode": "received", + "recvMode": "received", + } + }, + } + }, + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_check_if_orf_capability_is_absent, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Failed to disable ORF capability" + if __name__ == "__main__": args = ["-s"] + sys.argv[1:] diff --git a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_role.py b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_role.py index 700d4c130d33..aa9ad5f0e8c9 100644 --- a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_role.py +++ b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_role.py @@ -84,7 +84,7 @@ def _bgp_converge(): step("Set local-role 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. + # change the role. r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats") r1.vtysh_cmd( """ @@ -132,6 +132,41 @@ def _bgp_check_if_session_not_reset(): _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) assert result is None, "Session was reset after setting role capability" + # Clear message stats to check if we receive a notification or not after we + # change the role. + r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats") + r1.vtysh_cmd( + """ + configure terminal + router bgp + no neighbor 192.168.1.2 local-role customer + """ + ) + + def _bgp_check_if_role_capability_is_absent(): + output = json.loads(r1.vtysh_cmd("show bgp neighbor json")) + expected = { + "192.168.1.2": { + "bgpState": "Established", + "localRole": "undefined", + "remoteRole": "provider", + "neighborCapabilities": { + "dynamic": "advertisedAndReceived", + "role": "received", + }, + "messageStats": { + "notificationsRecv": 0, + }, + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_check_if_role_capability_is_absent, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Failed to disable role capability" + if __name__ == "__main__": args = ["-s"] + sys.argv[1:] diff --git a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_software_version.py b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_software_version.py index 11840b4c61b8..737e69470170 100644 --- a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_software_version.py +++ b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_software_version.py @@ -86,7 +86,7 @@ def _bgp_converge(): step("Enable software version capability 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. + # enable software-version capability. r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats") r1.vtysh_cmd( """ @@ -155,6 +155,41 @@ def _bgp_software_version(): result is None ), "Session was reset after enabling software version capability" + # Clear message stats to check if we receive a notification or not after we + # disable software-version capability. + r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats") + r1.vtysh_cmd( + """ + configure terminal + router bgp + no neighbor 192.168.1.2 capability software-version + """ + ) + + def _bgp_check_if_software_version_capability_is_absent(): + output = json.loads(r1.vtysh_cmd("show bgp neighbor json")) + expected = { + "192.168.1.2": { + "bgpState": "Established", + "neighborCapabilities": { + "dynamic": "advertisedAndReceived", + "softwareVersion": { + "advertisedSoftwareVersion": None, + }, + }, + "messageStats": { + "notificationsRecv": 0, + }, + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_check_if_software_version_capability_is_absent, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Failed to disable software version capability" + if __name__ == "__main__": args = ["-s"] + sys.argv[1:]