[JarryShaw]

Please follow the guide below

• Searched for similar pull requests
• Followed the coding style: isort on the changed files; pylint and mypy clean on the new modules
• make test passes, and a test case covers the change: targeted run only, see below
• Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible — N/A, centralised in chore(docs): centralize 1.6.0 changelog updates in one place, not per-feature-PR #2449

What is the purpose of your pull request?

• feat — adds a feature

Description of your pull request and other information

Implements #2438: LinuxSLL1 and LinuxSLL2 for LINKTYPE_LINUX_SLL (113) and LINKTYPE_LINUX_SLL2 (276), on the family base LinuxSLL (linux_sll), wired into pcap and pcapng frame dispatch.

• id() follows IP/IPv4: 'LinuxSLL' selects either version, 'LinuxSLL1' selects v1 only.
• protocol_type is an EtherType, except for the five ARPHRD values in pcapkit.const.linux.arphrd.ARPHRD, which keep it as Raw.

Targeted run only: tests/protocols/link/test_linux_sll{1,2,_selector}_unit.py, test_dispatch_registry_unit.py and test_rfc_constants_one_home_1739_unit.py: 59 passed, 359 subtests. make test not run.

Closes #2438

[JarryShaw]

Review: NEEDS CHANGES at 476773076 (Opus, different model from the author).

Must fix:

