bgpd: Add multiple clusters for bgp route-reflectors by PierreNeltner6WIND · Pull Request #22038 · FRRouting/frr

PierreNeltner6WIND · 2026-05-22T11:32:56Z

Add several commands that grant the possibility
to configure cluster-id on a per-neighbor basis
for route reflectors.

This allows more possibility in client-to-client
reflection management.

these commands are the following:
neighbor X.X.X.X cluster-id CLUSTER_ID
bgp cluster-id <per neighbor CLUSTER_ID|global>
client-to-client-reflection <true|fase>
show bgp clusters

two other command can change the behavior to be
closer to either juniper or cisco:
bgp cluster-id non-client-to-client prefer-global-cluster-id
bgp cluster-id loose_cluster-list-check

greptile-apps · 2026-06-22T13:42:03Z

Thanks for the detailed responses. All three explanations are technically sound:

Teardown guard: The BGP_FLAG_DELETE_IN_PROGRESS check is the standard FRR pattern for this exact scenario, and the point about peer_delete removing the peer from bgp->peer before any soft-clear loop could reach it is a valid belt-and-suspenders argument. Accepted.
Stale group leader flags: If the PEER_STATUS_GROUP check only gates the peer_clear_soft (not the flag assignment itself), then the group leader template stays current and new members inheriting via peer_group2peer_config_copy_af will pick up the correct cluster_flags. That directly addresses the concern. Accepted.
AS-change loop: If the AS-change callback is already dispatched to each member peer individually by the existing peer-group infrastructure (rather than once for the group leader), a redundant inner loop would be wrong. Accepted.

No further action needed from my side on these three points.

PierreNeltner6WIND · 2026-06-22T13:43:37Z

@greptile review

PierreNeltner6WIND · 2026-06-22T14:21:25Z

@greptile review

PierreNeltner6WIND · 2026-06-22T15:36:50Z

@greptile review

PierreNeltner6WIND · 2026-06-22T17:26:09Z

@greptile review

greptile-apps · 2026-06-22T17:42:10Z

Greptile Summary

This PR adds support for multiple cluster-ids per BGP route-reflector, enabling per-neighbor cluster identity assignment per AFI/SAFI. It introduces a new struct cluster with refcount lifecycle management, a doubly-linked list (per_neighbor_cluster_list) per peer, new client-to-client reflection policies ("always"/"never") configurable per cluster, and three new BGP instance-level flags (BGP_FLAG_PREFER_GLOBAL_CLUSTER, BGP_FLAG_LOOSE_CLUSTER_LIST_CHECK, BGP_FLAG_CLIENT_TO_CLIENT_GLOBAL_CLUSTER).

Adds bgp_neighbor_cluster_id_set/unset and bgp_per_neighbor_cluster_id_add/delete with symmetric refcount management gated on CLUSTER_FLAG_CLIENT_TO_CLIENT_INTRA_CLUSTER_CONFIGURED
Extends bgp_cluster_filter to scan all per-neighbor cluster-ids for RFC 4456 loop prevention and adds ibgp_reflection_check_from_per_neighbor/ibgp_reflection_check_from_global helpers in bgp_route.c
Includes a 9-router topotest covering per-neighbor cluster-id, c2c reflection policies, prefer-global-cluster-id, and loose-cluster-list-check scenarios

Confidence Score: 3/5

PR introduces substantial new feature with generally sound refcount/lifecycle management but contains a misleading redundant condition in the cluster-id stamping logic and an O(N) hot-path scan that may need profiling at scale before merging

Core lifecycle (alloc/free/refcount), peer-group propagation, teardown guards, and update-group hashing are all correct. The redundant negation in bgp_attr.c case 2 is a P2 clarity issue but not a runtime bug. The O(N) cluster scan in bgp_cluster_filter is architecturally intentional (RFC 4456) but warrants a comment noting the complexity. No data-loss or security issues found. Score of 3 reflects the feature complexity and the attr.c logic clarity issue.

bgpd/bgp_attr.c (redundant condition in cluster-id stamping chain), bgpd/bgp_route.c (O(N) scan in bgp_cluster_filter on inbound path)

Important Files Changed

Filename	Overview
bgpd/bgp_route.c	Adds per-cluster and global c2c reflection helpers; bgp_cluster_filter now does O(N) per-neighbor cluster scan on the hot inbound path — intentional per RFC 4456 but may be costly at scale
bgpd/bgp_attr.c	Four-case cluster-id stamping chain; case 2 contains a redundant negated flag sub-condition that is always true at that branch, making the logic misleading
bgpd/bgpd.c	Cluster lifecycle (alloc/free/refcount) is symmetric and correct; peer-group member loops use correct variable; BGP_FLAG_DELETE_IN_PROGRESS guard prevents spurious soft-clears during teardown
bgpd/bgp_vty.c	VTY commands and config-write use proper FRR versioned-defaults pattern; optional bracket on cluster-id token makes global c2c config parseable without id
bgpd/bgpd.h	New struct cluster, CLUSTER_FLAG_* and BGP_FLAG_* definitions look consistent with usage across the codebase
bgpd/bgp_updgrp.c	conf_copy, updgrp_hash_key_make, and updgrp_hash_cmp correctly include per-neighbor cluster-id in update-group identity
tests/topotests/bgp_multiple_clusters/test_bgp_multiple_clusters.py	Comprehensive 9-router topotest covering the main feature scenarios; sequential state dependencies are acceptable per FRR topotest convention

