reassembly: make the four IPv6 adapters agree, and analyse datagram payloads lazily by JarryShaw · Pull Request #424 · JarryShaw/PyPCAPKit

JarryShaw · 2026-09-16T21:52:06Z

Closes #415. Also lands the second of the three performance findings from #420, which lives in the same files.

The correctness fix — and a data-corruption defect the issue did not name

#415 reported that the four toolkit adapters disagree about whether the 8-octet IPv6 Fragment header belongs to ihl, header and tl. They now agree. Measured on examples/captures/ipv6.pcap frame 13, and independently re-measured:

adapter	`ihl`	`len(header)`	`tl`	→	`ihl`	`len(header)`	`tl`
`toolkit/pcap.py`	48	48	1496	→	40	40	1488
`toolkit/pcapng.py`	48	48	1496	→	40	40	1488
`toolkit/dpkt.py`	40	40	1488	→	40	40	1488
`toolkit/scapy.py`	40	40	1496	→	40	40	1488

That scapy row was silently corrupting reassembled payloads. Its ihl was right, but tl was len(ipv6), which counts the Fragment header — and the machinery writes each fragment over the span tl - ihl (reassembly/ip.py:99-100), so the span was 8 octets too wide. On main:

default  payload=4778
dpkt     payload=4778
scapy    payload=4786      <-- eight stray zeroes per fragment

A reassembled datagram that no other engine agreed with, and nothing reported it. tl is now derived from the payload actually handed over, so tl - ihl == len(payload) holds structurally rather than by coincidence.

ipv6_info.hdr_len keeps its documented meaning ("including extension headers") and protocols/internet/ipv6.py is untouched — pcap/pcapng subtract ipv6_frag.length, undoing exactly the addition ipv6.py:342 makes before the loop breaks.

A second defect #415 conflated with the first

Excluding the Fragment header's octets still left the field pointing at it, which is why header[6] == 44 on all four engines, dpkt and scapy included. :rfc:8200#section-4.5 also requires the Fragment header's Next Header value to move into the last header of the unfragmentable part.

That is a reassembly rule rather than a per-fragment fact — an individual fragment's header genuinely does say 44 — so it went into IP._rectify_header, an identity hook that ipv6.py overrides, applied once for all four engines. It walks the extension-header chain rather than assuming offset 6, since Hop-by-Hop, Routing or Destination Options headers may precede the Fragment header, and it handles AH's different length encoding.

Verified on every engine:

default  len(header)=40  header[6]=17 (UDP)  payload=4778
dpkt     len(header)=40  header[6]=17 (UDP)  payload=4778
scapy    len(header)=40  header[6]=17 (UDP)  payload=4778

The performance fix — `analyze()` was running on every frame

Datagram.packet is now analysed on first read. http.pcap, best of 5, one interpreter per shape:

shape	before	after
`reassembly=True, ip=True`	1881.1 ms	1111.2 ms	−40.9%
the feature's own cost	+834.4 ms	+77.3 ms	−90.7%
GC bill	+79.8 ms	~0

test.pcap: 40.7 → 29.7 ms, feature cost +12.8 → +1.3 ms. Forcing every packet gives 1793.8 ms, confirming the work is moved, not lost.

The mechanism worth knowing: listing packet in __additional__ makes Info store it under a mangled key and map it back, so it stays in to_dict(), keys() and repr() under its own name while attribute reads fall through to __getattr__. __contains__ is overridden so 'packet' in datagram does not trigger a full parse just to answer.

Behaviour-neutrality checked three ways: every observable of a Datagram — attribute read, len, iteration, to_dict, dict(), keys, items, get, in, str, repr, hasattr, copy/deepcopy/pickle — identical before and after, for complete and incomplete datagrams, with packet read first and not at all; all reassembly results over all 14 fixtures in both ip and tcp shapes identical (1509 lines); and all 14 fixtures serialised to tree and json ± reassembly byte-identical over 60 files / 33 MB.

