oxidecomputer / oxidecomputer/maghemite

BGP FSM handles changes to passive_tcp_establishment poorly

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

Nobody has claimed this yet.

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

Description

Passive TCP Establishment for a BGP FSM indicates that it will not attempt to make any outbound connections, instead waiting for inbound connections.

In our implementation:

  1. an active (!passive) peer will have its FSM park in the Connect state while it kicks off asynchronous connection attempts when the ConnectRetryTimer fires, and it waits for either inbound or outbound connections to complete.
  2. a passive peer will have its FSM park in the Active state while it waits for inbound connections and the ConnectRetryTimer will not run.

While changes to passive_tcp_establishment are visible (via lock!(self.session).passive_tcp_establishment) from the SessionRunner (FSM), there isn't consistent and well-planned logic to ensure the FSM changes its behavior appropriately.

We need some kind of FSM event to notify the SessionRunner it needs to take action. Its handler should react by checking passive_tcp_establishment: stay in Active if false, kick off new outbound connection and transition to Connect if true.

A couple options:

  1. Add a new Admin FSM event indicating there's been a change to passive_tcp_establishment
  2. Piggyback on ConnectRetryTimer.

If we go with (2) then we need to start running that timer in Active regardless of whether the session is passive.

Regardless of which FSM event we use, we need the handler to check passive_tcp_establishment and transition between Active/Connect appropriately.

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 SessionRunner FSM and its handling of the Active and Connect states, then trace ConnectRetryTimer and the passive_tcp_establishment value. Define an FSM event or timer path that observes changes and transitions appropriately; done means passive peers remain in Active while enabled outbound establishment starts a connection and transitions to Connect.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.