_{Reviews (26): Last reviewed commit: "doc: Add documentation to bgp client-to-..." | Re-trigger Greptile}

riw777

looks good ... let's give @ton31337 a bit to see if he wants to go back through this one

riw777 · 2026-06-24T17:47:12Z

@greptile can you verify all your comments have been resolved?

greptile-apps · 2026-06-24T17:48:14Z

All of my (Greptile) comments are resolved. Here's the summary:

My comments — all resolved (✅):

Wrong peer used for reset notifications in peer-group loop (bgpd/bgpd.c) — resolved
Wrong peer used for reset notifications in peer-group unset loop (bgpd/bgpd.c) — resolved
French comment in bgpd.h — resolved
Typos in CLI help strings (bgp_vty.c) — resolved
Non-client → client reflection stamps destination peer's cluster-id (bgp_attr.c) — resolved
cluster_ptr NULL guard missing (bgpd.c) — resolved
Soft-clear during BGP teardown (bgpd.c) — resolved (acknowledged as valid)
bgp_cluster_client_to_client_unset stale peer references (bgpd.c) — resolved
Refcount underflow on c2c toggle (bgpd.c) — resolved (false positive, acknowledged)
Global path cluster->flags NULL dereference (bgpd.c) — resolved
Stale cluster_flags on peer-group leaders (bgpd.c) — resolved
Double session reset for peer-group members (bgpd.c) — resolved
Excessive soft-in clear scope (bgpd.c) — resolved (design acknowledged as correct)
No IBGP validation in neighbor cluster-id VTY handler (bgp_vty.c) — resolved
per_neighbor_cluster_free missing list removal (bgpd.c) — resolved
Missing reflector-client guard (bgp_vty.c) — resolved

Unresolved threads (not mine — from @riw777):

Blank line consistency in bgp_route.c — still open
Double negative wording in bgp_vty.c — still open

You may want to address those two remaining threads from @riw777 before merging.

github-actions · 2026-06-30T12:13:55Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

lsang6WIND

3rd commit should be fixup to the first

riw777 · 2026-07-10T12:03:19Z

Can we get the linter errors fixed on this?

riw777 · 2026-07-10T12:28:46Z

@greptile can you check to see if all your comments have been resolved?

riw777 · 2026-07-10T12:29:26Z

===============================================
< WARNING: Block comments should align the * on each line
< #5871: FILE: /tmp/f1-193444/bgp_attr.c:5871:\nDetails at [https://ci1.netdef.org/browse/FRR-PULLREQ3-CHECKOUT-19518](https://ci1.netdef.org/browse/FRR-PULLREQ3-CHECKOUT-19518)```

Bah, humbug. :-) This needs to be fixed.

greptile-apps · 2026-07-10T12:30:01Z

After checking all 61 review threads, there is 1 unresolved comment — and it's not from me (Greptile), it's from @riw777:

"The double negative here seems like it might be a little confusing (?)" — bgpd/bgp_vty.c

It is marked as outdated (the underlying code has changed since the comment was posted), but it hasn't been formally resolved. All 60 of my (Greptile's) comments are resolved.

You may want to either address @riw777's concern or resolve that thread if the code change already addressed it.