Tests

tests/integration/test_reassembly_engine_parity.py — all four adapters over identical octets. Fails on the pristine tree with 9 subtest failures reproducing IPv6 reassembly: the default/pcapng adapters keep the Fragment header in ihl and header, dpkt/scapy do not #415's table verbatim.
tests/foundation/reassembly/test_ipv6.py — chain walk, AH encoding, idempotence, short headers.
tests/toolkit/test_pcap_unit.py is the only existing test changed: two assertions pinned v6.header == b'V' * 48, i.e. a header with the Fragment header still in it — exactly the value IPv6 reassembly: the default/pcapng adapters keep the Fragment header in ihl and header, dpkt/scapy do not #415 reports as wrong. Now 40, with ihl, tl and the tl - ihl invariant pinned alongside. Its IPv6 fake gained the length attribute the real IPv6_Frag has, and its raw_len now agrees with its own payload.

Full suite: 851 passed, 17 skipped, 0 failed. mypy identical to pristine (124 errors). pylint message multiset identical — cyclic-import excluded from that comparison because it is non-deterministic: two consecutive runs of the unchanged pristine tree gave 115 then 124.

The design change deliberately not made — your call

Filtering unfragmented frames out entirely means adding if not flags.mf and not offset: return None beside the existing DF check in toolkit/pcap.py:56 and pcapng.py.

It buys another two-thirds of what laziness left: reasm-ip on http.pcap 1111.2 → 1050.6 ms, feature cost +77.3 → +24.5 ms. Most of that is the per-frame bytearray(8191) + bytearray(65535) allocation, which is also where the residual GC pressure lives.
It costs consumers this: across all 15 fixtures IP reassembly currently yields 1237 datagrams, of which exactly one is genuinely fragmented (ipv6.pcap frames 13–16). Anyone reading extractor.reassembly.ipv4 as "one entry per IP packet, reassembled where needed" would see http.pcap go 1117 → 0, test.pcap 21 → 0, many_interfaces.pcapng 34 → 0.

It may well be intended — a trivially-complete datagram is the reassembly of a one-fragment packet — which is why it is untouched.

Two follow-ups, neither in scope here: docs/source/pep.rst:293-298 still lists this finding as fully open, and its "making packet lazy is mechanical" half is now done; and reassembly/tcp.py:298 has the same eager analyze, though it is FIN/RST-driven (222 submits per http.pcap pass against 1117), so it wants its own change.

