bgpd: fix link-bandwidth AS selection in confederations by srinivasan-nexthop · Pull Request #22872 · FRRouting/frr

srinivasan-nexthop · 2026-07-30T14:07:19Z

Summary

Fix incorrect AS numbers in link-bandwidth extended communities when BGP confederations are configured. External eBGP peers should see the confederation AS in the link-bandwidth EC; iBGP and confederation peers should see the local sub-AS — mirroring confederation AS_PATH semantics.

Problem

Link-bandwidth extended communities embed an AS number. In a confederation, that AS was wrong in several cases:

Automatic cumulative link-bandwidth (WECMP) used the local sub-AS when advertising to external eBGP peers.
Route-map set extcommunity bandwidth always used the local sub-AS, with no way to know at route-map-evaluation time which peer the route would ultimately be sent to — so the same (wrong, for external peers) AS was sent to iBGP, confederation, and external peers alike.
Non-transitive link-bandwidth set via route-map never got an AS rewrite: ecommunity_replace_linkbw() returned early for non-transitive communities, so external peers saw the sub-AS instead of the confederation AS.
Origination-only route-maps (network … route-map, no outbound route-map on the destination peer): BATTR_RMAP_LINK_BW_SET was cleared during the per-destination attribute reset, so even after adding the per-destination export-time correction (below), it was still skipped for peers with no outbound route-map of their own.

Solution

`bgp_link_bw_as_for_peer()`

New helper in bgpd.c / bgpd.h — centralizes AS selection:

Confederation ID → external eBGP peers (not in confederation peer list)
Local sub-AS → iBGP, confederation peers, and origination (peer_self)

Export-time rewrite in `subgroup_announce_check()`

Automatic cumulative link-bandwidth: use bgp_link_bw_as_for_peer() when building/replacing the EC for each destination peer.
Route-map-set link-bandwidth: when BATTR_RMAP_LINK_BW_SET is set, rewrite only the AS in the existing EC (bandwidth unchanged), again per destination peer.

Other fixes

ecommunity_replace_linkbw(): add ignore_non_transitive parameter so callers that only need an AS correction (not a bandwidth change) can rewrite non-transitive link-bandwidth ECs while preserving the non-transitive flag. WECMP cumulative path keeps existing behavior (ignore_non_transitive=false).
BATTR_RMAP_LINK_BW_SET: preserve across the per-destination attribute reset so origination-only route-map cases still get export-time AS correction.
UAF guard: only ecommunity_free(old_ecom) when ecommunity_replace_linkbw() returns a new, unshared object (new_ecom != old_ecom && !old_ecom->refcnt) — a no-op replace, or one still referenced elsewhere, must not free the live EC.
route_set_ecommunity_lb(): use bgp_link_bw_as_for_peer() at origination; AS is still rewritten on export in subgroup_announce_check().

Topotest

tests/topotests/bgp_confederation_linkbw_as_test/

7-router confederation topology (r1 sub-AS 65101, confederation AS 65100). Covers IPv4 and IPv6:

Scenario	Peers exercised
Automatic transitive cumulative LB	iBGP (r2, r7), confed (r3), external (r5)
Route-map outbound LB	confed (r4), external (r6)
Non-transitive route-map LB	confed + external
Origination-only route-map (`network … route-map`)	confed + external (no per-neighbor outbound RM)

Files changed

bgpd/bgpd.c, bgpd/bgpd.h — bgp_link_bw_as_for_peer()
bgpd/bgp_route.c — export-time AS rewrite, flag preservation
bgpd/bgp_routemap.c — origination path uses helper
bgpd/bgp_ecommunity.c, bgpd/bgp_ecommunity.h — ignore_non_transitive on ecommunity_replace_linkbw()
tests/topotests/bgp_confederation_linkbw_as_test/ — new topotest

Test plan

bgp_confederation_linkbw_as_test (IPv4 + IPv6, automatic / route-map / non-transitive / origination-only)

greptile-apps · 2026-07-30T14:10:40Z

Greptile Summary

