Skip to content

feat(link): add a radiotap dissector for IEEE80211_RADIOTAP on LinuxSLL - #2509

Open
JarryShaw wants to merge 6 commits into
mainfrom
feat/2502-radiotap-dissector
Open

feat(link): add a radiotap dissector for IEEE80211_RADIOTAP on LinuxSLL#2509
JarryShaw wants to merge 6 commits into
mainfrom
feat/2502-radiotap-dissector

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 31, 2026

Copy link
Copy Markdown
Owner

Please follow the guide below

  • Searched for similar pull requests
  • Followed the coding style (make pylint, make mypy, make isort)
  • make test passes, and a test case covers the change
  • Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible

What is the purpose of your pull request?

  • feat — adds a feature

Description of your pull request and other information

Part of #2460. Adds pcapkit.protocols.link.radiotap.Radiotap, dispatched from LinuxSLL/LinuxSLL1/LinuxSLL2 for ARPHRD_IEEE80211_RADIOTAP instead of falling back to Raw.

Scope (radiotap.org verified; cross-checked against the Linux kernel's ieee80211_radiotap.h/radiotap.c and Wireshark's dissector), as #2502 itself asked for, since the full 802.11 MAC frame behind radiotap is its own protocol family with no dissector here yet:

  • Decoded: the fixed 8-octet start (it_version/it_pad/it_len) and the present-flags chain, including bit-31 extended-word chaining — every word recorded in present, no individual field decoded.
  • Skipped whole: the optional-field region those bits select, kept raw as fields, bounded by it_len.
  • Left opaque: the 802.11 MAC frame from it_len on — Raw/NoPayload, unconditionally, same choice Netlink makes for its own payloads.

it_len is clamped defensively where it disagrees with the actual present-flags chain or the captured length (tested both directions), and every case round-trips byte-exact including the truncated-capture sweep.

Checklist: all 8 registration points, Link's docstring, and LinuxSLL's dispatch.

Tests: new test_radiotap_unit.py (13 tests/46 subtests) covering minimal/skipped/extended-presence/malformed-it_len×2/truncated/make(); updated SLL1/SLL2 dispatch tests, the short-read sweep, and one UNRELATED entry for it_len's 16-bit bound. tests/protocols/link/ is clean (166/924 subtests). The three project tests named in review guidance pass clean in isolation (24/149 subtests); a full tests/project/ run hit the same contention-driven spread the guidance warned about (235 failed, including test_benchmark_workflow_2125.py/test_sweeps_on_latest_only_1732.py — pure CI-script tests, unrelated), so those were isolated instead. pylint/mypy clean on the three new files.

FRAD (#2499), NETLINK (#2507) and GRE (#2508) are each still open on their own PR, so this stays "Part of #2460" rather than closing it.

@JarryShaw
feat(link): add a radiotap dissector for IEEE80211_RADIOTAP on LinuxS…
7f8a832 
…LL (#2502)

- Add `pcapkit.protocols.link.radiotap.Radiotap`: decodes the fixed 8-octet
  radiotap start (it_version/it_pad/it_len) and walks the present-flags
  chain, including bit-31 extended-word chaining, far enough to count its
  words. The optional-field region those bits select is skipped whole
  rather than decoded field by field; the 802.11 MAC frame behind it has no
  dissector in this package yet and is kept as Raw/NoPayload.
- Register the new protocol in all 8 places (pcapkit/__init__.py,
  pcapkit/all.py, protocols/__init__.py, protocols/data/__init__.py,
  protocols/data/link/__init__.py, protocols/schema/__init__.py,
  protocols/schema/link/__init__.py, protocols/link/__init__.py), plus the
  Link base class docstring.
- Wire LinuxSLL's `_import_next_layer` to dispatch ARPHRD_IEEE80211_RADIOTAP
  to Radiotap instead of falling back to Raw.
- Add a docs page, index entry, and changelog entry (1.6.0.rst,
  regenerated CHANGELOG.md).
- Tests: a new unit module covering the minimal header, present-field
  skipping, extended-presence chaining, malformed it_len (both too-small
  and too-large), truncated-capture round trips, and make()/validation;
  updated LinuxSLL1/SLL2 dispatch tests and the short-read round-trip
  sweep; one new UNRELATED entry in the RFC-constant-home test for
  radiotap's own 16-bit it_len bound.
- Build: full tests/protocols/link/ (166 tests, 924 subtests) and the
  targeted tests/project/ modules (test_all_exports_unit,
  test_module_isolation, test_rfc_constants_one_home_1739_unit: 24 tests,
  149 subtests) pass clean; pylint 10/10 and mypy clean on the new files.

The last of #2460's five arphrd_type follow-ups to get a dissector of its
own; FRAD (#2499), NETLINK (#2507) and IPGRE/IP6GRE (#2501) are tracked on
their own reviews.
@JarryShaw JarryShaw added feat Pull requests that add a new capability (feat: subject prefix) review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 31, 2026
@JarryShaw JarryShaw mentioned this pull request Oct 31, 2026
Closed
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 31, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 31, 2026
@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 31, 2026
@JarryShaw

JarryShaw commented Oct 31, 2026

Copy link
Copy Markdown
Owner Author

Cross-review (Sonnet) at 7f8a83267: GOOD TO GO. Wire format confirmed against radiotap.org and the Linux kernel's ieee80211_radiotap.h directly (little-endian layout, it_len's semantics, bit-31 chaining). Independently built the exact malformed it_len-shorter-than-chain case and confirmed the previously-found double-subtraction bug stays fixed -- no octets lost, payload starts at the right offset. All 8 registrations present, dispatch wired correctly in both LinuxSLL/LinuxSLL2. Test counts reproduced exactly (166/924 subtests on the link suite, full tests/project/ clean with no contention this run). PR body correctly says 'Part of #2460'. One wording nit: the changelog calls this 'the last of #2460's five follow-ups' while FRAD/NETLINK/GRE are each still their own open PR -- not a blocker. Ready for merge -- your call.

@github-actions

github-actions Bot commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

Coverage: 94.47% (unit tier, Python 3.15, 7f8a83267, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19738 133 2642 120 98.85%
pcapkit/corekit 2605 74 818 37 96.52%
pcapkit/dumpkit 281 3 100 3 98.43%
pcapkit/foundation 3183 27 1110 31 98.65%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 18585 201 4894 186 98.34%
pcapkit/toolkit 924 36 334 10 95.55%
pcapkit/utilities 502 7 144 5 98.14%
pcapkit/vendor 4510 1888 1034 169 54.89%

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

Copilot AI and others added 2 commits November 1, 2026 05:28
@JarryShaw
Merge remote-tracking branch 'origin/main' into feat/2502-radiotap-di…
850cedd 
…ssector

# Conflicts:
#	CHANGELOG.md
#	docs/source/changelog/1.6.0.rst
#	pcapkit/__init__.py
#	pcapkit/all.py
#	pcapkit/protocols/__init__.py
#	pcapkit/protocols/link/__init__.py
#	pcapkit/protocols/link/linux_sll.py
#	tests/protocols/link/test_linux_sll1_unit.py
#	tests/protocols/link/test_linux_sll2_unit.py
#	tests/protocols/test_short_read_roundtrip_unit.py

Co-authored-by: JarryShaw <15666417+JarryShaw@users.noreply.github.com>
Co-authored-by: JarryShaw <15666417+JarryShaw@users.noreply.github.com>
7917cd9
@JarryShaw

JarryShaw commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

@copilot resolve the merge conflicts in this pull request

Copilot AI commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved the merge conflicts by merging the current base and preserving both FRAD and Radiotap dispatch paths, then validating the targeted Linux SLL/short-read tests. Commit: 7917cd9.

@JarryShaw JarryShaw mentioned this pull request Nov 1, 2026
Open
@JarryShaw

JarryShaw commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

This PR conflicts again. Copilot's merge (7917cd96a) landed at 05:50Z, and #2466 (SSLv2) merged straight after it, also touching pcapkit/all.py.

In flight on this branch: merge current main, keeping both sides, and drop this PR's changelog hunks (per the owner, entries now consolidate in #2483). The rebase note, with the patch-id check and whether the existing verdict still stands, follows the push.

@JarryShaw JarryShaw mentioned this pull request Nov 1, 2026
Open
1 of 5 tasks
@JarryShaw
Merge remote-tracking branch 'origin/main' into feat/2502-radiotap-di…
587bf6a 
…ssector

Conflicts resolved by keeping both sides, main's entries first:
- exports and imports: Netlink and Radiotap side by side, plus GRE from main
- LinuxSLL._import_next_layer: Netlink, FRAD, GRE, then Radiotap
- link/index.rst toctree, short-read CASES, 1739 UNRELATED map
- LinuxSLL1/2 tests: both dispatch tests kept; the kept-raw loop is
  now empty, every _ARPHRD_NOT_ETHERTYPE member having a dissector
- changelogs: both sides' entries kept (dropped in the next commit)
@JarryShaw
docs(changelog): drop the Radiotap entries, centralised in #2483 (#2502)
671102e 
Restore CHANGELOG.md and docs/source/changelog/1.6.0.rst to main's
content; the Radiotap entry moves to the shared changelog pull request.
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: good-to-go Cross-review at the current head says ready; CI state is separate labels Nov 1, 2026
@JarryShaw

JarryShaw commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

Merge with origin/main

Old head 7917cd96a (Copilot's merge) → new head 671102ef6, a fast-forward with no force: 587bf6aaa merges 94185da93 and 671102ef6 drops this PR's changelog hunks (entries now consolidate in #2483).

Patch-id: not identical. Outside the changelog files, the resolution changed LinuxSLL._import_next_layer's Returns docstring (main's Netlink/FRAD/GRE wording kept, plus a Radiotap clause) and test_the_non_ethertype_arphrds_are_kept_raw in the SLL1/SLL2 tests: every member of the set now has a dissector, so the loop is (). Everything else only moved position: Radiotap now sits after Netlink in __all__, the imports and the toctree.

Found in Copilot's merge: it removed ARPHRD.FRAD from _ARPHRD_NOT_ETHERTYPE. That contradicts the set's comment and frad.py. Parses are unchanged, but _resolve_next_layer_proto(ARPHRD.FRAD, 0x0800) now returns an EtherType. A one-line restore with a test follows on this branch. The now-vacuous test goes to #2510, which is rebuilding this dispatch.

Verdict: the earlier GOOD TO GO was at 7f8a83267 and never covered 7917cd96a or this merge, so the label is now review: running and a Sonnet review is in flight. Tests run on the new head: the radiotap, SLL1/SLL2, FRAD, GRE, NETLINK and SSLv2 modules, the dispatch-registry and short-read suites, and the tests/project export/census/changelog checks all pass.

@JarryShaw
fix(link): keep ARPHRD.FRAD in LinuxSLL's non-EtherType set (#2502)
b1f1f57 
7917cd9 dropped ARPHRD.FRAD from _ARPHRD_NOT_ETHERTYPE, so
_resolve_next_layer_proto turned FRAD's protocol_type into an EtherType.
Restore it where main has it, and pin it in both LinuxSLL test modules.
@JarryShaw

JarryShaw commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

Pushed b1f1f57b7 (fast-forward over 671102ef6): ARPHRD.FRAD is back in _ARPHRD_NOT_ETHERTYPE, so the set is identical to main's again. A new SLL1/SLL2 test asserts that FRAD is in the set and that _resolve_next_layer_proto(ARPHRD.FRAD, 0x0800) stays a plain int. It fails on 671102ef6 and passes now. The label stays review: running. A Sonnet review of the merge is still in flight, and a focused one covers this commit.

JarryShaw added a commit that referenced this pull request Nov 1, 2026
@JarryShaw
docs(changelog): consolidate the 1.6.0 entries (#2449)
db01d1b 
- add entries for the user-visible changes merged since 1.5.1 that had
  none: LinuxSLL, ICMP/ICMPv6, IGMP, TLS/DTLS, SSLv2, DTLS reassembly,
  IP_Pseudo, the IPv6 extension-header walk and the IPv6_Ext id() and
  membership fixes
- add the Radiotap entry from #2509
- drop the FRAD, Netlink and GRE entries' stale "remain Raw" and
  "tracked separately" notes, and cite each entry's own PR and issue
- record the centralised-changelog rule this PR introduces
- regenerate CHANGELOG.md; util/changelog_md.py --check passes
@JarryShaw

JarryShaw commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

Cross-review (Sonnet) of the merge at 671102ef6: NEEDS CHANGES. The merge itself is faithful: it differs from git's own auto-merge only in the conflicted files, and every Radiotap, Netlink, GRE, FRAD and SSLv2 entry is present. Of its two major findings, the FRAD regression was fixed by b1f1f57b7, whose focused review is GOOD TO GO. Still open:

Fix round: delete the dead fallback and its empty tests (#2510 removes that chain anyway), add the truncated-FRAD test, and fix the citations. It is pushed once queued runs drop below 20.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: running A cross-review is in flight against the current head - no verdict yet labels Nov 1, 2026
This was referenced Nov 1, 2026
Open
Open

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

Copilot code review Copilot

Labels

feat Pull requests that add a new capability (feat: subject prefix) review: needs-changes Cross-review at the current head says changes are required; see the verdict comment

Projects

PyPCAPKit
Status: In review

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@JarryShaw