…Pv6-Frag (#415) The four toolkit adapters disagreed about whether the 8-octet IPv6 Fragment header belongs to a reassembly packet's `ihl`, `header` and `tl`. On `examples/captures/ipv6.pcap` frame 13, `(ihl, len(header), tl)` read `(48, 48, 1496)` for `pcap` and `pcapng`, `(40, 40, 1488)` for `dpkt` and `(40, 40, 1496)` for `scapy` -- no two adapters agreeing on all three, and a three-way split on `tl` alone. RFC 8200 section 4.5 settles it: the Fragment header is not present in the reassembled packet, so none of the three may count it. All four now report `(40, 40, 1488)`. * `toolkit/pcap.py`, `toolkit/pcapng.py`: `ihl` and `header` were `ipv6_info.hdr_len` and the whole of `ipv6_info.fragment.header`, both of which count the Fragment header, because `ipv6.py` adds each extension header's length before the Fragment-header check breaks its loop. Subtract the Fragment header's own length back off, undoing exactly that addition. `hdr_len` keeps its documented meaning. * `toolkit/scapy.py`: `tl` was `len(ipv6)`, which counts the Fragment header while its `ihl` did not. The reassembly machinery writes each fragment's payload over the span `tl - ihl`, so that overstated it by 8 and the reassembled datagram came out 4786 octets instead of 4778 -- eight stray zeroes per fragment. Derive `tl` from the payload handed over instead, which is what makes the invariant structural rather than a coincidence. * `toolkit/dpkt.py`: already correct on all three fields; only its `header` comment, which called a `bytes` value a `bytearray`, needed fixing. The `# header length, only headers before IPv6-Frag` comment repeated in all four was already true here and in `scapy`, and is now true in the other two. * `reassembly/ip.py`, `reassembly/ipv6.py`: excluding the Fragment header's octets still left the field *pointing* at it, so every engine reassembled a datagram whose `header[6]` was 44. RFC 8200 section 4.5 also moves the Fragment header's Next Header value into the last header of the unfragmentable part, so `IP._rectify_header` is a hook the IPv6 subclass overrides to do that once for all four engines. It walks the extension header chain rather than assuming offset 6, since Hop-by-Hop, Routing and Destination Options headers may precede the Fragment header. Deliberately not changed: the reassembled header's Payload Length field still describes the first fragment. It cannot be computed from one fragment, and `len(payload)` already gives the datagram's length; the docs now say so. Tests: `tests/integration/test_reassembly_engine_parity.py` runs all four adapters over the same octets -- the frames of `ipv6.pcap`, rewrapped as PCAP-NG for the adapter with no fragmented sample of its own -- and asserts they agree; on the pristine tree it fails with the table above. `tests/foundation/reassembly/test_ipv6.py` covers the chain walk, including the Authentication Header's different length encoding. `tests/toolkit/test_pcap_unit.py` was pinning the old behaviour: it asserted `v6.header == b'V' * 48`, i.e. a header with the Fragment header still in it, for both the PCAP and PCAP-NG adapters. Updated to 40, with `ihl`, `tl` and the `tl - ihl == len(payload)` invariant pinned alongside; its IPv6 fake gained the `length` attribute the real `IPv6_Frag` has and its `raw_len` now agrees with its own payload. 822 passed, 17 skipped. mypy 124 errors either side, pylint message multiset identical (bar `cyclic-import`, which is non-deterministic run to run: 115 then 124 on two consecutive runs of the unchanged tree).

) `IP.submit` called `Protocol.analyze` eagerly on every datagram it built, and IP reassembly builds one for *every* frame -- not only the fragmented ones, because `toolkit/pcap.py` dismisses an IPv4 frame only when its DF flag is set, so a frame with `DF=0, MF=0, FO=0` reaches `ip.py:73`, allocates a buffer and is submitted as a trivially complete datagram. `http.pcap` holds 1117 IPv4 frames, none of them fragmented, and yielded 1117 "datagrams" -- each one a second full parse of a payload most callers never look at. `Datagram.packet` is now analysed on first read. `Deferred` holds the bound analyser, the protocol type and the payload, and `Datagram.__analyse__` runs it once and keeps the result. `Info` builds its mapping view out of `__dict__`, so a naively lazy field disappears from `to_dict()`, `keys()` and `repr()` -- or shows up there as the placeholder. Listing `packet` in `__additional__` is what avoids that: `Info` already stores a field whose name collides with a builtin name under a mangled key and maps it back on the way out, so the key stays in every view under its own name while attribute reads fall through to `__getattr__`. `__getitem__`, `__str__`, `__repr__` and `to_dict` resolve before answering; `__contains__` does not, since `Mapping.__contains__` would otherwise run a full parse just to decide the field exists. `bytes(datagram[:TDL])` is now taken once rather than twice, which is why forcing every `packet` is slightly cheaper than the old eager path rather than equal to it. Measured, `http.pcap`, best of 5, one interpreter per shape: shape before after delta baseline (no reassembly) 1046.7ms 1031.1ms - reassembly=True ip=True 1881.1ms 1111.2ms -40.9% i.e. the feature's own cost +834.4ms +77.3ms -90.7% ... with every packet read - 1793.8ms reassembly=True ip=True, gc off 1801.3ms 1114.7ms i.e. the GC bill +79.8ms ~0ms `test.pcap` (34 frames, 21 trivial datagrams): 40.7ms -> 29.7ms, -27.0%; feature cost +12.8ms -> +1.3ms, -89.8%. Behaviour-neutral, checked three ways rather than assumed: every observable of a `Datagram` -- attribute read, `len`, iteration, `to_dict`, `dict()`, `keys`, `items`, `get`, `in`, `str`, `repr`, `hasattr`, and `copy`/`deepcopy`/`pickle` -- is identical before and after, for a complete and an incomplete datagram and with `packet` read first and not at all; every reassembly result over all 14 fixtures in both the `ip` and `tcp` shapes is identical (1509 lines); and serialising all 14 fixtures to `tree` and `json`, with and without reassembly, is byte-identical to the pre-#415 tree over 60 files and 33 MB. Not changed, deliberately: whether an unfragmented frame should be emitted as a datagram at all. That is a design decision about what consumers see, and it is the owner's to make -- see the report accompanying this branch. `reassembly/tcp.py:298` has the same eager `analyze`, but it is driven by FIN/RST and submits 222 times per `http.pcap` pass rather than 1117, so it is left for a separate change. 827 passed, 17 skipped. mypy 124 errors either side, pylint message multiset identical (bar `cyclic-import`, non-deterministic run to run).