Fixes per-destination AS selection for link-bandwidth extended communities in BGP confederations.

Adds a shared helper selecting the confederation AS for external peers and the local sub-AS for internal/confederation peers.
Rewrites route-map-set and cumulative link-bandwidth communities during export while preserving bandwidth and transitivity.
Preserves route-map link-bandwidth metadata for origination-only policy.
Adds IPv4 and IPv6 topotests covering internal, confederation, external, transitive, and non-transitive advertisements.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete changed-code defect identified.

The export path consistently selects the destination-facing AS, preserves configured bandwidth and non-transitive encoding, and follows established extended-community ownership conventions.

Important Files Changed

Filename	Overview
bgpd/bgp_route.c	Preserves route-map link-bandwidth state and performs safe per-peer AS correction during announcement.
bgpd/bgp_ecommunity.c	Extends link-bandwidth replacement to optionally rewrite non-transitive communities while retaining their flag.
bgpd/bgpd.c	Adds centralized confederation-aware link-bandwidth AS selection consistent with reachable peer classifications.
bgpd/bgp_routemap.c	Uses the centralized AS selector when constructing route-map link-bandwidth communities.
tests/topotests/bgp_confederation_linkbw_as_test/test_bgp_confederation_linkbw_as.py	Exercises IPv4 and IPv6 link-bandwidth AS behavior across internal, confederation, and external peers.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Route or multipath attribute] --> B[subgroup_announce_check]
  B --> C{Link bandwidth source}
  C -->|Automatic cumulative| D[Calculate cumulative bandwidth]
  C -->|Route-map set| E[Preserve configured bandwidth]
  D --> F[Select AS for destination peer]
  E --> F
  F --> G{Peer relationship}
  G -->|External eBGP| H[Confederation AS]
  G -->|iBGP or confederation| I[Local sub-AS]
  H --> J[Encode outbound link-bandwidth EC]
  I --> J

_{Reviews (1): Last reviewed commit: "bgpd: fix link-bandwidth AS selection in..." | Re-trigger Greptile}

ton31337 · 2026-07-30T14:14:18Z

+ * confederation peers. At origination (peer_self), use local sub-AS;
+ * subgroup_announce_check() rewrites per destination peer on export.
+ */
+as_t bgp_link_bw_as_for_peer(struct bgp *bgp, struct peer *peer)


Let's rename this function to be more generic - it doesn't do anything with link bandwidth at all...

Good catch — renamed to bgp_local_as_for_peer() (single struct peer *peer parameter now, bgp is no longer needed). While addressing this I also found we could drop the reimplementation entirely: peer->local_as is already maintained correctly across every confederation
config-change path (bgp_confederation_id_set()/unset(), bgp_confederation_peers_add()/remove(), and the peer AS-change path), so the function now just reads that field plus peer->change_local_as rather than recomputing the confed_id/local-AS decision from scratch.
Details in the reply to the comment below.

Pushed in 8d69c57.

ton31337 · 2026-07-30T14:14:44Z

+	    peer != bgp->peer_self &&
+	    peer->as != bgp->as &&
+	    !bgp_confederation_peers_check(bgp, peer->as))
+		return bgp->confed_id;


We MUST also evaluate the case when neighbor X local-as Y is used.

Good point, thank you. Fixed — bgp_local_as_for_peer() now honors change_local_as:

as_t bgp_local_as_for_peer(struct peer *peer)
{
if (!CHECK_FLAG(peer->bgp->config, BGP_CONFIG_CONFEDERATION) &&
peer->change_local_as)
return peer->change_local_as;

return peer->local_as;

}

I matched the priority order used in AS_PATH construction (bgp_packet_attribute())

Added tests/topotests/bgp_linkbw_local_as_test/ — a standalone (non-confederation) topology with neighbor X local-as Y configured, confirming the link-bandwidth AS reflects the override.

Pushed in 8d69c57.

donaldsharp · 2026-07-30T14:18:28Z

@ton31337 do we need a topotest?

