Skip to content

swbus: retry listener bind failures - #214

Open
BYGX-wcr wants to merge 1 commit into
sonic-net:masterfrom
BYGX-wcr:fix/retry-swbus-listener-bind
Open

swbus: retry listener bind failures#214
BYGX-wcr wants to merge 1 commit into
sonic-net:masterfrom
BYGX-wcr:fix/retry-swbus-listener-bind

Conversation

@BYGX-wcr

@BYGX-wcr BYGX-wcr commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Description of PR

Summary:
Keep swbusd alive through transient TCP listener bind failures. Bind the listener before starting route-announcer and peer-connection tasks, retry once per second for up to 180 seconds, and pass the successfully bound listener to tonic. If binding still fails at the deadline, log an error and exit.

Fixes: N/A

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Documentation update
  • Test improvement

Approach

What is the motivation for this PR?

During a SONiC/container restart on a physical SmartSwitch, all four swbusd instances on one NPU failed to bind their configured Loopback0 listener. Each process exited with status 101, Supervisor exhausted its short restart budget and marked swbusd FATAL, and dependent hamgrd processes never started. The peer switch then could not route HA actor messages.

The listener addresses became usable later, but Supervisor no longer retried the processes. Manually starting swbusd restored all services and inter-switch communication.

How did you do it?

  • Explicitly create the Tokio TCP listener before starting route and peer tasks.
  • Log failed attempts at WARN and retry listener binding once per second for up to 180 seconds.
  • Log ERROR and return the bind failure if the deadline expires.
  • Feed the successfully bound listener to tonic using TcpListenerStream.
  • Add unit tests covering both eventual recovery and timeout failure.

How did you verify/test it?

  • cargo build --workspace --all-features
  • cargo test -p swbus-core (37 unit tests and 2 integration tests passed)
  • cargo fmt --check --all
  • Strict cargo clippy for all swbus-core targets, allowing only the pre-existing items_after_test_module warning in conn.rs
  • Reproduced the physical failure: swbusd entered FATAL and hamgrd stayed stopped after transient listener failures. Manual retries after the address stabilized succeeded and restored 64 established inter-switch swbus sessions and bidirectional swbus pings.

Any platform specific information?

This specifically protects SONiC container startup/reload sequences where the host-network Loopback0 listener address or port is transiently unavailable.

Documentation

No documentation update is required for this bug fix.

Copilot AI lite review requested due to automatic review settings August 7, 2026 18:16
@mssonicbld

Copy link
Copy Markdown

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Improves swbusd startup robustness by avoiding process exit on transient TCP listener bind failures. This fits into swbus-core’s mux/service hosting by changing how the gRPC server obtains its listening socket and adding coverage for the retry behavior.

Changes:

  • Add a retrying TCP listener bind helper and bind the listener before starting route-announcer/peer setup.
  • Switch tonic serving from serve_with_shutdown(addr, ...) to serve_with_incoming_shutdown(TcpListenerStream, ...) using the pre-bound listener.
  • Add a unit test covering “port occupied → keep retrying → port released → bind succeeds”; enable tokio-stream’s net feature.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
crates/swbus-core/src/mux/service.rs Adds listener bind retry logic, feeds a pre-bound listener into tonic, and introduces a unit test for retry behavior.
Cargo.toml Enables tokio-stream net feature to use TcpListenerStream.

Comment thread crates/swbus-core/src/mux/service.rs Outdated
Comment on lines +27 to +31
async fn bind_listener_with_retry(addr: SocketAddr, retry_interval: std::time::Duration) -> TcpListener {
loop {
match TcpListener::bind(addr).await {
Ok(listener) => return listener,
Err(error) => {
Comment thread crates/swbus-core/src/mux/service.rs
@vivekrnv

vivekrnv commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Which tests would fail without this fix?

@zjswhhh

zjswhhh commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Which tests would fail without this fix?

Any test? I was running a steady state case, due to this binding failure, swbusd exited, hence hamgrd couldn't start either.

@vivekrnv

vivekrnv commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Which tests would fail without this fix?

Any test? I was running a steady state case, due to this binding failure, swbusd exited, hence hamgrd couldn't start either.

I ran a few tests and i didn't see this issue

Signed-off-by: BYGX-wcr <wcr@live.cn>
Copilot AI review requested due to automatic review settings August 7, 2026 21:04
@BYGX-wcr
BYGX-wcr force-pushed the fix/retry-swbus-listener-bind branch from 037c028 to 6759faf Compare August 7, 2026 21:04
@mssonicbld

Copy link
Copy Markdown

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

crates/swbus-core/src/mux/service.rs:43

  • bind_listener_with_retry retries/sleeps without observing any shutdown signal. If SwbusServiceHost::shutdown() is triggered while the listener is still failing to bind, start() will keep retrying until timeout and delay shutdown/termination unnecessarily.

Consider threading a CancellationToken (or the existing oneshot receiver) into bind_listener_with_retry and using tokio::select! to break out immediately on shutdown (returning an Interrupted error or similar), so the process can stop promptly when asked.

    loop {
        match TcpListener::bind(addr).await {
            Ok(listener) => return Ok(listener),
            Err(error) if tokio::time::Instant::now() < deadline => {
                warn!(%addr, %error, "Failed to bind swbus listener; retrying");
                let remaining = deadline.saturating_duration_since(tokio::time::Instant::now());
                tokio::time::sleep(retry_interval.min(remaining)).await;
            }

crates/swbus-core/src/mux/service.rs:28

  • The retry logic still returns an error after LISTENER_RETRY_TIMEOUT (currently hard-coded to 180s). That means swbusd can still exit and potentially hit Supervisor restart-budget/FATAL behavior during longer-than-3-minute address unavailability—the same failure mode this change is trying to avoid.

If the intended behavior is to keep the process alive until the address becomes available (no matter how long), consider making the timeout configurable (or removing it) and/or aligning it with the expected Supervisor restart policy.

const LISTENER_RETRY_INTERVAL: Duration = Duration::from_secs(1);
const LISTENER_RETRY_TIMEOUT: Duration = Duration::from_secs(180);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants