From 1f464dd3a271851a3a6cbd8b51079777ed4dc9fb Mon Sep 17 00:00:00 2001 From: ryskn Date: Fri, 22 May 2026 01:19:59 +0900 Subject: [PATCH 1/2] fix(srv6): implement DelConnectivity with NLRI-key teardown and candidate-path failover SRv6Provider.DelConnectivity was a FIXME no-op. Implement SR Policy withdraw/teardown and the candidate-path failover that follows it (RFC 9012 NLRI key, RFC 9256 candidate paths): - A withdraw is handled per NLRI key , so it tears down only the candidate path it identifies and never collaterally removes other policies on the same endpoint. - After teardown the orphaned prefix is re-steered onto the next-best surviving candidate of the same behavior on the same endpoint; the survivor is installed on demand (it was masked by the higher-priority winner and never reached CreateSRv6Tunnel). - The SR Policy NLRI cache is keyed by the NLRI key and upserted in place; on a BSID change the prior BSID is torn down after the steering has been re-pointed, and a pendingBsidCleanup queue retries cleanups that VPP still resolves through so a superseded BSID never leaks. - NO_SUCH_INNER_FIB and UNSPECIFIED from VPP are treated as idempotent "already gone" in a delete context (debug, not warn); other errors warn. - getPolicyNode skips candidates with a nil SrPolicy (nil-deref guard) and uses a strict priority comparison so a tie deterministically keeps the first candidate. common.SRv6Tunnel carries Color and Distinguisher; getSRPolicy populates them on success, on the withdraw short-circuit, and on the mixed-behavior reject so the SRv6PolicyDeleted dispatch is NLRI-key targeted. Signed-off-by: Ryosuke Nakayama --- calico-vpp-agent/common/common.go | 20 +- calico-vpp-agent/connectivity/srv6.go | 337 ++++++++- calico-vpp-agent/connectivity/srv6_test.go | 784 +++++++++++++++++++++ calico-vpp-agent/routing/bgp_watcher.go | 22 +- 4 files changed, 1139 insertions(+), 24 deletions(-) create mode 100644 calico-vpp-agent/connectivity/srv6_test.go diff --git a/calico-vpp-agent/common/common.go b/calico-vpp-agent/common/common.go index 39c7e5146..b79b83176 100644 --- a/calico-vpp-agent/common/common.go +++ b/calico-vpp-agent/common/common.go @@ -520,14 +520,20 @@ func (cn *NodeConnectivity) String() string { return fmt.Sprintf("%s-%s-%s", cn.Dst.String(), cn.NextHop.String(), fmt.Sprint(cn.Vni)) } -// SRv6Tunnel contains info needed to create all SRv6 tunnel components (Steering, Policy, Localsids) +// SRv6Tunnel contains info needed to create all SRv6 tunnel components (Steering, Policy, Localsids). +// Color and Distinguisher form the NLRI key from +// RFC 9012 that uniquely identifies one SR Policy candidate; DelConnectivity matches +// on that key so a withdraw cannot collaterally tear down unrelated policies on the +// same endpoint. type SRv6Tunnel struct { - Dst net.IP - Bsid net.IP - Policy *types.SrPolicy - Sid net.IP - Behavior uint8 - Priority uint32 + Dst net.IP + Bsid net.IP + Policy *types.SrPolicy + Sid net.IP + Behavior uint8 + Priority uint32 + Color uint32 + Distinguisher uint32 } func GetBGPSpecAddresses(nodeBGPSpec *LocalNodeSpec) (ip4 *net.IP, ip6 *net.IP) { diff --git a/calico-vpp-agent/connectivity/srv6.go b/calico-vpp-agent/connectivity/srv6.go index f15a81bf2..b4bfdfec3 100644 --- a/calico-vpp-agent/connectivity/srv6.go +++ b/calico-vpp-agent/connectivity/srv6.go @@ -9,6 +9,7 @@ import ( "github.com/projectcalico/calico/libcalico-go/lib/ipam" cnet "github.com/projectcalico/calico/libcalico-go/lib/net" "github.com/projectcalico/calico/libcalico-go/lib/options" + govppapi "go.fd.io/govpp/api" "github.com/projectcalico/vpp-dataplane/v3/calico-vpp-agent/common" "github.com/projectcalico/vpp-dataplane/v3/config" @@ -17,6 +18,19 @@ import ( "github.com/projectcalico/vpp-dataplane/v3/vpplink/types" ) +// isAlreadyGoneOnDelete: VPP returns NO_SUCH_INNER_FIB (-4) / UNSPECIFIED (-1) +// when the steering/policy is already absent — an idempotent delete. +func isAlreadyGoneOnDelete(err error) bool { + if err == nil { + return false + } + var vppErr govppapi.VPPApiError + if !errors.As(err, &vppErr) { + return false + } + return vppErr == govppapi.NO_SUCH_INNER_FIB || vppErr == govppapi.UNSPECIFIED +} + // NodeToPrefixes is data holder for node and traffic destination prefixes (subnets) that should end in the given node type NodeToPrefixes struct { Node net.IP @@ -29,10 +43,28 @@ type NodeToPolicies struct { SRv6Tunnel []common.SRv6Tunnel } +// srv6VppAPI is the subset of *vpplink.VppLink SRv6Provider uses, so tests can +// substitute a fake. +type srv6VppAPI interface { + ListSRv6Localsid() ([]*types.SrLocalsid, error) + AddSRv6Localsid(*types.SrLocalsid) error + AddModSRv6Policy(*types.SrPolicy) error + AddSRv6Steering(*types.SrSteer) error + DelSRv6Steering(*types.SrSteer) error + DelSRv6Policy(*types.SrPolicy) error + ListSRv6Steering() ([]*types.SrSteer, error) + SetEncapSource(net.IP) error + RouteAdd(*types.Route) error + RouteDel(*types.Route) error +} + // SRv6Provider is node connectivity provider that uses segment routing over IPv6 (SRv6) to connect the nodes // For more info about SRv6, see https://datatracker.ietf.org/doc/html/rfc8986. type SRv6Provider struct { *ConnectivityProviderData + // vpp shadows the embedded ConnectivityProviderData.vpp so tests can inject + // a fake; production wires the real *vpplink.VppLink through here. + vpp srv6VppAPI // nodePrefixes is internal data holder for information from common.NodeConnectivity data // from common.ConnectivityAdded event @@ -44,10 +76,18 @@ type SRv6Provider struct { policyIPPool net.IPNet // localSidIPPool is IP pool for LocalSID's SIDs (SID = IPv6 address in SRv6) localSidIPPool net.IPNet + // pendingBsidCleanup holds prior BSIDs not yet freeable (a steering still + // resolves through them); drained on later SR-policy events to avoid leaks. + pendingBsidCleanup []ip_types.IP6Address } func NewSRv6Provider(d *ConnectivityProviderData) *SRv6Provider { - p := &SRv6Provider{d, make(map[string]*NodeToPrefixes), make(map[string]*NodeToPolicies), net.IPNet{}, net.IPNet{}} + p := &SRv6Provider{ + ConnectivityProviderData: d, + vpp: d.vpp, + nodePrefixes: make(map[string]*NodeToPrefixes), + nodePolices: make(map[string]*NodeToPolicies), + } if *config.GetCalicoVppFeatureGates().SRv6Enabled { p.localSidIPPool = cnet.MustParseNetwork(config.GetCalicoVppSrv6().LocalsidPool).IPNet p.policyIPPool = cnet.MustParseNetwork(config.GetCalicoVppSrv6().PolicyPool).IPNet @@ -131,16 +171,21 @@ func (p *SRv6Provider) CreateSRv6Tunnnel(dst net.IP, prefixDst ip_types.Prefix, // The SRv6 tunnel info is propagated from tunnel-ending node using BGP(see bgp_watcher.go and // srv6_localsid_watcher.go). After these 3 calls (and the RescanState call) // you get fully configured SRv6 tunnel with SR steering, SR policy, SR localsids an SRv6 traffic forwarding. -func (p *SRv6Provider) AddConnectivity(cn *common.NodeConnectivity) (err error) { +func (p *SRv6Provider) AddConnectivity(cn *common.NodeConnectivity) error { p.log.Infof("SRv6Provider AddConnectivity %s", cn.String()) var nodeip string + // Set by the upsert below when it supersedes a prior BSID; freed (or queued) + // by drainPendingBsidCleanup at function end, after the steering re-point. + var orphanedBsid ip_types.IP6Address + var orphanedBsidValid bool + // processing normal NodeConnectivity data only IPv6 destination if vpplink.IsIP6(cn.NextHop) && !p.isSRv6TunnelInfoFromBGP(cn) { // destination IP can't be from policy IPPool, because this IPPool is reserved for policy BSIDs if p.policyIPPool.Contains(cn.Dst.IP) { p.log.Infof("SRv6Provider AddConnectivity no valid prefix %s", cn.Dst.String()) - return err + return nil } // variables processing @@ -176,7 +221,7 @@ func (p *SRv6Provider) AddConnectivity(cn *common.NodeConnectivity) (err error) // from the destination node (BGP transportation) if p.nodePolices[nodeip] == nil { p.log.Infof("SRv6Provider no policies for %s", nodeip) - return err + return nil } } else if p.isSRv6TunnelInfoFromBGP(cn) && cn.Custom != nil { // getting SRv6 tunnel data from BGP @@ -195,12 +240,34 @@ func (p *SRv6Provider) AddConnectivity(cn *common.NodeConnectivity) (err error) } p.log.Debugf("SRv6Provider new policy %s with behavior %d on node %s and priority %d", policyData.Bsid.String(), policyData.Behavior, nodeip, policyData.Priority) - p.nodePolices[policyData.Dst.String()].SRv6Tunnel = append(p.nodePolices[policyData.Dst.String()].SRv6Tunnel, *policyData) + // RFC 9012 NLRI key : same key replaces + // the prior candidate in place (endpoint = map key), never appends. + entry := p.nodePolices[policyData.Dst.String()] + replaced := false + for i := range entry.SRv6Tunnel { + if entry.SRv6Tunnel[i].Color != policyData.Color || entry.SRv6Tunnel[i].Distinguisher != policyData.Distinguisher { + continue + } + // BSID changed: hand the prior one to the deferred cleanup + // (freed after the steering is re-pointed, never while live). + oldBsid, oldOk := tunnelBsid(&entry.SRv6Tunnel[i]) + newBsid, newOk := tunnelBsid(policyData) + if oldOk && newOk && oldBsid != newBsid { + orphanedBsid = oldBsid + orphanedBsidValid = true + } + entry.SRv6Tunnel[i] = *policyData + replaced = true + break + } + if !replaced { + entry.SRv6Tunnel = append(entry.SRv6Tunnel, *policyData) + } - // stopping processing until we have also needed normal common.NodeConnectivity data if p.nodePrefixes[nodeip] == nil { p.log.Debugf("SRv6Provider no prefixes for %s", nodeip) - return err + // Fall through so the function-end drain still runs; the + // CreateSRv6Tunnel block below is gated on nodePrefixes != nil. } } @@ -226,15 +293,255 @@ func (p *SRv6Provider) AddConnectivity(cn *common.NodeConnectivity) (err error) } } - return err + p.drainPendingBsidCleanup(orphanedBsid, orphanedBsidValid) + return nil } -func (p *SRv6Provider) DelConnectivity(cn *common.NodeConnectivity) (err error) { +// drainPendingBsidCleanup deletes queued BSIDs no steering resolves through +// (one ListSRv6Steering classifies all); still-referenced ones stay queued. +func (p *SRv6Provider) drainPendingBsidCleanup(orphanedBsid ip_types.IP6Address, orphanedBsidValid bool) { + if orphanedBsidValid { + p.pendingBsidCleanup = append(p.pendingBsidCleanup, orphanedBsid) + } + if len(p.pendingBsidCleanup) == 0 { + return + } + steering, listErr := p.vpp.ListSRv6Steering() + if listErr != nil { + p.log.Warnf("SRv6Provider drainPendingBsidCleanup: ListSRv6Steering failed: %v; %d BSID cleanups deferred", + listErr, len(p.pendingBsidCleanup)) + return + } + referenced := make(map[ip_types.IP6Address]struct{}, len(steering)) + for _, st := range steering { + referenced[st.Bsid] = struct{}{} + } + queue := p.pendingBsidCleanup + p.pendingBsidCleanup = nil + for _, bsid := range queue { + if _, stillSteered := referenced[bsid]; stillSteered { + p.log.Debugf("SRv6Provider drainPendingBsidCleanup: BSID %s still steered; re-queued", bsid) + p.pendingBsidCleanup = append(p.pendingBsidCleanup, bsid) + continue + } + err := p.vpp.DelSRv6Policy(&types.SrPolicy{Bsid: bsid}) + if err == nil || isAlreadyGoneOnDelete(err) { + p.log.Debugf("SRv6Provider drainPendingBsidCleanup: BSID %s freed: %v", bsid, err) + continue + } + // Hard error: keep the BSID queued to retry on the next event. + p.log.Warnf("SRv6Provider drainPendingBsidCleanup: BSID %s cleanup failed: %v; re-queued", bsid, err) + p.pendingBsidCleanup = append(p.pendingBsidCleanup, bsid) + } +} + +// DelConnectivity tears down state from AddConnectivity. cn.Custom set = +// SRv6PolicyDeleted (NLRI-key teardown); cn.Dst set = ConnectivityDeleted +// (prefix steering). Per-step failures are logged, not fatal. +func (p *SRv6Provider) DelConnectivity(cn *common.NodeConnectivity) error { p.log.Infof("SRv6Provider DelConnectivity %s", cn.String()) - // FIXME SRv6 node connectivity removal not supported + if cn.Custom != nil { + return p.delSRPolicy(cn) + } + if cn.Dst.IP != nil { + return p.delPrefixSteering(cn) + } + return fmt.Errorf("SRv6Provider DelConnectivity: cn has neither Custom nor Dst.IP") +} + +func (p *SRv6Provider) delSRPolicy(cn *common.NodeConnectivity) error { + policyData, ok := cn.Custom.(*common.SRv6Tunnel) + if !ok || policyData == nil { + return fmt.Errorf("SRv6Provider DelConnectivity: cn.Custom is not a *common.SRv6Tunnel: %T", cn.Custom) + } + // A withdraw may free a queued BSID; retry the drain on any return path. + defer p.drainPendingBsidCleanup(ip_types.IP6Address{}, false) + nodeip := policyData.Dst.String() + entry := p.nodePolices[nodeip] + if entry == nil { + p.log.Infof("SRv6Provider DelConnectivity: no cached policies for endpoint %s", nodeip) + return nil + } + + // Match cached tunnels by NLRI key. + // Withdraws carry only the NLRI key (no BSID); the cached tunnel preserves + // the BSID we installed, which is what VPP needs to delete. + var matched []ip_types.IP6Address + remaining := entry.SRv6Tunnel[:0] + for _, tun := range entry.SRv6Tunnel { + if tun.Color == policyData.Color && tun.Distinguisher == policyData.Distinguisher { + if b, ok := tunnelBsid(&tun); ok { + matched = append(matched, b) + } + continue + } + remaining = append(remaining, tun) + } + if len(matched) == 0 { + p.log.Infof("SRv6Provider DelConnectivity: no cached policy matched endpoint=%s color=%d distinguisher=%d", + nodeip, policyData.Color, policyData.Distinguisher) + return nil + } + + steering, listErr := p.vpp.ListSRv6Steering() + if listErr != nil { + p.log.Warnf("SRv6Provider DelConnectivity: failed to list steering: %v", listErr) + } + // logDel: silent on success, debug when VPP says it's already gone, warn otherwise. + logDel := func(what string, err error) { + if err == nil { + return + } + log := p.log.Warnf + if isAlreadyGoneOnDelete(err) { + log = p.log.Debugf + } + log("SRv6Provider DelConnectivity: %s: %v", what, err) + } + + // Track which prefixes lose their steering: after we delete this BSID, + // the RFC 9256 candidate-path failover wants the next-best surviving + // policy of the same behavior to take over. Re-steer happens below, after + // the cache prune, so getPolicyNode sees the post-withdraw state. + var orphaned []ip_types.Prefix + for _, bsid := range matched { + for _, st := range steering { + if st.Bsid != bsid { + continue + } + orphaned = append(orphaned, st.Prefix) + logDel(fmt.Sprintf("DelSRv6Steering bsid=%s prefix=%s", st.Bsid, st.Prefix), p.vpp.DelSRv6Steering(st)) + } + logDel(fmt.Sprintf("DelSRv6Policy bsid=%s", bsid), p.vpp.DelSRv6Policy(&types.SrPolicy{Bsid: bsid})) + } + + if len(remaining) == 0 { + delete(p.nodePolices, nodeip) + } else { + entry.SRv6Tunnel = remaining + } + + // AddConnectivity only installs the highest-priority candidate per behavior; + // lower-priority survivors are cached but absent from VPP. Track which we + // install on demand here so multiple orphaned prefixes targeting the same + // surviving BSID don't churn the install. + installed := make(map[ip_types.IP6Address]struct{}) + for _, prefix := range orphaned { + p.resteerOrphan(nodeip, prefix, installed) + } + return nil +} + +// resteerOrphan re-points a prefix whose steering BSID just got deleted at the +// next-best surviving policy of the matching behavior on the same endpoint. The +// chosen policy may have never been installed in VPP (it was masked by the +// withdrawn higher-priority candidate), so install it on demand — guarded by +// `installed` so we install at most once per delSRPolicy call. If no candidate +// remains the prefix is left unsteered and AddConnectivity picks it up when a +// new candidate is later advertised. +func (p *SRv6Provider) resteerOrphan(nodeip string, prefix ip_types.Prefix, installed map[ip_types.IP6Address]struct{}) { + behavior := types.SrBehaviorDT4 + if vpplink.IsIP6(prefix.Address.ToIP()) { + behavior = types.SrBehaviorDT6 + } + policy, err := p.getPolicyNode(nodeip, behavior) + if err != nil || policy == nil { + p.log.Infof("SRv6Provider DelConnectivity: no surviving policy for endpoint=%s prefix=%s behavior=%d; prefix left unsteered", + nodeip, prefix.String(), behavior) + return + } + if _, ok := installed[policy.Bsid]; !ok { + if err := p.vpp.AddModSRv6Policy(policy); err != nil { + p.log.Warnf("SRv6Provider DelConnectivity: AddModSRv6Policy bsid=%s for failover: %v", + policy.Bsid.String(), err) + return + } + installed[policy.Bsid] = struct{}{} + } + srSteer := &types.SrSteer{ + TrafficType: types.SrSteerIPv4, + Prefix: prefix, + Bsid: policy.Bsid, + } + if vpplink.IsIP6(prefix.Address.ToIP()) { + srSteer.TrafficType = types.SrSteerIPv6 + } + if err := p.vpp.AddSRv6Steering(srSteer); err != nil { + p.log.Warnf("SRv6Provider DelConnectivity: AddSRv6Steering prefix=%s bsid=%s: %v", + prefix.String(), policy.Bsid.String(), err) + return + } + p.log.Infof("SRv6Provider DelConnectivity: re-steered prefix=%s onto surviving bsid=%s behavior=%d", + prefix.String(), policy.Bsid.String(), behavior) +} + +func (p *SRv6Provider) delPrefixSteering(cn *common.NodeConnectivity) error { + if p.policyIPPool.Contains(cn.Dst.IP) { + p.log.Debugf("SRv6Provider DelConnectivity skip policyIPPool prefix %s", cn.Dst.String()) + return nil + } + prefix, err := ip_types.ParsePrefix(cn.Dst.String()) + if err != nil { + return errors.Wrapf(err, "SRv6Provider DelConnectivity unable to parse prefix %s", cn.Dst.String()) + } + if p.localSidIPPool.Contains(cn.Dst.IP) { + if delErr := p.vpp.RouteDel(&types.Route{ + Dst: prefix.ToIPNet(), + Paths: []types.RoutePath{{Gw: cn.NextHop.To16(), SwIfIndex: common.VppManagerInfo.GetMainSwIfIndex()}}, + }); delErr != nil { + p.log.Warnf("SRv6Provider DelConnectivity: RouteDel localSidIPPool %s: %v", cn.Dst.String(), delErr) + } + return nil + } + + nodeip := cn.NextHop.String() + prefixKey := prefix.String() + steering, listErr := p.vpp.ListSRv6Steering() + if listErr != nil { + p.log.Warnf("SRv6Provider DelConnectivity: failed to list steering: %v", listErr) + } + for _, st := range steering { + if st.Prefix.String() != prefixKey { + continue + } + if err := p.vpp.DelSRv6Steering(st); err != nil { + log := p.log.Warnf + if isAlreadyGoneOnDelete(err) { + log = p.log.Debugf + } + log("SRv6Provider DelConnectivity: DelSRv6Steering prefix=%s bsid=%s: %v", st.Prefix, st.Bsid, err) + } + } + + if entry := p.nodePrefixes[nodeip]; entry != nil { + remaining := entry.Prefixes[:0] + for _, px := range entry.Prefixes { + if px.String() != prefixKey { + remaining = append(remaining, px) + } + } + if len(remaining) == 0 { + delete(p.nodePrefixes, nodeip) + } else { + entry.Prefixes = remaining + } + } return nil } +// tunnelBsid prefers Policy.Bsid (already ip_types.IP6Address) over the net.IP +// form. Returns ok=false only for a malformed cached tunnel where neither field +// is set — caller should skip it. +func tunnelBsid(t *common.SRv6Tunnel) (ip_types.IP6Address, bool) { + if t.Policy != nil && (t.Policy.Bsid != ip_types.IP6Address{}) { + return t.Policy.Bsid, true + } + if len(t.Bsid) != 0 { + return types.ToVppIP6Address(t.Bsid), true + } + return ip_types.IP6Address{}, false +} + // isSRv6TunnelInfoFromBGP checks whether given NodeConnectivity data is from BGP watcher that should pass // SRv6 tunnel information from node where the tunnel should end func (p *SRv6Provider) isSRv6TunnelInfoFromBGP(cn *common.NodeConnectivity) bool { @@ -246,14 +553,22 @@ func (p *SRv6Provider) getPolicyNode(nodeip string, behavior types.SrBehavior) ( p.log.Infof("SRv6Provider getPolicyNode node: %s, with beahvior: %d", nodeip, behavior) if p.nodePolices[nodeip] != nil { var priority uint32 + found := false p.log.Infof("SRv6Provider getPolicyNode: found %d tunnels for node %s", len(p.nodePolices[nodeip].SRv6Tunnel), nodeip) for i, tunnel := range p.nodePolices[nodeip].SRv6Tunnel { converted := types.FromGoBGPSrBehavior(tunnel.Behavior) p.log.Infof("SRv6Provider getPolicyNode: tunnel[%d] behavior=%d converted=%d want=%d match=%v policy=%v", i, tunnel.Behavior, converted, behavior, converted == behavior, tunnel.Policy != nil) - if converted == behavior && tunnel.Priority >= priority { + // Skip a candidate with no SrPolicy object (nil-deref guard; a nil + // Policy here does not imply not-installed-in-VPP). Strict > keeps + // the first candidate on a priority tie. + if tunnel.Policy == nil || converted != behavior { + continue + } + if !found || tunnel.Priority > priority { priority = tunnel.Priority policy = tunnel.Policy + found = true } } } else { diff --git a/calico-vpp-agent/connectivity/srv6_test.go b/calico-vpp-agent/connectivity/srv6_test.go new file mode 100644 index 000000000..f144714e3 --- /dev/null +++ b/calico-vpp-agent/connectivity/srv6_test.go @@ -0,0 +1,784 @@ +package connectivity + +import ( + "io" + "net" + "testing" + + "github.com/sirupsen/logrus" + govppapi "go.fd.io/govpp/api" + + "github.com/projectcalico/vpp-dataplane/v3/calico-vpp-agent/common" + "github.com/projectcalico/vpp-dataplane/v3/vpplink/generated/bindings/ip_types" + "github.com/projectcalico/vpp-dataplane/v3/vpplink/types" +) + +// fakeSRv6VPP records every srv6VppAPI call so tests can assert what reached +// the dataplane and seed ListSRv6Steering output. callLog records the +// interleaving across methods so ordering-sensitive tests can verify e.g. +// "AddSRv6Steering happened before DelSRv6Policy". +type fakeSRv6VPP struct { + steering []*types.SrSteer + + addModPolicy []*types.SrPolicy + delPolicy []*types.SrPolicy + addSteering []*types.SrSteer + delSteering []*types.SrSteer + routeAdd []*types.Route + routeDel []*types.Route + callLog []string + + listSteeringErr error + addModPolicyErr error + delSteeringErr error + delPolicyErr error +} + +func (f *fakeSRv6VPP) ListSRv6Localsid() ([]*types.SrLocalsid, error) { return nil, nil } +func (f *fakeSRv6VPP) AddSRv6Localsid(*types.SrLocalsid) error { return nil } +func (f *fakeSRv6VPP) SetEncapSource(net.IP) error { return nil } +func (f *fakeSRv6VPP) RouteAdd(r *types.Route) error { f.routeAdd = append(f.routeAdd, r); return nil } +func (f *fakeSRv6VPP) RouteDel(r *types.Route) error { f.routeDel = append(f.routeDel, r); return nil } + +func (f *fakeSRv6VPP) AddModSRv6Policy(p *types.SrPolicy) error { + f.addModPolicy = append(f.addModPolicy, p) + f.callLog = append(f.callLog, "AddModSRv6Policy:"+p.Bsid.String()) + return f.addModPolicyErr +} +func (f *fakeSRv6VPP) DelSRv6Policy(p *types.SrPolicy) error { + f.delPolicy = append(f.delPolicy, p) + f.callLog = append(f.callLog, "DelSRv6Policy:"+p.Bsid.String()) + return f.delPolicyErr +} +func (f *fakeSRv6VPP) AddSRv6Steering(s *types.SrSteer) error { + f.addSteering = append(f.addSteering, s) + f.callLog = append(f.callLog, "AddSRv6Steering:"+s.Bsid.String()) + return nil +} +func (f *fakeSRv6VPP) DelSRv6Steering(s *types.SrSteer) error { + f.delSteering = append(f.delSteering, s) + f.callLog = append(f.callLog, "DelSRv6Steering:"+s.Bsid.String()) + return f.delSteeringErr +} +func (f *fakeSRv6VPP) ListSRv6Steering() ([]*types.SrSteer, error) { + return f.steering, f.listSteeringErr +} + +func newTestProvider(fake *fakeSRv6VPP) *SRv6Provider { + logger := logrus.New() + logger.SetOutput(io.Discard) + return &SRv6Provider{ + ConnectivityProviderData: &ConnectivityProviderData{log: logrus.NewEntry(logger)}, + vpp: fake, + nodePrefixes: make(map[string]*NodeToPrefixes), + nodePolices: make(map[string]*NodeToPolicies), + } +} + +func mustBsid(t *testing.T, s string) ip_types.IP6Address { + t.Helper() + ip := net.ParseIP(s) + if ip == nil || ip.To16() == nil { + t.Fatalf("invalid ipv6 %q", s) + } + return types.ToVppIP6Address(ip) +} + +func mustPrefix(t *testing.T, s string) ip_types.Prefix { + t.Helper() + pr, err := ip_types.ParsePrefix(s) + if err != nil { + t.Fatalf("ParsePrefix(%q): %v", s, err) + } + return pr +} + +// ---------- tunnelBsid ---------- + +func TestTunnelBsid(t *testing.T) { + policyBsid := mustBsid(t, "cafe::1") + netBsid := net.ParseIP("cafe::2") + + cases := []struct { + name string + tun common.SRv6Tunnel + wantOK bool + wantStr string + }{ + {"policy wins", common.SRv6Tunnel{Policy: &types.SrPolicy{Bsid: policyBsid}, Bsid: netBsid}, true, policyBsid.String()}, + {"net.IP fallback", common.SRv6Tunnel{Bsid: netBsid}, true, types.ToVppIP6Address(netBsid).String()}, + {"policy with zero bsid falls back to net.IP", common.SRv6Tunnel{Policy: &types.SrPolicy{}, Bsid: netBsid}, true, types.ToVppIP6Address(netBsid).String()}, + {"neither set", common.SRv6Tunnel{}, false, ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, ok := tunnelBsid(&tc.tun) + if ok != tc.wantOK { + t.Fatalf("ok = %v, want %v", ok, tc.wantOK) + } + if ok && got.String() != tc.wantStr { + t.Fatalf("bsid = %s, want %s", got.String(), tc.wantStr) + } + }) + } +} + +// ---------- delSRPolicy ---------- + +func TestDelSRPolicy_TypeAssertError(t *testing.T) { + p := newTestProvider(&fakeSRv6VPP{}) + cn := &common.NodeConnectivity{Custom: "not a tunnel"} + if err := p.delSRPolicy(cn); err == nil { + t.Fatal("expected error for non-SRv6Tunnel Custom") + } +} + +func TestDelSRPolicy_NoCache(t *testing.T) { + fake := &fakeSRv6VPP{} + p := newTestProvider(fake) + dst := net.ParseIP("fd00:1::11") + cn := &common.NodeConnectivity{Custom: &common.SRv6Tunnel{Dst: dst, Color: 4}} + if err := p.delSRPolicy(cn); err != nil { + t.Fatalf("delSRPolicy: %v", err) + } + if len(fake.delPolicy)+len(fake.delSteering) > 0 { + t.Fatalf("expected no VPP calls; got delPolicy=%d delSteering=%d", len(fake.delPolicy), len(fake.delSteering)) + } +} + +func TestDelSRPolicy_NLRIKeyMismatchLeavesSiblingsAlone(t *testing.T) { + // Two cached tunnels for the same endpoint, different NLRI keys. Withdrawing + // one (color=4) must not touch the other (color=6). + fake := &fakeSRv6VPP{} + p := newTestProvider(fake) + dst := net.ParseIP("fd00:1::11") + dt4Bsid := mustBsid(t, "cafe::4") + dt6Bsid := mustBsid(t, "cafe::6") + p.nodePolices[dst.String()] = &NodeToPolicies{ + Node: dst, + SRv6Tunnel: []common.SRv6Tunnel{ + {Dst: dst, Color: 4, Policy: &types.SrPolicy{Bsid: dt4Bsid}, Priority: 100}, + {Dst: dst, Color: 6, Policy: &types.SrPolicy{Bsid: dt6Bsid}, Priority: 100}, + }, + } + cn := &common.NodeConnectivity{Custom: &common.SRv6Tunnel{Dst: dst, Color: 4}} + if err := p.delSRPolicy(cn); err != nil { + t.Fatalf("delSRPolicy: %v", err) + } + if len(fake.delPolicy) != 1 || fake.delPolicy[0].Bsid != dt4Bsid { + t.Fatalf("expected exactly DelSRv6Policy(dt4); got %+v", fake.delPolicy) + } + remaining := p.nodePolices[dst.String()].SRv6Tunnel + if len(remaining) != 1 || remaining[0].Color != 6 { + t.Fatalf("expected dt6 sibling to survive; got %+v", remaining) + } +} + +func TestDelSRPolicy_NoMatch(t *testing.T) { + fake := &fakeSRv6VPP{} + p := newTestProvider(fake) + dst := net.ParseIP("fd00:1::11") + p.nodePolices[dst.String()] = &NodeToPolicies{ + Node: dst, + SRv6Tunnel: []common.SRv6Tunnel{ + {Dst: dst, Color: 4, Policy: &types.SrPolicy{Bsid: mustBsid(t, "cafe::4")}}, + }, + } + cn := &common.NodeConnectivity{Custom: &common.SRv6Tunnel{Dst: dst, Color: 99}} // unknown color + if err := p.delSRPolicy(cn); err != nil { + t.Fatalf("delSRPolicy: %v", err) + } + if len(fake.delPolicy)+len(fake.delSteering) > 0 { + t.Fatalf("expected no VPP calls; got delPolicy=%d delSteering=%d", len(fake.delPolicy), len(fake.delSteering)) + } +} + +func TestDelSRPolicy_DeletesPolicyAndAssociatedSteering(t *testing.T) { + dst := net.ParseIP("fd00:1::11") + bsid := mustBsid(t, "cafe::4") + otherBsid := mustBsid(t, "cafe::dead") // unrelated steering, must survive + prefixA := mustPrefix(t, "fd20::aaaa/128") + prefixB := mustPrefix(t, "fd20::bbbb/128") + prefixC := mustPrefix(t, "fd20::cccc/128") + + fake := &fakeSRv6VPP{ + steering: []*types.SrSteer{ + {Bsid: bsid, Prefix: prefixA, TrafficType: types.SrSteerIPv6}, + {Bsid: bsid, Prefix: prefixB, TrafficType: types.SrSteerIPv6}, + {Bsid: otherBsid, Prefix: prefixC, TrafficType: types.SrSteerIPv6}, + }, + } + p := newTestProvider(fake) + p.nodePolices[dst.String()] = &NodeToPolicies{ + Node: dst, + SRv6Tunnel: []common.SRv6Tunnel{{Dst: dst, Color: 6, Policy: &types.SrPolicy{Bsid: bsid}}}, + } + + cn := &common.NodeConnectivity{Custom: &common.SRv6Tunnel{Dst: dst, Color: 6}} + if err := p.delSRPolicy(cn); err != nil { + t.Fatalf("delSRPolicy: %v", err) + } + + if len(fake.delSteering) != 2 { + t.Fatalf("expected 2 steering deletions; got %d", len(fake.delSteering)) + } + for _, st := range fake.delSteering { + if st.Bsid != bsid { + t.Fatalf("DelSRv6Steering targeted wrong BSID %s, want %s", st.Bsid.String(), bsid.String()) + } + } + if len(fake.delPolicy) != 1 || fake.delPolicy[0].Bsid != bsid { + t.Fatalf("DelSRv6Policy wrong: %+v", fake.delPolicy) + } + if _, ok := p.nodePolices[dst.String()]; ok { + t.Fatalf("expected nodePolices entry to be removed") + } +} + +// Codex round #2 + #3 regression: withdrawing the top-priority candidate must +// (a) re-steer orphaned prefixes onto the surviving lower-priority candidate +// and (b) install that candidate in VPP on demand, since AddConnectivity never +// pushed it during normal operation. +func TestDelSRPolicy_FailoverOntoSurvivingCandidate(t *testing.T) { + dst := net.ParseIP("fd00:1::11") + winnerBsid := mustBsid(t, "cafe::aa") + loserBsid := mustBsid(t, "cafe::bb") + prefixA := mustPrefix(t, "fd20::aaaa/128") + prefixB := mustPrefix(t, "fd20::bbbb/128") + + fake := &fakeSRv6VPP{ + steering: []*types.SrSteer{ + {Bsid: winnerBsid, Prefix: prefixA, TrafficType: types.SrSteerIPv6}, + {Bsid: winnerBsid, Prefix: prefixB, TrafficType: types.SrSteerIPv6}, + }, + } + p := newTestProvider(fake) + // Both candidates DT6 (uint8(SRv6Behavior_END_DT6) == 18 in gobgp). Use the + // raw uint that types.FromGoBGPSrBehavior maps to types.SrBehaviorDT6. + dt6Behavior := uint8(18) // bgpapi.SRv6Behavior_END_DT6 + p.nodePolices[dst.String()] = &NodeToPolicies{ + Node: dst, + SRv6Tunnel: []common.SRv6Tunnel{ + {Dst: dst, Color: 6, Distinguisher: 0, Behavior: dt6Behavior, Priority: 100, Policy: &types.SrPolicy{Bsid: winnerBsid}}, + {Dst: dst, Color: 6, Distinguisher: 1, Behavior: dt6Behavior, Priority: 50, Policy: &types.SrPolicy{Bsid: loserBsid}}, + }, + } + + cn := &common.NodeConnectivity{Custom: &common.SRv6Tunnel{Dst: dst, Color: 6, Distinguisher: 0}} + if err := p.delSRPolicy(cn); err != nil { + t.Fatalf("delSRPolicy: %v", err) + } + + // Top candidate teardown + if len(fake.delPolicy) != 1 || fake.delPolicy[0].Bsid != winnerBsid { + t.Fatalf("DelSRv6Policy: got %+v", fake.delPolicy) + } + if len(fake.delSteering) != 2 { + t.Fatalf("expected 2 steering deletes; got %d", len(fake.delSteering)) + } + // On-demand install of the surviver, once even for multiple orphaned prefixes + if len(fake.addModPolicy) != 1 || fake.addModPolicy[0].Bsid != loserBsid { + t.Fatalf("expected one AddModSRv6Policy(loser); got %+v", fake.addModPolicy) + } + // Re-steering for each orphaned prefix + if len(fake.addSteering) != 2 { + t.Fatalf("expected 2 AddSRv6Steering calls; got %d", len(fake.addSteering)) + } + for _, st := range fake.addSteering { + if st.Bsid != loserBsid { + t.Fatalf("AddSRv6Steering retargeted wrong BSID %s, want %s", st.Bsid.String(), loserBsid.String()) + } + } + // Surviver remains in cache + rem := p.nodePolices[dst.String()].SRv6Tunnel + if len(rem) != 1 || rem[0].Distinguisher != 1 { + t.Fatalf("expected loser to survive in cache; got %+v", rem) + } +} + +func TestDelSRPolicy_NoSurvivingCandidateLeavesPrefixUnsteered(t *testing.T) { + dst := net.ParseIP("fd00:1::11") + bsid := mustBsid(t, "cafe::4") + prefix := mustPrefix(t, "fd20::1/128") + fake := &fakeSRv6VPP{steering: []*types.SrSteer{{Bsid: bsid, Prefix: prefix, TrafficType: types.SrSteerIPv6}}} + p := newTestProvider(fake) + p.nodePolices[dst.String()] = &NodeToPolicies{ + Node: dst, + SRv6Tunnel: []common.SRv6Tunnel{{Dst: dst, Color: 6, Policy: &types.SrPolicy{Bsid: bsid}}}, + } + + cn := &common.NodeConnectivity{Custom: &common.SRv6Tunnel{Dst: dst, Color: 6}} + if err := p.delSRPolicy(cn); err != nil { + t.Fatalf("delSRPolicy: %v", err) + } + if len(fake.addSteering) != 0 || len(fake.addModPolicy) != 0 { + t.Fatalf("expected no re-steer / install when no surviving candidate; got addSteering=%d addModPolicy=%d", len(fake.addSteering), len(fake.addModPolicy)) + } +} + +// ---------- delPrefixSteering ---------- + +func TestDelPrefixSteering_SkipsPolicyIPPool(t *testing.T) { + fake := &fakeSRv6VPP{} + p := newTestProvider(fake) + _, ipNet, _ := net.ParseCIDR("cafe::/64") + p.policyIPPool = *ipNet + cn := &common.NodeConnectivity{ + Dst: net.IPNet{IP: net.ParseIP("cafe::1"), Mask: net.CIDRMask(128, 128)}, + NextHop: net.ParseIP("fd00:1::11"), + } + if err := p.delPrefixSteering(cn); err != nil { + t.Fatalf("delPrefixSteering: %v", err) + } + if len(fake.delSteering)+len(fake.routeDel) > 0 { + t.Fatal("expected no VPP calls for policy-pool address") + } +} + +func TestDelPrefixSteering_NormalPrefixDeletesSteeringAndPrunesCache(t *testing.T) { + prefix := mustPrefix(t, "fd20::aaaa/128") + other := mustPrefix(t, "fd20::bbbb/128") + fake := &fakeSRv6VPP{ + steering: []*types.SrSteer{ + {Bsid: mustBsid(t, "cafe::4"), Prefix: prefix, TrafficType: types.SrSteerIPv6}, + {Bsid: mustBsid(t, "cafe::4"), Prefix: other, TrafficType: types.SrSteerIPv6}, + }, + } + p := newTestProvider(fake) + node := net.ParseIP("fd00:1::11") + p.nodePrefixes[node.String()] = &NodeToPrefixes{Node: node, Prefixes: []ip_types.Prefix{prefix, other}} + cn := &common.NodeConnectivity{ + Dst: net.IPNet{IP: net.ParseIP("fd20::aaaa"), Mask: net.CIDRMask(128, 128)}, + NextHop: node, + } + if err := p.delPrefixSteering(cn); err != nil { + t.Fatalf("delPrefixSteering: %v", err) + } + if len(fake.delSteering) != 1 { + t.Fatalf("expected exactly one DelSRv6Steering; got %d", len(fake.delSteering)) + } + if fake.delSteering[0].Prefix.String() != prefix.String() { + t.Fatalf("DelSRv6Steering targeted wrong prefix %s, want %s", fake.delSteering[0].Prefix.String(), prefix.String()) + } + remaining := p.nodePrefixes[node.String()].Prefixes + if len(remaining) != 1 || remaining[0].String() != other.String() { + t.Fatalf("expected sibling prefix to survive cache prune; got %+v", remaining) + } +} + +// ---------- DelConnectivity dispatcher ---------- + +// Re-advertising an SR Policy with the SAME NLRI key (Color, Distinguisher, +// Endpoint) must REPLACE the cached candidate in-place, not append a duplicate. +// Without this, BGP path refresh would silently grow the cache and delSRPolicy +// would iterate the same BSID multiple times — the second pass hits VPP with +// an already-gone steering / policy and used to warn with NO_SUCH_INNER_FIB +// and UNSPECIFIED. +func TestAddConnectivity_ReAdvertiseSameNLRIKeyReplacesInPlace(t *testing.T) { + dst := net.ParseIP("fd00:1::12") + bsidA := mustBsid(t, "cafe::aaa") + bsidB := mustBsid(t, "cafe::bbb") + + fake := &fakeSRv6VPP{} + p := newTestProvider(fake) + + // First advertisement. + if err := p.AddConnectivity(&common.NodeConnectivity{ + NextHop: dst, + Custom: &common.SRv6Tunnel{ + Dst: dst, + Color: 6, + Distinguisher: 1, + Priority: 100, + Policy: &types.SrPolicy{Bsid: bsidA}, + }, + }); err != nil { + t.Fatalf("AddConnectivity (first): %v", err) + } + + // Re-advertisement with same NLRI key but different attrs (e.g. new BSID + // after a path refresh). RFC 9252 says this must REPLACE the prior entry. + if err := p.AddConnectivity(&common.NodeConnectivity{ + NextHop: dst, + Custom: &common.SRv6Tunnel{ + Dst: dst, + Color: 6, + Distinguisher: 1, + Priority: 150, + Policy: &types.SrPolicy{Bsid: bsidB}, + }, + }); err != nil { + t.Fatalf("AddConnectivity (second): %v", err) + } + + cache := p.nodePolices[dst.String()].SRv6Tunnel + if len(cache) != 1 { + t.Fatalf("expected cache to dedup to 1 entry; got %d (%+v)", len(cache), cache) + } + if cache[0].Priority != 150 || cache[0].Policy.Bsid != bsidB { + t.Fatalf("expected re-advertisement to replace in place (prio=150, bsid=%s); got prio=%d bsid=%s", + bsidB.String(), cache[0].Priority, cache[0].Policy.Bsid.String()) + } +} + +// Re-advertising with the SAME NLRI key but a DIFFERENT BSID (BGP path +// refresh updates path attributes including the BSID TLV) must tear down the +// prior BSID in VPP. Otherwise the old SR Policy stays installed with no cache +// reference, and a later withdraw — matched against only the new BSID — leaks +// it permanently. +func TestAddConnectivity_BsidChangeOnUpsertCleansUpOldBsid(t *testing.T) { + dst := net.ParseIP("fd00:1::12") + oldBsid := mustBsid(t, "cafe::aaa1") + newBsid := mustBsid(t, "cafe::aaa2") + + fake := &fakeSRv6VPP{} + p := newTestProvider(fake) + + // First advertisement: cached, no VPP install (no nodePrefixes wired up + // for the test — we just exercise the cache + cleanup path). + if err := p.AddConnectivity(&common.NodeConnectivity{ + NextHop: dst, + Custom: &common.SRv6Tunnel{ + Dst: dst, + Color: 6, + Distinguisher: 1, + Priority: 100, + Policy: &types.SrPolicy{Bsid: oldBsid}, + }, + }); err != nil { + t.Fatalf("AddConnectivity (first): %v", err) + } + if len(fake.delPolicy) != 0 { + t.Fatalf("expected no DelSRv6Policy on first advertisement; got %+v", fake.delPolicy) + } + + // Re-advertisement with same NLRI key but new BSID. + if err := p.AddConnectivity(&common.NodeConnectivity{ + NextHop: dst, + Custom: &common.SRv6Tunnel{ + Dst: dst, + Color: 6, + Distinguisher: 1, + Priority: 100, + Policy: &types.SrPolicy{Bsid: newBsid}, + }, + }); err != nil { + t.Fatalf("AddConnectivity (second): %v", err) + } + + // Old BSID must be torn down so it doesn't leak. + if len(fake.delPolicy) != 1 || fake.delPolicy[0].Bsid != oldBsid { + t.Fatalf("expected exactly one DelSRv6Policy(oldBsid=%s) on BSID change; got %+v", + oldBsid.String(), fake.delPolicy) + } + + // Cache holds only the new candidate. + cache := p.nodePolices[dst.String()].SRv6Tunnel + if len(cache) != 1 || cache[0].Policy.Bsid != newBsid { + t.Fatalf("expected cache to hold only new BSID %s; got %+v", newBsid.String(), cache) + } +} + +// When the upsert changes the BSID AND prefixes are wired up for the endpoint, +// the steering MUST be re-pointed at the new BSID BEFORE the old SR Policy is +// deleted. Otherwise VPP's steering hash holds steer_pl->sr_policy = freed +// pool index for the duration of the gap and packets transiting the steering +// land on undefined state. Verified by asserting the call sequence. +func TestAddConnectivity_BsidChangeCleansUpAfterRePoint(t *testing.T) { + dst := net.ParseIP("fd00:1::12") + oldBsid := mustBsid(t, "cafe::aaa1") + newBsid := mustBsid(t, "cafe::aaa2") + prefix := mustPrefix(t, "fd20::5506:688f:1e5:6f80/122") + + fake := &fakeSRv6VPP{} + p := newTestProvider(fake) + // Pre-populate nodePrefixes so CreateSRv6Tunnel runs on each AddConnectivity. + p.nodePrefixes[dst.String()] = &NodeToPrefixes{Node: dst, Prefixes: []ip_types.Prefix{prefix}} + + dt6Behavior := uint8(18) // bgpapi.SRv6Behavior_END_DT6 + if err := p.AddConnectivity(&common.NodeConnectivity{ + NextHop: dst, + Custom: &common.SRv6Tunnel{ + Dst: dst, Color: 6, Distinguisher: 1, Behavior: dt6Behavior, Priority: 100, + Policy: &types.SrPolicy{Bsid: oldBsid}, + }, + }); err != nil { + t.Fatalf("AddConnectivity (old): %v", err) + } + + // Reset call log so we observe only the upsert's calls. + fake.callLog = nil + + if err := p.AddConnectivity(&common.NodeConnectivity{ + NextHop: dst, + Custom: &common.SRv6Tunnel{ + Dst: dst, Color: 6, Distinguisher: 1, Behavior: dt6Behavior, Priority: 100, + Policy: &types.SrPolicy{Bsid: newBsid}, + }, + }); err != nil { + t.Fatalf("AddConnectivity (new): %v", err) + } + + addNewSteer := -1 + delOld := -1 + for i, op := range fake.callLog { + if op == "AddSRv6Steering:"+newBsid.String() && addNewSteer == -1 { + addNewSteer = i + } + if op == "DelSRv6Policy:"+oldBsid.String() && delOld == -1 { + delOld = i + } + } + if addNewSteer == -1 { + t.Fatalf("expected AddSRv6Steering(newBsid=%s) in call log; got %v", newBsid.String(), fake.callLog) + } + if delOld == -1 { + t.Fatalf("expected DelSRv6Policy(oldBsid=%s) in call log; got %v", oldBsid.String(), fake.callLog) + } + if !(addNewSteer < delOld) { + t.Fatalf("AddSRv6Steering(new) must precede DelSRv6Policy(old); got log %v (newSteer@%d, delOld@%d)", + fake.callLog, addNewSteer, delOld) + } +} + +// Failure mode: CreateSRv6Tunnel below the upsert can fail (getPolicyNode +// returned nil, AddModSRv6Policy errored, AddSRv6Steering errored), leaving +// the steering still resolving through the prior BSID. The deferred cleanup +// MUST NOT delete that BSID — VPP's sr_policy entry is still in use by a live +// steering. Simulated here by seeding ListSRv6Steering with an entry that +// continues to point at the old BSID after the upsert. +func TestAddConnectivity_BsidChangeSkipsCleanupWhenStillReferenced(t *testing.T) { + dst := net.ParseIP("fd00:1::12") + oldBsid := mustBsid(t, "cafe::aaa1") + newBsid := mustBsid(t, "cafe::aaa2") + prefix := mustPrefix(t, "fd20::5506:688f:1e5:6f80/122") + + // fake.steering reports what ListSRv6Steering returns. By leaving the + // pre-existing entry pointing at oldBsid we simulate VPP's view after a + // failed AddSRv6Steering — steer_pl->sr_policy never got re-pointed. + fake := &fakeSRv6VPP{ + steering: []*types.SrSteer{{Bsid: oldBsid, Prefix: prefix, TrafficType: types.SrSteerIPv6}}, + } + p := newTestProvider(fake) + + // Make CreateSRv6Tunnel a no-op for this test by not wiring nodePrefixes; + // the defer should still consult ListSRv6Steering before deleting. + if err := p.AddConnectivity(&common.NodeConnectivity{ + NextHop: dst, + Custom: &common.SRv6Tunnel{ + Dst: dst, Color: 6, Distinguisher: 1, Priority: 100, + Policy: &types.SrPolicy{Bsid: oldBsid}, + }, + }); err != nil { + t.Fatalf("AddConnectivity (old): %v", err) + } + if err := p.AddConnectivity(&common.NodeConnectivity{ + NextHop: dst, + Custom: &common.SRv6Tunnel{ + Dst: dst, Color: 6, Distinguisher: 1, Priority: 100, + Policy: &types.SrPolicy{Bsid: newBsid}, + }, + }); err != nil { + t.Fatalf("AddConnectivity (new): %v", err) + } + + for _, p := range fake.delPolicy { + if p.Bsid == oldBsid { + t.Fatalf("DelSRv6Policy(oldBsid=%s) must NOT fire while steering still resolves through it; got call log %v", + oldBsid.String(), fake.callLog) + } + } +} + +// Eventual consistency: when the prior BSID couldn't be released on the +// first upsert (steering still referenced it), it must NOT be silently +// dropped from the cache. The next SR-policy event must re-attempt the +// cleanup; once VPP reports the BSID is no longer steered, the deferred +// DelSRv6Policy fires. Without the pendingBsidCleanup queue, the upsert +// would replace the cache entry, lose the prior BSID reference, and leak +// the SR Policy in VPP forever. +func TestAddConnectivity_BsidChangePendingRetryOnNextEvent(t *testing.T) { + dst := net.ParseIP("fd00:1::12") + oldBsid := mustBsid(t, "cafe::aaa1") + newBsid := mustBsid(t, "cafe::aaa2") + otherBsid := mustBsid(t, "cafe::bbb") + prefix := mustPrefix(t, "fd20::5506:688f:1e5:6f80/122") + + // First upsert leaves OLD still referenced (re-point "failed"). + fake := &fakeSRv6VPP{ + steering: []*types.SrSteer{{Bsid: oldBsid, Prefix: prefix, TrafficType: types.SrSteerIPv6}}, + } + p := newTestProvider(fake) + + if err := p.AddConnectivity(&common.NodeConnectivity{ + NextHop: dst, + Custom: &common.SRv6Tunnel{ + Dst: dst, Color: 6, Distinguisher: 1, Priority: 100, + Policy: &types.SrPolicy{Bsid: oldBsid}, + }, + }); err != nil { + t.Fatalf("AddConnectivity (old): %v", err) + } + if err := p.AddConnectivity(&common.NodeConnectivity{ + NextHop: dst, + Custom: &common.SRv6Tunnel{ + Dst: dst, Color: 6, Distinguisher: 1, Priority: 100, + Policy: &types.SrPolicy{Bsid: newBsid}, + }, + }); err != nil { + t.Fatalf("AddConnectivity (new): %v", err) + } + + // After the first upsert, OLD should be queued — VPP still steers it. + if len(p.pendingBsidCleanup) != 1 || p.pendingBsidCleanup[0] != oldBsid { + t.Fatalf("expected pendingBsidCleanup=[%s] after first upsert; got %v", + oldBsid.String(), p.pendingBsidCleanup) + } + for _, pol := range fake.delPolicy { + if pol.Bsid == oldBsid { + t.Fatalf("DelSRv6Policy(oldBsid) must not fire while steering still resolves through it; call log %v", fake.callLog) + } + } + + // Simulate VPP catching up: the steering is now repointed at otherBsid, + // freeing OLD for cleanup. + fake.steering = []*types.SrSteer{{Bsid: otherBsid, Prefix: prefix, TrafficType: types.SrSteerIPv6}} + + // A subsequent SR-policy event (different NLRI key, not related to OLD) + // must drain the pending queue and finally release OLD in VPP. + if err := p.AddConnectivity(&common.NodeConnectivity{ + NextHop: dst, + Custom: &common.SRv6Tunnel{ + Dst: dst, Color: 6, Distinguisher: 99, Priority: 50, + Policy: &types.SrPolicy{Bsid: otherBsid}, + }, + }); err != nil { + t.Fatalf("AddConnectivity (third event): %v", err) + } + + if len(p.pendingBsidCleanup) != 0 { + t.Fatalf("expected pendingBsidCleanup to drain to empty; got %v", p.pendingBsidCleanup) + } + sawOldCleanup := false + for _, pol := range fake.delPolicy { + if pol.Bsid == oldBsid { + sawOldCleanup = true + break + } + } + if !sawOldCleanup { + t.Fatalf("expected DelSRv6Policy(oldBsid=%s) on retry; got call log %v", oldBsid.String(), fake.callLog) + } +} + +// Re-advertising the SAME NLRI key with the SAME BSID (only priority or SID +// list changed) must NOT trigger cleanup — there is nothing to clean up. +// Guards against the BSID-change cleanup over-firing. +func TestAddConnectivity_SameBsidOnUpsertSkipsCleanup(t *testing.T) { + dst := net.ParseIP("fd00:1::12") + bsid := mustBsid(t, "cafe::aaa") + + fake := &fakeSRv6VPP{} + p := newTestProvider(fake) + + for _, prio := range []uint32{100, 150} { + if err := p.AddConnectivity(&common.NodeConnectivity{ + NextHop: dst, + Custom: &common.SRv6Tunnel{ + Dst: dst, + Color: 6, + Distinguisher: 1, + Priority: prio, + Policy: &types.SrPolicy{Bsid: bsid}, + }, + }); err != nil { + t.Fatalf("AddConnectivity prio=%d: %v", prio, err) + } + } + + if len(fake.delPolicy) != 0 { + t.Fatalf("expected no DelSRv6Policy when BSID unchanged; got %+v", fake.delPolicy) + } +} + +// A second advertisement with a DIFFERENT NLRI key on the same endpoint must +// coexist as a candidate path — RFC 9256 candidate-path failover relies on +// this. This guards against the dedup logic over-applying. +func TestAddConnectivity_DifferentNLRIKeysCoexist(t *testing.T) { + dst := net.ParseIP("fd00:1::12") + bsidA := mustBsid(t, "cafe::aaa") + bsidB := mustBsid(t, "cafe::bbb") + + fake := &fakeSRv6VPP{} + p := newTestProvider(fake) + + for _, tun := range []common.SRv6Tunnel{ + {Dst: dst, Color: 6, Distinguisher: 1, Priority: 100, Policy: &types.SrPolicy{Bsid: bsidA}}, + {Dst: dst, Color: 6, Distinguisher: 2, Priority: 50, Policy: &types.SrPolicy{Bsid: bsidB}}, + } { + tun := tun + if err := p.AddConnectivity(&common.NodeConnectivity{NextHop: dst, Custom: &tun}); err != nil { + t.Fatalf("AddConnectivity: %v (tun=%+v)", err, tun) + } + } + + cache := p.nodePolices[dst.String()].SRv6Tunnel + if len(cache) != 2 { + t.Fatalf("expected 2 candidate paths to coexist; got %d (%+v)", len(cache), cache) + } +} + +// Idempotent delete: when DelSRv6Steering reports NO_SUCH_INNER_FIB (the L3 +// key has already been removed from VPP's steering hash) and DelSRv6Policy +// reports UNSPECIFIED (the BSID has already been removed from VPP's policy +// hash), delSRPolicy must still complete its work. Failover re-steer must +// run; nodePolices must be pruned. +func TestDelSRPolicy_IdempotentWhenVPPAlreadyMissing(t *testing.T) { + dst := net.ParseIP("fd00:1::11") + bsid := mustBsid(t, "cafe::aaa") + prefix := mustPrefix(t, "fd20::aaaa/128") + surviverBsid := mustBsid(t, "cafe::bbb") + + fake := &fakeSRv6VPP{ + steering: []*types.SrSteer{{Bsid: bsid, Prefix: prefix, TrafficType: types.SrSteerIPv6}}, + delSteeringErr: govppapi.NO_SUCH_INNER_FIB, + delPolicyErr: govppapi.UNSPECIFIED, + } + p := newTestProvider(fake) + dt6Behavior := uint8(18) // bgpapi.SRv6Behavior_END_DT6 + p.nodePolices[dst.String()] = &NodeToPolicies{ + Node: dst, + SRv6Tunnel: []common.SRv6Tunnel{ + {Dst: dst, Color: 6, Distinguisher: 0, Behavior: dt6Behavior, Priority: 100, Policy: &types.SrPolicy{Bsid: bsid}}, + {Dst: dst, Color: 6, Distinguisher: 1, Behavior: dt6Behavior, Priority: 50, Policy: &types.SrPolicy{Bsid: surviverBsid}}, + }, + } + + cn := &common.NodeConnectivity{Custom: &common.SRv6Tunnel{Dst: dst, Color: 6, Distinguisher: 0}} + if err := p.delSRPolicy(cn); err != nil { + t.Fatalf("delSRPolicy: %v", err) + } + + if len(fake.delSteering) != 1 || fake.delSteering[0].Bsid != bsid { + t.Fatalf("expected DelSRv6Steering(bsid=%s); got %+v", bsid.String(), fake.delSteering) + } + if len(fake.delPolicy) != 1 || fake.delPolicy[0].Bsid != bsid { + t.Fatalf("expected DelSRv6Policy(bsid=%s); got %+v", bsid.String(), fake.delPolicy) + } + // Failover still runs despite the VPP errors. + if len(fake.addModPolicy) != 1 || fake.addModPolicy[0].Bsid != surviverBsid { + t.Fatalf("expected AddModSRv6Policy(surviver=%s) for failover; got %+v", surviverBsid.String(), fake.addModPolicy) + } + if len(fake.addSteering) != 1 || fake.addSteering[0].Bsid != surviverBsid { + t.Fatalf("expected AddSRv6Steering(surviver=%s) for failover; got %+v", surviverBsid.String(), fake.addSteering) + } +} + +func TestDelConnectivity_DispatcherRoutesByEventShape(t *testing.T) { + fake := &fakeSRv6VPP{} + p := newTestProvider(fake) + // Empty cn (no Custom, no Dst.IP) must error so a malformed event is loud. + if err := p.DelConnectivity(&common.NodeConnectivity{}); err == nil { + t.Fatal("expected error for empty cn") + } +} diff --git a/calico-vpp-agent/routing/bgp_watcher.go b/calico-vpp-agent/routing/bgp_watcher.go index 96aa1975c..0fde135b3 100644 --- a/calico-vpp-agent/routing/bgp_watcher.go +++ b/calico-vpp-agent/routing/bgp_watcher.go @@ -211,7 +211,11 @@ func (s *Server) getSRPolicy(path *bgpapi.Path) (srv6Policy *types.SrPolicy, srv if err := path.Nlri.UnmarshalTo(srnrli); err != nil { return nil, nil, nil, err } + // is the RFC 9012 NLRI key and is what + // downstream DelConnectivity matches on; carry it on every srv6tunnel we return. srv6tunnel.Dst = net.IP(srnrli.Endpoint) + srv6tunnel.Color = srnrli.Color + srv6tunnel.Distinguisher = srnrli.Distinguisher // Withdraws carry only the NLRI key per RFC 9012; short-circuit so the // caller can dispatch SRv6PolicyDeleted instead of failing the empty-segments check. @@ -268,9 +272,11 @@ func (s *Server) getSRPolicy(path *bgpapi.Path) (srv6Policy *types.SrPolicy, srv // Mixed-behavior reject: VPP installs all SidLists under one sr_policy with // one Behavior, so ECMP onto a wrong-behavior list would drop/mis-decap. + // The populated srv6tunnel is returned alongside the error so the caller can + // tear down the prior install (if any) for this exact NLRI key. for i := 1; i < len(listBehaviors); i++ { if listBehaviors[i] != listBehaviors[0] { - return nil, nil, srnrli, fmt.Errorf( + return srv6Policy, srv6tunnel, srnrli, fmt.Errorf( "sr policy endpoint=%s: segment list 0 ends with endpoint behavior %d, segment list %d ends with %d: %w", net.IP(srnrli.Endpoint), listBehaviors[0], i, listBehaviors[i], errSRPolicyMixedBehavior) } @@ -285,17 +291,21 @@ func (s *Server) injectSRv6Policy(path *bgpapi.Path) error { _, srv6tunnel, srnrli, err := s.getSRPolicy(path) if err != nil { - // Mixed-behavior reject can land on an endpoint that already has a - // prior install; signal teardown via the same event as a normal withdraw. - if srnrli != nil && srnrli.Endpoint != nil && errors.Is(err, errSRPolicyMixedBehavior) { - s.log.Warnf("injectSRv6Policy: rejecting mixed-behavior SR Policy for endpoint=%s and signalling teardown of any prior install under the same endpoint", net.IP(srnrli.Endpoint)) + // A mixed-behavior advertisement supersedes the prior install for the + // SAME NLRI key — but since the new advertisement is unsafe to install, + // the prior install for that key must be torn down. Dispatch carries + // the NLRI key (Color+Distinguisher) so DelConnectivity targets exactly + // that candidate path, not every policy on the endpoint. + if srv6tunnel != nil && srnrli != nil && srnrli.Endpoint != nil && errors.Is(err, errSRPolicyMixedBehavior) { + s.log.Warnf("injectSRv6Policy: rejecting mixed-behavior SR Policy for endpoint=%s color=%d distinguisher=%d; signalling teardown of any prior install for this NLRI key", + net.IP(srnrli.Endpoint), srnrli.Color, srnrli.Distinguisher) common.SendEvent(common.CalicoVppEvent{ Type: common.SRv6PolicyDeleted, Old: &common.NodeConnectivity{ Dst: net.IPNet{}, NextHop: srnrli.Endpoint, ResolvedProvider: "", - Custom: &common.SRv6Tunnel{Dst: net.IP(srnrli.Endpoint)}, + Custom: srv6tunnel, }, }) } From f0f66ad63b00f7aa40da9de156e17d01eab44730 Mon Sep 17 00:00:00 2001 From: ryskn Date: Wed, 3 Jun 2026 19:28:42 +0900 Subject: [PATCH 2/2] fix(srv6): preserve FibTable when re-steering on candidate-path failover resteerOrphan rebuilt the steering without a FibTable, so a candidate-path failover re-pointed every orphaned prefix into the main table. Pod-prefix steerings live there so they were unaffected, but the node-IP /128 steering (steerNodeIPViaSID, for host-network-backed ClusterIPs) lives in PodVRFIndex and would have been silently moved to table 0 on failover, breaking the host plane. Carry the full SrSteer (including FibTable) in the orphan set and preserve it on re-steer, so pod-prefix (main table) and node-IP /128 (PodVRFIndex) steerings each fail over into their original table. The same delSRPolicy BSID match also tears the node-IP /128 steering down when no candidate survives. Tests: TestDelSRPolicy_FailoverPreservesNodeIPSteeringFibTable, TestDelSRPolicy_TearsDownNodeIPSteeringWhenNoSurvivor. Signed-off-by: Ryosuke Nakayama --- calico-vpp-agent/connectivity/srv6.go | 25 ++++-- calico-vpp-agent/connectivity/srv6_test.go | 90 ++++++++++++++++++++++ 2 files changed, 107 insertions(+), 8 deletions(-) diff --git a/calico-vpp-agent/connectivity/srv6.go b/calico-vpp-agent/connectivity/srv6.go index b4bfdfec3..26cee8527 100644 --- a/calico-vpp-agent/connectivity/srv6.go +++ b/calico-vpp-agent/connectivity/srv6.go @@ -403,13 +403,18 @@ func (p *SRv6Provider) delSRPolicy(cn *common.NodeConnectivity) error { // the RFC 9256 candidate-path failover wants the next-best surviving // policy of the same behavior to take over. Re-steer happens below, after // the cache prune, so getPolicyNode sees the post-withdraw state. - var orphaned []ip_types.Prefix + // Keep the full steering entry (not just the prefix): the failover re-steer + // must preserve the FibTable it was installed in. Pod prefixes are steered + // in the main table, but the node-IP /128 steering (steerNodeIPViaSID, for + // host-network-backed ClusterIPs) lives in PodVRFIndex; re-steering it into + // the wrong table would silently break that path after failover. + var orphaned []*types.SrSteer for _, bsid := range matched { for _, st := range steering { if st.Bsid != bsid { continue } - orphaned = append(orphaned, st.Prefix) + orphaned = append(orphaned, st) logDel(fmt.Sprintf("DelSRv6Steering bsid=%s prefix=%s", st.Bsid, st.Prefix), p.vpp.DelSRv6Steering(st)) } logDel(fmt.Sprintf("DelSRv6Policy bsid=%s", bsid), p.vpp.DelSRv6Policy(&types.SrPolicy{Bsid: bsid})) @@ -426,8 +431,8 @@ func (p *SRv6Provider) delSRPolicy(cn *common.NodeConnectivity) error { // install on demand here so multiple orphaned prefixes targeting the same // surviving BSID don't churn the install. installed := make(map[ip_types.IP6Address]struct{}) - for _, prefix := range orphaned { - p.resteerOrphan(nodeip, prefix, installed) + for _, st := range orphaned { + p.resteerOrphan(nodeip, st, installed) } return nil } @@ -438,8 +443,11 @@ func (p *SRv6Provider) delSRPolicy(cn *common.NodeConnectivity) error { // withdrawn higher-priority candidate), so install it on demand — guarded by // `installed` so we install at most once per delSRPolicy call. If no candidate // remains the prefix is left unsteered and AddConnectivity picks it up when a -// new candidate is later advertised. -func (p *SRv6Provider) resteerOrphan(nodeip string, prefix ip_types.Prefix, installed map[ip_types.IP6Address]struct{}) { +// new candidate is later advertised. The orphan's FibTable is preserved so the +// node-IP /128 steering stays in PodVRFIndex (and pod prefixes in the main +// table) across the failover. +func (p *SRv6Provider) resteerOrphan(nodeip string, orphan *types.SrSteer, installed map[ip_types.IP6Address]struct{}) { + prefix := orphan.Prefix behavior := types.SrBehaviorDT4 if vpplink.IsIP6(prefix.Address.ToIP()) { behavior = types.SrBehaviorDT6 @@ -460,6 +468,7 @@ func (p *SRv6Provider) resteerOrphan(nodeip string, prefix ip_types.Prefix, inst } srSteer := &types.SrSteer{ TrafficType: types.SrSteerIPv4, + FibTable: orphan.FibTable, // preserve the table the orphan was steered in Prefix: prefix, Bsid: policy.Bsid, } @@ -471,8 +480,8 @@ func (p *SRv6Provider) resteerOrphan(nodeip string, prefix ip_types.Prefix, inst prefix.String(), policy.Bsid.String(), err) return } - p.log.Infof("SRv6Provider DelConnectivity: re-steered prefix=%s onto surviving bsid=%s behavior=%d", - prefix.String(), policy.Bsid.String(), behavior) + p.log.Infof("SRv6Provider DelConnectivity: re-steered prefix=%s onto surviving bsid=%s behavior=%d table=%d", + prefix.String(), policy.Bsid.String(), behavior, orphan.FibTable) } func (p *SRv6Provider) delPrefixSteering(cn *common.NodeConnectivity) error { diff --git a/calico-vpp-agent/connectivity/srv6_test.go b/calico-vpp-agent/connectivity/srv6_test.go index f144714e3..b4aa19b8c 100644 --- a/calico-vpp-agent/connectivity/srv6_test.go +++ b/calico-vpp-agent/connectivity/srv6_test.go @@ -296,6 +296,96 @@ func TestDelSRPolicy_FailoverOntoSurvivingCandidate(t *testing.T) { } } +// The node-IP /128 steering installed by steerNodeIPViaSID (host-network-backed +// ClusterIPs, #1028) lives in PodVRFIndex, not the main table. When its DT6 +// policy is withdrawn, the failover must re-steer it into the SAME table — +// otherwise the host plane silently breaks after a candidate-path switch. +func TestDelSRPolicy_FailoverPreservesNodeIPSteeringFibTable(t *testing.T) { + dst := net.ParseIP("fd00:1::14") + winnerBsid := mustBsid(t, "cafe::aa") + loserBsid := mustBsid(t, "cafe::bb") + nodeIP := mustPrefix(t, "fd00:1::14/128") + + fake := &fakeSRv6VPP{ + steering: []*types.SrSteer{ + // node-IP /128 steered in PodVRFIndex onto the DT6 winner. + {Bsid: winnerBsid, Prefix: nodeIP, TrafficType: types.SrSteerIPv6, FibTable: common.PodVRFIndex}, + }, + } + p := newTestProvider(fake) + dt6Behavior := uint8(18) // bgpapi.SRv6Behavior_END_DT6 + p.nodePolices[dst.String()] = &NodeToPolicies{ + Node: dst, + SRv6Tunnel: []common.SRv6Tunnel{ + {Dst: dst, Color: 6, Distinguisher: 0, Behavior: dt6Behavior, Priority: 100, Policy: &types.SrPolicy{Bsid: winnerBsid}}, + {Dst: dst, Color: 6, Distinguisher: 1, Behavior: dt6Behavior, Priority: 50, Policy: &types.SrPolicy{Bsid: loserBsid}}, + }, + } + + cn := &common.NodeConnectivity{Custom: &common.SRv6Tunnel{Dst: dst, Color: 6, Distinguisher: 0}} + if err := p.delSRPolicy(cn); err != nil { + t.Fatalf("delSRPolicy: %v", err) + } + + if len(fake.addSteering) != 1 { + t.Fatalf("expected 1 re-steer; got %d", len(fake.addSteering)) + } + re := fake.addSteering[0] + if re.Bsid != loserBsid { + t.Fatalf("re-steer BSID = %s, want surviving %s", re.Bsid.String(), loserBsid.String()) + } + if re.FibTable != common.PodVRFIndex { + t.Fatalf("re-steer FibTable = %d, want PodVRFIndex (%d): the node-IP steering must stay in the pod VRF after failover", + re.FibTable, common.PodVRFIndex) + } +} + +// When the node's last DT6 candidate is withdrawn, the node-IP /128 steering +// (steerNodeIPViaSID, #1028) must be torn down on the same teardown path and +// NOT re-steered — leaving the host plane to fall back to native routing. This +// is the teardown half the #1028 note deferred to #1025. +func TestDelSRPolicy_TearsDownNodeIPSteeringWhenNoSurvivor(t *testing.T) { + dst := net.ParseIP("fd00:1::14") + bsid := mustBsid(t, "cafe::aa") + podPrefix := mustPrefix(t, "fd20::aaaa/128") + nodeIP := mustPrefix(t, "fd00:1::14/128") + + fake := &fakeSRv6VPP{ + steering: []*types.SrSteer{ + {Bsid: bsid, Prefix: podPrefix, TrafficType: types.SrSteerIPv6}, // pod prefix in main table + {Bsid: bsid, Prefix: nodeIP, TrafficType: types.SrSteerIPv6, FibTable: common.PodVRFIndex}, // node-IP /128 in PodVRFIndex + }, + } + p := newTestProvider(fake) + dt6Behavior := uint8(18) // bgpapi.SRv6Behavior_END_DT6 + p.nodePolices[dst.String()] = &NodeToPolicies{ + Node: dst, + SRv6Tunnel: []common.SRv6Tunnel{ + {Dst: dst, Color: 6, Distinguisher: 0, Behavior: dt6Behavior, Priority: 100, Policy: &types.SrPolicy{Bsid: bsid}}, + }, + } + + cn := &common.NodeConnectivity{Custom: &common.SRv6Tunnel{Dst: dst, Color: 6, Distinguisher: 0}} + if err := p.delSRPolicy(cn); err != nil { + t.Fatalf("delSRPolicy: %v", err) + } + + // No surviving candidate → nothing re-steered (no leak into any table). + if len(fake.addSteering) != 0 { + t.Fatalf("expected no re-steer when no survivor; got %d", len(fake.addSteering)) + } + // The node-IP /128 steering must have been torn down, in PodVRFIndex. + foundNodeIP := false + for _, st := range fake.delSteering { + if st.Prefix.String() == nodeIP.String() && st.FibTable == common.PodVRFIndex { + foundNodeIP = true + } + } + if !foundNodeIP { + t.Fatalf("node-IP /128 steering in PodVRFIndex was not torn down; deletions=%+v", fake.delSteering) + } +} + func TestDelSRPolicy_NoSurvivingCandidateLeavesPrefixUnsteered(t *testing.T) { dst := net.ParseIP("fd00:1::11") bsid := mustBsid(t, "cafe::4")