Merge pull request #17151 from opensourcerouting/fix/send_link_bw_as_…

…non-transitive_to_ebgp

bgpd: Handle non-transitive extended communities

bgpd/bgp_attr.c

@@ -4482,66 +4482,22 @@ static bool bgp_append_local_as(struct peer *peer, afi_t afi, safi_t safi)
 }
 
 static void bgp_packet_ecommunity_attribute(struct stream *s, struct peer *peer,
-					    struct ecommunity *ecomm,
-					    bool transparent, int attribute)
+					    struct ecommunity *ecomm, int attribute)
 {
-	if (peer->sort == BGP_PEER_IBGP || peer->sort == BGP_PEER_CONFED ||
-	    peer->sub_sort == BGP_PEER_EBGP_OAD || transparent) {
-		if (ecomm->size * ecomm->unit_size > 255) {
-			stream_putc(s, BGP_ATTR_FLAG_OPTIONAL |
-					       BGP_ATTR_FLAG_TRANS |
-					       BGP_ATTR_FLAG_EXTLEN);
-			stream_putc(s, attribute);
-			stream_putw(s, ecomm->size * ecomm->unit_size);
-		} else {
-			stream_putc(s, BGP_ATTR_FLAG_OPTIONAL |
-					       BGP_ATTR_FLAG_TRANS);
-			stream_putc(s, attribute);
-			stream_putc(s, ecomm->size * ecomm->unit_size);
-		}
-		stream_put(s, ecomm->val, ecomm->size * ecomm->unit_size);
-	} else {
-		uint8_t *pnt;
-		int tbit;
-		int ecom_tr_size = 0;
-		uint32_t i;
-
-		for (i = 0; i < ecomm->size; i++) {
-			pnt = ecomm->val + (i * ecomm->unit_size);
-			tbit = *pnt;
-
-			if (CHECK_FLAG(tbit, ECOMMUNITY_FLAG_NON_TRANSITIVE))
-				continue;
-
-			ecom_tr_size++;
-		}
-
-		if (ecom_tr_size) {
-			if (ecom_tr_size * ecomm->unit_size > 255) {
-				stream_putc(s, BGP_ATTR_FLAG_OPTIONAL |
-						       BGP_ATTR_FLAG_TRANS |
-						       BGP_ATTR_FLAG_EXTLEN);
-				stream_putc(s, attribute);
-				stream_putw(s, ecom_tr_size * ecomm->unit_size);
-			} else {
-				stream_putc(s, BGP_ATTR_FLAG_OPTIONAL |
-						       BGP_ATTR_FLAG_TRANS);
-				stream_putc(s, attribute);
-				stream_putc(s, ecom_tr_size * ecomm->unit_size);
-			}
-
-			for (i = 0; i < ecomm->size; i++) {
-				pnt = ecomm->val + (i * ecomm->unit_size);
-				tbit = *pnt;
-
-				if (CHECK_FLAG(tbit,
-					       ECOMMUNITY_FLAG_NON_TRANSITIVE))
-					continue;
+	if (!ecomm || !ecomm->size)
+		return;
 
-				stream_put(s, pnt, ecomm->unit_size);
-			}
-		}
+	if (ecomm->size * ecomm->unit_size > 255) {
+		stream_putc(s, BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS | BGP_ATTR_FLAG_EXTLEN);
+		stream_putc(s, attribute);
+		stream_putw(s, ecomm->size * ecomm->unit_size);
+	} else {
+		stream_putc(s, BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS);
+		stream_putc(s, attribute);
+		stream_putc(s, ecomm->size * ecomm->unit_size);
 	}
+
+	stream_put(s, ecomm->val, ecomm->size * ecomm->unit_size);
 }
 
 /* Make attribute packet. */
@@ -4848,19 +4804,11 @@ bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer, struct strea
 
 	/* Extended IPv6/Communities attributes. */
 	if (CHECK_FLAG(peer->af_flags[afi][safi], PEER_FLAG_SEND_EXT_COMMUNITY)) {
-		bool transparent = CHECK_FLAG(peer->af_flags[afi][safi],
-					      PEER_FLAG_RSERVER_CLIENT) &&
-				   from &&
-				   CHECK_FLAG(from->af_flags[afi][safi],
-					      PEER_FLAG_RSERVER_CLIENT);
-
 		if (CHECK_FLAG(attr->flag,
 			       ATTR_FLAG_BIT(BGP_ATTR_EXT_COMMUNITIES))) {
 			struct ecommunity *ecomm = bgp_attr_get_ecommunity(attr);
 
-			bgp_packet_ecommunity_attribute(s, peer, ecomm,
-							transparent,
-							BGP_ATTR_EXT_COMMUNITIES);
+			bgp_packet_ecommunity_attribute(s, peer, ecomm, BGP_ATTR_EXT_COMMUNITIES);
 		}
 
 		if (CHECK_FLAG(attr->flag,
@@ -4869,7 +4817,6 @@ bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer, struct strea
 				bgp_attr_get_ipv6_ecommunity(attr);
 
 			bgp_packet_ecommunity_attribute(s, peer, ecomm,
-							transparent,
 							BGP_ATTR_IPV6_EXT_COMMUNITIES);
 		}
 	}

bgpd/bgp_attr.h

@@ -515,7 +515,7 @@ static inline void bgp_attr_set_ecommunity(struct attr *attr,
 {
 	attr->ecommunity = ecomm;
 
-	if (ecomm)
+	if (ecomm && ecomm->size)
 		SET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_EXT_COMMUNITIES));
 	else
 		UNSET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_EXT_COMMUNITIES));
@@ -566,7 +566,7 @@ static inline void bgp_attr_set_ipv6_ecommunity(struct attr *attr,
 {
 	attr->ipv6_ecommunity = ipv6_ecomm;
 
-	if (ipv6_ecomm)
+	if (ipv6_ecomm && ipv6_ecomm->size)
 		SET_FLAG(attr->flag,
 			 ATTR_FLAG_BIT(BGP_ATTR_IPV6_EXT_COMMUNITIES));
 	else

bgpd/bgp_debug.c

@@ -449,6 +449,10 @@ bool bgp_dump_attr(struct attr *attr, char *buf, size_t size)
 			 ", extcommunity %s",
 			 ecommunity_str(bgp_attr_get_ecommunity(attr)));
 
+	if (CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_IPV6_EXT_COMMUNITIES)))
+		snprintf(buf + strlen(buf), size - strlen(buf), ", ipv6-extcommunity %s",
+			 ecommunity_str(bgp_attr_get_ipv6_ecommunity(attr)));
+
 	if (CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_ATOMIC_AGGREGATE)))
 		snprintf(buf + strlen(buf), size - strlen(buf),
 			 ", atomic-aggregate");

bgpd/bgp_ecommunity.c

@@ -1408,30 +1408,26 @@ char *ecommunity_ecom2str(struct ecommunity *ecom, int format, int filter)
 					 "FS:marking %u", *(pnt + 5));
 			} else
 				unk_ecom = true;
-		} else if (type == ECOMMUNITY_ENCODE_AS_NON_TRANS) {
+		} else if (CHECK_FLAG(type, ECOMMUNITY_FLAG_NON_TRANSITIVE) ||
+			   type == ECOMMUNITY_ENCODE_OPAQUE_NON_TRANS) {
 			sub_type = *pnt++;
-			if (sub_type == ECOMMUNITY_LINK_BANDWIDTH)
+			if (sub_type == ECOMMUNITY_ORIGIN_VALIDATION_STATE)
+				ecommunity_origin_validation_state_str(encbuf, sizeof(encbuf), pnt);
+			else if (sub_type == ECOMMUNITY_LINK_BANDWIDTH)
 				ecommunity_lb_str(encbuf, sizeof(encbuf), pnt,
 						  ecom->disable_ieee_floating);
 			else if (sub_type == ECOMMUNITY_EXTENDED_LINK_BANDWIDTH)
 				ipv6_ecommunity_lb_str(encbuf, sizeof(encbuf),
 						       pnt, len);
 			else
 				unk_ecom = true;
-		} else if (type == ECOMMUNITY_ENCODE_IP_NON_TRANS) {
+		} else if (CHECK_FLAG(type, ECOMMUNITY_ENCODE_IP_NON_TRANS)) {
 			sub_type = *pnt++;
 			if (sub_type == ECOMMUNITY_NODE_TARGET)
 				ecommunity_node_target_str(
 					encbuf, sizeof(encbuf), pnt, format);
 			else
 				unk_ecom = true;
-		} else if (type == ECOMMUNITY_ENCODE_OPAQUE_NON_TRANS) {
-			sub_type = *pnt++;
-			if (sub_type == ECOMMUNITY_ORIGIN_VALIDATION_STATE)
-				ecommunity_origin_validation_state_str(
-					encbuf, sizeof(encbuf), pnt);
-			else
-				unk_ecom = true;
 		} else {
 			sub_type = *pnt++;
 			unk_ecom = true;
@@ -1587,6 +1583,57 @@ bool ecommunity_strip(struct ecommunity *ecom, uint8_t type,
 	return true;
 }
 
+static bool ecommunity_non_transitive(uint8_t type)
+{
+	return (CHECK_FLAG(type, ECOMMUNITY_FLAG_NON_TRANSITIVE) ||
+		CHECK_FLAG(type, ECOMMUNITY_ENCODE_IP_NON_TRANS) ||
+		type == ECOMMUNITY_ENCODE_OPAQUE_NON_TRANS);
+}
+
+/* Delete all non-transitive extended communities */
+bool ecommunity_strip_non_transitive(struct ecommunity *ecom)
+{
+	uint8_t *p, *q, *new;
+	uint32_t c, found = 0;
+
+	if (!ecom || !ecom->val)
+		return false;
+
+	/* Certain extended communities like the Route Target can be present
+	 * multiple times, handle that.
+	 */
+	c = 0;
+	for (p = ecom->val; c < ecom->size; p += ecom->unit_size, c++)
+		if (ecommunity_non_transitive(*p))
+			found++;
+
+	if (!found)
+		return false;
+
+	/* Handle the case where everything needs to be stripped. */
+	if (found == ecom->size) {
+		XFREE(MTYPE_ECOMMUNITY_VAL, ecom->val);
+		ecom->size = 0;
+		return true;
+	}
+
+	/* Strip extended communities with non-transitive flag set */
+	new = XMALLOC(MTYPE_ECOMMUNITY_VAL, (ecom->size - found) * ecom->unit_size);
+	q = new;
+	for (c = 0, p = ecom->val; c < ecom->size; c++, p += ecom->unit_size) {
+		if (!ecommunity_non_transitive(*p)) {
+			memcpy(q, p, ecom->unit_size);
+			q += ecom->unit_size;
+		}
+	}
+
+	XFREE(MTYPE_ECOMMUNITY_VAL, ecom->val);
+	ecom->val = new;
+	ecom->size -= found;
+
+	return true;
+}
+
 /*
  * Remove specified extended community value from extended community.
  * Returns 1 if value was present (and hence, removed), 0 otherwise.
@@ -1883,9 +1930,7 @@ const uint8_t *ecommunity_linkbw_present(struct ecommunity *ecom, uint64_t *bw)
 		if (len < ecom->unit_size)
 			return NULL;
 
-		if ((type == ECOMMUNITY_ENCODE_AS ||
-		     type == ECOMMUNITY_ENCODE_AS_NON_TRANS) &&
-		    sub_type == ECOMMUNITY_LINK_BANDWIDTH) {
+		if ((type == ECOMMUNITY_ENCODE_AS) && sub_type == ECOMMUNITY_LINK_BANDWIDTH) {
 			uint32_t bwval;
 
 			pnt += 2; /* bandwidth is encoded as AS:val */

bgpd/bgp_ecommunity.h

@@ -32,9 +32,7 @@
 #define ECOMMUNITY_EXTENDED_COMMUNITY_PART_3 0x82
 
 /* Non-transitive extended community types. */
-#define ECOMMUNITY_ENCODE_AS_NON_TRANS      0x40
 #define ECOMMUNITY_ENCODE_IP_NON_TRANS      0x41
-#define ECOMMUNITY_ENCODE_AS4_NON_TRANS     0x42
 #define ECOMMUNITY_ENCODE_OPAQUE_NON_TRANS  0x43
 
 /* Low-order octet of the Extended Communities type field.  */
@@ -398,6 +396,7 @@ extern struct ecommunity *ecommunity_new(void);
 extern bool ecommunity_strip(struct ecommunity *ecom, uint8_t type,
 			     uint8_t subtype);
 extern struct ecommunity *ecommunity_new(void);
+extern bool ecommunity_strip_non_transitive(struct ecommunity *ecom);
 extern bool ecommunity_del_val(struct ecommunity *ecom,
 			       struct ecommunity_val *eval);
 struct bgp_pbr_entry_action;

bgpd/bgp_route.c

@@ -2836,6 +2836,50 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi,
 		}
 	}
 
