Skip to content

fix(protocols): dissect LINKTYPE_RAW by version nibble; add the BSD loopback layer for LINKTYPE_NULL/LOOP (#1502, #1574) - #1594

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1502-1574-raw-null-linktypes
Oct 10, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1502-1574-raw-null-linktypes

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 10, 2026 •

Copy link
Copy Markdown
Owner

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly

  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])

  • Use Preview tab to see how your pull request will actually look like

  • Searched for similar pull requests

  • Followed the coding style (make pylint, make mypy, make isort) — Makefile flags on the changed modules, no new findings

  • make test passes, and a test case covers the change — narrow selections only, not the full suite

  • Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible — N/A — centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • release — bumps the version or rolls up a distribution
  • chore — anything else

Description of your pull request and other information

  • fix(protocols): the default engine does not dissect LINKTYPE_RAW (101) in PCAP and PCAP-NG #1502: Frame.__proto__ and PCAPNG.__proto__ had no LinkType.RAW entry. Both now dispatch raw-IP data as IPV4/IPV6 by its version nibble, unless RAW is registered. The dpkt engine's _get_protocol does the same.
  • feat(protocols): the default engine has no LINKTYPE_NULL (BSD loopback) dissector #1574: there was no BSD loopback dissector. The new pcapkit.protocols.link.loopback.Loopback reads the 4-octet address family: big-endian under LINKTYPE_LOOP, inferred from the octets under LINKTYPE_NULL, with the byte order kept in info. It dispatches 2 to IPv4 and 24/28/30 to IPv6, and is registered for LINKTYPE_NULL and LINKTYPE_LOOP in both tables and in the dpkt engine.
  • Gap(1502) is deleted. All four labels pass, and many_interfaces.pcapng now gives 36 IPv4 datagrams in default, scapy and dpkt alike. The agreement module shows dpkt and scapy matching default on all 207 labels each.
  • Parse→rebuild, make→parse and from_data(info.to_dict()) are byte-exact in both byte orders, in PCAP and PCAP-NG.
  • examples/generators/dispatch.py now pins the new entries and walks Loopback.__proto__.
  • Header.make defaults network to LinkType.NULL, so a frame built without a link type now parses as Loopback:… rather than NULL. Round trips are unchanged.

Closes #1502
Closes #1574

