diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c index 866930c73223..545da7c55940 100644 --- a/bgpd/bgp_open.c +++ b/bgpd/bgp_open.c @@ -340,15 +340,14 @@ static void bgp_capability_orf_not_support(struct peer *peer, iana_afi_t afi, peer->host, afi, safi, type, mode); } -static const struct message orf_type_str[] = { - {ORF_TYPE_RESERVED, "Reserved"}, - {ORF_TYPE_PREFIX, "Prefixlist"}, - {0}}; - -static const struct message orf_mode_str[] = {{ORF_MODE_RECEIVE, "Receive"}, - {ORF_MODE_SEND, "Send"}, - {ORF_MODE_BOTH, "Both"}, - {0}}; +const struct message orf_type_str[] = { { ORF_TYPE_RESERVED, "Reserved" }, + { ORF_TYPE_PREFIX, "Prefixlist" }, + { 0 } }; + +const struct message orf_mode_str[] = { { ORF_MODE_RECEIVE, "Receive" }, + { ORF_MODE_SEND, "Send" }, + { ORF_MODE_BOTH, "Both" }, + { 0 } }; static int bgp_capability_orf_entry(struct peer *peer, struct capability_header *hdr) diff --git a/bgpd/bgp_open.h b/bgpd/bgp_open.h index a92c56d1b508..34f4b7619edc 100644 --- a/bgpd/bgp_open.h +++ b/bgpd/bgp_open.h @@ -108,5 +108,7 @@ extern void bgp_capability_vty_out(struct vty *vty, struct peer *peer, bool use_json, json_object *json_neigh); extern as_t peek_for_as4_capability(struct peer *peer, uint16_t length); extern const struct message capcode_str[]; +extern const struct message orf_type_str[]; +extern const struct message orf_mode_str[]; #endif /* _QUAGGA_BGP_OPEN_H */ diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index fe39488d567c..b585591e2f69 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -1214,6 +1214,8 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi, 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, "Unknown"); @@ -1458,8 +1460,78 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi, iana_safi2str(pkt_safi)); break; - case CAPABILITY_CODE_REFRESH: case CAPABILITY_CODE_ORF: + /* Convert AFI, SAFI to values for packet. */ + bgp_map_afi_safi_int2iana(afi, safi, &pkt_afi, &pkt_safi); + + stream_putc(s, action); + stream_putc(s, CAPABILITY_CODE_ORF); + cap_len = stream_get_endp(s); + stream_putc(s, 0); + + stream_putw(s, pkt_afi); /* Address Family Identifier */ + stream_putc(s, 0); /* Reserved */ + stream_putc(s, + pkt_safi); /* Subsequent Address Family Identifier */ + + number_of_orfs_p = + stream_get_endp(s); /* Number of ORFs pointer */ + stream_putc(s, 0); /* Number of ORFs */ + + /* Address Prefix ORF */ + if (CHECK_FLAG(peer->af_flags[afi][safi], + PEER_FLAG_ORF_PREFIX_SM) || + CHECK_FLAG(peer->af_flags[afi][safi], + PEER_FLAG_ORF_PREFIX_RM)) { + stream_putc(s, ORF_TYPE_PREFIX); + + if (CHECK_FLAG(peer->af_flags[afi][safi], + PEER_FLAG_ORF_PREFIX_SM) && + CHECK_FLAG(peer->af_flags[afi][safi], + PEER_FLAG_ORF_PREFIX_RM)) { + SET_FLAG(peer->af_cap[afi][safi], + PEER_CAP_ORF_PREFIX_SM_ADV); + SET_FLAG(peer->af_cap[afi][safi], + PEER_CAP_ORF_PREFIX_RM_ADV); + stream_putc(s, ORF_MODE_BOTH); + } else if (CHECK_FLAG(peer->af_flags[afi][safi], + PEER_FLAG_ORF_PREFIX_SM)) { + SET_FLAG(peer->af_cap[afi][safi], + PEER_CAP_ORF_PREFIX_SM_ADV); + UNSET_FLAG(peer->af_cap[afi][safi], + PEER_CAP_ORF_PREFIX_RM_ADV); + stream_putc(s, ORF_MODE_SEND); + } else { + SET_FLAG(peer->af_cap[afi][safi], + PEER_CAP_ORF_PREFIX_RM_ADV); + UNSET_FLAG(peer->af_cap[afi][safi], + PEER_CAP_ORF_PREFIX_SM_ADV); + stream_putc(s, ORF_MODE_RECEIVE); + } + number_of_orfs++; + } else { + UNSET_FLAG(peer->af_cap[afi][safi], + PEER_CAP_ORF_PREFIX_SM_ADV); + UNSET_FLAG(peer->af_cap[afi][safi], + PEER_CAP_ORF_PREFIX_RM_ADV); + } + + /* Total Number of ORFs. */ + stream_putc_at(s, number_of_orfs_p, number_of_orfs); + + 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)); + break; + case CAPABILITY_CODE_REFRESH: case CAPABILITY_CODE_AS4: case CAPABILITY_CODE_DYNAMIC: case CAPABILITY_CODE_ENHANCED_RR: @@ -3044,6 +3116,123 @@ static void bgp_dynamic_capability_addpath(uint8_t *pnt, int action, } } +static void bgp_dynamic_capability_orf(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; + + struct capability_mp_data mpc; + uint8_t num; + iana_afi_t pkt_afi; + afi_t afi; + iana_safi_t pkt_safi; + safi_t safi; + uint8_t type; + uint8_t mode; + uint16_t sm_cap = PEER_CAP_ORF_PREFIX_SM_RCV; + uint16_t rm_cap = PEER_CAP_ORF_PREFIX_RM_RCV; + int i; + + if (data + CAPABILITY_CODE_ORF_LEN > end) { + flog_warn(EC_BGP_CAPABILITY_INVALID_LENGTH, + "ORF: Received invalid length %zu, less than %d", len, + CAPABILITY_CODE_ORF_LEN); + return; + } + + /* ORF Entry header */ + memcpy(&mpc, data, sizeof(mpc)); + data += sizeof(mpc); + num = *data++; + pkt_afi = ntohs(mpc.afi); + pkt_safi = mpc.safi; + + /* Convert AFI, SAFI to internal values, check. */ + if (bgp_map_afi_safi_iana2int(pkt_afi, pkt_safi, &afi, &safi)) { + zlog_info("%pBP Addr-family %d/%d not supported. Ignoring the ORF capability", + peer, pkt_afi, pkt_safi); + return; + } + + /* validate number field */ + if (CAPABILITY_CODE_ORF_LEN + (num * 2) > hdr->length) { + zlog_info("%pBP ORF Capability entry length error, Cap length %u, num %u", + peer, hdr->length, num); + return; + } + + if (action == CAPABILITY_ACTION_UNSET) { + UNSET_FLAG(peer->af_cap[afi][safi], sm_cap); + UNSET_FLAG(peer->af_cap[afi][safi], rm_cap); + return; + } + + for (i = 0; i < num; i++) { + if (data + 1 > end) { + flog_err(EC_BGP_CAPABILITY_INVALID_LENGTH, + "%pBP ORF Capability entry length (type) error, Cap length %u, num %u", + peer, hdr->length, num); + return; + } + type = *data++; + + if (data + 1 > end) { + flog_err(EC_BGP_CAPABILITY_INVALID_LENGTH, + "%pBP ORF Capability entry length (mode) error, Cap length %u, num %u", + peer, hdr->length, num); + return; + } + mode = *data++; + + /* ORF Mode error check */ + switch (mode) { + case ORF_MODE_BOTH: + case ORF_MODE_SEND: + case ORF_MODE_RECEIVE: + break; + default: + if (bgp_debug_neighbor_events(peer)) + zlog_debug("%pBP Addr-family %d/%d has ORF type/mode %d/%d not supported", + peer, afi, safi, type, mode); + continue; + } + + if (!((afi == AFI_IP && safi == SAFI_UNICAST) || + (afi == AFI_IP && safi == SAFI_MULTICAST) || + (afi == AFI_IP6 && safi == SAFI_UNICAST))) { + if (bgp_debug_neighbor_events(peer)) + zlog_debug("%pBP Addr-family %d/%d unsupported AFI/SAFI received", + peer, afi, safi); + continue; + } + + if (bgp_debug_neighbor_events(peer)) + zlog_debug("%pBP OPEN has %s ORF capability as %s for afi/safi: %s/%s", + peer, lookup_msg(orf_type_str, type, NULL), + lookup_msg(orf_mode_str, mode, NULL), + iana_afi2str(pkt_afi), + iana_safi2str(pkt_safi)); + + switch (mode) { + case ORF_MODE_BOTH: + SET_FLAG(peer->af_cap[afi][safi], sm_cap); + SET_FLAG(peer->af_cap[afi][safi], rm_cap); + break; + case ORF_MODE_SEND: + SET_FLAG(peer->af_cap[afi][safi], sm_cap); + UNSET_FLAG(peer->af_cap[afi][safi], rm_cap); + break; + case ORF_MODE_RECEIVE: + SET_FLAG(peer->af_cap[afi][safi], rm_cap); + UNSET_FLAG(peer->af_cap[afi][safi], sm_cap); + break; + } + } +} + static void bgp_dynamic_capability_llgr(uint8_t *pnt, int action, struct capability_header *hdr, struct peer *peer) @@ -3401,8 +3590,10 @@ static int bgp_capability_msg_parse(struct peer *peer, uint8_t *pnt, case CAPABILITY_CODE_ADDPATH: bgp_dynamic_capability_addpath(pnt, action, hdr, peer); break; - case CAPABILITY_CODE_REFRESH: case CAPABILITY_CODE_ORF: + bgp_dynamic_capability_orf(pnt, action, hdr, peer); + break; + case CAPABILITY_CODE_REFRESH: case CAPABILITY_CODE_AS4: case CAPABILITY_CODE_DYNAMIC: case CAPABILITY_CODE_ENHANCED_RR: diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 8144d6e7b3bc..fca9e2ad8e3c 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -5831,24 +5831,37 @@ DEFUN (neighbor_capability_orf_prefix, struct peer *peer; afi_t afi = bgp_node_afi(vty); safi_t safi = bgp_node_safi(vty); + int ret; peer = peer_and_group_lookup_vty(vty, peer_str); if (!peer) return CMD_WARNING_CONFIG_FAILED; - if (strmatch(argv[idx_send_recv]->text, "send")) - return peer_af_flag_set_vty(vty, peer_str, afi, safi, - PEER_FLAG_ORF_PREFIX_SM); + if (strmatch(argv[idx_send_recv]->text, "send")) { + ret = peer_af_flag_set_vty(vty, peer_str, afi, safi, + PEER_FLAG_ORF_PREFIX_SM); + bgp_capability_send(peer, afi, safi, CAPABILITY_CODE_ORF, + CAPABILITY_ACTION_SET); + return ret; + } - if (strmatch(argv[idx_send_recv]->text, "receive")) - return peer_af_flag_set_vty(vty, peer_str, afi, safi, - PEER_FLAG_ORF_PREFIX_RM); + if (strmatch(argv[idx_send_recv]->text, "receive")) { + ret = peer_af_flag_set_vty(vty, peer_str, afi, safi, + PEER_FLAG_ORF_PREFIX_RM); + bgp_capability_send(peer, afi, safi, CAPABILITY_CODE_ORF, + CAPABILITY_ACTION_SET); + return ret; + } - if (strmatch(argv[idx_send_recv]->text, "both")) - return peer_af_flag_set_vty(vty, peer_str, afi, safi, - PEER_FLAG_ORF_PREFIX_SM) - | peer_af_flag_set_vty(vty, peer_str, afi, safi, - PEER_FLAG_ORF_PREFIX_RM); + if (strmatch(argv[idx_send_recv]->text, "both")) { + ret = peer_af_flag_set_vty(vty, peer_str, afi, safi, + PEER_FLAG_ORF_PREFIX_SM) | + peer_af_flag_set_vty(vty, peer_str, afi, safi, + PEER_FLAG_ORF_PREFIX_RM); + bgp_capability_send(peer, afi, safi, CAPABILITY_CODE_ORF, + CAPABILITY_ACTION_SET); + return ret; + } return CMD_WARNING_CONFIG_FAILED; } @@ -5883,24 +5896,37 @@ DEFUN (no_neighbor_capability_orf_prefix, struct peer *peer; afi_t afi = bgp_node_afi(vty); safi_t safi = bgp_node_safi(vty); + int ret; peer = peer_and_group_lookup_vty(vty, peer_str); if (!peer) return CMD_WARNING_CONFIG_FAILED; - if (strmatch(argv[idx_send_recv]->text, "send")) - return peer_af_flag_unset_vty(vty, peer_str, afi, safi, - PEER_FLAG_ORF_PREFIX_SM); + if (strmatch(argv[idx_send_recv]->text, "send")) { + ret = peer_af_flag_unset_vty(vty, peer_str, afi, safi, + PEER_FLAG_ORF_PREFIX_SM); + bgp_capability_send(peer, afi, safi, CAPABILITY_CODE_ORF, + CAPABILITY_ACTION_UNSET); + return ret; + } - if (strmatch(argv[idx_send_recv]->text, "receive")) - return peer_af_flag_unset_vty(vty, peer_str, afi, safi, - PEER_FLAG_ORF_PREFIX_RM); + if (strmatch(argv[idx_send_recv]->text, "receive")) { + ret = peer_af_flag_unset_vty(vty, peer_str, afi, safi, + PEER_FLAG_ORF_PREFIX_RM); + bgp_capability_send(peer, afi, safi, CAPABILITY_CODE_ORF, + CAPABILITY_ACTION_UNSET); + return ret; + } - if (strmatch(argv[idx_send_recv]->text, "both")) - return peer_af_flag_unset_vty(vty, peer_str, afi, safi, - PEER_FLAG_ORF_PREFIX_SM) - | peer_af_flag_unset_vty(vty, peer_str, afi, safi, - PEER_FLAG_ORF_PREFIX_RM); + if (strmatch(argv[idx_send_recv]->text, "both")) { + ret = peer_af_flag_unset_vty(vty, peer_str, afi, safi, + PEER_FLAG_ORF_PREFIX_SM) | + peer_af_flag_unset_vty(vty, peer_str, afi, safi, + PEER_FLAG_ORF_PREFIX_RM); + bgp_capability_send(peer, afi, safi, CAPABILITY_CODE_ORF, + CAPABILITY_ACTION_UNSET); + return ret; + } return CMD_WARNING_CONFIG_FAILED; } diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 12e462a8ca73..04c06b94d8ac 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -4979,6 +4979,16 @@ static int peer_af_flag_modify(struct peer *peer, afi_t afi, safi_t safi, else if (flag == PEER_FLAG_ORF_PREFIX_RM) peer->last_reset = PEER_DOWN_CAPABILITY_CHANGE; + /* We should not reset the session if + * dynamic capability is enabled and we + * are changing the ORF prefix flags. + */ + if ((CHECK_FLAG(peer->cap, PEER_CAP_DYNAMIC_RCV) && + CHECK_FLAG(peer->cap, PEER_CAP_DYNAMIC_ADV)) && + (flag == PEER_FLAG_ORF_PREFIX_RM || + flag == PEER_FLAG_ORF_PREFIX_SM)) + action.type = peer_change_none; + peer_change_action(peer, afi, safi, action.type); } } @@ -5039,6 +5049,18 @@ static int peer_af_flag_modify(struct peer *peer, afi_t afi, safi_t safi, member->last_reset = PEER_DOWN_CAPABILITY_CHANGE; + /* We should not reset the session if + * dynamic capability is enabled and we + * are changing the ORF prefix flags. + */ + if ((CHECK_FLAG(peer->cap, + PEER_CAP_DYNAMIC_RCV) && + CHECK_FLAG(peer->cap, + PEER_CAP_DYNAMIC_ADV)) && + (flag == PEER_FLAG_ORF_PREFIX_RM || + flag == PEER_FLAG_ORF_PREFIX_SM)) + action.type = peer_change_none; + peer_change_action(member, afi, safi, action.type); } diff --git a/tests/topotests/bgp_dynamic_capability/r1/frr.conf b/tests/topotests/bgp_dynamic_capability/r1/frr.conf index 6ff69037bde5..aa5c3db9a68f 100644 --- a/tests/topotests/bgp_dynamic_capability/r1/frr.conf +++ b/tests/topotests/bgp_dynamic_capability/r1/frr.conf @@ -17,3 +17,5 @@ router bgp 65001 neighbor 192.168.1.2 addpath-tx-all-paths exit-address-family ! +ip prefix-list r2 seq 5 permit 10.10.10.10/32 +! diff --git a/tests/topotests/bgp_dynamic_capability/r2/frr.conf b/tests/topotests/bgp_dynamic_capability/r2/frr.conf index 16b83a5c58d8..7f25665ed572 100644 --- a/tests/topotests/bgp_dynamic_capability/r2/frr.conf +++ b/tests/topotests/bgp_dynamic_capability/r2/frr.conf @@ -3,6 +3,7 @@ ! int lo ip address 10.10.10.10/32 + ip address 10.10.10.20/32 ! int r2-eth0 ip address 192.168.1.2/24 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 new file mode 100644 index 000000000000..49c7e554eb7e --- /dev/null +++ b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_orf.py @@ -0,0 +1,172 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + +# Copyright (c) 2023 by +# Donatas Abraitis +# + +""" +Test if ORF capability is adjusted 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_orf(): + 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", + }, + } + } + 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( + "Apply incoming prefix-list to r1 and check if we advertise only 10.10.10.20/32 from r2" + ) + + # Clear message stats to check if we receive a notification or not after we + # enable ORF capability. + r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats") + r1.vtysh_cmd( + """ + configure terminal + router bgp + address-family ipv4 unicast + neighbor 192.168.1.2 prefix-list r2 in + neighbor 192.168.1.2 capability orf prefix-list both + """ + ) + + r2.vtysh_cmd( + """ + configure terminal + router bgp + address-family ipv4 unicast + neighbor 192.168.1.1 capability orf prefix-list both + """ + ) + + def _bgp_check_if_session_not_reset(): + 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, + "capabilityRecv": 1, + "capabilitySent": 1, + }, + "addressFamilyInfo": { + "ipv4Unicast": { + "afDependentCap": { + "orfPrefixList": { + "sendMode": "advertisedAndReceived", + "recvMode": "advertisedAndReceived", + } + }, + "incomingUpdatePrefixFilterList": "r2", + } + }, + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _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 up ORF capability" + + r1.vtysh_cmd( + """ + configure terminal + ip prefix-list r2 seq 5 permit 10.10.10.20/32 + """ + ) + + def _bgp_check_if_we_send_correct_prefix(): + output = json.loads( + r2.vtysh_cmd( + "show bgp ipv4 unicast neighbors 192.168.1.1 advertised-routes json" + ) + ) + expected = { + "advertisedRoutes": { + "10.10.10.20/32": { + "valid": True, + }, + }, + "totalPrefixCounter": 1, + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_check_if_we_send_correct_prefix, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert ( + result is None + ), "Only 10.10.10.20/32 SHOULD be advertised due to ORF filtering" + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args))