Add the command : neighbor <A.B.C.D|X:X::X:X|WORD> cluster-id CLUSTER_ID This command can be executed after router bgp AS, address-family AFI SAFI. This command assigns a per-neighbor cluster for that specific peer and afi safi combination. If the peer is not previously declared that command fails. For that purpose the peer struct has one new attribute per_neighbor_cluster that is an in_addr list the content of which can be accessed through afi and safi indexes. This list is associated with a new af_flag that indicates whether it is configured. Another new attribute is added: cluster_flags that is also accessed throug afi safi indexes and ensures that the checking of cluster_flags can be performed in O(1) in the hot-path. A new structure is added to represent the new per-neighbor clusters: struct cluster, it contains the cluster-id of the cluster, flags that define its behavior and a count of how many times a cluster is referenced in the configuration, when that count goes down to 0 the cluster is deleted. That count takes into account every peers and groups that are part of the cluster and any reflecetion policy for that group. The CLUSTER_FLAG_GLOBAL marks any per-neighbor cluster whose cluster-id is identical to the global cluster-id: the unique historical cluster-id of route-reflectors. Whenever a cluster is marked as global,its flags are ignored and the ones of the global cluster are used instead. bgp instances have a new attribute: per_neighbor_clusters: a list that contains cluster objects. A peer that is part of a per-neighbor cluster and is configured as a route-reflector-client, behaves as a classic route-reflector-client peer. However, prefixes received from that peer will be advertised to other client and non-client peers with the per-neighbor cluster-id of that peer in the prefix's cluster-list. In the same spirit prefix reflected to one such peer from a non-client peer will have the per-neighbor cluster-id of the client-peer added to the cluster list. When a prefix is reflected between two client peers part of different per-neighbor clusters only the cluster-id of the sender is added to the cluster-list. If not configured as route-reflector client, a peer with a per-neighbor cluster-id behaves as a normal peer. Because the per-neighbor cluster-ids have an influence on out-going prefixes, this commit also modifies update-groups more specificaly ```updgrp_hash_key_make``` and ```updgrp_hash_cmp``` in order to have peers from different per-neighbor clusters sorted in different update groups. In the spirit of the rfc4456 any incoming prefix that contains one of the cluster-ids of the route-reflector clients is dropped. This implies changes to the ```bgp_cluster_filter``` function in order to check all clusters and not only the global one. A cluster-id can be assigned to a peer that is not a route-reflector-client in that case that peer will behave normaly but the cluster-id will still be taken into account for filtering. This makes sure that commands do not depend on the execution order. Information about cluster-ids that are configured on peers can be found in the running config and in the display of that peer's information. Signed-off-by: Pierre Neltner <pierre.neltner@6wind.com>

the topology of the test is the following : +------+ +------+ | R1--|- -|--R6 | | | \ R9 / | | C1 | | \ / \ / | | C3 | R2--|---R4--R5---|--R7 | +------+ / \ +------+ +------+ / \ +------+ C2 | R3--|- -|--R8 | C2 +------+ +------+ All routers are in AS 100 (IBGP). R4 and R5 are configured as a route reflector with their neighbors as clients. They form with r9 a full-mesh of non-client peers. They each oversee two clusters of clients Test that reflection in the default setup works as intended and that cluster-lists are correct, verifies that client-to-client configurations works well both in the per-neighbor and global clusters. Test that the non-client-to-client prefer-global-cluster-id and loose-cluster-list-check configuration work as intentded. These checks are achieved through verifying the prefixes in the bgp tables of each client router and on R9 as well as their cluster-lists. Signed-off-by: Pierre Neltner <pierre.neltner@6wind.com>

Refactor the route-reflector reflection-check in bgp_route.c in order to prepare for next commit. Signed-off-by: Pierre Neltner <pierre.neltner@6wind.com>

Add 2 new flags: CLUSTER_FLAG_CLIENT_TO_CLIENT_INTRA_CLUSTER and CLUSTER_FLAG_CLIENT_TO_CLIENT_INTRA_CLUSTER_CONFIGURED. These flags represent the three states that are possible for a cluster configuration and apply to the flags attribute of the cluster object: the cluster-to-cluster behavior is not configured, the cluster-to-cluster behavior is configured and enabled, the cluster-to-cluster behavior is configured and disabled Add 2 new flags: BGP_FLAG_CLIENT_TO_CLIENT_GLOBAL_CLUSTER and BGP_FLAG_CLIENT_TO_CLIENT_GLOBAL_CLUSTER_CONFIGURED. These flags have same role as the ones described before but for the global cluster, they apply to bgp->flags. The behavior of client-to-client reflection is now the following: reflection inside of a cluster is always allowed if it is configured as allowed for that cluster, it is always forbidden if it is configured as forbidden for that cluster. If nothing is configured for that cluster then the legacy client-to-client reflection configuration applies. Between two different clusters the legacy client-to-client reflection configuration applies. This allows any pattern of reflection to be easily set up. This configuration is achieved through the new command: ```[no] bgp cluster <per-neighbor X.X.X.X|global> \ client-to-client-reflection <always|never>``` That command configures client-to-client reflection on any per-neighbor cluster or the global cluster. In its negative form it unconfigures client-to-client reflection. If it is configured on a per-neighbor cluster then adds one to the reference of that cluster and if it doesn't exist creates it in a similar fashion to the neighbor X.X.X.X cluster-id command. These configurations are shown in the running config Signed-off-by: Pierre Neltner <pierre.neltner@6wind.com>

