Skip to content

fix(engines): dpkt float timestamps, Simple Packet Blocks and raw-IP tracing (#1501, #1520, #1548) - #1550

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1501-1520-dpkt-engine
Oct 10, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1501-1520-dpkt-engine

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 10, 2026 •

Copy link
Copy Markdown
Owner

Please follow the guide below

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

Three dpkt-engine defects, one commit.

  • fix(engines): the dpkt engine passes a Decimal timestamp for nanosecond PCAP #1501: the dpkt reassembly and flow-tracing adapters (pcapkit/toolkit/dpkt.py) passed the reader's timestamp through unchanged, so a nanosecond PCAP handed them a Decimal. They now hand on float(timestamp), which is what pcapkit/toolkit/pcap.py and the scapy toolkit do. The PCAP frame record is still built from the exact timestamp attached to the frame, so nanosecond traces stay byte-exact and their labels now match the default engine's.
  • fix(engines): the dpkt engine's own PCAPNGReader drops Simple Packet Blocks #1520: PCAPNGReader now yields Simple Packet Blocks. Each one is parsed with pcapkit's PCAPNG against interface 0, with data length min(original_len, snaplen). Like the default engine, it dates the block at the epoch, and it keeps the block's original length. The stale .. important:: block in docs/source/pcapkit/foundation/engines/3rdparty.rst that said dpkt drops these blocks is replaced by one sentence describing this.
  • fix(toolkit): the dpkt engine traces no TCP on a raw-IP link type #1548: on LINKTYPE 228/229 dpkt reads a frame as a bare IP/IP6, and a bare packet has no .ip/.ip6 attribute. All four adapters now treat such a packet as its own network layer. Stripping Ethernet from tcp.pcap (228) and http6.cap (229) now gives PCAP traces byte-identical to the default engine's.

Deletes Gap(1501) and Gap(1520) from the engine-agreement table. Every new test fails on main.

Closes #1501
Closes #1520
Closes #1548

@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 force-pushed the fix/1501-1520-dpkt-engine branch from dc52be3 to 46a9e1e Compare October 10, 2026 02:27
@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
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 46a9e1ee7. Cross-reviewed on Sonnet; the author ran Opus.

Not blocking:

@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
@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Coverage: 89.66% (unit tier, Python 3.14, b4702f17c, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18858 957 2366 871 91.03%
pcapkit/corekit 2240 67 722 34 96.32%
pcapkit/dumpkit 258 2 90 2 98.85%
pcapkit/foundation 2780 129 984 47 94.26%
pcapkit/interface 108 7 36 5 91.67%
pcapkit/protocols 16993 230 4546 191 98.02%
pcapkit/toolkit 702 93 232 7 85.65%
pcapkit/utilities 431 4 124 4 98.56%
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

resolve conflicts

@JarryShaw

Copy link
Copy Markdown
Owner Author

On it. #1531 merged, so the worker is rebasing onto main. The conflict is in the Gap table of test_engine_agreement_runtime.py, on adjacent rows. The rebased head gets a delta review.

@JarryShaw JarryShaw added review: pending No verdict for the current head - never reviewed, or the head moved since the last one and removed review: good-to-go Cross-review at the current head says ready; CI state is separate labels Oct 10, 2026
@JarryShaw
JarryShaw force-pushed the fix/1501-1520-dpkt-engine branch from 46a9e1e to f0e0802 Compare October 10, 2026 03:42
@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
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at f0e0802fd. This was a Sonnet delta review; the author ran Opus. The rebase over #1531 is clean.

@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
@JarryShaw

Copy link
Copy Markdown
Owner Author

resolve conflicts

…tracing (#1501, #1520, #1548)

- The dpkt reassembly and flow-tracing adapters passed the reader's
  timestamp straight through, so a nanosecond PCAP gave reassembly and the
  flow tracer a Decimal where every other engine gives a float, and flow
  labels were spelled with nine places. They now hand on float(timestamp),
  as the default and scapy toolkits do; the PCAP frame record is still built
  from the exact timestamp attached to the frame, so traces stay byte-exact.
- PCAPNGReader skipped Simple Packet Blocks, dropping their packets and
  renumbering every later frame. It now parses an SPB with pcapkit's own
  PCAPNG against interface 0, dates it at the epoch as the default engine
  does, and keeps the block's original length. The engine docs no longer
  say that dpkt drops them.
- On a raw-IP link type (228, 229) dpkt reads a frame as a bare IP or IP6,
  which names no ip or ip6 layer, so all four adapters returned None and
  nothing was reassembled or traced. They now take such a packet as its own
  network layer.
- Deletes the Gap(1501) and Gap(1520) rows from the engine-agreement table,
  and the labels=False workaround from the nanosecond PCAP trace test.
@JarryShaw

Copy link
Copy Markdown
Owner Author

On it. #1535 merged as ea883a648, and the author is rebasing onto main (2770404b9). The conflict is in the Gap table in tests/foundation/engines/test_engine_agreement_runtime.py: #1535 deleted its #1513 row next to this PR's #1501 and #1520 rows, and the rebase keeps both deletions. It also re-checks against #1556 and #1560, which merged at the same time. The verdict resets to review: pending on the new head.

@JarryShaw
JarryShaw force-pushed the fix/1501-1520-dpkt-engine branch from f0e0802 to b4702f1 Compare October 10, 2026 04:42
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at b4702f17c, carried forward from f0e0802fd after the rebase onto a3da376dd.

@JarryShaw
JarryShaw deleted the fix/1501-1520-dpkt-engine branch October 10, 2026 05:17
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 10, 2026
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