Skip to content

feat(link): add a FRAD dissector for Frame Relay over LinuxSLL (#2460) - #2499

Merged
JarryShaw merged 2 commits into
mainfrom
feat/2460-frad-dissector
Nov 1, 2026
Merged

JarryShaw merged 2 commits into
mainfrom
feat/2460-frad-dissector

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Part of #2460.

What

Adds pcapkit.protocols.link.frad.FRAD, a dissector for Frame Relay's
multiprotocol encapsulation (RFC 2427, obsoleting RFC 1490), and wires
LinuxSLL/LinuxSLL1/LinuxSLL2 to dispatch to it for the ARPHRD_FRAD
arphrd_type instead of falling back to Raw.

Why this slice

#2460 lists five arphrd_type values that LinuxSLL/LinuxSLL2 keep whole
as Raw: NETLINK, IPGRE, IP6GRE, IEEE80211_RADIOTAP, FRAD. The
issue itself calls each one "effectively its own new-protocol
implementation, not a tweak" and too much for one PR. FRAD is the
narrowest and rarest of the five -- no Netlink framing, no GRE tunnelling,
no 802.11/radiotap -- just a thin link-layer header, which keeps this PR
reviewable. The other four are filed as separate issues (linked from #2460)
rather than implemented here.

What the dissector covers

The frame, per RFC 2427 §3/§4.1: a 2-octet Q.922 address (DLCI, C/R, FECN,
BECN, DE -- the 2-octet form only), a 1-octet control field, then either

  • direct NLPID (the common case) -- a single NLPID octet selects the
    encapsulated protocol directly. 0xCC (IPv4) and 0x8E (IPv6) dispatch
    onward; every other NLPID (Q.933 signalling, ISO CLNP/ES-IS/IS-IS,
    FRF.9/FRF.12 compression, PPP-in-Frame-Relay) has no dissector in this
    package and is kept as Raw, exactly as the five arphrd_types were kept
    before this PR.
  • SNAP-encapsulated -- a 0x00 pad, then NLPID 0x80, then a 3-octet
    OUI and 2-octet PID. An OUI of 00-00-00 carries an EtherType in the PID,
    resolved through the same family registry (Link.__proto__) that
    Ethernet, VLAN and LinuxSLL already share -- which is how ARP (PID
    0x0806) over Frame Relay reaches ARP, not just IPv4/IPv6. Any other
    OUI (RFC 2427 §4.2's bridged-medium encapsulation) is kept as Raw: this
    package has no bridging dissector.

No FCS is parsed or expected: per tcpdump.org's own LINUX_SLL page, the
ARPHRD_FRAD payload is "a Frame Relay LAPF frame... without an FCS at the
end of the frame".

Tests

  • tests/protocols/link/test_frad_unit.py -- the address-field bit layout
    (checked against RFC 2427 §3's own worked example, DLCI 50 = 0x0C21),
    both encapsulation forms, the SNAP/EtherType dispatch, make()
    validation, and byte-exact round trips.
  • tests/protocols/link/test_linux_sll1_unit.py /
    test_linux_sll2_unit.py -- ARPHRD_FRAD moved out of the "kept raw"
    sweep into its own dispatch test; the other four arphrd_types are
    unaffected.
  • tests/protocols/test_short_read_roundtrip_unit.py -- two FRAD cases
    (direct and SNAP) added to the generic "cut at every offset" sweep.

make test passes; mypy/pylint are clean on every new/changed file.

Checklist

  • Searched for similar pull requests
  • Followed the coding style (pylint, mypy, isort clean on the
    changed files)
  • make test passes, and a test case covers the change
  • Added a changelog entry under docs/source/changelog/1.6.0.rst and
    regenerated CHANGELOG.md

feat

@JarryShaw JarryShaw added the feat Pull requests that add a new capability (feat: subject prefix) label Oct 31, 2026
@JarryShaw JarryShaw added the review: pending No verdict for the current head - never reviewed, or the head moved since the last one label Oct 31, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 31, 2026
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one review: running A cross-review is in flight against the current head - no verdict yet labels Oct 31, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review (Sonnet) at 286a78352: NEEDS CHANGES, two defects.

  1. FRAD is registered in pcapkit/protocols/link/__init__.py but missing from pcapkit/__init__.py, pcapkit/protocols/__init__.py, pcapkit/protocols/data/__init__.py, pcapkit/protocols/data/link/__init__.py, pcapkit/protocols/schema/__init__.py, and pcapkit/protocols/schema/link/__init__.py -- breaks 9 tests (test_link_package_reexports_every_child_export, test_public_api.py, 6 test_module_isolation.py cases). The narrow test file run (55 passed) doesn't cover the export registries, which is why it looked clean.
  2. test_rfc_constants_one_home_1739_unit.py::test_sweep flags frad.py:258's 0 <= snap_pid <= 65535 and several BitField namespace entries in schema/link/frad.py as copies of constants that already have a home elsewhere -- needs reusing the homed constants or the sweep's allow-list convention.

RFC 2427 framing itself confirmed correct (Q.922 address layout, NLPID dispatch, SNAP OUI/PID handling), LinuxSLL wiring correct, changelog present, PR body correctly says 'Part of #2460' not 'Closes'. Fixing both now.

Part of #2460's five arphrd_type follow-ups: LinuxSLL/LinuxSLL2 fell back
to Raw for ARPHRD_FRAD, the narrowest of the five, since no Frame Relay
dissector existed.

- Add pcapkit.protocols.link.frad.FRAD, parsing the Q.922 address, control
  field and NLPID of RFC 2427's multiprotocol encapsulation (obsoleting
  RFC 1490), both the direct-NLPID and SNAP-encapsulated forms.
- Wire LinuxSLL1/LinuxSLL2 to dispatch ARPHRD_FRAD to it instead of Raw.
- Direct NLPID 0xCC/0x8E dispatch onward to IPv4/IPv6; a SNAP header with
  OUI 00-00-00 dispatches by its PID through the EtherType registry
  Ethernet/VLAN/LinuxSLL already share (reaching ARP, among others). Every
  other NLPID, and a SNAP header naming a bridged medium, stays Raw.
- Add tests/protocols/link/test_frad_unit.py; move ARPHRD_FRAD out of the
  LinuxSLL "kept raw" sweep into its own dispatch test; add two FRAD cases
  to the short-read round-trip sweep.
- Register FRAD in every export registry it was missing from: pcapkit,
  pcapkit.all, pcapkit.protocols, and the data/schema registries under
  pcapkit.protocols.{data,schema}[.link].
- Add NOT_A_COPY entries to the #1739 constant-sweep test for FRAD's bit
  offsets and field bounds that coincidentally match homed RFC constants.

Build: mypy and pylint clean; FRAD, LinuxSLL, export-registry, #1739
sweep, and protocol-convention test suites all pass.
@JarryShaw
JarryShaw force-pushed the feat/2460-frad-dissector branch from 286a783 to a8b5c0e Compare October 31, 2026 21:26
@JarryShaw

Copy link
Copy Markdown
Owner Author

Pushed a8b5c0ec7: fixed both defects. FRAD was missing from 6 of 7 export registries (and a 7th, pcapkit/all.py, caught by test_every_aggregated_export_is_listed_or_excluded that wasn't in the original scope) -- added to all 7. Added 4 NOT_A_COPY entries to test_rfc_constants_one_home_1739_unit.py for the two re-inlined-constant false positives (the SNAP PID bound and 3 BitField offset literals in frad.py/schema/link/frad.py).

Verified: tests/protocols/link/ tests/project/ tests/const/ -- 2691 passed, 0 failed, 35843 subtests. One false alarm along the way (235 failures from a PYTHONSAFEPATH=1 invocation breaking util/*.py subprocess calls) correctly diagnosed and ruled out before the real run. Re-review owed on this head.

@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment review: running A cross-review is in flight against the current head - no verdict yet labels Oct 31, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review (Sonnet) at a8b5c0ec7: GOOD TO GO. FRAD confirmed present in all 8 registries, all 4 NOT_A_COPY entries confirmed genuine false positives (FRAD's own Q.922 bit offsets and SNAP PID bound, not copies of homed constants). Full suite reproduced exactly: 2691 passed, 0 failed, 35843 subtests, no collection errors. No conflict markers or pasted text. mergeStateStatus: BEHIND is just the strict-ruleset effect of recent pushes to main (no actual conflict) -- the owner bypasses that on merge. Ready for merge -- your call.

@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 94.47% (unit tier, Python 3.15, a8b5c0ec7, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19738 133 2642 120 98.85%
pcapkit/corekit 2605 74 818 37 96.52%
pcapkit/dumpkit 281 3 100 3 98.43%
pcapkit/foundation 3183 27 1110 31 98.65%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 18597 203 4898 187 98.33%
pcapkit/toolkit 924 36 334 10 95.55%
pcapkit/utilities 502 7 144 5 98.14%
pcapkit/vendor 4510 1888 1034 169 54.89%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw
JarryShaw merged commit c5a8c97 into main Nov 1, 2026
12 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Nov 1, 2026
@JarryShaw
JarryShaw deleted the feat/2460-frad-dissector branch November 1, 2026 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat Pull requests that add a new capability (feat: subject prefix) review: good-to-go Cross-review at the current head says ready; CI state is separate

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant