oxidecomputer / oxidecomputer/maghemite

BFD hot loop when required_rx is 0

Open
#862 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

BFD configuration permits required_rx = 0.

The state machine currently computes its receive timeout approximately
as:

local.required_min_rx * local.detection_multiplier

When required_rx is zero, every receive deadline equals the current
instant. The session driver repeatedly:

  1. observes an immediately expired timeout;
  2. re-arms another immediate timeout;
  3. increments the timeout counter;
  4. loops again.

The synchronous implementation had the same basic timer defect, but it
consumed a dedicated BFD thread. The async implementation runs this loop
on mgd's shared Tokio runtime, increasing the impact on unrelated BFD
sessions and API work.

Impact

One configuration can cause persistent CPU consumption and scheduler
pressure. Multiple such sessions may affect unrelated work running on
the Tokio runtime.

Protocol consideration

RFC 5880 gives Required Min RX Interval zero a meaning: the system does
not want to receive periodic BFD Control packets. It does not mean that
the receive deadline should continually expire.

Correct detection-time calculation also depends on the remote Desired
Min TX interval, so fixing timer negotiation may naturally eliminate
the zero deadline.

Proposed direction

This should be coordinated with #798.

Possible approaches:

  • represent a disabled periodic receive expectation explicitly; or
  • calculate the detection deadline from the negotiated interval rather
    than local Required Min RX alone; or
  • reject zero at the management boundary if Maghemite intentionally
    chooses not to support its RFC meaning.

The driver should also avoid repeatedly expiring a receive timer while
the session is already Down.

Testing

Add a current-thread Tokio test containing:

  • a session configured with required_rx = 0; and
  • an independent heartbeat task.

Verify that the heartbeat continues to run and the BFD driver does not
produce an unbounded stream of expiration events.

Also test zero in combination with a nonzero remote Desired Min TX
interval after timer negotiation is implemented.

Related issues

  • #798: BFD timer negotiation is non-existent
  • oxidecomputer/omicron#10657: BFD configuration validation

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

Start with the async BFD session driver and its receive-timeout calculation, then read related issue #798 about timer negotiation. Add a current-thread Tokio test with required_rx = 0 and an independent heartbeat, plus coverage for a nonzero remote Desired Min TX interval. Done means no unbounded expiration loop and the heartbeat continues to run.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
networking
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.