Use the confederation ID for link-bandwidth extended communities when advertising to external eBGP peers, and the local sub-AS for iBGP and confederation peers. Cover both automatic cumulative link-bandwidth and route-map-set link-bandwidth by adjusting AS per destination peer in subgroup_announce_check(). bgp_local_as_for_peer() centralizes this AS selection instead of duplicating it across export paths. Rather than reimplementing the confederation-vs-local decision, it uses peer->local_as directly, which FRR already maintains correctly across every confederation config-change path (bgp_confederation_id_set()/unset(), bgp_confederation_peers_add()/ remove(), and peer AS-change handling). It also honors an explicit "neighbor X local-as Y" override (peer->change_local_as) when confederation is not configured for that peer, mirroring the priority order used in AS_PATH construction (bgp_packet_attribute()): confederation membership is a structural property of the peer relationship and takes precedence over a session-presentation override. Only free replaced extended communities when ecommunity_replace_linkbw() returns a new object, avoiding use-after-free when replacement is a no-op. Non-transitive link-bandwidth communities need the same AS correction as transitive ones, but ecommunity_replace_linkbw() bailed out early for non-transitive communities entirely, so the route-map-set AS fix silently never applied to them. External eBGP peers ended up seeing the local sub-AS instead of the confederation AS whenever the operator set link-bandwidth as non-transitive via route-map. Add an ignore_non_transitive parameter so callers that only need to fix the encoded AS (not replace the bandwidth value) can still do so, while preserving the non-transitive marking on the rebuilt community. The existing WECMP cumulative-bandwidth path keeps its original behavior unchanged. The AS correction for route-map-set link-bandwidth also failed to apply when the route-map only ran at origination (e.g. "network ... route-map") and the destination peer had no outbound route-map of its own, since the tracking flag for "link-bandwidth was route-map-set" was unconditionally cleared per destination before being rechecked. Preserve that flag across the per-destination attribute reset so the AS gets corrected for every destination, not only ones with their own outbound route-map. Add a topotest covering automatic, route-map, non-transitive, and origination-only link-bandwidth AS selection, over iBGP, confederation, and external peers, in IPv4 and IPv6. Add a second, standalone topotest covering the "neighbor X local-as Y" override with no confederation configured. Signed-off-by: Srinivasan Koona Lokabiraman <srinivasan@nexthop.ai>

frrbot Bot added bgp bugfix labels Jul 30, 2026

github-actions Bot added size/XXL master labels Jul 30, 2026

ton31337 requested changes Jul 30, 2026

View reviewed changes

srinivasan-nexthop force-pushed the srinivasan.Fix-LB-AS-in-confed branch from e6659fd to 8d69c57 Compare July 30, 2026 14:51

srinivasan-nexthop requested a review from ton31337 July 30, 2026 15:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

bgpd: fix link-bandwidth AS selection in confederations - #22872

bgpd: fix link-bandwidth AS selection in confederations#22872
srinivasan-nexthop wants to merge 1 commit into
FRRouting:masterfrom
srinivasan-nexthop:srinivasan.Fix-LB-AS-in-confed

srinivasan-nexthop commented Jul 30, 2026

Uh oh!

greptile-apps Bot commented Jul 30, 2026

Uh oh!

ton31337 Jul 30, 2026

Uh oh!

srinivasan-nexthop Jul 30, 2026

Uh oh!

ton31337 Jul 30, 2026

Uh oh!

srinivasan-nexthop Jul 30, 2026

Uh oh!

donaldsharp commented Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

srinivasan-nexthop commented Jul 30, 2026

Summary

Problem

Solution

bgp_link_bw_as_for_peer()

Export-time rewrite in subgroup_announce_check()

Other fixes

Topotest

Files changed

Test plan

Uh oh!

greptile-apps Bot commented Jul 30, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

ton31337 Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

srinivasan-nexthop Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

ton31337 Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

srinivasan-nexthop Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

donaldsharp commented Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

`bgp_link_bw_as_for_peer()`

Export-time rewrite in `subgroup_announce_check()`