refactor(toolkit): revert dpkt's agreement-only code (#1708)

[JarryShaw]

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)

• make test passes, and a test case covers the change. Narrow runs only, listed below.

• 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

Phase 2 of #1708, for dpkt, per the plan on #1687.

• Reverted: fix(toolkit): dpkt walks IP layers off the octets as the default engine does (#1609, #1626, #1651, #1654) #1662, fix(toolkit): dpkt reads IPv6 in IPv4, and nothing behind MPLS or Cisco ISL (#1637, #1638, #1646) #1655, fix(toolkit): dpkt and scapy find TCP in IPv4 where the default engine does (#1606, #1617) #1624, fix(toolkit): dpkt hands IPv4 reassembly what the default engine does (#1615, #1616) #1622 and fix(toolkit): dpkt and scapy check IPv4 headers as the default engine does (#1607) #1614, plus the dpkt hunks of fix(toolkit): key tunnelled TCP by the innermost IP header (#1581) #1605. That removes:
  • the octet walk and its link-type stash;
  • IPv6 in IPv4 and IPv4 in IPv6;
  • the refusal of MPLS and ISL;
  • the tunnel and extension-header walks;
  • the IPv4 header checks;
  • the layer bounds.
• Re-landed as guards whose tests assert constants or the capture's own octets:
  • a Total Length below the IHL keeps the header whole (fix(toolkit): dpkt cuts the IPv4 reassembly input at Total Length, truncating the header when it is below the IHL #1616);
  • a header the capture cuts short is no input;
  • packet2dict and packet2chain accept a str payload and a raw frame (fix(toolkit): dpkt and scapy find TCP in IPv4 where the default engine does (#1606, #1617) #1624, fix(toolkit): dpkt walks IP layers off the octets as the default engine does (#1609, #1626, #1651, #1654) #1662);
  • the stacked-VLAN ProtocolError (fix(toolkit): dpkt ends the extraction with ProtocolError on 802.1Q then 802.1ad, two 802.1ad, or three VLAN tags #1646): see the decision below.
• Kept: fix(toolkit): dpkt and pypcapfile hand over the correctly rounded timestamp (#1668) #1690's timestamps, fix(protocols): dissect LINKTYPE_RAW by version nibble; add the BSD loopback layer for LINKTYPE_NULL/LOOP (#1502, #1574) #1594's parser choice by link type, fix(toolkit): dpkt reads SYN, FIN and RST from the flags octet whatever the NS bit (#1618) #1621 and fix(toolkit): a copied DecimalTimestamp keeps orig_len under _pydecimal (#1647) #1653.
• Dropped:
  • the modules added by fix(toolkit): dpkt walks IP layers off the octets as the default engine does (#1609, #1626, #1651, #1654) #1662 and fix(toolkit): dpkt reads IPv6 in IPv4, and nothing behind MPLS or Cisco ISL (#1637, #1638, #1646) #1655;
  • pypcapfile's VLAN_TAGS and VLAN_TAG_LEN, which only dpkt imported;
  • the fix(toolkit): dpkt and pypcapfile round a PCAP timestamp twice, so a small ts_sec hands over a float 1 ulp off the default engine's #1668 module's _Agreement base.
• Now silent: TCP in an IP-in-IP tunnel, or behind an AH or IPv6 extension header carried in IPv4, is left out with no warning. So are IPv4 in IPv6 and IPv6 in IPv4. The docs say so.
• Conflicts with refactor(toolkit): revert scapy's agreement-only code (#1708) #1724 (the scapy revert) in five docs files and two shared tests. Whichever merges second gets rebased.

Decision needed (#1646). dpkt's own lengths can misplace a layer, after 802.1Q then 802.1ad, or ISL then 802.1Q. This PR then re-serialises that layer from dpkt's fields, so the frame's data is kept, but those octets are dpkt's rather than the capture's. A zero checksum or length is recomputed. The alternative is to return None and drop the frame from reassembly and tracing. Re-serialising is recommended. Which do you want?

Tests:

• The dpkt modules, the shared modules that run dpkt, and tests/foundation/engines:
  • 3.14: 272 passed;
  • 3.11 with dpkt 1.9.8: 286 passed.
• Every guard mutation is caught.
• Sphinx gives 63 warnings, the same as main.

[JarryShaw]

Verdict: GOOD TO GO at 3ab380768. Sonnet cross-reviews (author: Opus) checked two rounds. Round 1 confirmed the content and blocked only on #1721 merging. Round 2 checked the rebase.

• Plan coverage: every row of the plan is reverted and nothing on the keep list is. toolkit/dpkt.py is byte-identical to round 1's, and its diff from 6c17c7e1b^ shows only the kept hunks plus the guards.
• Guards: removing each one makes its test fail.
• Rebase: nothing from refactor(toolkit): revert pypcapfile's agreement-only code (#1708) #1721 is undone. Main's pypcapfile sentences are kept and dpkt's are appended. VLAN_TAGS, VLAN_TAG_LEN and _Agreement have no readers left.
• Prose against behaviour: crafted frames confirm each new sentence, with warnings turned into errors:
  • TCP in IP-in-IP or behind headers carried in IPv4 is dropped silently;
  • 4in6 and 6in4 give no IP input;
  • TCP is read past 40 extension headers and 70 nested tunnels, so dpkt applies neither bound.
• Sphinx: 63 warnings, the same set as main.
• For you: the fix(toolkit): dpkt ends the extraction with ProtocolError on 802.1Q then 802.1ad, two 802.1ad, or three VLAN tags #1646 choice in the description.

[github-actions]

Coverage: 89.55% (unit tier, Python 3.15, e79bd2659, Unit Tests run success)

Package	Statements	Missed	Branches	Partial	Cover
pcapkit (top level)	104	3	20	3	95.16%
pcapkit/const	18860	954	2368	868	91.06%
pcapkit/corekit	2384	67	764	35	96.51%
pcapkit/dumpkit	275	3	100	3	98.40%
pcapkit/foundation	2884	144	1010	34	94.20%
pcapkit/interface	138	7	52	5	93.68%
pcapkit/protocols	17174	226	4590	191	98.07%
pcapkit/toolkit	865	198	298	6	74.38%
pcapkit/utilities	508	4	154	4	98.79%
pcapkit/vendor	4417	2342	1010	157	43.38%

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

[JarryShaw]

Resolve conflicts.

[JarryShaw]

Rebasing onto main now, after #1723 and #1724. The conflicts are in five docs files and two shared tests, where #1724 and this PR rewrote the same engine sentences and ENGINES lists. A Sonnet check runs on the result before I push it.

[JarryShaw]

And what exactly is re-serialising? I cannot make the decision without knowing the exact difference there.

[JarryShaw]

On an 802.1Q-then-802.1ad frame (the #1646 shape), dpkt's own header lengths put the IPv4 header at the wrong offset in the captured octets. The adapter then cannot slice that layer from the capture. "Re-serialising" means handing over bytes(copy(layer)) instead: dpkt packs back the fields it parsed (packet2bytes, toolkit/dpkt.py:258).

I measured the difference on the PR head (3ab380768) with hand-built frames, comparing the captured IPv4 octets with what is handed over:

Frame	Handed over vs captured
Checksums set, no trailer	identical
IPv4 Total Length 0 (TSO)	identical (dpkt does not fill in the length)
Ethernet padding after the datagram	the padding is dropped; the captured path trims it by Total Length anyway, so the datagram is the same
IPv4 or TCP checksum 0	differs: dpkt computes and fills in the checksum (0000 becomes 66c8 and 32d0), and everything else is identical

So the only observable change is a zero checksum arriving filled in. The other option, returning None, drops the whole frame from IP reassembly, TCP reassembly and flow tracing. I recommend re-serialising, because it keeps the data and changes only checksums that were zero.

[JarryShaw]

okay, go with re-serialising

[JarryShaw]

Recorded: re-serialising stays, which is what this PR already does. A layer that dpkt's own lengths misplace is handed over as dpkt's packing of it, and a zero checksum arrives filled in. The rebase onto main is in progress, and its Sonnet check follows before the push.

[JarryShaw]

resolve conflicts

[JarryShaw]

Conflicts are resolved locally: rebased onto fb7027b25 as 6418bdb14. Its narrow tests and the Sonnet check on the result are running now, and I push once the check comes back. #1728, which merged just now, does not touch this branch's files.

[JarryShaw]

Pushed: rebased onto 59dd2613d as e79bd2659, and conflict-free with main.

• Docs: the engine sentences in ext.rst, index.rst, internet.rst and protocol.rst keep main's scapy wording and add this branch's dpkt wording.
• The 1666 test: it keeps main's fix(protocols): IPv6.read accepts a header whose Version is not 6 #1680 rewrite with dpkt dropped from ENGINES.
• Two cleanups the rebase forced: pypcapfile's IPV6_EXTENSION_HEADERS is gone, since nothing imports it after refactor(toolkit): revert scapy's agreement-only code (#1708) #1724, and so are the 1615/1616 module's helpers that only the deleted scapy tests used.
• Tests: they pass on 3.14 and 3.11. Sphinx gives 63 warnings, the same as main.

The Sonnet check on this head is still running, so the label reads review: running until it lands.

[JarryShaw]

Verdict: GOOD TO GO at e79bd2659. A Sonnet cross-review checked the rebase (author: Opus).

• Unchanged parts: the patch bodies of the 10 files main did not touch are byte-identical to 3ab380768, including toolkit/dpkt.py and engines/dpkt.py.
• Earlier merges intact: fix(protocols): reject an IPv6 header whose Version is not 6 (#1680) #1723 and refactor(toolkit): revert scapy's agreement-only code (#1708) #1724 are not undone. toolkit/scapy.py and the IPv6 Version check are as main has them.
• Resolved sentences: each one is true of main's scapy and pypcapfile adapters and of this dpkt adapter. A 14-frame probe on 3.11 confirms the dpkt wording.
• The two forced cleanups are correct: pypcapfile's IPV6_EXTENSION_HEADERS has no importer left, and the trimmed 1615/1616 helpers had none either.
• Tests and Sphinx: tests pass on 3.14 and 3.11, and Sphinx gives 63 warnings, the same as main.