Skip to content

fix(toolkit): pyshark traces no TCP inside encapsulations the default engine leaves raw (#1639) - #1670

Merged
JarryShaw merged 2 commits into
mainfrom
fix/1639-pyshark-encapsulated-tcp
Oct 13, 2026
Merged

JarryShaw merged 2 commits into
mainfrom
fix/1639-pyshark-encapsulated-tcp

Conversation

@JarryShaw

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

Cause. pyshark traced whatever TCP tshark dissects. tshark dissects TCP inside DLT 12 and 14 (it reads both as raw IP), Linux SLL, MPLS, PPPoE, LLC/SNAP, Cisco ISL, GRE, VXLAN and L2TP, and the default engine leaves all of these raw.

Fix. Every frame tshark finds TCP in now takes #1642's octet path (pcapkit/toolkit/pyshark.py:958): the outermost IP layer by the capture file's link type, then IP-in-IP only. Nothing is traced where the default engine reads no IP, or where tshark's IP layer is elsewhere. _network matches that layer's destination address where tshark reports it (:835), because behind an LSRR or SSRR route not yet used up tshark's ip.dst is the route's last address, in the option. OCTETS_FILTER gains tcp (:356). The second tshark pass costs about 0.1 s per capture with TCP: 0.12 s against 0.23 s median on 3 frames. On 5,000 TCP frames it is lost in run-to-run noise (28.8–29.2 s on both). If the octets can't be read, the frame is traced as tshark dissects it, as before.

Measured on 3.11 with tshark 4.6.9, against main. The sweep covers 18,050 frames on DLTs 0, 1, 12, 14, 101, 108, 113, 228, 229 and 276 plus PCAP-NG, and 10,080 more behind LSRR, SSRR and Record Route options (live, used up and malformed). Result: 0 new disagreements, 10,685 fixed, 126 left (#1626 and FRAME_LAYER_LIMIT, below). 2,357 of the fixed frames are in 12 source-route captures that main's pyshark aborted with a bare AttributeError, because tshark gives no ip.dst behind a length-9 route. This branch reads them through.

Tests. New tests/toolkit/test_pyshark_encapsulated_tcp_1639_unit.py. With pcapkit/ at main, all 11 tests fail (32 failures). On this branch's previous revision, its source-route cases fail (3 tests, 5 failures). Three existing files pinned #1642's interim fallback and are updated, as before.

Not fixed here, unchanged from main:

  • TCP behind Mobility or HIP in IPv6, which is fix(toolkit): dpkt, scapy and pyshark drop TCP and IPv4 behind some IPv6 extension-header chains #1626, and IP-in-IP past FRAME_LAYER_LIMIT.
  • Where the octets can't be read, the fallback keys a source-routed segment by tshark's ip.dst, and raises AttributeError where there is none.
  • pyshark 0.6's TSharkCrashException (retcode 255) is a race that also happens on main. It hits test_engine_agreement_runtime.py about 0.3 times per run on both: 11 in 37 runs on main, 12 in 42 here.

Closes #1639

@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: good-to-go Cross-review at the current head says ready; CI state is separate labels Oct 13, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 6890f38f6. Round 2 passed a Sonnet cross-review: three fresh sweeps of 28,352 independently built frames showed 0 new disagreements and 0 new errors against main. Round 1 had come back NEEDS CHANGES for an LSRR/SSRR disagreement in _network. I re-ran the 1639, 1625, test_pyshark_unit and 1604 agreement tests on 3.11 with tshark 4.6.9: 11, 28, 38 and 3 passed.

Round 2's fix: tshark moves ip.dst to the route's last address while an LSRR or SSRR route is still live. _network now matches the destination at the position tshark reports for it (ip.dst.pos), rather than at the header's place.

Unverified:

  • behaviour on tshark versions other than 4.6.9, which CI's engine tests will exercise;
  • the cost, about +0.1 s per capture with TCP, which was not re-measured.

A TSharkCrashException in test_engine_agreement_runtime.py predates this PR and occurs at the same rate on main.

Left for a follow-up, being filed: the _from_layers fallback still raises a bare AttributeError when tshark reports no ip.dst.

@JarryShaw

Copy link
Copy Markdown
Owner Author

resolve conflicts

@JarryShaw

Copy link
Copy Markdown
Owner Author

Recorded: resolving the conflicts. With #1663 merged, the only conflict is in tests/foundation/engines/test_ext_chain_limit_agreement_1604_unit.py, whose docstring #1663 rewrote. A worker is rebasing onto main, keeping #1663's state and adding this PR's pyshark edits on top. The rebased head then gets a short Sonnet check, and runs the pyshark tests on 3.11 with tshark, before the label is renewed.

… engine leaves raw (#1639)

- tshark dissects TCP inside link types 12 and 14, which it reads as raw
  IP, Linux SLL, MPLS, PPPoE, 802.3 LLC/SNAP, Cisco ISL, GRE, VXLAN and
  L2TP, all of which the default engine leaves raw, so pyshark traced TCP
  the default engine does not.
- tcp_traceflow now reads every frame tshark finds TCP in from its octets,
  by #1625's path: the outermost IP layer the default engine dissects, by
  the capture file's link type, then IP-in-IP alone. Where it dissects no
  IP there, or tshark's IP layer is elsewhere, nothing is traced.
  OCTETS_FILTER gains tcp, so the second tshark pass serves these frames.
- _network matches tshark's destination address where tshark reports it:
  behind an LSRR or SSRR route not used up, ip.dst is the route's last
  address, in the option, so such frames were not traced.
- A frame whose octets cannot be had is still traced as tshark dissects it.
- Update the #1625 tests that pinned the interim fallback, and drop the
  ipv6-33/pyshark/trace gap from the #1604 table, which this closes.

Tests: new tests/toolkit/test_pyshark_encapsulated_tcp_1639_unit.py.

Closes #1639
@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Coverage: 89.46% (unit tier, Python 3.14, f0c02f0e5, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 3 20 3 95.16%
pcapkit/const 18858 954 2366 868 91.06%
pcapkit/corekit 2366 67 760 35 96.48%
pcapkit/dumpkit 275 3 100 3 98.40%
pcapkit/foundation 3031 152 1074 35 94.23%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 17166 226 4580 190 98.07%
pcapkit/toolkit 1354 275 558 6 77.25%
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: GOOD TO GO at f0c02f0e5. It is rebased onto main at 257580444, after #1663 and #1669, and a Sonnet check confirmed the result. I re-ran the 1604 agreement and 1639 tests on 3.11 with tshark 4.6.9: 3 and 11 passed.

git range-diff shows five of the six files byte-identical. The sixth, test_ext_chain_limit_agreement_1604_unit.py, keeps #1663's state and drops the last Gap, ipv6-33/pyshark/trace. KNOWN_FAILURES is now empty. On main's code that case still fails ("INPUTS: frame 1: extra"), so this PR is what closes it.

@JarryShaw
JarryShaw force-pushed the fix/1639-pyshark-encapsulated-tcp branch from 6890f38 to f0c02f0 Compare October 13, 2026 19:38
@JarryShaw
JarryShaw merged commit 5e7a736 into main Oct 13, 2026
14 of 15 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 13, 2026
@JarryShaw
JarryShaw deleted the fix/1639-pyshark-encapsulated-tcp branch October 13, 2026 20:50
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 13, 2026
JarryShaw added a commit that referenced this pull request Oct 14, 2026
Third-party engines are best effort (#1687), so the pyshark code that
exists only to match the default engine comes out:

- revert #1686, #1670 and #1642: the second tshark pass that read a
  frame's octets, the link type read from the capture file, the 64-layer
  bound, reading past tshark's 802.1Q limit, and keying a source-routed
  segment by ip.cur_rt
- revert #1613's tshark preferences that turned IP reassembly off, with
  its FileCapture assertion in test_runtime_engines.py and its sentence
  in the engines index
- keep #1613's partial-TCP-header guard and #1605's keying by the IP
  header that carries the segment
- re-land #1671's crash fix: a TCP layer whose IP layer lacks ip.src or
  ip.dst is not traced, rather than raising AttributeError
- delete the three test modules those reverts added and the agreement
  checks in test_pyshark_unit.py; the #1611 and #1671 checks assert
  constants

The pyshark test modules, test_runtime_engines.py, test_engine_runtime.py
and the project guards pass on 3.11 (tshark 4.6.9) and 3.14.
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

Development

Successfully merging this pull request may close these issues.

fix(toolkit): pyshark traces TCP inside GRE, VXLAN, MPLS, PPPoE, L2TP, LLC/SNAP and ISL

1 participant