Repository navigation
monorail: Reduce management network workaround frequency - #2739
Draft
jamesmunns wants to merge 2 commits into
Draft
jamesmunns wants to merge 2 commits into
jamesmunns wants to merge 2 commits into
Conversation
jamesmunns
marked this pull request as draft
October 19, 2026 08:27
evan-oxide
reviewed
Oct 23, 2026
Comment on lines
+616
to
627
| if now.wrapping_sub(link_down_at) >= limit { | ||
| self.retry_10g_count = | ||
| self.retry_10g_count.saturating_add(1); | ||
| self.link_down_at = None; | ||
| // This logs Trace::Reinit in the ringbuf | ||
| self.reinit()?; | ||
| } | ||
| } else { | ||
| // re-init to the default attempt count once the link goes | ||
| // down. | ||
| self.retry_10g_count = 0; | ||
| self.link_down_at = Some(now); |
Contributor
There was a problem hiding this comment.
It looks like the retry_10g_count will never get past 1. Since link_down_at is set to None when re-initing, next time wake() runs we'll take this else and set retry_10g_count back to 0.
| // The LONG_WAIT time (ms) | ||
| const REINIT_LONG_WAIT: u64 = 5 * 60_000; | ||
| // How many times should we SHORT_WAIT | ||
| const REINIT_SHORT_ATTEMPTS: u32 = ((60_000 |
Contributor
There was a problem hiding this comment.
I'm wondering if we should use timestamps to check whether 15 minutes has passed instead of counting attempts. Do you know how fast reinit() is? If it's slow (or there's some other unexpected delay) then these attempts could end up taking somewhat longer than 15 minutes.
|
|
||
| // Follow-up workaround for the other link-stuck issue discussed in | ||
| // https://github.com/oxidecomputer/bf_sde/issues/27 | ||
| // If we remain down for 20 seconds, then reinitialize everything. |
Contributor
There was a problem hiding this comment.
Maybe mention the backoff
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a targeted hotfix-ish patch for an issue we've seen in the field.
There are some cases where the tofino fails to come up entirely, and from the sidecar's perspective, this is first noticed by an absence of a successful 10g link between the tofino and the vsc7448 management switch.
In the olden days, this symptom was indicative of a different problem, and one that was remedied by restarting the vsc7448 until it eventually the condition improved, and we were able to autonegotiate the link.
However, more recently, we see some cases where this never resolved (because the tofino itself does not come up fully), which leaves us more or less restarting the management network every 20s, forever. This makes using the management network to recover the switch difficult or not possible.
So, this fix keeps the old behavior for the first 15 minutes (a reasonable time for a scrimlet to boot, at least if we don't need to do a bunch of DIMM retraining), then relaxes and only tries restarting the vsc7448 every 5m until the condition resolves, instead of every 20s. That gives an operator a lot more breathing time to command a reboot, etc.
I'm starting to look at the transceiver/monorail code in a wider scope instead of just putting a bandaid on this one symptom, but in case that goes a little long for the next release, a small qol bandaid might still be good to have.
I could also make this an exponential backoff if we want, but the step function seemed a bit easier to put in place.