+	/* 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.).
+	 */
+	if (from->sort == BGP_PEER_EBGP && peer->sort == BGP_PEER_EBGP &&
+	    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);
+			}
+		}
+	}
+
 	return true;
 }
 

tests/topotests/bgp_extended_link_bandwidth/r1/frr.conf

@@ -22,7 +22,7 @@ ip prefix-list p200 seq 5 permit 10.10.10.200/32
 !
 route-map r2 permit 10
  match ip address prefix-list p40
- set extcommunity bandwidth 40000
+ set extcommunity bandwidth 40000 non-transitive
 route-map r2 permit 20
  match ip address prefix-list p100
  set extcommunity bandwidth 100000

tests/topotests/bgp_extended_link_bandwidth/r2/frr.conf

@@ -2,9 +2,15 @@
 int r2-eth0
  ip address 192.168.1.2/24
 !
+int r2-eth1
+ ip address 192.168.2.2/24
+!
 router bgp 65000
  no bgp ebgp-requires-policy
  neighbor 192.168.1.1 remote-as internal
  neighbor 192.168.1.1 timers 1 3
  neighbor 192.168.1.1 timers connect 1
+ neighbor 192.168.2.1 remote-as external
+ neighbor 192.168.2.1 timers 1 3
+ neighbor 192.168.2.1 timers connect 1
 !

tests/topotests/bgp_extended_link_bandwidth/r3/frr.conf

@@ -0,0 +1,16 @@
+!
+int r3-eth0
+ ip address 192.168.2.1/24
+!
+int r3-eth1
+ ip address 192.168.3.1/24
+!
+router bgp 65003
+ no bgp ebgp-requires-policy
+ 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.3.2 remote-as external
+ neighbor 192.168.3.2 timers 1 3
+ neighbor 192.168.3.2 timers connect 1
+!

tests/topotests/bgp_extended_link_bandwidth/r4/frr.conf

@@ -0,0 +1,10 @@
+!
+int r4-eth0
+ ip address 192.168.3.2/24
+!
+router bgp 65004
+ no bgp ebgp-requires-policy
+ neighbor 192.168.3.1 remote-as external
+ neighbor 192.168.3.1 timers 1 3
+ neighbor 192.168.3.1 timers connect 1
+!