Per review on #424. `Deferred` was in `data/ip.py`, which misfiled it: it holds an analyser, a protocol and a payload, and calls the analyser -- nothing in it is IP-specific. `ReassemblyData` was in `data/__init__.py`, which is the same problem from the other side, a class living in a package initialiser. Both now sit in `pcapkit/foundation/reassembly/data/data.py`, following `pcapkit/protocols/data/data.py`, and `data/__init__.py` is re-exports only. `Deferred` remains importable from `data.ip` for anyone who already had it. Its docstring now separates the mechanism from the case that motivated it: IP reassembly submits a datagram per frame, which is why the eager parse cost 86% of IP reassembly over a capture with no fragments, but nothing about postponing the parse is IP-only. It also records that TCP reassembly builds its `packet` eagerly too and can use this unmodified -- being FIN/RST-driven, 222 submits per `http.pcap` pass against 1117, it is a far smaller cost and wants its own change. Flow tracing was considered and needs nothing: its data models carry no parsed protocol at all -- `Packet` holds the already-extracted frame it was handed, and `Buffer`/`Index` hold a dumper, indices and a label -- and there is no `analyze()` call anywhere in `foundation/traceflow/`. Its per-packet cost was the dumper rebuilding a `Frame`, a different problem fixed separately. Full suite 851 passed, 17 skipped.

…sembly Symmetry, per review on #424. `TraceFlowData` sat in `traceflow/data/__init__.py` exactly as `ReassemblyData` sat in the reassembly one; both packages now keep their shared model in `data/data.py` and their `__init__` as re-exports only, following `pcapkit/protocols/data/data.py`. No `Deferred` here, because there is nothing yet to defer: flow tracing holds no parsed protocol and no payload bytes. `Index` carries a *filename*, a tuple of frame indices and a label, and `submit()` never sees packet data at all -- frames go straight to the dumper as they arrive rather than accumulating. Wiring application-layer analysis into flow tracing is a design change rather than a refactor, and is being raised separately. Full suite unchanged.

…ith IP Per the decision on #424: flow tracing keeps streaming and gains nothing, while TCP reassembly -- which already reassembles the stream and already calls `analyze`, eagerly, in `TCP.submit` -- takes the deferral instead. The reading half of the arrangement is now a `DeferredPacket` mixin in `data/data.py` beside `Deferred`, rather than copied into a second `Datagram`. `IP_Datagram` and `TCP_Datagram` both inherit it and both declare `__additional__ = ['packet']`, which is what makes the field lazy: `Info` stores a builtin-named field under a mangled key and maps it back, so `packet` never lands in `__dict__` and reading it routes through `__getattr__`. `tcp=True, reassembly=True` over `http.pcap`, best of 5: **1441.0 ms to 1099.0 ms, -23.7%**. Smaller in relative terms than IP's -90.7%, as expected -- this path is FIN/RST-driven, 222 submits against 1117 -- but 222 HTTP parses is still 222 parses most callers never read. Output is unchanged, checked rather than assumed: 229 datagram lines across `http.pcap`, `tcp.pcap` and `http6.cap` -- completion, indices, payload lengths and parsed type -- hash identically before and after. All 222 completed datagrams still resolve to `HTTP`, memoised on first read, and `'packet' in datagram` still answers without triggering a parse. Full suite 851 passed, 17 skipped.

