Skip to content

fix(mh): keep the reserved bits of 24 BitField flag namespaces on rebuild - #1421

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1418-mh-reserved-bits
Oct 8, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1418-mh-reserved-bits

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

Closes #1418
Refs #1288

All 24 sites name a reserved sub-field, read into the data model (flags_reserved on the ten messages, since MH.reserved is the header octet; reserved on the options, Multi-Prefix extension and per-session bit-rate attribute) and written by make(), default 0. Follows #1345.

Breaking: each of the 24 data classes gains a required field.

Before/after: each frame sets every reserved bit of its site. On main, from_data rebuilt all 24 with those bits zeroed (e.g. Heartbeat fffc → 0000, FBU cfff → c000). Now all 24 are byte-exact.

Tests: new tests/protocols/internet/test_mh_flags_reserved_unit.py, table-driven over all 24 sites: parse→from_data, make→parse, default 0, over-wide value rejected. Without the fix: 120 subtests fail. With it: 6 passed / 120 subtests. test_mh_unit.py stubs updated for the new field (52 passed). test_mh_roundtrip_unit 6 passed, test_option_roundtrip_unit 6 passed, leg protocols/internet 572 OK, tests/project 399 passed.

Rebase note: #1416 is not merged yet. Its KNOWN_GAPS lists these 24 MH entries; delete them when rebasing over #1416.

…uild

Ten messages (BRM, FBack, FBU, FB, HAck, HI, Heartbeat, LRA, SR, UPN),
twelve options (DNS-UPDATE, DMNP, DynIP Multicast Selector, IPv4 Address
Ack, IPv4 DHCP Support Mode, IPv4 HoA Reply/Request, IPv4 Traffic
Offload Selector, MAG Multipath Binding, NAT Detection, Redirect,
Transient Binding), the Multi-Prefix CGA extension and the per-session
bit-rate QoS attribute left bits of their flags word unnamed, so they
were packed back as zero.

Each site now names a 'reserved' sub-field, read into the data model
(flags_reserved on messages, reserved elsewhere) and written by make(),
defaulting to 0.

Closes #1418
Refs #1288
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) breaking Breaks public-facing behaviour or API (apply alongside the type label) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 8, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 009e5f4e0 (Sonnet cross-review; the author ran on Opus).

  • Repro: on main, all 24 sites zero their reserved bits. Two examples: Heartbeat fffc→0000, and FBU cfff→c000. All 24 are byte-exact here, including the widest sites (Traffic Offload Selector and Multi-Prefix, (1,31)) and the odd offsets (IPv4 Address Ack (6,2), Redirect (2,14), Delegated MNP).
  • The new module: with the 3 source files reverted, it fails all 120 site subtests.
  • RFCs: all 24 sites were checked against the fetched RFCs (5846, 5568, 5949, 5847, 5555, 5844, 5535, 5026, 6058, 6463, 6909, 7028, 7148, 7222, 8278, 6705, 7077, 7109, 7161). The new reserved bits are exactly each RFC's reserved bits, and the neighbouring named flags sit at the right offsets. No defined flag is mislabelled.
  • Naming: flags_reserved on the 10 messages and reserved elsewhere, matching fix(mh): keep zero-length PadN, reserved fields and BU/BA flag bits on rebuild #1345.
  • Blast radius: no constructor or hand-built dict outside this PR breaks. The other MH, option, schema and make modules pass. The test_mh_unit.py edits only add reserved.
  • Fixtures: 22 captures, 1588 frames. They differ only by the new keys. All 99 MH layers in options-internet.pcap rebuild byte-exact.
  • Leg: protocols/internet gives 572 tests, 0 failures.
  • test(schema): every BitField namespace covers all of its bits #1416: with its test applied on top, the only failure is the 24 MH KNOWN_GAPS entries going stale. Whichever PR merges second deletes them.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 8, 2026
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Coverage: 89.43% (unit tier, Python 3.14, 009e5f4e0, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18791 968 2352 869 90.90%
pcapkit/corekit 2038 89 634 25 94.76%
pcapkit/dumpkit 234 2 84 2 98.74%
pcapkit/foundation 2637 98 928 48 94.78%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 16529 219 4420 183 98.04%
pcapkit/toolkit 539 74 168 3 84.58%
pcapkit/utilities 429 4 122 4 98.55%
pcapkit/vendor 4409 2342 1006 157 43.25%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw
JarryShaw merged commit 07c74b3 into main Oct 8, 2026
64 of 66 checks passed
@JarryShaw
JarryShaw deleted the fix/1418-mh-reserved-bits branch October 8, 2026 17:23
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 8, 2026
JarryShaw added a commit that referenced this pull request Oct 8, 2026
#1419 (HIP, #1417) and #1421 (MH, #1418) merged after #1416, so all 27
allowlisted sites now name every bit and the exact-match assertion failed
on main. The allowlist stays, empty, for future filed gaps.
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Breaks public-facing behaviour or API (apply alongside the type label) fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(mh): 24 BitField flag namespaces leave reserved bits unnamed, so they rebuild as zero

1 participant