fix(trace-topology): taprio GCL readback honours sched-entry cmd (S/H/R) (#390) - #394
Conversation
…/R) (#390) extract_taprio_gcl read every taprio schedule entry as an 802.1Qbv gate-control entry, using only gatemask and interval and never looking at cmd. iproute2 emits cmd as one of S (SetGates), H (Hold) or R (Release); only S entries are gate entries. H/R belong to 802.1Qbu/802.3br frame preemption and their gatemask is not a schedule state the Qcc YANG gate-control-list should carry. Read a schedule entry as a GCL entry only when cmd == "S"; drop H/R rather than fold their masks in; error (naming what was seen) on a missing or unknown cmd, on the same principle as the rest of the function that absence must not read as the expected case. Three lib tests, each proven non-vacuous by neutering its arm and observing the failure: - tc_taprio_skips_preemption_hold_release_entries: an S/H/R/S schedule yields exactly the two S masks in order, with neither preemption mask present. - tc_taprio_missing_cmd_is_an_error - tc_taprio_unknown_cmd_is_an_error REQ-TRACE-TOPOLOGY-013 (traces-to REQ-TRACE-TOPOLOGY-005) records the decision; the VM-level preemption fixture (gen-fixtures installs only sched-entry S today) is honestly deferred to a successor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Automated review for PR #394pulseengine/spar: Verdict: 💬 Comment Summary: The pull request is approved as it addresses the issue of ensuring that Findings: 0 mechanical (rivet) · 2 from local AI model. Findings (2):
Generated by a local AI model and post-validated against a strict JSON contract. Each finding includes the verbatim line being criticised — verify by reading the file at the cited location. Reviewed at |
Rivet verification gate✅ 20/20 passed
Filter: Failed artifacts(none) Updated automatically by |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Closes #390.
Problem
extract_taprio_gcl(crates/spar-trace-topology/src/fixtures/transform.rs) read every taprio schedule entry as an 802.1Qbv gate-control-list entry, using onlygatemaskandintervaland never looking atcmd. iproute2'sentry_cmd_to_str(tc/q_taprio.c) emitscmdas one ofS(SetGates),H(Hold),R(Release). OnlySentries are gate entries in the 802.1Qbv sense;H/Rbelong to 802.1Qbu/802.3br frame preemption and their gate mask is not a schedule state the Qcc YANGgate-control-listshould carry.It has not bitten because
bin/gen-fixtures.rsinstalls onlysched-entry S, so every entry read back is anS— which is exactly why the distinction should be written down rather than trusted to stay that way (a future preemption fixture would silently foldH/Rmasks into its GCL).Change
In
extract_taprio_gcl, dispatch oncmdbefore extracting a gate entry:cmd"S"gatemask/intervalas before"H"/"R""S"The missing/unknown handling follows the same discipline already in this function (and the
pmc/lldptransforms): absence must not read as the expected case, and an error names what it saw.Oracle — non-vacuous by construction
Three lib tests, each proven to fail if its arm is removed (verified by neutering each in turn and observing red):
tc_taprio_skips_preemption_hold_release_entries— anS/H/R/Sschedule (the shape a preemption-configured port reads back) yields exactly the twoSmasks in order, and neither preemption mask (0x0f,0xf0) appears. Drop the filter and this goes red (GCL length 4).tc_taprio_missing_cmd_is_an_errortc_taprio_unknown_cmd_is_an_errorextract_taprio_gclis a pure function, so this is a real test of the preemption path rather than an assertion about code that cannot run — no VM required.Requirement
REQ-TRACE-TOPOLOGY-013(traces-toREQ-TRACE-TOPOLOGY-005) records the decision, with PROVEN (the three executed tests) separated honestly from ASSUMED/DEFERRED:gen-fixtures.rsstill installs onlysched-entry S, so the preemption path is proven on the pure transform, not end-to-end through the netns VM. A fixture that installssched-entry Hso the golden readback exercises the drop on realtcoutput is a heavier, separate step and is left to a successor.Verification
cargo test -p spar-trace-topology --lib fixtures::transform→ 18 passed, 0 failedcargo fmt --all -- --checkclean;cargo clippy -p spar-trace-topology --all-targets -- -D warningscleanrivet validate→ PASS;REQ-TRACE-TOPOLOGY-013parses and itstraces-toresolves (no broken cross-refs)🤖 Generated with Claude Code
Generated by Claude Code