…eader values Per review on #424. The header walk had `44` and `51` as module literals when the library already names them: they are now `Enum_TransType.IPv6_Frag` and `Enum_TransType.AH`, so a reader does not have to trust a comment to know which protocol a number means. `_NH_IPV6_FRAG` is gone entirely, since the comparison reads better against the enum member. `_NH_AH` stays as a name -- bound to `Enum_TransType.AH` -- because its comment carries the reason the walk singles that header out at all: AH is the one extension header that does not measure its length in 8-octet units (:rfc:`4302#section-2.2`). `__protocol_type__` is `None` on `IPv6`, `IPv6_Frag` and `AH`, so there is no protocol-class attribute to prefer over the const enum here; checked rather than assumed. The two remaining literals stay literals, and now say why: `_IPV6_HDR_LEN` (40) and `_IPV6_NEXT_HEADER` (6) are byte offsets into the header rather than protocol numbers, so no enumeration carries them. Full suite 851 passed, 17 skipped; IPv6 and TCP reassembly both still resolve their deferred payloads (UDP and HTTP).

…the class Two review findings on #424. `_NH_AH` is gone; the comparison reads `Enum_TransType.AH` directly, with the reason the walk singles that header out moved to the use site rather than living on a constant that existed only to carry it. Four-adapter parity re-checked after the change: 40/40/1488 and `header[6] == 17` on default, dpkt and scapy alike. The docs gap was mine and larger than the comment suggested. `DeferredPacket` was in `data/data.py`'s `__all__` but never re-exported from the package, so autodoc could not import it at all; and `Deferred` was documented in `reassembly/ip/ip.rst` from when it lived in `data/ip.py`, so my new entry beside `ReassemblyData` made it a duplicate registration. The stale `ip.rst` entry is removed -- the class moved, so its documentation moves with it -- and the two `:class:` references in `ipv4.rst` and `ipv6.rst` are repointed. `TraceFlowData` was already documented; what it lacked was the same treatment for the module it now lives in. I also added `.. module::` directives for both `data.data` modules and then removed them again: they double-register everything the autoclass paths already cover, which is what produced the duplicate warning. Clean build: 85 warning/error lines, exactly main's baseline, with all four of `ReassemblyData`, `Deferred`, `DeferredPacket` and `TraceFlowData` rendering and no warning naming `data.data`. Full suite 857 passed, 17 skipped.

Per review on #424: the docs described `ReassemblyData`, `Deferred`, `DeferredPacket` and `TraceFlowData` by their re-export path, which said where to import them from rather than where they are. They are now documented as `...data.data.<class>`, under a `.. module::` directive for each of the two new modules. That is also what the sibling pages already do -- `reassembly/tcp.rst` documents `...data.tcp.Packet` and `ip/ip.rst` documents `...data.ip.Datagram`, both under a `.. module::` naming the defining module -- so the re-export paths were the odd ones out, mine included. The `.. module::` directives are back and no longer duplicate anything: the earlier duplicate-registration warning came from `Deferred` being documented in two places at once (the stale `ip/ip.rst` entry, since removed, plus the new one), not from the directive. Imports are deliberately unchanged. `extraction.py` still imports from `...data`, the public re-export, so the documentation reflects where a class lives while consumers keep a path that does not move when it does. Clean build: 85 warning/error lines, exactly main's baseline, nothing naming `data.data`, all four classes rendering under the canonical path and the `ipv4`/`ipv6` cross-references resolving to it. Foundation tests 163 passed, 11 skipped.

