From b062c31dd1303e66e822a6998e1bfc6899b63d94 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 5 Feb 2025 11:47:04 +0200 Subject: [PATCH 1/2] Revert "bgpd: Send non-transitive extended communities from/to OAD peers" This reverts commit a73b0f88c7715bfcf5dc8ffd46d2fd0bbc9a8aff. --- bgpd/bgp_route.c | 89 ------------------------------------------------ 1 file changed, 89 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index e39a97f84a..aa2c3a6a56 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -2700,7 +2700,6 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi, if (nh_reset && bgp_path_info_mpath_chkwtd(bgp, pi) && (cum_bw = bgp_path_info_mpath_cumbw(pi)) != 0 && -<<<<<<< HEAD !CHECK_FLAG(attr->rmap_change_flags, BATTR_RMAP_LINK_BW_SET)) bgp_attr_set_ecommunity( attr, @@ -2709,94 +2708,6 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi, CHECK_FLAG( peer->flags, PEER_FLAG_DISABLE_LINK_BW_ENCODING_IEEE))); -======= - !CHECK_FLAG(attr->rmap_change_flags, BATTR_RMAP_LINK_BW_SET)) { - if (CHECK_FLAG(peer->flags, PEER_FLAG_EXTENDED_LINK_BANDWIDTH)) - bgp_attr_set_ipv6_ecommunity( - attr, - ecommunity_replace_linkbw(bgp->as, - bgp_attr_get_ipv6_ecommunity( - attr), - cum_bw, false, true)); - else - bgp_attr_set_ecommunity( - attr, - ecommunity_replace_linkbw( - bgp->as, bgp_attr_get_ecommunity(attr), - cum_bw, - CHECK_FLAG(peer->flags, - PEER_FLAG_DISABLE_LINK_BW_ENCODING_IEEE), - false)); - } - - /* - * Adjust AIGP for propagation when the nexthop is set to ourselves, - * e.g., using "set ip nexthop peer-address" or when advertising to - * EBGP. Note in route reflection the nexthop is usually unmodified - * and the AIGP should not be adjusted in that case. - */ - if (CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP)) && AIGP_TRANSMIT_ALLOWED(peer)) { - if (nh_reset || - CHECK_FLAG(attr->rmap_change_flags, BATTR_RMAP_NEXTHOP_PEER_ADDRESS)) { - uint64_t aigp = bgp_aigp_metric_total(pi); - - bgp_attr_set_aigp_metric(attr, aigp); - } - } - - /* Extended communities can be transitive and non-transitive. - * If the extended community is non-transitive, strip it off, - * unless it's a locally originated route (static, aggregate, - * redistributed, etc.). - * draft-uttaro-idr-bgp-oad says: - * Extended communities which are non-transitive across an AS - * boundary MAY be advertised over an EBGP-OAD session if allowed - * by explicit policy configuration. If allowed, all the members - * of the OAD SHOULD be configured to use the same criteria. - * For example, the Origin Validation State Extended Community, - * defined as non-transitive in [RFC8097], can be advertised to - * peers in the same OAD. - */ - if (from->sort == BGP_PEER_EBGP && from->sub_sort != BGP_PEER_EBGP_OAD && - peer->sort == BGP_PEER_EBGP && peer->sub_sort != BGP_PEER_EBGP_OAD && - pi->sub_type == BGP_ROUTE_NORMAL) { - struct ecommunity *new_ecomm; - struct ecommunity *old_ecomm; - - old_ecomm = bgp_attr_get_ecommunity(attr); - if (old_ecomm) { - new_ecomm = ecommunity_dup(old_ecomm); - if (ecommunity_strip_non_transitive(new_ecomm)) { - bgp_attr_set_ecommunity(attr, new_ecomm); - if (!old_ecomm->refcnt) - ecommunity_free(&old_ecomm); - if (bgp_debug_update(NULL, p, subgrp->update_group, 0)) - zlog_debug("%pBP: %pFX stripped non-transitive extended communities", - peer, p); - } else { - ecommunity_free(&new_ecomm); - } - } - - /* Extended link-bandwidth communities are encoded as IPv6 - * address-specific extended communities. - */ - old_ecomm = bgp_attr_get_ipv6_ecommunity(attr); - if (old_ecomm) { - new_ecomm = ecommunity_dup(old_ecomm); - if (ecommunity_strip_non_transitive(new_ecomm)) { - bgp_attr_set_ipv6_ecommunity(attr, new_ecomm); - if (!old_ecomm->refcnt) - ecommunity_free(&old_ecomm); - if (bgp_debug_update(NULL, p, subgrp->update_group, 0)) - zlog_debug("%pBP: %pFX stripped non-transitive ipv6 extended communities", - peer, p); - } else { - ecommunity_free(&new_ecomm); - } - } - } ->>>>>>> f2759c46c (bgpd: Send non-transitive extended communities from/to OAD peers) return true; } From f950fff2f519215414175092a948f500c8980395 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 5 Feb 2025 11:49:16 +0200 Subject: [PATCH 2/2] Revert "tests: Check if OAD peers can exchange non-transitive extended communities" This reverts commit fa438172d868b0da4c99cd972356dc8ed9f1f016. --- tests/topotests/bgp_oad/r3/frr.conf | 6 ----- tests/topotests/bgp_oad/test_bgp_oad.py | 33 ------------------------- 2 files changed, 39 deletions(-) diff --git a/tests/topotests/bgp_oad/r3/frr.conf b/tests/topotests/bgp_oad/r3/frr.conf index 164267d74d..02dd5adfe1 100644 --- a/tests/topotests/bgp_oad/r3/frr.conf +++ b/tests/topotests/bgp_oad/r3/frr.conf @@ -7,14 +7,12 @@ int r3-eth0 ! router bgp 65003 no bgp ebgp-requires-policy - no bgp network import-check neighbor 192.168.2.2 remote-as external neighbor 192.168.2.2 timers 1 3 neighbor 192.168.2.2 timers connect 1 neighbor 192.168.2.2 oad ! address-family ipv4 unicast - network 10.10.10.20/32 route-map static redistribute connected route-map connected exit-address-family ! @@ -22,7 +20,3 @@ route-map connected permit 10 set local-preference 123 set metric 123 ! -route-map static permit 10 - set extcommunity bandwidth 100 non-transitive -exit -! diff --git a/tests/topotests/bgp_oad/test_bgp_oad.py b/tests/topotests/bgp_oad/test_bgp_oad.py index 9c00422f7d..c21b3793c3 100644 --- a/tests/topotests/bgp_oad/test_bgp_oad.py +++ b/tests/topotests/bgp_oad/test_bgp_oad.py @@ -60,7 +60,6 @@ def test_bgp_dynamic_capability_role(): r2 = tgen.gears["r2"] r3 = tgen.gears["r3"] r4 = tgen.gears["r4"] - r5 = tgen.gears["r5"] def _bgp_converge(): output = json.loads(r1.vtysh_cmd("show bgp ipv4 unicast 10.10.10.10/32 json")) @@ -126,38 +125,6 @@ def test_bgp_dynamic_capability_role(): _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) assert result is None, "10.10.10.1/32 should not be advertised to r4 (not OAD peer)" - def _bgp_check_non_transitive_extended_community( - router, arg={"string": "LB:65003:12500000 (100.000 Mbps)"} - ): - output = json.loads( - router.vtysh_cmd("show bgp ipv4 unicast 10.10.10.20/32 json") - ) - expected = { - "paths": [ - { - "extendedCommunity": arg, - } - ] - } - return topotest.json_cmp(output, expected) - - test_func = functools.partial( - _bgp_check_non_transitive_extended_community, - r4, - ) - _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) - assert ( - result is None - ), "10.10.10.20/32 should be received at r4 with non-transitive extended community" - - test_func = functools.partial( - _bgp_check_non_transitive_extended_community, r5, None - ) - _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) - assert ( - result is None - ), "10.10.10.20/32 should NOT be received at r5 with non-transitive extended community" - if __name__ == "__main__": args = ["-s"] + sys.argv[1:]