Skip to content

BFD session del/add may race to update RIB nexthop shutdown status #858

Description

@taspelund

The following is an issue identified by AI when I asked it to do an adversarial code review of #795. This was pre-existing in the sync/pthread impl, which is why it was not addressed as part of the async swap-out.

Title: BFD: prevent stale RIB writes from superseded sessions

Problem

BFD session teardown does not wait for all RIB work associated with the
session to finish.

Session::drop() aborts the session's async tasks, but RibTask performs
RDB updates through tokio::task::spawn_blocking(). Once a blocking task
has started, aborting the async task waiting for it does not cancel the
blocking operation.

The shutdown handle returned when removing a peer waits only for the UDP
listener. It does not wait for the session's RIB task or an in-flight
blocking RDB update.

Failure scenario

  1. Session A begins a RIB update for peer P.
  2. A is removed.
  3. The async RIB task is aborted, but its blocking update remains live.
  4. Session B is created for the same peer.
  5. B publishes its current state to the RIB.
  6. A's stale update completes afterward and becomes the final RIB state.

For example, session B may be Down while a stale shutdown = false write
from A leaves routes through the peer enabled. Because B already believes
it synchronized shutdown = true, no further write is guaranteed until
another state transition occurs.

The inverse is also possible: a stale shutdown = true write can suppress
routes belonging to a new Up session.

Impact

The forwarding state can permanently disagree with the currently visible
BFD session until another BFD transition happens.

Proposed direction

Give peer removal an awaitable, session-level shutdown operation that:

  1. stops and awaits the driver and egress tasks;
  2. closes the state channel;
  3. allows any current blocking RIB operation to complete;
  4. awaits the RIB task;
  5. awaits the listener task.

The same peer must not be re-added until shutdown of the prior generation
has completed. Alternatively, serialize RIB updates through a
generation-aware per-peer worker that rejects writes from superseded
sessions.

Testing

Add a fake RIB sink with barriers that can:

  1. block an old session's write;
  2. remove and recreate the session;
  3. allow the new session's write to complete;
  4. release the old write;
  5. verify that the old generation cannot overwrite the new state.

Metadata

Metadata

Assignees

No one assigned

    Labels

    BugbfdBidirectional Forwarding DetectionmgdMaghemite daemonneeds testingrustPull requests that update rust code

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions