Skip to content

feat(link): decode radiotap's optional fields (#2576) - #2657

Merged
JarryShaw merged 1 commit into
mainfrom
feat/2576-radiotap-fields
Nov 2, 2026
Merged

feat(link): decode radiotap's optional fields (#2576)#2657
JarryShaw merged 1 commit into
mainfrom
feat/2576-radiotap-fields

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Nov 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • feat — adds a feature

Description of your pull request and other information

Radiotap decodes its optional fields as radiotap.org defines them, with each field's alignment counted from the header start. Extended presence words, namespace switches, vendor namespaces and TLV mode are handled. Unknown and vendor fields stay raw, and captured octets round-trip exactly.

Structure, per the ruling in review here:

  • Radiotap.__field__ is a class-level map from FieldType to its reader and maker, registered through Radiotap.register_field and register_radiotap_field (shaped like register_sctp_chunk).
  • Each layout is a concrete RadiotapField schema, re-exported as Radiotap_*.
  • registry-protocol.rst records the rule.

The owner's rulings on #2576:

  • Bits 16 and 17 (RTS and data retries) decode.
  • Bit-by-bit bitmaps decode into IntFlag registries under pcapkit.const.radiotap, crawled from radiotap.org, with undefined bits kept.
  • Member names come from Linux's ieee80211_radiotap.h, or from Wireshark or radiotap.org where Linux has none.

Mixed bitmaps are split by #2647, and short TLV items are decoded by #2655.

A Sonnet cross-review checked the field layouts against scapy and radiotap.org. A second review, of the rework, found decoding identical to the first reviewed head across 6,505 vectors. The two changes that review asked for are in this head.

Closes #2576

@JarryShaw JarryShaw added this to the PyPCAPKit v1.6 milestone Nov 2, 2026
@JarryShaw JarryShaw added feat Pull requests that add a new capability (feat: subject prefix) review: good-to-go Cross-review at the current head says ready; CI state is separate labels Nov 2, 2026
@JarryShaw

JarryShaw commented Nov 2, 2026

Copy link
Copy Markdown
Owner Author

GOOD TO GO at fd4adbbef, from a Sonnet cross-review.

  • Fields: checked against scapy frames and the live radiotap.org layouts, covering alignment after odd fields, extended presence words, namespace switches, vendor namespaces and TLV items. Scapy differs in three places, and in each it is scapy that departs from radiotap.org.
  • Registries: every registry regenerates byte-identical from the live pages. Every name matches Linux at the pinned commit.
  • Guard change: following subclasses is sound. Copying the template into 13 crawlers would create drift points.

@JarryShaw JarryShaw mentioned this pull request Nov 2, 2026
Closed
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Nov 2, 2026
@JarryShaw

JarryShaw commented Nov 2, 2026

Copy link
Copy Markdown
Owner Author

I think FIELDS should be something like __field__ registry map in the Radiotap class with register methods and APIs.

Layout and SubFields should be concrete Scehma classes defined instead of dynamically generated in the registry map.

@JarryShaw

JarryShaw commented Nov 2, 2026

Copy link
Copy Markdown
Owner Author

The owner asks for two structural changes, paraphrased:

  1. FIELDS becomes a class-level registry. It turns into a __field__ map on Radiotap, with register methods and the registry APIs that registry-protocol.rst describes for __proto__ and __option__, instead of a module-level table.
  2. Concrete schemas. Each field's layout is a concrete schema class, defined like any other schema, instead of being built from Layout and Subfield entries inside the map.

Decoded output, round trips and the flag registries stay as reviewed. The label is review: needs-changes and CI on this head is cancelled while the rework is done. A cross-review follows.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: good-to-go Cross-review at the current head says ready; CI state is separate labels Nov 2, 2026
@JarryShaw
JarryShaw force-pushed the feat/2576-radiotap-fields branch from fd4adbb to 2392363 Compare November 2, 2026 18:59
@JarryShaw JarryShaw added review: pending No verdict for the current head - never reviewed, or the head moved since the last one and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Nov 2, 2026
@JarryShaw JarryShaw mentioned this pull request Nov 2, 2026
Closed
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one review: running A cross-review is in flight against the current head - no verdict yet labels Nov 2, 2026
@JarryShaw

JarryShaw commented Nov 2, 2026

Copy link
Copy Markdown
Owner Author

NEEDS CHANGES at 239236327, from a Sonnet cross-review. Decoding is fine: across 6,505 vectors, its output is identical to fd4adbbef. __field__, register_radiotap_field and the concrete schemas match the #2657 ruling, and every leg passes. Three items:

  1. Test Radiotap.register_field's overwrite warning and its silent no-op for an equal entry. The review removed each of those behaviours in turn and no test failed.
  2. Write the feat(link): decode radiotap's optional fields (#2576) #2657 ruling (class-level __field__ with register APIs, concrete RadiotapField schemas) onto registry-protocol.rst under Registration Wrappers.
  3. Refresh this PR's body for the rework; I'll do that.

@JarryShaw
feat(link): decode radiotap's optional fields (#2576)
53e71b4 
- Decode the field-data region into Radiotap.decoded, one entry per presence
  bit, bits 16 and 17 included: alignment from the header start, namespaces,
  raw vendor data, extended words and TLVs; the rest ends as an Unknown.
- Read and make each type through Radiotap.__field__, keyed by FieldType, with
  register_field and the register_radiotap_field wrapper for new types.
- Give each type a RadiotapField schema whose fields, size and __align__ are
  its radiotap.org layout; bitmaps are EnumFields of their flag registries.
- Add pcapkit.const.radiotap, FieldType and fourteen flag registries named
  after Linux's macros, each with a crawler scraping its radiotap.org page.
- Tests: spec, radiotap-library and tcpdump vectors, both round trips, a
  registered type, a radiotap-field option family, the registrar tables.

The protocols/link, const, vendor, project, foundation and corekit legs pass.
@JarryShaw
JarryShaw force-pushed the feat/2576-radiotap-fields branch from 2392363 to 53e71b4 Compare November 2, 2026 19:49
@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Nov 2, 2026
@JarryShaw

JarryShaw commented Nov 2, 2026

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 53e71b47a, rebased onto fc407c383 with no conflicts. Both review changes are in, and I checked the diff myself:

  1. test_register_field_warns_on_overwrite checks the warning on overwrite and the silent no-op when the entry is equal. Each of the two mutations the review used now fails it.
  2. registry-protocol.rst records the feat(link): decode radiotap's optional fields (#2576) #2657 ruling under Registration Wrappers.

The body is refreshed for the rework. Decoding is unchanged, and the link, foundation, dumpkit, const and vendor legs pass.

@JarryShaw
JarryShaw merged commit 0c66303 into main Nov 2, 2026
59 checks passed
@JarryShaw
JarryShaw deleted the feat/2576-radiotap-fields branch November 2, 2026 20:05
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Nov 2, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Nov 2, 2026
@JarryShaw JarryShaw mentioned this pull request Nov 2, 2026
Closed
This was referenced Nov 2, 2026
Closed
Merged
JarryShaw added a commit that referenced this pull request Nov 2, 2026
@JarryShaw
docs(changelog): fold radiotap's field decoding, the MH and DSS lengt…
25659fd 
…h refusals and the Windows leg's argument file into 1.6.0 (#2449)

- Breaking: MH(raw) and TCP(raw) (#2683) raise ProtocolError for an MH
  message whose Header Len is short of its fields, or a DSS option of a
  length its flags do not give, where 1.5.x read the gap as zeros and
  rebuilt it longer; a cut MH keeps its declared Header Len. The entry gives
  the old and new results, measured against v1.5.1.
- Folded into unreleased entries: radiotap's decoded optional fields,
  Radiotap.__field__ with register_field and register_radiotap_field, the
  RadiotapField schemas and pcapkit.const.radiotap with its crawlers (#2657),
  and cut TLV items decoding zero-padded (#2690), into the Radiotap entry,
  with the IEEE 802.11 entry's Flags clause corrected; #2657's ruling on code
  to layout tables, into the conventions entry; the Windows leg's argument
  file (#2681), into the CI legs entry, as the cycle's other CI changes have
  entries.
- No entry for the test-only 42a6133.
- CHANGELOG.md regenerated; util/changelog_md.py --check passes.
This was referenced Nov 2, 2026
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

feat Pull requests that add a new capability (feat: subject prefix)

Projects

PyPCAPKit
Status: Done

Milestone

PyPCAPKit v1.6

Development

Successfully merging this pull request may close these issues.

feat(link): decode radiotap's optional fields

1 participant

@JarryShaw