Skip to content

Drop() in adj-RIB-In creates withdrawals for rejected paths, causing spurious "No matching path for withdraw" warnings #3455

Description

@avx-vkhare

Summary

The AdjRib.Drop() function in internal/pkg/table/adj.go does not filter out rejected paths (paths marked with IsRejected=true) when generating withdrawal paths. This causes the downstream processWithdraw() to fail to find these paths in the actual RIB (since rejected paths were never installed there), resulting in repeated "No matching path for withdraw found, may be path was not installed into table" warnings.

In large-scale BGP deployments — particularly HA transit topologies with many spokes — this can flood logs with tens of thousands of warnings per peer-flap event and waste memory storing rejected paths that never need to be withdrawn.

Environment

  • GoBGP version: v3.37.0 (issue exists on master as well)
  • Affected file: internal/pkg/table/adj.go
  • Affected function: AdjRib.Drop()

Reproduction Scenario

  1. Deploy two BGP routers (Router A and Router B) sharing the same AS number (HA pair scenario).
  2. Connect ~200 BGP peers (spokes) to both routers.
  3. Each spoke advertises routes to Router A.
  4. Router A advertises routes to all other spokes; spokes re-advertise back to Router B.
  5. Router B receives the routes, sees its own AS in the AS-path → rejects them (loop prevention) → marks them as IsRejected=true in adj-RIB-In.
  6. With 200 spokes, ~200×199 ≈ ~40,000 rejected ghost paths accumulate in Router B's adj-RIB-In.
  7. Trigger any BGP session flap on Router B.
  8. Drop() is called → generates withdrawal entries for ALL paths (including rejected ones).
  9. processWithdraw() cannot find the rejected paths in the actual RIB (they were never installed).
  10. Result: ~40,000+ "No matching path for withdraw" warnings flood the log.

Root Cause

In internal/pkg/table/adj.go lines 158–173:

func (adj *AdjRib) Drop(rfList []bgp.RouteFamily) []*Path {
    l := make([]*Path, 0, adj.Count(rfList))
    adj.walk(rfList, func(d *Destination) bool {
        for _, p := range d.knownPathList {
            w := p.Clone(true)      // ← no IsRejected() guard
            w.SetDropped(true)
            l = append(l, w)
        }
        return false
    })
    ...
}

This iterates over all paths in knownPathList without checking IsRejected(). Rejected paths are included in the withdrawal list, but they were never installed into the RIB, so processWithdraw() cannot find them.

Inconsistency with Existing Code

Other functions in the same file already correctly skip rejected paths:

  • StaleAll() (lines 191–206) — skips rejected paths from the returned list:

    if !n.IsRejected() {
        pathList = append(pathList, n)
    }
  • MarkLLGRStaleOrDrop() (lines 208–231) — handles rejected paths separately:

    if p.IsRejected() {
        d.knownPathList[i] = n
    } else {
        pathList = append(pathList, n)
    }
  • PathList() (lines 125–137) — has an accepted flag that skips rejected paths.

The graceful restart code path also correctly handles rejected paths. Drop() is the outlier.

Proposed Fix

Add an IsRejected() guard in Drop() to skip rejected paths, matching the pattern already established in StaleAll(), MarkLLGRStaleOrDrop(), and PathList():

func (adj *AdjRib) Drop(rfList []bgp.RouteFamily) []*Path {
    l := make([]*Path, 0, adj.Count(rfList))
    adj.walk(rfList, func(d *Destination) bool {
        for _, p := range d.knownPathList {
            if p.IsRejected() {
                continue
            }
            w := p.Clone(true)
            w.SetDropped(true)
            l = append(l, w)
        }
        return false
    })
    ...
}

Impact

Without the fix:

  • Log spam: ~40,000+ "No matching path for withdraw" warnings per peer flap in 200-spoke HA setups
  • Wasted memory: ~19 MB per event at 200 spokes; potentially up to ~1.67 GB at 2000-spoke scale
  • Difficult to debug actual issues due to log noise

With the fix:

  • No spurious warnings
  • Reduced memory footprint
  • Cleaner, more reliable adj-RIB-In behavior on peer flaps

Test Case

A test case can be added that:

  1. Adds two paths to the AdjRib — one regular, one with SetRejected(true).
  2. Calls Drop().
  3. Asserts the returned path list contains only the non-rejected path.

Willing to Contribute

I'm happy to submit a PR with the fix and a test case if maintainers agree with the approach.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions