From 5a938f576e9bb0c4e904c01d80088d3c4269a025 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 16 Sep 2026 15:11:36 -0400 Subject: [PATCH 1/2] docs: record the protocol, reassembly and crypto gaps on the Help Wanted page Catalogues what PyPCAPKit does not yet implement, so the gaps are tracked rather than rediscovered. Every entry below was verified against the code at 5e4378d9b; nothing here is inferred from documentation. New sections in docs/source/pep.rst ----------------------------------- * "DTLS" (under More Protocols, More!!!) -- no dissector and no stub anywhere in the tree; only TLS/SSL has a stub. The registries have moved ahead of it: SCTP DTLS chunk type 65 (pcapkit/const/sctp/chunk.py:77), DTLS Key Management chunk parameter 0x8006 (const/sctp/parameter.py:86), error causes 100-103 (const/sctp/cause_code.py:62,66,70,74), and seven PPIDs -- 47, 66, 67, 68, 69 and 4242 (const/sctp/payload_protocol_identifier.py:181,240,243,246,249,265). DTLS blocks NGAP over DTLS (PPID 66) independently of NGAP itself. * "Registered, But Not Dissected" (under More Protocols, More!!!) -- registry values with no dissector behind them, all counts measured by importing the worktree copy: - LinkType 3 of 219 bound (ETHERNET 1, IPV4 228, IPV6 229), declared twice: pcapkit/protocols/misc/pcap/frame.py:89-96 and misc/pcapng.py:555-562 - TransType 15 of 151 bound, pcapkit/protocols/internet/internet.py:89-107 - EtherType 6 of 160 bound, pcapkit/protocols/link/link.py:74-87 - AppType 2 port numbers of 8182 -- TCP 21, TCP 80 (transport/tcp.py:309-315) and UDP 80 (transport/udp.py:72-78) - SCTP PPID 0 of 75; SCTP.__proto__ ships empty, transport/sctp.py:344-347, and no in-tree caller of register_sctp exists Cheap wins named: OSPF (link/ospf.py:70) and L2TP (link/l2tp.py:76) are implemented but bound to nothing (TransType 89, 115 unbound; both make __index__ raise at ospf.py:229-237 / l2tp.py:231-239); FTP_DATA (application/ftp.py:195) exists but TCP 20 is unbound; HTTP is bound on port 80 only; EtherType 0x88A8 unbound though VLAN parses that shape; LinkType NULL/LOOP/RAW carry bare IPv4/IPv6 which pcapkit dissects. * "Reassembly Beyond IP and TCP" (new top-level section) -- SCTP has no reassembly; grep -rni sctp pcapkit/foundation/ returns nothing. - The DATA chunk fields a reassembler needs are all exposed already: tsn/stream_id/stream_seq/ppid/data and flags.B/flags.E at protocols/transport/sctp.py:1143-1157; the only readers of B/E are in _make_chunk_data (sctp.py:1725-1726), i.e. the construction path. - Only the first DATA chunk of a bundle is dispatched, sctp.py:517-524. - I-DATA (RFC 8260, chunk 64) is not dissected -- SCTP.__chunk__[Chunk(64)] is 'donone'; read() matches only Payload_Data (sctp.py:520), so an I-DATA-only packet yields no payload at all. FORWARD TSN 192 and I-FORWARD-TSN 194 likewise unhandled. - Where the work goes: pcapkit/foundation/reassembly/sctp.py, subclassing Reassembly and implementing reassembly() and submit() (the only two abstract methods; reassembly/reassembly.py:180-195). Registering it is inert today -- Extractor.register_reassembly accepts any name (extraction.py:396-416) but Extractor instantiates only three literal keys (extraction.py:840,848,856) and ReassemblyManager is a fixed-field Info container (reassembly/__init__.py:37-49). Also needs the read-side twin (reassembly/data/__init__.py:37-48), five toolkit adapters, five engine call sites, and interface/core.py:148-171. - Flow tracing is TCP only (Extractor.__traceflow__ has one key; TraceFlowManager one field). TCP tracer closes on FIN not RST -- rst is absent from traceflow/data/tcp.py:43-46 -- and flows are unidirectional (traceflow/tcp.py:99). - No reassembly timeout or eviction anywhere in pcapkit/foundation/; the buffer models carry no timestamp (reassembly/data/ip.py:29-51, data/tcp.py:29-61). 8191 + 65535 bytes preallocated per in-flight IP datagram id (reassembly/ip.py:82-88). - Engines: pyshark disables reassembly (engines/pyshark.py:207-213); pypcap (pypcap.py:295-306) and pcap_ct (pcap_ct.py:380-391) disable reassembly and traceflow; pypcapfile disables IPv6 (pypcapfile.py:225-229, toolkit/pypcapfile.py:348-350). Corrections to text that was already on the page ------------------------------------------------ * ESP. "Only five encryption and five integrity algorithms" is right (verified: 36 Cipher members, 15 Integrity, 5 entries each in CIPHER_SUITES at esp.py:405-426 and INTEGRITY_SUITES at esp.py:432-448) but two of the ten are the no-ops ENCR_NULL and NONE. The "not implemented" list was incomplete; added AES-CTR (RFC 3686), AES-CMAC-96 (RFC 4494), the AES-GMAC family (RFC 4543) with ENCR_NULL_AUTH_AES_GMAC, and the Camellia, RFC 8750 implicit-IV and RFC 9227 MGM families. Added the two structural blockers: IntegritySuite records the digest as a hashlib constructor name (esp.py:360-362, 687-693) so a non-HMAC MAC needs the record widened, and load_cryptography imports only the ciphers submodules (esp.py:223-234). * Mobility Header. The three counts are accurate (24 messages / 14 dispatched, 71 options / 20, 4 CGA extensions / 1; tables at mh.py:527, 552, 583) but the claim that the "# TODO markers sit at the exact dispatch tables" was wrong -- the six markers are at mh.py:1510, 2310, 2422, 2986, 3757, 3871, i.e. the end of each handler block. Both places need editing. Also noted that read and make are symmetric (37 each) so every gap is a gap in both directions, and that mh.py:32-78 imports 32 const/mh sub-registry enums it never uses -- the enumerations for the missing options are already generated. "NEMO" dropped from the option-group description: Mobile Network Prefix (option 6) is implemented, while DNS-UPDATE-TYPE (17), Vendor Specific (19) and Service Selection (20) are missing and belong to none of the named groups. * Fixed one pre-existing rendering bug in the Logging Integration bullet list: a literal nested inside bold. RST has no nested inline markup and does not warn, so the backticks were rendering as visible characters. Checked and confirmed still accurate, so deliberately left alone ---------------------------------------------------------------- * The NotImplemented stub list -- 33 files, all 0 bytes except link/NotImplemented/eapol.py which holds one comment line. Layer grouping in the page matches the tree exactly, including NDP being under link/ and QUIC having no stub. * SCTP registered-but-unimplemented type codes: 30 chunk types / 13 in __chunk__, 32 parameters / 8, 23 cause codes / 13 -- so 17, 24 and 10 missing, exactly as the page says. * Test suite: 91 modules matching tests/test_*.py. * The warnings double-report item: pcapkit/utilities/warnings.py documents the two-channel emission as deliberate, and warn() does still log and then call warnings.warn, so an application routing warnings into logging sees each twice. * ESP/SCTP/PCAPNG/logging/engines all genuinely done, as the page states. * No read/make asymmetry anywhere: NotImplementedError appears only in utilities/exceptions.py definitions, and every UnsupportedCall in pcapkit/protocols/ is a deliberate design decision (extension headers with no payload of their own, container classes with no registry index, abstract bases). Code defects found while auditing, not fixed here -- to be filed as issues ------------------------------------------------------------------------- 1. __proto__ registry pollution. protocol.py:1300 does self.__proto__[proto] on a defaultdict, so a lookup miss inserts the key; internet/internet.py:248 and internet/ipv6.py:409 repeat it. Reproduced: parsing one 24-byte IPv4 packet with proto=1 inserts TransType.ICMP -> Raw into the class-level (shared) Internet.__proto__, after which register_transtype(TransType.ICMP, ...) warns "protocol 1 already registered, overwriting". SCTP._import_next_layer overrides the base method precisely to avoid this and documents the bug at sctp.py:818-831 -- a ready-made template for Frame, PCAPNG, Link, Internet, IPv6, TCP and UDP. 2. beholder's Raw fallback breaks on payload-less schemas. utilities/decorators.py:137 calls self.__header__.get_payload(), which raises ProtocolUnbound (schema/schema.py:461) for SCTP, whose schema has no payload field. Any registered SCTP upper layer that throws takes the whole SCTP layer down to Raw. Already fixed on the open NGAP branch (PR #417), so main only. 3. register_apptype cannot register an SCTP-only service. It raises RegistryError (foundation/registry/protocols.py:633-634) for the ~41 AppType members whose proto is SCTP-only or DCCP-only, and register_sctp is PPID-keyed, so there is no alternative path. Intent unclear. 4. ESP AEAD dispatch falls through. esp.py:743-756 and 782-806 treat anything that is not ENCR_NULL or ENCR_AES_CBC as AES-GCM. Safe today because CIPHER_SUITES holds nothing else, but adding AES-CCM or ChaCha20-Poly1305 to the table without touching decrypt/encrypt would silently mis-process them. 5. Cipher.get() / Integrity.get() invent members for an unknown string via extend_enum(..., -1) (const/esp/cipher.py:229-233, integrity.py:118-122). ESP's own _resolve avoids them, so not reachable through ESP. 6. vendor/pcapng/filter_type.py:25 has DATA = {} with a TODO URL, so the generated enum has no members at all. 7. DPKT and Scapy trace output is affected by the mapping-vs-frame defect that extraction.py:879-881 documents and excludes. PR #412 is in flight over that area. Docs build: 91 WARNING/ERROR lines before this change. The after-count is still running at the time of this commit and is NOT yet confirmed; re-run `PCAPKIT_SPHINX=1 PYTHONPATH=. python -m sphinx -b html docs/source /tmp/out` and compare before treating this as verified. Not done yet: the summary posts to discussion #106 (one reply under the "More Protocols, More!!!" topic comment 2859991, one new top-level comment for the reassembly theme, which no existing topic covers) and the DTLS note under #251. Nothing has been posted to either discussion. --- docs/source/pep.rst | 218 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 205 insertions(+), 13 deletions(-) diff --git a/docs/source/pep.rst b/docs/source/pep.rst index a9cf98b541..6998e04a7c 100644 --- a/docs/source/pep.rst +++ b/docs/source/pep.rst @@ -81,6 +81,10 @@ As things stand that is 17 of the 30 registered chunk types, 24 of the 32 chunk parameters and 10 of the 23 error causes; :doc:`pcapkit/const/sctp` lists them all. +The other thing wanted for SCTP is reassembly, which no protocol beyond IP and +TCP has -- see `Reassembly Beyond IP and TCP`_ below, since it needs work in +:mod:`pcapkit.foundation` rather than in the protocol. + ESP ~~~ @@ -91,12 +95,35 @@ Association is supplied through the protocol keyed :mod:`pcapkit.corekit.context` channel. What is still wanted there is wider algorithm coverage. The two enumerations -under :doc:`pcapkit/const/esp` carry every transform IANA has registered, but -only five encryption and five integrity algorithms are actually applied, as -listed by :data:`~pcapkit.protocols.internet.esp.CIPHER_SUITES` and -:data:`~pcapkit.protocols.internet.esp.INTEGRITY_SUITES`. Specifically not -implemented: ChaCha20-Poly1305 [:rfc:`7634`], AES-CCM [:rfc:`4309`], AES-XCBC -integrity [:rfc:`3566`], and Extended Sequence Numbers. +under :doc:`pcapkit/const/esp` carry every transform IANA has registered -- 36 +:class:`~pcapkit.const.esp.cipher.Cipher` members and 15 +:class:`~pcapkit.const.esp.integrity.Integrity` members -- but +:data:`~pcapkit.protocols.internet.esp.CIPHER_SUITES` and +:data:`~pcapkit.protocols.internet.esp.INTEGRITY_SUITES` apply only five of +each, and two of those ten are the no-ops ``ENCR_NULL`` and ``NONE``. So the +real coverage is AES-CBC and AES-GCM at all three tag lengths for encryption, +and HMAC-SHA1-96 plus the three :rfc:`4868` HMAC-SHA2 truncations for +integrity. + +Not implemented, roughly in the order a capture is likely to want them: +AES-CTR [:rfc:`3686`], AES-CCM at all three tag lengths [:rfc:`4309`], +ChaCha20-Poly1305 [:rfc:`7634`], AES-XCBC-MAC-96 [:rfc:`3566`], AES-CMAC-96 +[:rfc:`4494`], the AES-GMAC family and its ``ENCR_NULL_AUTH_AES_GMAC`` +counterpart [:rfc:`4543`], and then the Camellia, implicit-IV [:rfc:`8750`] and +MGM [:rfc:`9227`] families. Extended Sequence Numbers are not implemented +either, and are the one item on this list that is not simply a table entry: the +high-order 32 bits are never transmitted, so recovering them is stateful, and +they widen both the ICV coverage and the AEAD associated data. + +Two structural notes for anyone starting. Adding an integrity algorithm that is +not an HMAC needs more than a row -- +:class:`~pcapkit.protocols.internet.esp.IntegritySuite` records the digest as +the name of a :mod:`hashlib` constructor, which AES-XCBC and AES-CMAC are not. +And :func:`~pcapkit.protocols.internet.esp.load_cryptography` imports only the +``ciphers`` submodules of :mod:`cryptography`, so the AEAD and CMAC primitives +have to be added there before a cipher suite can reach them. Unsupported +algorithms are refused loudly, when the Security Association is constructed +rather than when a packet is read, so nothing decodes wrongly in the meantime. Mobility Header ~~~~~~~~~~~~~~~ @@ -111,15 +138,116 @@ registry: Heartbeat, Binding Revocation, Localized Routing Initiation and Acknowledgment, Update Notification and its Acknowledgement, Flow Binding, Subscription Query and Subscription Response. -* **51 of the 71 registered options** -- broadly the PMIPv6, NEMO and - flow-binding block, including Home Network Prefix, Handoff Indicator, Access - Technology Type, Timestamp, GRE Key, Binding Identifier and the QoS options. +* **51 of the 71 registered options** -- most of the PMIPv6 and flow-binding + block, including Home Network Prefix, Handoff Indicator, Access Technology + Type, Timestamp, GRE Key, Binding Identifier and the QoS options, together + with DNS-UPDATE-TYPE, Vendor Specific and Service Selection, which belong to + none of those groups. * **3 of the 4 CGA extensions**; only Multi-Prefix is implemented. Each of those falls through to a generic handler, so nothing breaks -- the -fields simply are not decoded. The ``# TODO`` markers in -``pcapkit/protocols/internet/mh.py`` sit at the exact dispatch tables that need -entries, and the file documents the shape each handler takes. +fields simply are not decoded. Read and construction are symmetric throughout, +so every gap above is a gap in both directions: a message type needs a +``_read_msg_`` and a ``_make_msg_`` handler, an option a ``_read_opt_`` and a +``_make_opt_``, and each has to be named in the matching dispatch table -- +:attr:`~pcapkit.protocols.internet.mh.MH.__message__`, +:attr:`~pcapkit.protocols.internet.mh.MH.__option__` or +:attr:`~pcapkit.protocols.internet.mh.MH.__extension__`. The six ``# TODO`` +markers in ``pcapkit/protocols/internet/mh.py`` sit at the end of each handler +block rather than at the tables, so both places need editing; the file documents +the shape each handler takes. + +Less of this is groundwork than the numbers suggest. The sub-registries the +missing options and messages need -- binding revocation types and triggers, +handoff indicators, access network identifier sub-options, flow identification +and flow binding sub-options, LMA-controlled MAG parameters, DNS update status, +traffic selector formats, QoS attributes -- are already generated in full under +:doc:`pcapkit/const/mh`, and ``mh.py`` already imports 32 of them without using +them. What is missing is the handlers, not the enumerations. + +DTLS +~~~~ + +**Not started**, and unlike everything in the list above it has no stub in the +tree at all -- only TLS/SSL does. It earns its own entry because the registries +have moved ahead of it, and a growing part of SCTP's surface now names a protocol +that does not exist: + +* the DTLS chunk, type 65 of :class:`~pcapkit.const.sctp.chunk.Chunk`, and the + DTLS Key Management chunk parameter, ``0x8006`` of + :class:`~pcapkit.const.sctp.parameter.Parameter` -- both fall through to the + generic handlers, so they parse as opaque; +* four of its error causes, 100 to 103 of + :class:`~pcapkit.const.sctp.cause_code.CauseCode`; +* seven payload protocol identifiers -- 47 (Diameter over DTLS/SCTP), 66 to 69 + (NGAP, XnAP, F1AP and E1AP, each over DTLS over SCTP) and 4242 (DTLS chunk + key management). + +That last group is the one that bites. NGAP over DTLS over SCTP is PPID 66, and +even with an NGAP dissector registered against it the bytes arriving are a DTLS +record rather than an NGAP PDU, so they can only degrade to +:class:`~pcapkit.protocols.misc.raw.Raw`. DTLS therefore blocks NGAP over DTLS, +asked for in `discussion #251 +`__, independently of +whether NGAP itself is implemented. + +A record-layer dissector is enough to unblock that -- content type, version, +epoch, sequence number, length, and the fragment offset and length DTLS adds to +handshake messages. Decryption is a separate question and is not needed to make +the record structure legible, in the same way :class:`ESP +` parses without keys. DTLS over UDP is the +other half of the same work and wants the same dissector. + +Registered, But Not Dissected +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +A different shape of gap from the empty stubs, and easy to miss because nothing +announces it. The :doc:`pcapkit/const/reg` enumerations are complete, but only a +small part of each is bound to a dissector; everything else resolves to +:class:`~pcapkit.protocols.misc.raw.Raw`, so the capture parses without +complaint and yields nothing useful. + +* **3 of the 219** :class:`~pcapkit.const.reg.linktype.LinkType` values -- + ``ETHERNET``, ``IPV4`` and ``IPV6``, declared identically in + :class:`~pcapkit.protocols.misc.pcap.frame.Frame` and + :class:`~pcapkit.protocols.misc.pcapng.PCAPNG`. +* **15 of the 151** :class:`~pcapkit.const.reg.transtype.TransType` values, in + :attr:`Internet.__proto__ + `. +* **6 of the 160** :class:`~pcapkit.const.reg.ethertype.EtherType` values, in + :attr:`Link.__proto__ `: ARP, + RARP, IPv4, IPv6, IPX and the customer VLAN tag. +* **2 port numbers, out of 8182** :class:`~pcapkit.const.reg.apptype.AppType` + members -- TCP 21 to FTP, and port 80 to HTTP on both TCP and UDP. +* **none of the 75** + :class:`~pcapkit.const.sctp.payload_protocol_identifier.PayloadProtocolIdentifier` + values. :attr:`SCTP.__proto__ + ` ships empty, so every DATA + chunk payload is ``Raw`` until something calls + :func:`~pcapkit.foundation.registry.protocols.register_sctp`. + +Most of those want a dissector written and are covered by the stub list above. +A handful want only a table entry, because the dissector is already there: + +* :class:`~pcapkit.protocols.link.ospf.OSPF` and + :class:`~pcapkit.protocols.link.l2tp.L2TP` are implemented but reachable from + no registry at all -- ``TransType`` 89 and 115 are both unbound. Note that + each class deliberately makes ``__index__`` raise, so registering them means + deciding that question first. +* :class:`~pcapkit.protocols.application.ftp.FTP_DATA` is implemented and + exported, but TCP port 20 is unbound. +* HTTP is bound on port 80 only, not on 8080 or 8443. +* The service VLAN tag identifier (S-Tag), ``0x88A8``, is unbound, though + :class:`~pcapkit.protocols.link.vlan.VLAN` already parses that shape and the + customer tag ``0x8100`` is bound to it. +* ``LinkType`` ``NULL``, ``LOOP`` and ``RAW`` carry bare IPv4 or IPv6, both of + which pcapkit dissects. ``NULL`` and ``LOOP`` need their four-octet address + family word skipped first, and ``RAW`` needs a version sniff. + +Beyond those, the gaps most likely to be met in a real capture are ICMP (1), +ICMPv6 (58) and IGMP (2) on the internet layer, all three of which have stubs; +and ``LINUX_SLL`` and ``LINUX_SLL2`` at the link layer, which every +``tcpdump -i any`` capture uses and which have no stub. PCAPNG Support -------------- @@ -174,7 +302,7 @@ with a hard-wired handler. It now provides: and the four :func:`print` calls that were marked ``# pylint: disable=logging-fstring-interpolation`` are now real logger calls; -- **``debug`` coverage of the extraction path** -- extractor construction, +- **debug coverage of the extraction path** -- extractor construction, engine selection and fallback, frame counts, cleanup, reassembly and flow-tracing setup -- so that ``DEBUG`` explains what PyPCAPKit did with a file without descending into per-field parsing. @@ -319,3 +447,67 @@ wheel stays lean because the suite could not run from an installed package anyway: the generated sample captures are not shipped, and ``tests/_tiers.py`` resolves paths from a repository root that an installed package does not have. Anyone wanting to run the tests wants the repository, which is where they are. + +Reassembly Beyond IP and TCP +---------------------------- + +**Still open**, and newer than the rest of this page -- +:doc:`pcapkit/foundation/reassembly/index` covers three protocols and no more. +IPv4 and IPv6 share the :rfc:`791` procedure, and TCP uses the :rfc:`815` +hole-descriptor algorithm, which does handle out-of-order and overlapping +segments. SCTP has nothing: a user message split across DATA chunks is never put +back together, and ``sctp`` appears nowhere in :mod:`pcapkit.foundation` at all. + +What that costs is concrete. The dissector already exposes everything a +reassembler needs, on +:class:`~pcapkit.protocols.data.transport.sctp.DATAChunk` -- ``tsn``, +``stream_id``, ``stream_seq``, ``ppid``, ``data``, and ``flags.B`` and +``flags.E`` for the beginning and ending fragment bits -- and nothing reads +those two bits outside the construction path. So each fragment is dispatched on +its own PPID as though it were a whole PDU, and a registered upper layer is +handed half a message. Two related holes compound it: only the *first* DATA +chunk of a bundle is dispatched at all, and I-DATA [:rfc:`8260`], chunk type 64, +is not dissected -- so the MID and FSN fields that exist precisely to make +interleaved reassembly tractable are never parsed, and an I-DATA-only packet +yields no payload whatsoever. FORWARD TSN (192) and I-FORWARD-TSN (194) are +likewise unhandled, so partial-reliability stream advancement is invisible. + +Writing the reassembler is the smaller half of the job. It would go at +``pcapkit/foundation/reassembly/sctp.py``, subclass +:class:`~pcapkit.foundation.reassembly.reassembly.Reassembly` and implement two +methods, +:meth:`~pcapkit.foundation.reassembly.reassembly.Reassembly.reassembly` and +:meth:`~pcapkit.foundation.reassembly.reassembly.Reassembly.submit`. Registering +it, though, is currently inert: +:meth:`~pcapkit.foundation.extraction.Extractor.register_reassembly` accepts any +protocol name, while +:class:`~pcapkit.foundation.extraction.Extractor` only ever instantiates the +three it names literally, and +:class:`~pcapkit.foundation.reassembly.ReassemblyManager` is a fixed-field +container with no room for a fourth. Making SCTP reachable therefore also means +touching that manager and its read-side twin, the construction branch in +:class:`~pcapkit.foundation.extraction.Extractor`, one adapter per engine in +:mod:`pcapkit.toolkit`, the call site in each of those engines, and +:func:`~pcapkit.interface.core.reassemble`. Generalising that wiring so a +registered reassembler is actually used is worth doing on its own account, and +would make the fourth protocol cheaper than the third rather than dearer. + +Two smaller items in the same subsystem: + +* **Flow tracing is TCP only**, and blocked on the same generalisation -- + :class:`~pcapkit.foundation.traceflow.TraceFlowManager` holds a single field, + so UDP, SCTP and IP conversation tracing have nowhere to go. The TCP tracer + itself closes a flow on FIN but never on RST, which is not in its packet model + at all, and treats each direction of a connection as a separate flow. +* **Nothing ever times a partial datagram out.** :rfc:`791` gives IP reassembly + a 15-second timer and :rfc:`8200` gives IPv6 60 seconds; neither is + implemented, and neither can be until the buffer models carry a timestamp. A + buffer is released only when its datagram completes or its flow is torn down, + and every in-flight IP datagram identifier holds a fixed 72 KiB of + preallocated space -- so a lossy capture, or one with spoofed identifiers, + grows the buffer monotonically. + +Reassembly is also unavailable on some engines rather than merely slower, which +is worth knowing before benchmarking against them: ``pyshark``, ``pypcap`` and +``pcap_ct`` disable it entirely, and ``pypcapfile`` disables the IPv6 half of it. +:doc:`pcapkit/foundation/engines/index` tabulates that. From 0bb245a8dcd0042fcb724ac3906346259aa8e1e6 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 16 Sep 2026 16:12:53 -0400 Subject: [PATCH 2/2] docs: reconcile the performance section with the profiling pass that answered it "Maybe Even Faster?" closed by asking for "a measured benchmark showing where the time actually goes". That benchmark now exists (#420) and cut extraction on a 1117-frame HTTP capture by ~46% with byte-identical output, so the section asks for something already delivered. Rewritten to record what was found instead: the four hot-path defects that were fixed, the three larger wins deliberately left for their own review (the flow dumper reopening its output per frame, `analyze()` running eagerly on unfragmented frames, and every option being parsed twice), and the two measurement traps that make a naive benchmark of this library meaningless -- `reassembly=`/`trace=` are no-ops without `ip=`/`tcp=`, and timing several capture shapes in one process inflates them by up to 73%. Two predictions the profiling contradicted are recorded as such, since both are the sort of thing a contributor would otherwise assume and spend a day on: `Info` construction, the usual suspect, is 0.0% of its own cost and 4.4% of a parse; and the logging integration is 0.03% even in the heaviest shape. The IO-batching idea the original thread proposed was never the bottleneck. Whether unfragmented frames should be emitted as datagrams at all is left as an open design question rather than answered here. Clean build: 91 warning/error lines, unchanged from this branch's baseline, none naming pep.rst. --- docs/source/pep.rst | 58 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 14 deletions(-) diff --git a/docs/source/pep.rst b/docs/source/pep.rst index 6998e04a7c..34e2619721 100644 --- a/docs/source/pep.rst +++ b/docs/source/pep.rst @@ -261,20 +261,50 @@ the thread raised when only PCAP was supported. Maybe Even Faster? ------------------ -**Still open.** Benchmarking put the builtin default engine at roughly 4x -Scapy and 10x DPKT, which is an acceptable price for what it decodes, but the -original proposal in the thread stands: fold consecutive ``_read_xxxxxx`` calls -into a single ``file.read`` so that the number of IO calls and the duplicated -:func:`struct.unpack` work both come down. - -Note that the parsing path has been rewritten since that was written. Protocols -no longer read fields inline; they declare a -:class:`~pcapkit.protocols.schema.schema.Schema` of field descriptors and let -:meth:`~pcapkit.protocols.schema.schema.Schema.unpack` drive it. The batching -idea still applies, but it belongs in the schema and field machinery now rather -than in each protocol's ``_read_`` methods, and the sketch in the thread no -longer maps onto the code. A measured benchmark showing where the time actually -goes would be the useful first contribution here. +**Partly done.** The measured benchmark this section used to ask for now exists, +and acting on it cut extraction time on a 1117-frame HTTP capture by about 46% +with byte-identical output. Four things were wrong on the hot path, none of them +the ones the thread predicted: + +* character-set detection was uncached, and accounted for 30% of an HTTP + extraction -- 3011 :func:`chardet.detect` calls over 163 distinct + bytestrings; +* every field of every protocol was copied through the generic + :func:`copy.copy` machinery, 63207 times per extraction; +* :attr:`Field.length ` recomputes + :func:`struct.calcsize` on each read and was read two to four times per field; +* :meth:`Schema.__setattr__ ` + re-entered itself once per field assigned. + +Two predictions the profiling **contradicted**, recorded so nobody spends time on +them again: :class:`~pcapkit.corekit.infoclass.Info` construction -- the usual +suspect -- is 0.0% of its own cost and 4.4% of a parse, and the logging +integration is 0.03% even in the heaviest shape, since every call site is a lazy +``%s``. The IO-batching idea the thread proposed was never the bottleneck. + +Three larger wins remain, all outside the parse path and each wanting its own +review: + +* **The flow dumper reopens its output file once per frame** and rebuilds a whole + :class:`~pcapkit.protocols.misc.pcap.frame.Frame` to obtain bytes it already + holds (``pcapkit/dumpkit/pcap.py:94,128``). That is 80% of the flow-tracing + cost, and a counterfactual left 330 of 331 output files byte-identical. This is + the best-evidenced and lowest-risk piece of work on this page. +* :meth:`analyze() ` + **runs eagerly on every frame, fragmented or not**, because + ``pcapkit/toolkit/pcap.py:53`` filters only on the *DF* flag. A capture with no + fragments at all still produces one "datagram" per frame -- 86% of the + IP-reassembly cost plus a 133 ms garbage-collection bill. Making the ``packet`` + field lazy is mechanical; whether unfragmented frames should be emitted at all + is a design question worth settling first. +* **Every option is parsed twice** (``pcapkit/corekit/fields/collections.py:262-270``): + 2274 schema unpacks for 1137 options, the pre-parse always discarded. About + 15% of a PCAP-NG extraction, and the riskiest of the three. + +Two traps for anyone benchmarking this library. ``reassembly=True`` and +``trace=True`` are **no-ops** without ``ip=``/``tcp=``, so a benchmark that passes +only the switch measures nothing; and timing several capture shapes in one +process inflates them by up to 73%, so each shape wants its own interpreter. Logging Integration -------------------