Skip to content

Fix remaining TXOP duration calculation - #1126

Open
mgonzalezlopezudc wants to merge 2 commits into
inet-framework:masterfrom
mgonzalezlopezudc:feat-ht
Open

Fix remaining TXOP duration calculation#1126
mgonzalezlopezudc wants to merge 2 commits into
inet-framework:masterfrom
mgonzalezlopezudc:feat-ht

Conversation

@mgonzalezlopezudc

Copy link
Copy Markdown
Contributor

Summary

  • fix TxopProcedure::getRemaining() to return the unspent TXOP duration
  • preserve the existing zero clamp once the TXOP limit has expired
  • add a focused unit test for timing boundaries and the TXOP lifecycle

Root cause

While a TXOP was active, getRemaining() returned simTime() - start, which is the elapsed duration. Callers therefore received the spent portion of the TXOP as though it were the remaining transmission budget.

The corrected expression returns start + limit - simTime() before expiry and zero afterward.

Impact

Code that uses the remaining TXOP duration can now make scheduling decisions from the actual available budget. The inactive-state behavior is unchanged: querying before the TXOP starts or after it ends still raises a runtime error.

Validation

  • source ./setenv -q && make MODE=release -j$(nproc)
  • source ./setenv -q && inet_run_unit_tests -m release -f 'Ieee80211TxopProcedure_1\.test'

The focused test covers the inactive state, the full limit at startup, partial elapsed time, exact expiry, post-expiry clamping, and the state after ending the TXOP.

TxopProcedure::getRemaining() incorrectly returns the elapsed TXOP duration while the TXOP is active. This causes callers to treat spent time as the remaining transmission budget.

Return the configured TXOP limit minus the elapsed duration while preserving the existing zero clamp after expiry.

Add a focused unit test covering the inactive state, the full limit at startup, partial elapsed time, exact expiry, post-expiry clamping, and the state after ending the TXOP.
The corrected TxopProcedure::getRemaining() now reports the remaining TXOP budget instead of elapsed duration. HcfFs::hasMoreTxOps() therefore makes different frame-continuation decisions in the MIPv6 roaming Wi-Fi scenario.

Record the deterministic trajectory produced by the fix: 7ed8-bee3/~tNlb replaces c8dc-27c2/~tNlb in the focused regression baseline. The narrow fingerprint test passed after the update.
@mgonzalezlopezudc
mgonzalezlopezudc marked this pull request as ready for review August 15, 2026 12:15

@devin-ai-integration devin-ai-integration Bot 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.

Devin Review found 1 potential issue.

View 1 additional finding in Devin Review.

Open in Devin Review

/examples/ipv6/mipv6/, -f omnetpp.ini -c Handover -r 0, 70s, aa29-a8c5/~tNlb, PASS, wireless EthernetMac
/examples/ipv6/mipv6/, -f omnetpp.ini -c RouteOptimizationTwoCNs -r 0, 60s, 068b-23aa/~tNlb, PASS, wireless EthernetMac
/examples/ipv6/mipv6roaming/, -f omnetpp.ini -c Roaming -r 0, 70s, c8dc-27c2/~tNlb, PASS, wireless EthernetMac
/examples/ipv6/mipv6roaming/, -f omnetpp.ini -c Roaming -r 0, 70s, 7ed8-bee3/~tNlb, PASS, wireless EthernetMac

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 A stored reference result for a wireless roaming test was replaced even though that test cannot be affected by the change

The expected result for the mobile-IPv6 roaming test is overwritten with a new value (7ed8-bee3/~tNlb at tests/fingerprint/mipv6-refactoring.csv:13) even though that simulation never exercises the changed prioritized-transmission code, so a genuine unrelated change in behavior is silently accepted as the new baseline.
Impact: A real behavior change (or nondeterminism) in the roaming scenario is hidden, and future runs will be compared against a value that was never justified.

Why the roaming scenario cannot exercise TxopProcedure

TxopProcedure only exists inside Edcaf, which is instantiated inside Hcf, and Hcf is created only when qosStation is true (src/inet/linklayer/ieee80211/mac/Ieee80211Mac.ned:86 and :116). examples/ipv6/mipv6roaming/omnetpp.ini never sets qosStation (it configures **.wlan*.mac.dcf.channelAccess.cwMin), so the non-QoS DCF path is used and TxopProcedure::getRemaining() is never called.

Consistently, the actually TXOP-sensitive baselines (e.g. tests/fingerprint/examples.csv:657-661, tests/fingerprint/showcases.csv:316-317,344) were left untouched — which is plausible because the only caller, src/inet/linklayer/ieee80211/mac/framesequence/HcfFs.cc:65, evaluates getRemaining() > 0 and old/new expressions differ only at the exact expiry instant. That makes the single updated non-QoS row inconsistent with the rest of the change.

Prompt for agents
The fingerprint baseline for /examples/ipv6/mipv6roaming/ (Roaming config) was changed from c8dc-27c2/~tNlb to 7ed8-bee3/~tNlb in tests/fingerprint/mipv6-refactoring.csv, but this scenario runs the non-QoS DCF path (qosStation defaults to false in src/inet/linklayer/ieee80211/mac/Ieee80211Mac.ned and the ini never enables it), so TxopProcedure is not even instantiated and the getRemaining() fix cannot alter it. Re-run the roaming fingerprint on the base commit and on this branch to determine whether the difference is real; if the hash differs without this code change, the scenario is nondeterministic or the change came from elsewhere and the baseline update should be reverted/investigated rather than re-recorded. Also confirm whether the genuinely TXOP-dependent baselines (examples.csv wireless/qos and adhoc/qos rows, showcases.csv wireless/qos, txop, blockack, aggregation rows) still pass.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@mgonzalezlopezudc mgonzalezlopezudc Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Root cause: bookkeeping failure to the July 802.11 on-wire serialization batch, which updated the duplicate examples.csv row’s data-sensitive fingerprint but omitted mipv6-refactoring.csv.

Historical isolation found the real cause:

  • Commit 632e460 reproduces c8dc-27c2.
  • The sequence-control endian change still reproduces c8dc-27c2.
  • Adding the association-ID allocation changes it exactly to 7ed8-bee3.
  • The AP now writes allocated AIDs into association/reassociation responses at Ieee80211MgmtAp.cc (line 266). Event count remains 59,408, proving this is serialized packet-content change, not changed scheduling.

The corresponding examples.csv row (line 385) already contains the correct network fingerprints; tplx, ~tNl, and ~tND all passed. Its graphical tyf ingredient differed under fake-GUI execution, but that ingredient is unrelated and intentionally unreliable.

I also reran all 20 relevant QoS/HCF baselines across examples.csv and showcases.csv, including the dedicated TXOP showcase. Every baseline passed unchanged, so the TXOP fix requires no additional fingerprint updates.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@levy ready for your review and approval

@tabgab

tabgab commented Aug 16, 2026 via email

Copy link
Copy Markdown

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.

2 participants