Commit ded9eec460 for frr
commit ded9eec4606a84385370bd27a3e29cd8101dfd25
Author: Enke Chen <enchen@paloaltonetworks.com>
Date: Sun Oct 18 20:48:37 2026 -0700
bgpd: set/clear originator-id and cluster-list in subgroup_announce_check
Set and clear route reflection related attributes (originator-id and
cluster-list) in subgroup_announce_check() instead of computing them
at packet formation time. With an accurate outbound attribute cache:
1) Avoid unnecessary attributes in the outbound attribute table
2) Reduce duplicate updates
3) Display the attributes correctly in "show bgp ipv4 unicast
neighbor x.x.x.x advertised-routes"
Signed-off-by: Enke Chen <enchen@paloaltonetworks.com>
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index e8fd2d8953..72dfbd3907 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -197,6 +197,49 @@ static void cluster_unintern(struct cluster_list **cluster)
}
}
+/* Prepend cluster_id to cluster list for route reflection */
+void bgp_attr_prepend_cluster_id(struct bgp *bgp, struct attr *attr)
+{
+ struct cluster_list *cluster = bgp_attr_get_cluster(attr);
+ struct in_addr cluster_id;
+ int new_length;
+ struct cluster_list *new_cluster;
+
+ /* Determine our cluster_id */
+ if (CHECK_FLAG(bgp->config, BGP_CONFIG_CLUSTER_ID))
+ cluster_id = bgp->cluster_id;
+ else
+ cluster_id = bgp->router_id;
+
+ /* Calculate new length: existing length + 4 bytes for cluster_id */
+ new_length = cluster ? cluster->length + 4 : 4;
+
+ /* Allocate new cluster list structure */
+ new_cluster = XCALLOC(MTYPE_CLUSTER, sizeof(struct cluster_list));
+ new_cluster->length = new_length;
+ new_cluster->list = XMALLOC(MTYPE_CLUSTER_VAL, new_length);
+
+ /* Prepend our cluster_id */
+ memcpy(new_cluster->list, &cluster_id, sizeof(struct in_addr));
+
+ /* Append existing cluster list if present */
+ if (cluster && cluster->length > 0)
+ memcpy((uint8_t *)new_cluster->list + sizeof(struct in_addr), cluster->list,
+ cluster->length);
+
+ /* Set the new cluster in attr (will be interned later by bgp_attr_intern) */
+ bgp_attr_set_cluster(attr, new_cluster);
+}
+
+/* Clear route reflection attributes (originator-id and cluster-list) */
+void bgp_attr_clear_reflection(struct attr *attr)
+{
+ if (bgp_attr_exists(attr, BGP_ATTR_ORIGINATOR_ID))
+ bgp_attr_set_originator_id(attr, (struct in_addr){ INADDR_ANY });
+ if (bgp_attr_get_cluster(attr))
+ bgp_attr_set_cluster(attr, NULL);
+}
+
static void cluster_init(void)
{
cluster_hash = hash_create(cluster_hash_key_make, cluster_hash_cmp,
@@ -6051,54 +6094,28 @@ bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer, struct strea
lcom_length(bgp_attr_get_lcommunity(attr)));
}
- /* Route Reflector. */
- if (peer->sort == BGP_PEER_IBGP && from
- && from->sort == BGP_PEER_IBGP) {
- struct cluster_list *cluster = bgp_attr_get_cluster(attr);
-
- /* Originator ID. */
+ /* Originator ID. */
+ if (bgp_attr_exists(attr, BGP_ATTR_ORIGINATOR_ID)) {
stream_putc(s, BGP_ATTR_FLAG_OPTIONAL);
stream_putc(s, BGP_ATTR_ORIGINATOR_ID);
stream_putc(s, 4);
+ stream_put_in_addr(s, &attr->originator_id);
+ }
- if (bgp_attr_exists(attr, BGP_ATTR_ORIGINATOR_ID))
- stream_put_in_addr(s, &attr->originator_id);
- else
- stream_put_in_addr(s, &from->remote_id);
+ /* Cluster list. */
+ if (bgp_attr_exists(attr, BGP_ATTR_CLUSTER_LIST)) {
+ struct cluster_list *cluster = bgp_attr_get_cluster(attr);
- /* Cluster list. */
- /* RFC 4271 4.3 says:
- * The fourth high-order bit (bit 3) of the Attribute Flags octet
- * is the Extended Length bit. It defines whether the Attribute
- * Length is one octet (if set to 0) or two octets (if set to 1).
- *
- * If the Extended Length bit of the Attribute Flags octet is set
- * to 0, the third octet of the Path Attribute contains the length
- * of the attribute data in octets.
- *
- * If the Extended Length bit of the Attribute Flags octet is set
- * to 1, the third and fourth octets of the path attribute contain
- * the length of the attribute data in octets.
- */
- if (cluster && cluster->length + 4 > 255) {
+ if (cluster->length > 255) {
stream_putc(s, BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_EXTLEN);
stream_putc(s, BGP_ATTR_CLUSTER_LIST);
- stream_putw(s, cluster->length + 4);
+ stream_putw(s, cluster->length);
} else {
stream_putc(s, BGP_ATTR_FLAG_OPTIONAL);
stream_putc(s, BGP_ATTR_CLUSTER_LIST);
- stream_putc(s, cluster ? cluster->length + 4 : 4);
+ stream_putc(s, cluster->length);
}
-
- /* If this peer configuration's parent BGP has
- * cluster_id. */
- if (CHECK_FLAG(bgp->config, BGP_CONFIG_CLUSTER_ID))
- stream_put_in_addr(s, &bgp->cluster_id);
- else
- stream_put_in_addr(s, &bgp->router_id);
-
- if (cluster)
- stream_put(s, cluster->list, cluster->length);
+ stream_put(s, cluster->list, cluster->length);
}
/* Extended IPv6/Communities attributes. */
diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h
index 837e5ccac8..a68b188c7b 100644
--- a/bgpd/bgp_attr.h
+++ b/bgpd/bgp_attr.h
@@ -464,6 +464,8 @@ extern enum bgp_attr_parse_ret bgp_attr_ignore(struct peer *peer, uint8_t type);
/* Cluster list prototypes. */
extern bool cluster_loop_check(struct cluster_list *cluster,
struct in_addr originator);
+extern void bgp_attr_prepend_cluster_id(struct bgp *bgp, struct attr *attr);
+extern void bgp_attr_clear_reflection(struct attr *attr);
/* Below exported for unit-test purposes only */
struct bgp_attr_parser_args {
@@ -777,6 +779,21 @@ static inline void bgp_attr_set_cluster(struct attr *attr,
bgp_attr_unset(attr, BGP_ATTR_CLUSTER_LIST);
}
+static inline struct in_addr bgp_attr_get_originator_id(const struct attr *attr)
+{
+ return attr->originator_id;
+}
+
+static inline void bgp_attr_set_originator_id(struct attr *attr, struct in_addr originator_id)
+{
+ attr->originator_id = originator_id;
+
+ if (originator_id.s_addr != INADDR_ANY)
+ bgp_attr_set(attr, BGP_ATTR_ORIGINATOR_ID);
+ else
+ bgp_attr_unset(attr, BGP_ATTR_ORIGINATOR_ID);
+}
+
static inline struct bgp_route_evpn *
bgp_attr_get_evpn_overlay(const struct attr *attr)
{
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 0fe1845f38..86004f987e 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -2865,11 +2865,10 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi,
}
/* If originator-id is not set and the route is to be reflected,
- set the originator id */
- if (ibgp_to_ibgp && (!bgp_attr_exists(attr, BGP_ATTR_ORIGINATOR_ID))) {
- IPV4_ADDR_COPY(&(attr->originator_id), &(from->remote_id));
- bgp_attr_set(attr, BGP_ATTR_ORIGINATOR_ID);
- }
+ * set the originator id (before route-map so it can be overridden).
+ */
+ if (ibgp_to_ibgp && !bgp_attr_exists(attr, BGP_ATTR_ORIGINATOR_ID))
+ bgp_attr_set_originator_id(attr, from->remote_id);
/* Remove MED if its an EBGP peer - will get overwritten by route-maps
*/
@@ -3537,6 +3536,15 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi,
}
}
+ /* Handle originator-id and cluster-list for route reflection */
+ if (ibgp_to_ibgp) {
+ /* Prepend cluster-id to cluster list */
+ bgp_attr_prepend_cluster_id(bgp, attr);
+ } else {
+ /* Clear originator-id and cluster-list for non-reflected routes */
+ bgp_attr_clear_reflection(attr);
+ }
+
return true;
}
diff --git a/bgpd/bgp_updgrp_adv.c b/bgpd/bgp_updgrp_adv.c
index 3fe637ef20..9c4324ef69 100644
--- a/bgpd/bgp_updgrp_adv.c
+++ b/bgpd/bgp_updgrp_adv.c
@@ -1038,6 +1038,10 @@ void subgroup_default_originate(struct update_subgroup *subgrp, bool withdraw)
if (new_pref < pref) {
pref = new_pref;
bgp_attr_flush(new_attr);
+ /* Clear reflection attributes - default-originate
+ * is self-originated, not reflected.
+ */
+ bgp_attr_clear_reflection(tmp_pi.attr);
new_attr = bgp_attr_intern(
tmp_pi.attr);
}