@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 10, 2026
@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 10, 2026
@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Coverage: 89.74% (unit tier, Python 3.14, ab543c46f, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 3 20 3 95.16%
pcapkit/const 18858 956 2366 870 91.04%
pcapkit/corekit 2366 67 760 35 96.48%
pcapkit/dumpkit 275 3 100 3 98.40%
pcapkit/foundation 2875 150 1008 38 93.87%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 17089 227 4566 192 98.05%
pcapkit/toolkit 770 96 266 6 86.68%
pcapkit/utilities 494 4 146 4 98.75%
pcapkit/vendor 4409 2342 1006 157 43.25%

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

@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: NEEDS CHANGES at 58a4ad7b4. The required change is small. This was a Sonnet cross-review; the author ran Opus.

Needed:

  • LOOP (108) must be network byte order, not guessed. The PR infers the byte order for LINKTYPE_LOOP as well as for NULL, but its own docstring and docs say LOOP is network order. Wireshark's dissect_loop reads it with tvb_get_ntohl, and scapy agrees. Because of the guess, a LOOP frame 02 00 00 00 reads as IPv4 here but as Raw in Wireshark and scapy. Two fixes:
    • Gate the inference on link type 0.
    • Correct test_captures_dissect_and_rebuild_every_layer, which feeds little-endian headers under 108.

Also worth folding in:

  • engines/dpkt.py's _get_loopback_protocol builds a new dpkt.loopback.Loopback subclass for every packet. That costs 20.6 µs against 5.8 µs to parse, so cache it.

Confirmed:

  • Round trips: 1,866 parse→rebuild cases are byte-exact, and so are from_data(info)/to_dict() and make→parse, for families 0–299 in both orders and for 1–3-octet truncated headers.
  • NULL byte-order inference: it can't misread a family. No family below 0x10000 is still below 0x10000 once byte-swapped, and the inference matches tcpdump's null_if_print.
  • Not breaking: Header.make already defaulted to NULL, and header bytes are unchanged. Only the parse side changes, which is the point of both issues.
  • Agreement: dpkt matches 207 of 207 cases and scapy 207 of 207. many_interfaces.pcapng gives 36 datagrams on default, dpkt and scapy alike.
  • Inventories: tests/project and the tier guard pass (582).
  • Merges: it merges cleanly with fix(protocols): drop the warnings of a next layer replaced by Raw (#1580) #1588 and fix(toolkit): the scapy adapters stop at the IP datagram's declared length (#1584) #1589.

@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 Oct 10, 2026
@JarryShaw
JarryShaw force-pushed the fix/1502-1574-raw-null-linktypes branch from 58a4ad7 to 789221b Compare October 10, 2026 07:31
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Oct 10, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: NEEDS CHANGES at 789221ba1: one small round-trip fix. This was a Sonnet delta review; the author ran Opus.

Needed: Loopback.from_data(info) must give back the same info. The bytes come back exact, but a LOOP frame stored in little-endian byte order changes meaning when rebuilt from its info:

  • Loopback(b'\x02\x00\x00\x00' + IPv4, alias=108) gives family=0x02000000, 'big', payload Raw.
  • Loopback.from_data(p.info) instead gives family=2, 'little', payload IPv4:UDP.

The cause is that the rebuild calls read with no alias, so the byte order is inferred as for NULL. The reviewer counted 32 of 320 direct cases, all of them LOOP-little-endian or family 0. At Frame/PCAPNG level nothing drifts.

The owner's rule is that from_data(info) round-trips exactly. So carry the parsed byte order through make/from_data, for example by honouring info.byteorder when it is given. Then pin info equality in test_little_endian_loop_family_is_not_ipv4; today it checks only .data.

Optional: the lru_cache key covers only dpkt.loopback.Loopback. After importlib.reload(dpkt.ip) alone, the cached class raises TypeError. That reload is contrived, but adding dpkt.ip.IP/IP6 to the key is cheap.

Confirmed:

  • Round trips: 320 direct and 336 PCAP/PCAP-NG round trips are byte-exact, mixed-interface PCAP-NG included.
  • LOOP byte order: LOOP is network order. A little-endian 02000000 under 108 stays Raw.
  • alias plumbing: alias is passed correctly from captures and from user-registered codes.
  • Cache: class identity is stable per (base, linktype), and a cached lookup takes 0.09 µs.
  • Fail-before: 7 failed with the LOOP branch removed.
  • Merges: only the known adjacent-row conflict with fix(engines): the pypcapfile engine reads the PCAP records pypcapfile drops (#1512) #1569 (Gap(1512)/Gap(1502)). Resolve it by deleting both.

@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 Oct 10, 2026
@JarryShaw
JarryShaw force-pushed the fix/1502-1574-raw-null-linktypes branch from 789221b to 4ede9e6 Compare October 10, 2026 07:56
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Oct 10, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 4ede9e652. Round 3 was a Sonnet delta review; the author ran Opus.

  • from_data(info) now round-trips exactly.
    • The round-2 probes give 0 info drift: 320 direct cases (32 before) and 336 PCAP/PCAP-NG cases.
    • I checked 84 direct from_data(info) and from_data(info.to_dict()) cases myself: info and bytes are identical.
  • Normal parsing is unchanged.
    • JSON output for 21 sample captures and synthetic NULL/LOOP captures in both byte orders is byte-identical to round 2.
    • byteorder never reaches read from a capture, so the new ProtocolError is reachable only through a direct Loopback(raw, n, byteorder='native'), which used to be silently ignored.
  • dpkt cache. Class identity is stable per key. After reload(dpkt.ip), ip6, loopback, or all of them, records parse into the current classes; round 2 raised TypeError there.
  • Fail-before: with read ignoring byteorder, 19 tests fail.
  • Tests: link (101), the PCAP/PCAP-NG sample (176), dpkt, the agreement module, tests/project (472) and the capture round trip all pass.

Note for merging: this PR and #1569 delete neighbouring known-failure rows, Gap(1502) and Gap(1512), in tests/foundation/engines/test_engine_agreement_runtime.py. Whichever merges second needs a trivial rebase that keeps both deletions.

@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 10, 2026
…oopback layer for LINKTYPE_NULL/LOOP (#1502, #1574)

- Frame and PCAPNG dispatch LINKTYPE_RAW (101) data as IPV4 or IPV6 by the
  version nibble of its first octet (_dispatch_linktype), unless RAW itself
  is registered; any other version stays Raw.
- New Loopback link layer (protocols/link, schema/link, data/link): a 4-octet
  address family, big-endian under LINKTYPE_LOOP (108) and inferred under
  LINKTYPE_NULL (0) (family_byteorder). The byte order is kept in info and
  read back by from_data, so a rebuild returns the same info. 2 goes to IPv4
  and 24/28/30 to IPv6 through its own __proto__. Registered for NULL and
  LOOP in both tables, exported, and documented.
- dpkt engine: RAW by version nibble; NULL/LOOP through a dpkt Loopback,
  cached per base/IP/IP6 class and link type, that reads the family as
  pcapkit does and names its ip/ip6 payload.
- Delete Gap(1502): all four labels now agree, many_interfaces.pcapng's two
  NULL frames included (36 datagrams in default, scapy and dpkt).
- Dispatch generator pins the new entries and walks Loopback.__proto__.
- New tests for RAW, Loopback (7 families x 2 byte orders x 3 link-type
  hints, info-exact from_data, PCAP and PCAP-NG) and the dpkt engine.

Closes #1502
Closes #1574
@JarryShaw

Copy link
Copy Markdown
Owner Author

Resolve conflicts

@JarryShaw
JarryShaw force-pushed the fix/1502-1574-raw-null-linktypes branch from 4ede9e6 to ab543c4 Compare October 10, 2026 12:24
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at ab543c46f. The verdict carries forward because this push is a rebase only.

Conflicts resolved: rebased onto main 589414eba, still one commit.

  • The only conflict was the adjacent Gap rows in tests/foundation/engines/test_engine_agreement_runtime.py. Both deletions are kept, so fix(engines): the pypcapfile engine reads the PCAP records pypcapfile drops (#1512) #1569's Gap(1512) and this PR's Gap(1502) are both gone.
  • No other change: git range-diff e9f3340f7...4ede9e652 589414eba...ab543c46f differs only in that hunk's context lines.
  • Agreement module: passes on 3.14 (3 passed, 828 skipped, 414 subtests) and on 3.11 (3 passed, 621 skipped, 621 subtests).
  • The PR's own test files: pass one at a time.

@JarryShaw
JarryShaw merged commit 588b24c into main Oct 10, 2026
35 checks passed
@JarryShaw
JarryShaw deleted the fix/1502-1574-raw-null-linktypes branch October 10, 2026 14:48
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 10, 2026
JarryShaw added a commit that referenced this pull request Oct 14, 2026
Third-party engines are best effort (#1687), so the dpkt code that exists
only to match the default engine comes out:

- revert #1662, #1655, #1624, #1622, #1614 and the dpkt hunks of #1605:
  the octet walk and its link-type stash, IPv6 in IPv4 and IPv4 in IPv6,
  refusing MPLS and ISL, the tunnel and extension-header walks, the IPv4
  header checks and the layer bounds
- keep #1690's timestamps, #1594's parser choice, #1621 and #1653
- re-land as guards: a layer dpkt's own lengths misplace (#1646) is
  re-serialised, not refused with ProtocolError; a Total Length below the
  IHL keeps the header whole (#1616), and a header the capture cuts is no
  input; packet2dict and packet2chain take a str payload and a raw frame
- delete the modules #1662 and #1655 added; the 1615/1616 module keeps
  only what its #1616 tests use, as no scapy test imports it now; dpkt's
  comparisons with the default engine assert constants or the capture's
  octets, or are dropped
- drop pypcapfile's VLAN_TAGS, VLAN_TAG_LEN and IPV6_EXTENSION_HEADERS,
  which only dpkt imported, and the 1668 module's _Agreement base, which
  nothing subclasses now
- the docs drop the depth claims and name what dpkt leaves out silently

The dpkt modules, the shared modules that run dpkt, tests/foundation/engines
and the project guards pass on 3.14 and 3.11 (dpkt 1.9.8, pypcapfile 0.12.0).
JarryShaw added a commit that referenced this pull request Oct 14, 2026
Third-party engines are best effort (#1687), so the dpkt code that exists
only to match the default engine comes out:

- revert #1662, #1655, #1624, #1622, #1614 and the dpkt hunks of #1605:
  the octet walk and its link-type stash, IPv6 in IPv4 and IPv4 in IPv6,
  refusing MPLS and ISL, the tunnel and extension-header walks, the IPv4
  header checks and the layer bounds
- keep #1690's timestamps, #1594's parser choice, #1621 and #1653
- re-land as guards: a layer dpkt's own lengths misplace (#1646) is
  re-serialised, not refused with ProtocolError; a Total Length below the
  IHL keeps the header whole (#1616), and a header the capture cuts is no
  input; packet2dict and packet2chain take a str payload and a raw frame
- delete the modules #1662 and #1655 added; the 1615/1616 module keeps
  only what its #1616 tests use, as no scapy test imports it now; dpkt's
  comparisons with the default engine assert constants or the capture's
  octets, or are dropped
- drop pypcapfile's VLAN_TAGS, VLAN_TAG_LEN and IPV6_EXTENSION_HEADERS,
  which only dpkt imported, and the 1668 module's _Agreement base, which
  nothing subclasses now
- the docs drop the depth claims and name what dpkt leaves out silently

The dpkt modules, the shared modules that run dpkt, tests/foundation/engines
and the project guards pass on 3.14 and 3.11 (dpkt 1.9.8, pypcapfile 0.12.0).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

1 participant