Every figure re-derived against `origin/main` rather than trusted; three claims came out different from the sweep that flagged them. - `pep.rst`: MH options are 71 of 71, not 70 of 71; the four CGA extension `EXPECTED_FAILURES` entries are gone; CGA Parameters is registered at `mh.py:1193`; `SCTP.__proto__` ships 2 PPIDs, not empty; #447 is fixed; the per-frame `Frame` rebuild is gone; options are no longer parsed twice; 105 test modules, not 91; version table gains a `1.5.0b3` row - `pep.rst:412-416`: the removed rebuild was **82%** of a flow-traced extraction, per the measurement in `dumpkit/pcap.py:173-175` -- the 80% figure described reopen plus rebuild together - `pep.rst:417`: retargeted to `ProtocolBase.analyze` (`protocol.py:403`); `analyze` is a member of neither `IP_Reassembly` nor `IP` - `pep.rst:409,418,421-422`: lazy payload analysis already landed in `7073f4343` (#424), which the page contradicted itself about at `:706-709` - `ipv4.rst`: the table documents the data model, so the legacy RFC 791 ToS row expands to five (`pre`/`del`/`thr`/`rel`/`ecn`) and `frag_offset` and `proto` become `offset` and `protocol`; there is no `dsfield` or `dscp` All 15 `ipv4.rst` rows were checked, not only the four reported. Both files parse with docutils; every new cross-reference target imports.

JarryShaw added 2 commits September 16, 2026 17:31

JarryShaw commented Sep 16, 2026

View reviewed changes

Comment thread pcapkit/foundation/reassembly/data/ip.py Outdated

JarryShaw commented Sep 16, 2026

View reviewed changes

Comment thread pcapkit/foundation/reassembly/data/data.py Outdated

Comment thread pcapkit/foundation/reassembly/ipv6.py Outdated

Comment thread pcapkit/foundation/reassembly/ipv6.py Outdated

Comment thread pcapkit/foundation/reassembly/ipv6.py

Comment thread pcapkit/foundation/reassembly/ipv6.py

JarryShaw added 3 commits September 16, 2026 19:22

JarryShaw commented Sep 17, 2026

View reviewed changes

Comment thread pcapkit/foundation/reassembly/ipv6.py Outdated

Comment thread pcapkit/foundation/traceflow/data/data.py

JarryShaw added 4 commits September 16, 2026 20:46

Merge branch 'main' into fix/ipv6-reasm-fragment-header

0f6ba1d

Merge branch 'main' into fix/ipv6-reasm-fragment-header

d9edaff

JarryShaw merged commit 7073f43 into main Sep 17, 2026
23 checks passed

JarryShaw mentioned this pull request Sep 17, 2026

foundation: give IP reassembly the RFC timeout, and trace TCP flows bidirectionally #435

Merged

JarryShaw deleted the fix/ipv6-reasm-fragment-header branch September 18, 2026 20:22

JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) breaking Breaks public-facing behaviour or API (apply alongside the type label) labels Sep 22, 2026

JarryShaw mentioned this pull request Sep 27, 2026

docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

Merged

JarryShaw added this to the 1.5 milestone Oct 6, 2026

Navigation Menu

Uh oh!

reassembly: make the four IPv6 adapters agree, and analyse datagram payloads lazily - #424

reassembly: make the four IPv6 adapters agree, and analyse datagram payloads lazily#424
JarryShaw merged 10 commits into
mainfrom
fix/ipv6-reasm-fragment-header

JarryShaw commented Sep 16, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

JarryShaw commented Sep 16, 2026

The correctness fix — and a data-corruption defect the issue did not name

A second defect #415 conflated with the first

The performance fix — analyze() was running on every frame

Tests

The design change deliberately not made — your call

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

The performance fix — `analyze()` was running on every frame