Merge pull request FRRouting#17896 from opensourcerouting/fix/bgp_oad…

…_extended_communities

bgpd: Send non-transitive extended communities from/to OAD peers

bgpd/bgp_route.c

@@ -2851,8 +2851,17 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi,
 	 * 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 && peer->sort == BGP_PEER_EBGP &&
+	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;

tests/topotests/bgp_oad/r3/frr.conf

@@ -7,16 +7,22 @@ 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
 !
 route-map connected permit 10
  set local-preference 123
  set metric 123
 !
+route-map static permit 10
+ set extcommunity bandwidth 100 non-transitive
+exit
+!

tests/topotests/bgp_oad/test_bgp_oad.py

@@ -56,6 +56,7 @@ def test_bgp_oad():
     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"))
@@ -121,6 +122,38 @@ def _bgp_check_no_export(router, arg=[{"valid": True}]):
     _, 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:]