bgpd: Fix missing SRv6 advertisement with distribute bgp-fabric-link-state#21912
bgpd: Fix missing SRv6 advertisement with distribute bgp-fabric-link-state#21912cscarpitta wants to merge 10 commits into
distribute bgp-fabric-link-state#21912Conversation
Greptile SummaryThis PR fixes missing SRv6 information in BGP-LS advertisements when
Confidence Score: 5/5Safe to merge; the core SRv6 BGP-LS origination logic is sound and two previously reported counter/flag issues are addressed. The locator-count tracking, DLIST memory management, seg6local data plumbing through zebra to bgp_route to bgp_ls, and the NULL-locator get-all path are all implemented correctly. The two inline comments flag unnecessary node NLRI re-originations when End.X SIDs are manipulated without any active locator, but neither produces incorrect data — only extra BGP updates — and both have straightforward fixes. bgpd/bgp_ls.c — the upsert/delete End.X SID paths around the bgp_ls_originate_bgp_node call guards. Important Files Changed
Sequence DiagramsequenceDiagram
participant Z as Zebra
participant BZ as bgp_zebra.c
participant BR as bgp_route.c
participant BLS as bgp_ls.c
Note over Z,BLS: distribute bgp-fabric-link-state enabled
Z->>BZ: ZEBRA_REDISTRIBUTE_ROUTE_ADD (static, seg6local)
BZ->>BR: bgp_redistribute_add(..., seg6local_action, seg6local_ctx)
BR->>BLS: bgp_ls_handle_route_add(...)
BLS->>BLS: "bgp_ls_handle_srv6_localsid_update (is_add=true)"
alt END action
BLS->>BLS: bgp_ls_originate_static_srv6_sid_from_seg6local()
BLS->>BLS: originate SRv6 SID NLRI
else END_X action
BLS->>BLS: bgp_ls_upsert_bgp_link_srv6_endx_sid()
BLS->>BLS: store in static_endx_sids list
BLS->>BLS: bgp_ls_refresh_bgp_link_endx_attrs(peer)
BLS->>BLS: originate Link NLRI with End.X SID attr
end
Z->>BZ: ZEBRA_SRV6_LOCATOR_ADD
BZ->>BLS: bgp_ls_originate_srv6_locator_prefix(default_bgp, loc)
BLS->>BLS: originate IPv6 Prefix NLRI with SRv6 Locator attr
BLS->>BLS: srv6_locator_nlri_count++
BLS->>BLS: bgp_ls_originate_bgp_node() with SRv6 Capabilities attr
Z->>BZ: ZEBRA_SRV6_LOCATOR_DEL
BZ->>BLS: bgp_ls_withdraw_srv6_locator_prefix(default_bgp, loc)
BLS->>BLS: bgp_ls_withdraw() prefix NLRI
BLS->>BLS: srv6_locator_nlri_count--
alt count reaches 0
BLS->>BLS: bgp_ls_originate_bgp_node() without SRv6 Capabilities
end
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
bgpd/bgp_ls.c:1685-1718
`bgp_ls_has_srv6_capability` only counts SRv6 locator NLRIs, so `had_srv6_cap` is always `false` when there are no locators — regardless of how many End.X SIDs are already in `static_endx_sids`. As a result, every call to `bgp_ls_upsert_bgp_link_srv6_endx_sid` when no locator is present unconditionally re-originates the node NLRI without any SRv6 capability attribute set (a no-op advertisement). The guard should fire only when SRv6 capability is actually gained — i.e., only when the End.X SID list was previously empty as well.
```suggestion
had_srv6_cap = bgp_ls_has_srv6_capability(bgp);
bool endx_was_empty = bgp_ls_endx_sid_list_empty(&bgp->ls_info->static_endx_sids);
entry = bgp_ls_static_endx_sid_lookup(bgp, sid, prefixlen);
if (!entry) {
entry = XCALLOC(MTYPE_BGP_LS, sizeof(*entry));
entry->sid = *sid;
entry->prefixlen = prefixlen;
bgp_ls_endx_sid_list_add_tail(&bgp->ls_info->static_endx_sids, entry);
} else {
old_peer = entry->peer;
}
entry->behavior = behavior;
entry->lb_len = ctx->block_len;
entry->ln_len = ctx->node_len;
entry->fn_len = ctx->function_len;
entry->arg_len = ctx->argument_len;
entry->nh6 = ctx->nh6;
entry->ifindex = ctx->ifindex;
entry->peer = NULL;
peer = bgp_ls_find_peer_by_nh6(bgp, &ctx->nh6, ctx->ifindex);
if (!peer) {
if (BGP_DEBUG(linkstate, LINKSTATE))
zlog_debug("BGP-LS: %pI6/%u stored pending End.X SID for nh6=%pI6 ifindex=%u until link exists",
sid, prefixlen, &ctx->nh6, ctx->ifindex);
} else {
entry->peer = peer;
if (BGP_DEBUG(linkstate, LINKSTATE))
zlog_debug("BGP-LS: %pI6/%u matched peer %s", sid, prefixlen, peer->host);
}
if (!had_srv6_cap && endx_was_empty)
bgp_ls_originate_bgp_node(bgp);
```
### Issue 2 of 2
bgpd/bgp_ls.c:1742-1747
The symmetrical problem exists on deletion: `bgp_ls_has_srv6_capability` only tests the locator count, so this guard fires whenever there are no locators — even when the `static_endx_sids` list still has other entries. That generates a spurious node re-origination (without SRv6 capability) for every End.X SID removal when no locator is configured. The re-origination should only happen when the last End.X SID is removed and there are no locators.
```suggestion
peer = entry->peer;
bgp_ls_endx_sid_list_del(&bgp->ls_info->static_endx_sids, entry);
XFREE(MTYPE_BGP_LS, entry);
if (!bgp_ls_has_srv6_capability(bgp) &&
bgp_ls_endx_sid_list_empty(&bgp->ls_info->static_endx_sids))
bgp_ls_originate_bgp_node(bgp);
```
Reviews (3): Last reviewed commit: "tests: Add topotest for BGP-LS SRv6 exte..." | Re-trigger Greptile |
When zebra sends redistributed routes, SRv6 seg6local metadata attached to nexthops is not propagated in the ZAPI payload. Encode seg6local metadata in zsend_redistribute_route() by setting ZAPI_NEXTHOP_FLAG_SEG6LOCAL and filling seg6local_action plus seg6local_ctx when a nexthop has a valid seg6local action. This preserves SRv6 LocalSID metadata across zebra-to-bgpd redistribution, so bgpd can derive the SRv6 endpoint behavior from redistributed routes when building BGP-LS SRv6 NLRIs/attributes. Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
…tate When BGP-LS fabric distribution is enabled, bgpd must receive IPv6 redistributed routes from zebra so SRv6-related route information can be exported through BGP-LS. On distribute bgp-fabric-link-state, register IPv6 ZEBRA_ROUTE_ALL redistribution by calling bgp_redist_add() and bgp_redistribute_set(). Log a warning if subscription setup fails. On no distribute bgp-fabric-link-state, remove the redistribution subscription with bgp_redistribute_unset() before disabling distribution and withdrawing exported BGP-LS routes. Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
Add BGP-LS support for static SRv6 LocalSIDs (END behaviors) learned via zebra route redistribution. Introduce BGP-LS handlers to process redistribute add/delete events and react to SRv6 localsid updates. For eligible static IPv6 routes, map seg6local action/context to SRv6 endpoint behavior and optional SID structure fields, then originate/withdraw SRv6 SID NLRIs. Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
Add BGP-LS support for static SRv6 END.X LocalSIDs and attach them to BGP link NLRIs as SRv6 End.X attributes. Maintain a per-instance END.X SID list keyed by SID/prefix and store peer matching keys (next-hop IPv6 and ifindex). When END.X localsids are added or removed, update the list and refresh the corresponding BGP link advertisement. Refactor link origination to accept optional attributes and build link updates with zero or more SRv6 End.X SIDs. If no END.X SID matches a peer, originate the link without End.X attributes. This enables BGP-LS export of per-link SRv6 END.X information derived from static seg6local routes. Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
Allow SRv6 get-locator requests to omit the locator name and treat that as a request for all locators. Update zclient and zapi message handling so NULL locator names are encoded/decoded safely and passed through to the SRv6 manager callback. In zebra SRv6 manager, when locator_name is NULL, iterate all configured locators and send each one to the requesting client. This adds bulk locator retrieval while preserving single-locator lookup behavior. This is needed by BGP-LS to retrieve all SRv6 locators and originate Prefix NLRIs with SRv6 Locator TLVs. Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
Extract common BGP-LS prefix NLRI construction into a new internal helper to avoid duplication between route-prefix and locator-prefix origination paths. Add bgp_ls_originate_prefix_internal() to centralize: - allocating the prefix NLRI and selecting the NLRI type from AFI - populating local-node descriptors (ASN and BGP Router-ID) - building prefix descriptors (BGP route type and IP reachability) - submitting the update and releasing temporary NLRI state This refactor does not introduce functional or protocol behavior changes. Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
ad4bb36 to
8be328f
Compare
|
@greptile review |
…LRIs When a new SRv6 locator is received from the SRv6 SID Manager, originate a BGP-LS Prefix NLRI carrying an SRv6 Locator TLV with the locator flags, metric and algorithm number. When a locator is removed, withdraw the corresponding Prefix NLRI. On the first locator advertisement, set has_srv6_locator on the BGP-LS instance and re-originate the Node NLRI so the SRv6 Capabilities TLV is included. Hook bgp_zebra_process_srv6_locator_add/delete in bgp_zebra.c to call the new originate/withdraw functions. Also trigger an immediate locator fetch from CLI when distribute bgp-fabric-link-state is enabled and the SRv6 SID Manager is already connected. Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
When a BGP instance with BGP-LS fabric distribution enabled registers with the SRv6 SID Manager, request all locators instead of a specific one. This ensures BGP-LS receives the full locator set at startup and can originate Prefix NLRIs with SRv6 Locator TLVs for each configured locator. Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
Add a helper bgp_ls_has_srv6_capability() that returns true when the BGP-LS instance has received at least one SRv6 locator. When originating the BGP Node NLRI, include the SRv6 Capabilities TLV in the node attributes if the instance has SRv6 capability. This signals to BGP-LS consumers that the node is SRv6-capable. Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
Add a dedicated topotest suite bgp_link_state_bgp_fabric_srv6 to verify BGP-LS export of SRv6 NLRIs and attributes in a BGP-only fabric scenario. The topology consists of four fabric routers (r1-r4) with SRv6 locators and static uN/uA SIDs configured, peering with a route reflector (rr) acting as the BGP-LS collector. r1/r2 form an iBGP pair (AS 65001), r3/r4 form an eBGP pair (AS 65002/65003), and each router advertises its BGP-LS topology to the rr. Test cases: - Verify BGP convergence across all routers - Verify SRv6 NLRIs and attributes (locator prefixes, SRv6 Capabilities, SRv6 Endpoint Behavior, SRv6 End.X SIDs) on the rr match expected JSON Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
8be328f to
1d6b17a
Compare
|
@greptile review |
|
@Mergifyio backport stable/10.6 stable/10.5 |
🟠 Waiting for conditions to matchDetails
|
When distribute
bgp-fabric-link-stateis enabled via CLI, BGP-LS advertises Node, Link, and Prefix NLRIs, but SRv6 information is missing.SRv6 capability and SRv6 attributes (locator, End, End.X) should be advertised together with the BGP-fabric topology, but currently are not.
This PR fixes SRv6 advertisement for BGP-LS BGP-fabric export and adds a dedicated topotest that validates SRv6 BGP-LS advertisements.