protocols: implement NGAP over SCTP, decoding aligned PER through pycrate by JarryShaw · Pull Request #417 · JarryShaw/PyPCAPKit

JarryShaw · 2026-09-16T18:04:51Z

Closes the long-queued NGAP work from #251. SCTP landing earlier removed the stated prerequisite; this adds the protocol on top of it.

The ASN.1 PER decision

Per your call: use an existing library as an optional extra, don't write a PER codec. pycrate 0.8.1 is pure Python, LGPL-2.1+, released 2026-05-11, and ships NGAP precompiled — a Release-18-era spec with 81 procedures and 438 protocol IE ids. Aligned PER round-trips exactly, and a decode costs 0.147 ms, the same order as pcapkit's own ~0.2 ms/packet.

NGAP = [ "pycrate" ], excluded from all with the reasoning written out: pip install pycrate lands ~238 MB of pycrate_asn1dir for the one 4.9 MB NGAP module, and it is LGPL where pcapkit is BSD-3. Imported lazily through the same three-state cache ESP uses for cryptography. Verified — import pcapkit, import pcapkit.all and importing NGAP itself pull in zero pycrate* modules; only a parse does.

A pre-existing crash this uncovered

beholder's recovery path read self.__header__.get_payload(). SCTP has no payload schema field — user data lives inside a DATA chunk — so it raised ProtocolUnbound("unknown field: 'payload'") from the recovery path, turning a next-layer failure that should degrade to Raw into a crash. Unreachable before, because SCTP.__proto__ was empty; registering anything at all exposes it. PCAP-NG has the same shape.

Fixed to self._get_payload() — the wrapper both of them override — and alias=proto is now forwarded to match the success path, without which registering NGAP made the output less informative than leaving PPID 60 unregistered. Own commit (764caa1f6), with two regression tests. Both fail against main's implementation with exactly that error, so the bug is pinned rather than asserted.

Decode strategy: generic by ASN.1 shape

Not per-procedure. dict→Sequence, list→list, (str, val)→Choice, (int, int)→BitString — that last one matters, since it is the bit-length pair a naive converter would flatten and lose. So all 81 procedures work on day one and a new 3GPP release needs no code change.

Surfaced as first-class fields: kind, procedure, criticality, message, ies, plus value holding the whole converted body. ies is the interpreted view with enums resolved; value is the faithful tree — documented as such, because the converter cannot know that id in one position means a ProtocolIE without exactly the per-procedure knowledge being avoided.

Enums stay in `ngap.py`

Per the standing rule — registry-derived enums go to const/ with a vendor crawler, spec-derived ones stay with the protocol. NGAP's ProcedureCode (81), ProtocolIE (438), Criticality (3, closed) and PDUKind (3) are 3GPP TS 38.413 values with no crawlable registry, so they live in the protocol module. Generated from NGAP_Constants and pasted in, so nothing needs pycrate at import time. The SCTP PPID enum is the IANA one and is reused, not duplicated.

Registration

Both PPIDs as SCTP.__proto__ defaults, mirroring how TCP.__proto__ declares 21 and 80, rather than a register_sctp call at import. SCTP's class docstring, which still claimed "No PPID is registered by default", is updated.

Tests

670 passed, 8 skipped, 633 subtests    # with pycrate
613 passed, 65 skipped, 594 subtests   # in a pycrate-free venv

Unit tier, no fixture on disk: the 58 APER bytes are inline and the SCTP frame is built with SCTP.make. The missing-dependency tests run unconditionally by resetting the cache, so they are covered in CI even where pycrate is installed.

Also pinned: that reset_val() is never called (mocked and asserted), that consecutive decodes don't share state, and that seven malformed or truncated payloads arrive as ProtocolError rather than raw ASN1PERDecodeErr/CharpyErr.

Why reset_val() matters enough to have a test: it costs 101.6 ms against the decode's 0.147 ms — roughly 690x — because it walks the whole 318-submodule spec tree. from_aper() overwrites the value anyway, so calling it would make NGAP dissection dominate a capture. Both a comment and a test guard it.

vermin reports minimum 3.6; mypy adds no new errors.

Docs

docs/source/pcapkit/protocols/application/ngap.rst is new, docstring copied under .. module:: with no automodule, per the convention #408 settled. One line added to the application toctree — Sphinx verified at 285 warnings before and 285 after, none added or removed.

Deliberately out of scope

PPID 66 is registered but not decoded. NGAP_over_DTLS_over_SCTP wraps the PDU in a DTLS record and pcapkit has no DTLS, so those bytes are not APER. Registered so the PPID is named; the payload degrades to Raw. Said plainly in the module docstring, the docs page and the __proto__ comment.
No SCTP fragmentation reassembly — a PDU split across DATA chunks fails to decode.
No per-IE typed models. The trade the generic conversion makes: an IE's value comes back in the specification's shape, not a pcapkit model. PrivateMessage contents are vendor-defined and unnameable.
The spec revision is pycrate's, not ours. A newer pycrate decodes IEs the pasted enums don't name; both extend at lookup to Unassigned_<n> rather than failing.
docs/source/ext.rst's protocol table and the mermaid diagram in docs/source/pcapkit/protocols/index.rst still omit NGAP — left alone to honour a single-line docs constraint. Worth a follow-up.

One divergence from ftp.py: NGAP.length returns the PDU length rather than raising UnsupportedCall, since the whole payload is the header and there is no next layer. Documented, including why it differs from __length_hint__, which reports the 4-octet prefix every PDU shares.

… schema `beholder` read the fallback payload from `self.__header__.get_payload()`. That is what `ProtocolBase._get_payload()` wraps, so the two agree for every protocol whose payload is a schema field -- and disagree for the two that override it. SCTP carries user data inside a DATA chunk and PCAP-NG inside a block, so neither header schema has a `payload` field at all, and reaching for the schema raises `ProtocolUnbound("unknown field: 'payload'")` *from the recovery path*. A next-layer parse failure that should have degraded to `Raw` became a crash that aborted the frame. It was unreachable until now only because `SCTP.__proto__` had no entries: with nothing registered on a payload protocol identifier, no next-layer parse could fail. Registering anything on one exposes it immediately. - `beholder` now calls `self._get_payload()`, the wrapper both overrides. - It also forwards `alias=proto`, which the success path in `_import_next_layer` already passes. Without it, a payload that failed to parse came back as a bare `Raw` while an *unregistered* number came back named -- so registering a protocol made the output less informative than leaving the number alone. A plain integer has no `name` and still renders as `Raw`, so this only adds a name where the registry key is an enumeration. The four `beholder` tests are rewritten onto shared stand-ins that carry a `_get_payload`, as a real protocol does, plus two new ones: the alias forwarding and an overridden `_get_payload` whose schema accessor raises, which is the SCTP shape. Unit tier green, and mypy reports the same five pre-existing errors in `decorators.py` as it does at HEAD -- no new ones.