add the command : ```Add command show bgp clusters [json]``` that shows information about all configured or referenced clusters including the global cluster and per-neighbor clusters this includes the cluster id, client-to-client reflection policies inside of each cluster and number of times referenced in the configuration as well as whether it is overwritten by the global cluster. the json format is the following: { "global":{ "cluster-id":"CLUSTER_ID", "client-to-client-reflection":<"always"|"never"|"not-configured"> }, "per-neighbor":[ "cluster-id":"CLUSTER_ID", "global":bool, "client-to-client-reflection":<"always"|"never"|"not-configured">, "refcnt":NUMBER_OF_REFERENCES ], } Signed-off-by: Pierre Neltner <pierre.neltner@6wind.com>

Add the command : bgp cluster-id non-client-to-client prefer-global-cluster This command configures the new flag BGP_FLAG_PREFER_GLOBAL_CLUSTER When this flag is configured, the global cluster-id is added to the cluster list of prefixes reflected from non-clients toward client-peers, default behavior is to add the destination peer's cluster-id instead. When configured the behavior is similar to the cisco one and further from a classic cluster-id behavior. This feature modifies the attributes of outgoing routes and so modifies the ```bgp_packet_attribute``` function from bgp_attribute.c This flag can be seen in the running config. Signed-off-by: Pierre Neltner <pierre.neltner@6wind.com>

add the command: bgp cluster-id loose-cluster-list-check This command configures the new bgp flag: BGP_FLAG_LOOSE_CLUSTER_LIST_CHECK When this is configured, prefixes that contain one or more of the cluster-ids of the per neighbor clusters of the router are no longer dropped when received, they are just not reflected to members of these clusters. This implies that the ```bgp_cluster_filter``` function from bgp_route.c is modified to no longer reject these and that ```subgroup_announce_check``` is modified to reject these prefixes only when advertising toward a member of that cluster. When configured this option shows a behavior closer to juniper behavior while still respecting rfc4456. This configuration can be seen in the running-config. Signed-off-by: Pierre Neltner <pierre.neltner@6wind.com>

Signed-off-by: Pierre Neltner <pierre.neltner@6wind.com>

riw777 · 2026-07-10T21:14:38Z

@lsang6WIND everything okay from your end at this point?

frrbot Bot added bgp documentation tests Topotests, make check, etc labels May 22, 2026

github-actions Bot added master size/XXL labels May 22, 2026

PierreNeltner6WIND force-pushed the multiple_clusters branch 25 times, most recently from 45eb1e1 to 4fbaccf Compare May 25, 2026 10:53

greptile-apps Bot reviewed Jun 22, 2026

View reviewed changes

Comment thread bgpd/bgp_vty.c

riw777 approved these changes Jun 24, 2026

View reviewed changes

lsang6WIND reviewed Jun 30, 2026

View reviewed changes

Comment thread bgpd/bgp_updgrp.c

Comment thread bgpd/bgpd.h Outdated

lsang6WIND reviewed Jun 30, 2026

View reviewed changes

Comment thread bgpd/bgp_vty.c Outdated

Comment thread bgpd/bgp_vty.c

Comment thread bgpd/bgp_vty.c

Comment thread bgpd/bgpd.c

lsang6WIND reviewed Jul 2, 2026

View reviewed changes

lsang6WIND reviewed Jul 3, 2026

View reviewed changes

Comment thread bgpd/bgp_route.c

Comment thread bgpd/bgpd.c Outdated

Comment thread doc/user/bgp.rst

PierreNeltner6WIND added 8 commits July 10, 2026 16:34

bgpd: refactor the route-reflector reflection-check in bgp_route.c

8d81855

Refactor the route-reflector reflection-check in bgp_route.c in order to prepare for next commit. Signed-off-by: Pierre Neltner <pierre.neltner@6wind.com>

doc: Add documentation to bgp client-to-client reflection

de7f7b2

Signed-off-by: Pierre Neltner <pierre.neltner@6wind.com>

Uh oh!

Conversation

PierreNeltner6WIND commented May 22, 2026

Uh oh!

greptile-apps Bot commented Jun 22, 2026

Uh oh!

PierreNeltner6WIND commented Jun 22, 2026

Uh oh!

PierreNeltner6WIND commented Jun 22, 2026

Uh oh!

Uh oh!

PierreNeltner6WIND commented Jun 22, 2026

Uh oh!

PierreNeltner6WIND commented Jun 22, 2026

Uh oh!

greptile-apps Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Uh oh!

riw777 left a comment

Choose a reason for hiding this comment

Uh oh!

riw777 commented Jun 24, 2026

Uh oh!

greptile-apps Bot commented Jun 24, 2026

Uh oh!

github-actions Bot commented Jun 30, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lsang6WIND left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

riw777 commented Jul 10, 2026

Uh oh!

riw777 commented Jul 10, 2026

Uh oh!

riw777 commented Jul 10, 2026

Uh oh!

greptile-apps Bot commented Jul 10, 2026

Uh oh!

riw777 commented Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

greptile-apps Bot commented Jun 22, 2026 •

edited

Loading

lsang6WIND left a comment •

edited

Loading