You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Copy link
×ばつ 8 counters) were exported; - ran 70 no-traffic/below-threshold invocations and 30 active-traffic invocations: 100/100 returned successfully, with zero hangs or timeouts; - observed command latency between 32 ms and 64 ms; - generated a controlled traffic burst on `eth1/1/1` and observed: ```text hwagent0 eth1/1/1.out_bytes.rate.60 52195 Interface Name Peer Intvl InMbps InPct InKpps OutMbps OutPct OutKpps eth1/1/1 -- 0:60 0.00 0.00% 0.00 0.42 0.00% 0.04 Total -- -- 0.00 0.00% 0.00 0.42 0.00% 0.04 ``` - Confirmed `fboss_sw_agent`, `fboss_hw_agent@0`, and `qsfp_service` remained active after validation. " data-view-component="true">
Copy Markdown
Summary
Fix fboss2 show interface traffic on OSS multi-switch systems where the command can hang indefinitely or return an empty/zero table while ports are carrying traffic.
This change:
uses deterministic synchronous RPCs for the command's sequential data collection;
exports the SUM and RATE types requested by MonotonicCounter callers, making port .rate.60 counters available from the HW agent;
fetches only the traffic-rate counter families needed by this command;
skips down ports, which may legitimately have no HW-agent rate counters;
reports a missing counter on an up port instead of silently converting it to zero; and
preserves fractional Kpps values and guards per-port zero bandwidth.
The aggregate zero-bandwidth fix from #1560 is already present on main and remains unchanged.
Root cause
There were three contributing problems:
CmdShowInterfaceTraffic::queryClient() mixed future-based RPCs attached to a short-lived local IOThreadPoolExecutor with blocking HW-agent fanout, followed by an unconditional wait(). If the future did not reach a terminal state on a cold/multi-switch path, the command had no forward progress and remained blocked. The operation is sequential at the point the model is created, so the local executor did not provide useful concurrency.
The OSS MonotonicCounter implementation created its backing stat with getLockableStatNoExport() and ignored both requested export types. Port counters were updated internally, but .rate.60 was therefore absent from FB303 output.
The CLI read rate counters with std::map::operator[]. Missing counters were inserted as zero, making unavailable telemetry indistinguishable from a real zero traffic rate. It also attempted to read current rate counters for down ports.
Fix details and compatibility
The Thrift APIs and output schema are unchanged.
Both monolithic and multi-switch paths remain supported.
MonotonicCounter now honors its existing constructor contract. Its swap() implementation also transfers the state, name, and lockable stat so clear/rename operations retain the correct counter lifecycle.
The FB303 regex is anchored to exact .rate.60 suffixes, reducing the counter payload compared with fetching every agent counter.
Down ports are excluded before rate lookup. An up port without required telemetry fails explicitly instead of displaying misleading zeros.
Existing error-counter reporting is preserved for both up and down ports.
Unit-test coverage
synchronous query path and exact .rate.60 regex;
missing rate counter on an up port;
down port without rate counters;
fractional Kpps conversion;
zero-bandwidth utilization;
monotonic counter rate export, removal, and port-name reinitialization; and
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
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.
Summary
Fix
fboss2 show interface trafficon OSS multi-switch systems where the command can hang indefinitely or return an empty/zero table while ports are carrying traffic.This change:
SUMandRATEtypes requested byMonotonicCountercallers, making port.rate.60counters available from the HW agent;The aggregate zero-bandwidth fix from #1560 is already present on
mainand remains unchanged.Root cause
There were three contributing problems:
CmdShowInterfaceTraffic::queryClient()mixed future-based RPCs attached to a short-lived localIOThreadPoolExecutorwith blocking HW-agent fanout, followed by an unconditionalwait(). If the future did not reach a terminal state on a cold/multi-switch path, the command had no forward progress and remained blocked. The operation is sequential at the point the model is created, so the local executor did not provide useful concurrency.MonotonicCounterimplementation created its backing stat withgetLockableStatNoExport()and ignored both requested export types. Port counters were updated internally, but.rate.60was therefore absent from FB303 output.std::map::operator[]. Missing counters were inserted as zero, making unavailable telemetry indistinguishable from a real zero traffic rate. It also attempted to read current rate counters for down ports.Fix details and compatibility
MonotonicCounternow honors its existing constructor contract. Itsswap()implementation also transfers the state, name, and lockable stat so clear/rename operations retain the correct counter lifecycle..rate.60suffixes, reducing the counter payload compared with fetching every agent counter.Unit-test coverage
.rate.60regex;Test Plan
pre-commit run --from-ref upstream/main --to-ref HEAD— passed, including clang-format and repository checks.fboss-gr2-stable-20260903-fbc276b.1-1-ge405635..rate.60counters (128 ports ×ばつ 8 counters) were exported;eth1/1/1and observed:fboss_sw_agent,fboss_hw_agent@0, andqsfp_serviceremained active after validation.