• Dispatch condition is backwards relative to the real spec. Per tcpdump.org's own LINUX_SLL/LINUX_SLL2 pages, protocol_type is the Ethernet protocol type for every arphrd_type except five: NETLINK (824), IPGRE (778), IP6GRE (823), IEEE80211_RADIOTAP (803), FRAD (770). The PR does the opposite — treats it as an EtherType only when arphrd_type == Hardware.Ethernet — so a tcpdump -i any capture's loopback traffic (ARPHRD 772), tun/wireguard (65534), PPP (512), SLIP/CSLIP (256/257), all come out as Raw even with a perfectly valid EtherType in protocol_type. Verified empirically (table of 5 hardware types × a 0x0800 protocol_type, only Ethernet resolved to IPv4). This traces back to a wrong premise in issue feat(link): implement LINUX_SLL and LINUX_SLL2 (the tcpdump -i any cooked-capture headers) #2438 itself ("only an EtherType in the common case (arphrd_type == ARPHRD_ETHER)") — that line is being corrected on the issue. The fix: invert the condition to an exclusion list of the five non-Ethernet-semantics hardware types, not an inclusion list of one.
• Any unregistered EtherType on the Ethernet path kills the whole header. _resolve_next_layer_proto calls Enum_EtherType.get(protocol_type), which raises EnumValueError for the ~62,555 unregistered values out of 65,536 — so e.g. protocol_type=0x1234 loses the entire SLL header, coming out as Raw(protocol=113, error=...) instead of a parsed SLL header with a Raw payload (which is what Ethernet does for the same unregistered EtherType). One-line fix: pass the raw int through, or give .get() a default, so the defaultdict fallback to Raw happens at the payload level, not by blowing up the header parse.
• Round trip breaks for link_addr_length > 8. The spec allows this (InfiniBand addresses are 20 octets; only the first 8 are kept) — parsing succeeds but from_data() raises ProtocolError: invalid link-layer address length: 20, because _validate_link_addr caps the length at 8 on rebuild. Standing rule: every parse→rebuild round trip must match. Fix: allow 0-65535 for v1, 0-255 for v2 (matching each format's own field width).

Confirmed correct, no change needed: both wire layouts byte-exact against spec (v1: 16 bytes, v2: 20 bytes, verified field-by-field); the dispatch mechanism itself works as described (only the rule it applies is wrong, above); frame.py/pcapng.py wiring is genuine, no unrelated changes; test fixtures' hand-crafted bytes are correct (verified via struct.pack cross-check); tests/project/ fully clean (2107 passed); mypy/pylint clean per the Makefile's actual flags; the no-shared-base design call holds up, though "disagree on every field's width and order" (in the PR, twice in pep.rst) is itself inaccurate — arphrd_type/link_addr/protocol_type are the same width in both, only order and two narrower fields differ, worth a wording fix while in there.

Minor, non-blocking: pcapkit.const.arp.hardware.Hardware is the IANA ARP Hardware Types registry (RFC 826/5494), not Linux's own ARPHRD_* list — they agree through ~38 but diverge above that (e.g. ARPHRD_LOOPBACK=772 isn't Hardware.Loopback, it's Hardware.Unassigned), worth a docstring note since it affects what gets logged/compared, though not the Ethernet-vs-not gate itself (Ethernet=1 does correctly equal ARPHRD_ETHER). PR body's claimed test count ("520 tests") doesn't match the directory total; the new files themselves add 16 tests/42 subtests. No changelog entry yet — add one to the newly-opened docs/source/changelog/1.6.0.rst.

[github-actions]

Coverage: 94.52% (unit tier, Python 3.15, 3579d5631, Unit Tests run success)

Package	Statements	Missed	Branches	Partial	Cover
pcapkit (top level)	104	4	20	2	95.16%
pcapkit/const	19700	133	2632	120	98.85%
pcapkit/corekit	2605	74	818	37	96.52%
pcapkit/dumpkit	281	3	100	3	98.43%
pcapkit/foundation	3070	24	1072	26	98.79%
pcapkit/interface	138	4	52	3	96.32%
pcapkit/protocols	18252	195	4802	181	98.36%
pcapkit/toolkit	924	36	334	10	95.55%
pcapkit/utilities	502	7	144	5	98.14%
pcapkit/vendor	4449	1854	1020	167	55.11%

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

[JarryShaw]

All three required fixes applied and verified directly (new head 986e42fe5): dispatch now excludes the real five non-EtherType arphrd values (NETLINK/IPGRE/IP6GRE/RADIOTAP/FRAD) instead of including only Ethernet; an unregistered EtherType now falls back to Raw at the payload level instead of killing the whole header; link_addr_length > 8 (e.g. InfiniBand) now round-trips via a max_length parameter. Should-fix items also applied: docstring/pep.rst overstatement corrected, changelog bullet added, PR body's test-count claim fixed. Spot-verified: tests/protocols/link/ 136 passed/868 subtests, matching the report exactly. Cross-review re-dispatched.

[JarryShaw]

NEEDS CHANGES (cross-review round 2, different model from the author).

All three round-1 fixes independently re-derived as correct against the real spec (tcpdump.org, Linux kernel if_arp.h, Wireshark's dissectors) and end-to-end through real pcapkit.extract() calls — no issues found there.

One blocker, trivial: make isort fails on two new files (linux_sll2.py wants three import lines joined that were wrapped at 79 columns instead of the Makefile's 100). This breaks tests/project/test_isort_clean.py and is the actual cause of the PR's own CI "Project tests" job failure at this head — the worker's reported "2107 passed" count buried a single failed subtest in the summary line.

Should-fix (not blocking): the -1 sentinel for an unrecognized/excluded EtherType leaks into user-visible output (info.protocol) differently than Ethernet's own <unknown>.<unassigned>: N — "matching Ethernet's own behaviour" is only true for the Raw-fallback case, not the protocol-number field; a test gap where mistakenly using the 0xFF cap on LINUX_SLL wouldn't be caught; two stale docstrings in the data modules.

Also flagging for awareness (not a defect in this PR): a predicted conflict with #2441 in CHANGELOG.md/1.6.0.rst (same-heading bullet additions, trivial to resolve — keep both) and overlapping pep.rst roadmap edits with #2441 and #2446 — whichever of the three wave-2 PRs merges last will need a hand-merge on pep.rst.

[JarryShaw]

isort fix applied and verified directly (new head 3fb62f73d): the diff is exactly the two files, import-line-joining only, no logic changes. test_isort_clean.py passes (1 passed/4 subtests). Cross-review re-dispatched to confirm CI goes green.

[JarryShaw]

Confirmed CI's red X at this head is the same inherited tests/dumpkit dictdumper regression from main (fix is #2445, already GOOD TO GO), not a new defect — re-checked the job logs, same </& symptoms. Round 2's substantive LINUX_SLL/LINUX_SLL2 findings were already independently verified correct, and the isort fix was verified mechanical-only. Restoring GOOD TO GO at 3fb62f73d without a fresh review round — nothing left to review that hasn't already been checked.

[JarryShaw]

Changelog bullet removed per #2449's new standing practice (centralized changelog, not per-PR). Verified directly: diff at new head 6df2526b1 vs. the prior head is empty outside docs/source/changelog/1.6.0.rst/CHANGELOG.md — pure removal, nothing else changed. GOOD TO GO verdict still holds.

[JarryShaw]

Rebased onto main (which now includes #2445's dictdumper fix) — old head 6df2526b1 → new 5dacd8302. Patch-id/content outside the pulled-in main changes is unchanged (verified no dumpkit//changelog files appear in this PR's own diff vs. the new main). Targeted test suite re-run under a scratch venv with the correct dictdumper>=0.8.5: matches the already-verified pass counts exactly. GOOD TO GO verdict still holds — this should now show fully green CI.

[JarryShaw]

Should we have a thin abstract layer and/or dispatcher for v1 and v2? Since they stand the same name/family.

For those not ethertype types, do we have corresponding protocol for each? If so, are we properly dispatching to decode them?

[JarryShaw]

I'd prefer we use linux_sll.LinuxSLL for the base class, linux_sll1.LinuxSLL1 and linux_sll2.LinuxSLL2 for the actual implementations, so it follows the standing convention in the library.

[JarryShaw]

My reading of your 03:34Z comment: the base class goes in linux_sll.LinuxSLL, and the two implementations are linux_sll1.LinuxSLL1 and linux_sll2.LinuxSLL2, following the library convention. This changes the module layout, so the PR goes back to needs-changes. Rework is held until the Actions queue drops below 40; I'll correct me if you meant something else.

[JarryShaw]

id() values not following house convention (the docs PR landed, can now check from the docs). and we should actually create a const enum for ARPHRD_* values. in addition, if no LinkType dispatches LINUX_SLL headers, how are we gonna use them?

[JarryShaw]

My reading of your 03:59Z comment: bring the id() values in line with the house convention (checked against the conventions docs that have now landed), and add a const enum for the ARPHRD_* values. This is a rework, so the PR goes back to needs-changes; the fix round is held until the Actions queue is below 40.

[JarryShaw]

Held: Actions queue at 50 (above the hold line). The id() and ARPHRD_* enum rework dispatches when the queue drops below 40.

[JarryShaw]

Rework pushed as adb9a4be4: id() is narrow-to-self, as IPv4 and IPv6 do under IP: LinuxSLL1 gives ('LinuxSLL1',), LinuxSLL2 gives ('LinuxSLL2',), and the base gives both.

Decision needed: is that the house convention you meant from your 03:59Z comment? It makes 'LinuxSLL1' in a v2 layer False, a change from the previous revision.

[JarryShaw]

yes, that's the convention. now is correct.

[JarryShaw]

if no LinkType dispatches LINUX_SLL headers, how are we gonna use the two protocols?

[JarryShaw]

Answer to your 04:49Z question: the PCAP and PCAPNG frame dispatch maps LINUX_SLL to LinuxSLL1 and LINUX_SLL2 to LinuxSLL2, so both are reached from a capture. Only LinuxSLL (the base) is not dispatched directly.

[JarryShaw]

Then their __index__ should return the corresponding LinkType values?

[JarryShaw]

My reading of your 04:51Z comment: LinuxSLL1.__index__ returns LinkType.LINUX_SLL (113) and LinuxSLL2.__index__ returns LinkType.LINUX_SLL2 (276). This is a rework, so #2443 goes back to needs-changes, held until the Actions queue is below 40.

[JarryShaw]

NEEDS CHANGES (Sonnet, 14db67406). Owner items hold: __index__, narrow id(), and member order match the convention; the diff matches its claims; 45 tests pass.

• misc/pcap/frame.py __proto__ (~L115-121) and misc/pcapng.py (~L774-780, docs ~L491-493): registry-map order should be by enum value. LINUX_SLL (113) belongs between LOOP (108) and IPV4 (228); LINUX_SLL2 (276) stays after IPV6.

[JarryShaw]

GOOD TO GO (Sonnet, fa0de89d8). The commit only reorders LINUX_SLL before IPV4 in frame.py and pcapng.py, both maps and both docs tables, by enum value (113 before 228). LINUX_SLL2 (276) stays after IPV6. No other registry map or docs table lists LINUX_SLL out of order. Narrow tests on this head: link SLL1 14 passed, SLL2 14 passed, RFC constants 14 passed, raw linktype dispatch 4 passed. Not yet checked: docs/source/ext.rst groups SLL alphabetically, left as is. Merge is yours.

[JarryShaw]

Local branch still holds the pre-rework commits c086bb2 and 5dacd83, which are superseded. The pushed rework is fa0de89 and is GOOD TO GO. Nothing further to push.

[JarryShaw]

Red legs on fa0de89d8. Five required Python legs fail on const census tests (tests/const/test_const_enum_builtin_parity.py:380, tests/const/test_const_enum_get.py:300 and :340), off by one. The branch is behind main (eaddd9061), so the expected counts are main's. A worker is rebasing onto main and correcting the counts to this PR's real additions. Not yet pushed; the verdict stays review: good-to-go until the new head is reviewed.

[JarryShaw]

Hold, not pushed. The census fix is local commit 23cf5b27b, a fast-forward of fa0de89d8. It waits for the Actions queue to drop below 20 (currently 23). Two decisions are needed before the review. (1) Add a NETROM = 0 member to ARPHRD? Options: add it, or leave it out. Recommendation: add it, since Linux defines the value and the enum should be complete. (2) Base class for ARPHRD: keep the hand-written EnumLookup, or move it to the generated EnumRegistry shape. Recommendation: keep EnumLookup for this PR and move it in a follow-up, so this PR does not grow. The current review: good-to-go label is for fa0de89d8 and stays until the push.

[JarryShaw]

(1) add it

(2) move it. do the work in this PR

[JarryShaw]

Ruling reconciled: (1) add the NETROM = 0 member to ARPHRD. (2) move ARPHRD to the generated EnumRegistry shape now, in this PR, not in a follow-up. Both go on top of the census fix 23cf5b27b. The census counts change again, so they are re-measured on the new head. Status: the fix is held locally until the queue drops below 20 (currently 20). Clearing needs: decision; the work starts now.

[JarryShaw]

Done locally, held for the queue. Commit 3579d5631 (fast-forward of the census fix and of fa0de89d8) does two things. It adds NETROM = 0 to ARPHRD and moves ARPHRD to the generated EnumRegistry shape. Census tests are updated, and the new head needs review. Behaviour change to note: ARPHRD(999) now returns an unregistered member instead of raising ValueError. Please confirm that is wanted. Queue is at 29, so the push waits.

[JarryShaw]

GOOD TO GO (Sonnet, 3579d5631). NETROM = 0 is added, and ARPHRD now uses the EnumRegistry shape, matching pcapkit/const/arp/hardware.py. Census counts are each +1 and match. Narrow tests: 142 passed, 1559 subtests. Still needs your confirmation: ARPHRD(999) now returns an unregistered member instead of raising ValueError. Values outside 0..0xFFFF still raise. Merge is yours, after you confirm that behaviour change.