oxidecomputer / oxidecomputer/maghemite

BFD makes FSM transition after building Poll response instead of before

Open
#864 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bfd Bug mgd needs testing rust
Dominant language
Rust
Stars
94
Forks
6
Avg merge
1d 8h
Merged PRs (30d)
12

Description

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.

Problem

When a control packet has the Poll bit set, the state machine currently
constructs and queues its Final response before applying the state
transition caused by the received packet.

Examples:

  • Local Down receives remote Down+Poll:

    • Final is created with State=Down;
    • local session then transitions to Init.
  • Local Init receives remote Init+Poll:

    • Final is created with State=Init;
    • local session then transitions to Up.

The queued response therefore advertises the state from before the
received packet was processed.

RFC 5880 §6.8.6 applies the received state transition before sending the
Final response. RFC 5880 §6.8.7 requires transmitted State to reflect
the current local session state.

Impact

The remote peer receives stale state in the mandatory immediate Poll
response. This can require additional handshake packets and may delay or
disrupt convergence, particularly when Demand mode suppresses subsequent
periodic packets.

Proposed direction

For an accepted packet:

  1. update remote session information;
  2. apply the local state transition;
  3. determine whether Demand mode is active;
  4. construct the Poll Final response from the resulting local state.

The Final response must still bypass the normal periodic transmission
timer.

Testing

Extend Poll tests to check the State field, not only the Poll and Final
bits:

  • Down + received Down/Poll produces an Init/Final response;
  • Init + received Init/Poll produces an Up/Final response;
  • Up + received Down/Poll produces a Down/Final response.

References

  • RFC 5880 §6.8.6
  • RFC 5880 §6.8.7

Context

This ordering behavior predated the async implementation and was retained
by #795 and #796.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Locate the BFD control-packet Poll handling and the existing Poll tests; inspect where remote session information, state transitions, Demand mode, and the immediate Final response are processed. Update the ordering so the response uses the resulting local state while still bypassing the periodic timer. Extend the Poll tests to verify Init/Final, Up/Final, and Down/Final state combinations.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
networking
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.