…rate (#251) NGAP (3GPP TS 38.413) is the 5G RAN-to-AMF control plane. It has no header of its own: an SCTP DATA chunk whose payload protocol identifier names NGAP carries exactly one aligned-PER `NGAP-PDU` and nothing else, so nothing about it is visible until an ASN.1 decoder has run. - `pcapkit.protocols.application.ngap`, with the matching schema and data models, and exports added everywhere `FTP` is listed. - Registered on `SCTP.__proto__` as a *default*, for PPID 60 (`NG_Application_Protocol`) and 66 (`NGAP_over_DTLS_over_SCTP`), following how `TCP.__proto__` declares 21 and 80 rather than calling `register_sctp` at import time from somewhere. - `pycrate` is an optional extra, `pip install pypcapkit[NGAP]`, imported inside the parse path and never at module import, and deliberately excluded from `all` -- it lands ~238 MB of `pycrate_asn1dir` (87 spec modules) to obtain the one 4.9 MB `NGAP.py`, and it is LGPL-2.1+ where this package is BSD-3-Clause. Absent, it fails the way `pcapkit.protocols.internet.esp` fails without `cryptography`: a `ProtocolError` that `beholder` degrades to `Raw`. - Decoding is generic rather than per-procedure: the value tree is mapped by ASN.1 *shape*, so all 81 elementary procedures and 438 protocol IEs work without 519 hand-written cases and a new 3GPP release needs no code change. The PDU kind, procedure code, criticality, message type name and IE list are surfaced as first-class fields. - `ProcedureCode`, `Criticality` and `ProtocolIE` live in `ngap.py`, not `pcapkit.const`: 3GPP publishes them in the ASN.1 of TS 38.413 rather than in a crawlable registry, so there is no vendor module either. The SCTP PPIDs are IANA's and are used from `pcapkit.const.sctp` as they stand. `reset_val()` is never called on the parse path, and there is a comment plus a test saying so: it walks all 318 submodules of the compiled specification and costs ~102 ms against the decode's ~0.15 ms, and `from_aper()` overwrites the value anyway. The module-level PDU object is stateful and `get_val()` hands back its own containers, so a lock is held across the decode *and* the conversion. Five SCTP tests used PPID 60 as an arbitrary placeholder with junk payloads and now describe the registered default instead; their `SCTP.__proto__.clear()` teardowns are replaced with a snapshot restore, since clearing the registry now destroys the defaults for every later test in the process. Unit tier: 670 passed with pycrate, 613 passed / 65 skipped without it, no failures either way. Round trip verified byte-exact on a real 58-octet `NGSetupRequest`.

Follows the convention the sibling application pages use: the module docstring copied into the `.rst` under a `.. module::` directive, then `autoclass` per class, rather than `automodule` -- which was the review feedback on #408. `ProcedureCode` and `ProtocolIE` are rendered with `:no-members:`. Between them they carry 519 members, each named for the 3GPP identifier it came from, and a page listing all of them is longer than the specification's own tables and no more useful; a note says where to read them instead, and why there is no matching `pcapkit.vendor` module. No `pycrate` intersphinx mapping: the project publishes no `objects.inv`, so the link is a plain external reference through the same `|pycrate|_` substitution `esp.rst` uses for `cryptography`. One line added to the application-layer toctree. Sphinx build verified against a clean export of HEAD with the same interpreter: 285 warnings before, 285 after -- none added, none removed.

Copilot

🟡 Changes recommended

PPID 66 is currently registered to NGAP in a way that guarantees exception-driven fallback and warning spam for DTLS-wrapped traffic, and a couple of documentation/test assertions should be aligned with the intended dispatch behavior.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR adds first-class NGAP (5G control plane) dissection on top of the existing SCTP support, using pycrate as an optional dependency for aligned PER decoding, and hardens the beholder recovery path so next-layer failures degrade to Raw instead of crashing.

Changes:

Implement NGAP protocol parsing/encoding with a generic ASN.1-shape converter and a lazy, cached pycrate import.
Register NGAP on SCTP PPIDs and adjust beholder to use the protocol payload accessor and preserve PPID naming on fallback.
Add unit/regression tests (NGAP decode/encode round-trip, missing-dependency behavior, and the beholder/SCTP registry interactions) plus docs and packaging extra.

File summaries

File	Description
tests/utilities/test_decorators.py	Expands `beholder` regression coverage (payload accessor + alias forwarding).
tests/protocols/transport/test_sctp_unit.py	Updates SCTP PPID dispatch tests and registry teardown helpers.
tests/protocols/application/test_ngap_unit.py	New NGAP unit tests covering enums, optional dependency behavior, and decode/make semantics.
pyproject.toml	Adds `NGAP = ["pycrate"]` optional extra and documents why it’s excluded from `all`.
pcapkit/utilities/decorators.py	Fixes `beholder` fallback to use `_get_payload()` and forwards `alias=proto`.
pcapkit/protocols/transport/sctp.py	Registers default PPID mappings (including NGAP) and updates SCTP docstring.
pcapkit/protocols/schema/application/ngap.py	New NGAP schema (payload is the full APER PDU).
pcapkit/protocols/schema/application/init.py	Exports NGAP schema from the application schema package.
pcapkit/protocols/schema/init.py	Adds NGAP to schema namespace exports.
pcapkit/protocols/data/application/ngap.py	New NGAP data models (IE/Choice/BitString/Sequence/NGAP).
pcapkit/protocols/data/application/init.py	Exports NGAP data models from application data package.
pcapkit/protocols/data/init.py	Adds NGAP symbols to top-level data exports.
pcapkit/protocols/application/ngap.py	New NGAP protocol implementation (pycrate-backed APER decode/encode + conversion).
pcapkit/protocols/application/init.py	Exports NGAP from application protocol namespace.
pcapkit/protocols/init.py	Adds NGAP to protocols public exports.
pcapkit/all.py	Includes NGAP in `pcapkit.all` export list.
pcapkit/init.py	Includes NGAP in top-level `pcapkit` exports.
docs/source/pcapkit/protocols/application/ngap.rst	New NGAP documentation page.
docs/source/pcapkit/protocols/application/index.rst	Adds NGAP page to the application protocols toctree.

Review details

Suppressed comments (2)

pcapkit/protocols/transport/sctp.py:354

Registering PPID 66 to NGAP forces every "NGAP over DTLS" packet down a decode path that cannot succeed (no DTLS support), which means an exception + beholder warning for every such frame. Consider mapping PPID 66 directly to Raw (it will still be named via alias=PPID) to avoid log spam and exception-driven control flow.

            # PPID 66 is NGAP wrapped in a DTLS record rather than a bare
            # NGAP-PDU, and pcapkit implements no DTLS. It is registered anyway
            # so that the PPID is *named*: the payload then fails in NGAP's own
            # decoder and `beholder` degrades it to Raw, which is where an
            # unregistered PPID would have left it regardless.

tests/protocols/transport/test_sctp_unit.py:890

This test currently asserts PPID 66 is registered to NGAP. If PPID 66 is DTLS-wrapped and intentionally not decoded, it should remain Raw (but still named by the PPID enum) to avoid exception-driven parsing and warning spam; adjust the assertions accordingly.

        for ppid in (NGAP_PPID, DTLS_PPID):
            with self.subTest(ppid=int(ppid)):
                self.assertIn(ppid, SCTP.__proto__)
                entry = SCTP.__proto__[ppid]
                module = getattr(entry, 'module', None)

Files reviewed: 19/19 changed files
Comments generated: 4
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…idates CI's Integration leg failed on all six interpreters with one real failure: `test_malformed_http_payload_falls_back_to_raw` asserted `tcp.payload.info.protocol is None`, and forwarding `alias` from `beholder` makes it 80. Neither the unit tier nor the local runs cover that tier, which is how it reached CI. The new value is the right one -- `Data_Raw.protocol` is "the original enumeration of this protocol", and 80 is what the payload arrived as -- so the assertion is updated rather than the fix reverted. The protochain still reads `Raw`, since a plain int has no name to render. The comment justifying the forward was too broad, though, and is now measured rather than asserted. It claimed an unregistered code keeps its name generally. True on SCTP (an unknown PPID gives `SCTP:Unassigned_4243` and `protocol=4243`, which is why registering NGAP would otherwise have made a failed parse *less* informative than leaving PPID 60 alone) and on IP, but false on TCP, where `Transport._decode_next_layer` resolves ports through `__proto__` and reaches Raw without an alias, so an unknown port already reports `None`. So the failure path is now uniform across layers while the unknown-code paths remain inconsistent with each other -- filed as #418. Full suite, the command CI's Integration leg runs: 806 passed, 17 skipped.

Four review findings on #417, all of them right. The "not 51 hand-written procedures" heading in `ngap.py` and `ngap.rst` was wrong: `ProcedureCode` has 81 members, and 51 was a bad number from the brief that the enums themselves already corrected. Now 81 in both, which is the same character count, so the RST underline is unaffected. The other two are accurate-but-misleading-by-omission. `SCTP`'s class docstring said both PPIDs are registered to NGAP without saying that only 60 can decode: a PPID 66 payload is an NGAP PDU inside a DTLS record, and with no DTLS implementation those bytes are never aligned PER, so 66 degrades to `Raw` on every capture rather than only when `pycrate` is absent. The module docstring and the docs page already said so; the class docstring, which is where someone reading `SCTP.__proto__` looks, did not. `test_ppid_dispatch_hook`'s docstring had the same gap, and its assertion is deliberately no stronger than "both dispatch" -- now stated, so the test is not read as claiming both decode. 56 tests pass across the SCTP and NGAP files.

JarryShaw · 2026-09-16T18:55:37Z

On the two findings about the "51 hand-written procedures" heading in ngap.py:44 and ngap.rst:42 — both correct, fixed in c3e8dce.

That number came from my own brief and was wrong twice over. Measured against the shipped spec:

ProcedureCode: 81 members
ProtocolIE:    438 members
Criticality:   3 members
PDUKind:       3 members

The enums were already right — the agent implementing this measured the split itself and corrected 519 into 81 procedure codes plus 438 protocol IE ids — but the prose heading kept the bad figure. It now reads 81 in both files, which happens to be the same character count, so the RST section underline needed no adjustment.

Copilot

🔵 Needs a closer look

It introduces a large new protocol implementation plus optional-dependency decoding and default SCTP dispatch behavior, which warrants final human review despite good test coverage.

Review details

Files reviewed: 20/20 changed files
Comments generated: 1
Review effort level: Lite

Copilot is right on #417: `Criticality.get` declared and documented a `default` it never used. An unknown name raised `KeyError`, an unknown value `ValueError` via `_missing_`, and `default` reached neither path. The behaviour is intended -- `Criticality` is an ASN.1 `ENUMERATED` with no extension marker, so a fourth value is unencodable and a lookup for one is a bug, which is why `_missing_` raises deliberately. It is the parameter that was wrong, not the closedness, so the parameter is gone. No caller passed it: all five call sites use one argument. Its siblings keep theirs, and the docstring now says why rather than leaving the difference to be rediscovered -- `ProcedureCode` and `ProtocolIE` extend through `extend_enum` for a code a newer specification names, which is right for a registry that grows and impossible for one that cannot. Also made the unknown-name path raise `ValueError` like the unknown-value path, so the two ways of getting this wrong no longer report differently. Verified: signature carries no `default`, valid int/str/member lookups unchanged, both unknown forms now `ValueError`. 22 NGAP tests and the SCTP file pass.

…ted page (#419) * 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 5e4378d; 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: 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.

…t sees The sixth and last instance of #421, missed by the first pass because it is not in a `_import_next_layer` at all: `callback_payload` in the Ethernet *schema* resolved the next layer by subscripting the registry directly. protocol = Ethernet.__proto__[type_] `Ethernet.__proto__` *is* `Link.__proto__`, a class-level `defaultdict`, so one frame with an unregistered EtherType recorded it: registered before: False payload: Raw protocol: 34997 registered after : True new keys: {<EtherType.IEEE_Std_802_Local_Experimental_Ethertype_0x88B5: 34997>} Link.register warns: ['protocol ... already registered, overwriting'] and afterwards, with the lookup routed through `ProtocolBase._lookup_next_layer` as the other five sites now are: registered after : False new keys: set() Link.register warns: [] The payload still resolves to `Raw` and still carries the EtherType as `Data_Raw.protocol` (34997) -- only the registry write is gone. Resolving a registered `ModuleDescriptor` now also memoises the imported class, which this callback never did, so it stops re-importing on every frame. The existing `callback_payload` test covered both hit branches and neither miss; it now covers the miss and the memoisation as well. `ModuleDescriptor` is no longer named in the module and its import goes. A sweep for the same shape across the package -- `grep -rn "__proto__\[" pcapkit/` -- finds nothing else: every other occurrence is a `cls.__proto__[code] = protocol` *write* inside a `register()` method, which cannot inject, and `pcapkit/protocols/schema/` holds no other reference to a `__proto__` at all. Six sites is the whole set for this registry. Every capture in `examples/captures/` serialises identically to the previous commit, byte for byte, in both `tree` and `json`. Also, for the next reader of #418: it cites `tests/protocols/transport/test_sctp_unit.py:966` for the SCTP behaviour it uses as its reference, and at the commit it was filed against that line was `def test_register_rejects_a_non_protocol`, which is not the test it means. The one it means is `test_unregistered_ppid_does_not_mutate_the_class_registry` -- lines 871-938 then, 951-1016 now that #417 has grown the file above it. Find it by name rather than by line.

JarryShaw added 3 commits September 16, 2026 13:40

JarryShaw requested a lite review from Copilot September 16, 2026 18:19

Copilot started reviewing on behalf of JarryShaw September 16, 2026 18:21 View session

Copilot AI reviewed Sep 16, 2026

View reviewed changes

Comment thread docs/source/pcapkit/protocols/application/ngap.rst Outdated

Comment thread pcapkit/protocols/application/ngap.py Outdated

Comment thread pcapkit/protocols/transport/sctp.py

Comment thread tests/protocols/transport/test_sctp_unit.py Outdated

JarryShaw mentioned this pull request Sep 16, 2026

Unknown next-layer codes keep their enumeration on SCTP and IP but not on TCP/UDP #418

Closed

JarryShaw added 3 commits September 16, 2026 14:33

Merge branch 'main' into feat/ngap-protocol

116fa4e

JarryShaw mentioned this pull request Sep 16, 2026

docs: record the protocol, reassembly and crypto gaps on the Help Wanted page #419

Merged

JarryShaw added 2 commits September 16, 2026 16:00

Merge branch 'main' into feat/ngap-protocol

9d2f327

Merge branch 'main' into feat/ngap-protocol

26739d3

JarryShaw requested a lite review from Copilot September 16, 2026 20:17

Copilot started reviewing on behalf of JarryShaw September 16, 2026 20:21 View session

Copilot AI reviewed Sep 16, 2026

View reviewed changes

Comment thread pcapkit/protocols/application/ngap.py

JarryShaw merged commit 659b259 into main Sep 16, 2026
24 checks passed

JarryShaw mentioned this pull request Sep 16, 2026

protocols: stop next-layer dispatch writing to shared registries, and fix two transport defects #426

Merged

JarryShaw deleted the feat/ngap-protocol branch September 17, 2026 01:06

JarryShaw added the feat Pull requests that add a new capability (feat: subject prefix) label Sep 22, 2026

JarryShaw mentioned this pull request Sep 27, 2026

fix(vendor): stop get() minting; _missing_ fixed for 21 of 105 (#775) #838

Merged

5 tasks done

JarryShaw added this to the 1.5 milestone Oct 6, 2026

Navigation Menu

Uh oh!

protocols: implement NGAP over SCTP, decoding aligned PER through pycrate - #417

protocols: implement NGAP over SCTP, decoding aligned PER through pycrate#417
JarryShaw merged 9 commits into
mainfrom
feat/ngap-protocol

JarryShaw commented Sep 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JarryShaw commented Sep 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

JarryShaw commented Sep 16, 2026

The ASN.1 PER decision

A pre-existing crash this uncovered

Decode strategy: generic by ASN.1 shape

Enums stay in ngap.py

Registration

Tests

Docs

Deliberately out of scope

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JarryShaw commented Sep 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Enums stay in `ngap.py`