Repository navigation
feat(layer3): decode VRRPv2 and VRRPv3, dispatched from IP protocol 112 - #243
Merged
Merged
Conversation
Add netprotocols.VRRP and its subclass VRRPv6 (RFC 3768, RFC 5798). One class decodes both versions and branches on the version nibble: the fixed fields, the 16 bits between Count and Checksum kept raw as adver_word and read through per-version accessors that answer None where the version has no such field (auth_type and adver_int for v2; reserved and max_adver_int for v3), the virtual addresses, and v2's 8 bytes of authentication data. Only a version other than 2 or 3 is rejected, because the version decides the layout; type, priority 0, count 0 and the reserved bits are carried verbatim. The address width depends on the IP family, not on the header, so the class comes as a pair wired like AH/AHv6: VRRPv6 is registered in ip.proto.v6 before VRRP in ip.proto, and FAMILY_SPECIFIC_NUMBERS now names 112 too. VRRPv6 accepts VRRPv3 only (RFC 3768 is IPv4-only). header_len comes from the version and count, so Ethernet padding and an FCS stay out of the message and out of its checksum. checksum.compute()/verify() and Packet.with_checksums() handle VRRP: the message alone for v2, behind the IP pseudo-header for v3 in both families (RFC 5798 §5.2.8), which is what keepalived 2.2.8 sends by default. RFC 9568, which obsoletes RFC 5798, drops the IPv4 pseudo-header; such an advert fails verify(), documented and tested. compute()'s UDP arm moves into a helper so compute() stays at B(10). tests/fixtures/vrrp.pcap (18 frames) is captured by the new scripts/capture_fixtures_vrrp.sh from two keepalived routers over a veth pair: v2 with PASS auth and two addresses (with failover and a priority-0 advert from each router), v2 without auth, v3 with one address (failover), and v3 unicast with three addresses. check_fixtures.py verifies every VRRP checksum independently and fails a length/count mismatch, an unknown version, v2 over IPv6 and the RFC 9568 IPv4 form. VRRPv3 over IPv6, Ethernet padding and a nonzero trailer are covered by constructed frames in test_vrrp.py, whose checksums it computes itself. Closes #181 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CEQCZGyuvpMyZwUbDucF3G
Conflicts resolved: - CHANGELOG: master turned [Unreleased] into [2.4.0]; the VRRP Added and Development entries move to a fresh [Unreleased] section instead of riding inside the released one. - README: the "Why" paragraph now says the 2.4.0 figures were measured on the 140 frames the corpus held then, so it no longer reads as the current count (158 across 23 scenarios after VRRP). - CLAIMS §5.1 (158 frames, 23 scenarios), §5.2 (26 protocols, 22 generalized round trips, VRRP's accessors never raise), §5.3 and the pyproject.toml comment (3,053 statements, 1 missed) re-measured. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CEQCZGyuvpMyZwUbDucF3G
…and checker Review found mutants the suite let through: - VRRP.advertisement_interval reading Adver Int from the high byte (`adver_word >> 8`): every asserted VRRPv2 word was 0x0101, so Auth Type and Adver Int were indistinguishable. test_no_authentication (word 0x0001) and the Auth Type 2/7 variants now assert the interval. - check_fixtures.py's VRRP length test weakened from `!=` to `<`: no test handed the checker an IP payload longer than the message. A new checker test grows the IPv4 Total Length by 4 (header checksum recomputed) and appends 4 zero bytes, for a VRRPv2 and a VRRPv3 frame, and expects exactly ["vrrp-length"]. - `or 0xFFFF` on _vrrp_checksum's VRRPv2 return: no advert summed to zero. A crafted VRRPv2 message whose checksum is 0x0000 now checks that compute() returns 0x0000 and that 0xFFFF does not verify. - checksum_hex_str's width 6 -> 4: no checksum below 0x1000 was asserted; 0x0abc now is. Each mutant was applied by hand and fails the new test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CEQCZGyuvpMyZwUbDucF3G
…t RFC 5798 The docs said RFC 5798 §5.2.8 puts the IP pseudo-header in VRRPv3's checksum over IPv4 as over IPv6. It does not: §5.2.8 sums the message "and a 'pseudo-header' as defined in Section 8.1 of [RFC2460]", which defines only IPv6's. RFC 9568 §1.1 (item 4) calls its own §5.2.8 a clarification "that it does not include the pseudo-header for IPv4". compute()'s behaviour is unchanged: it adds the IPv4 pseudo-header, keepalived's default and the form vrrp.pcap carries. The wording now says so, wherever it appeared: checksum.py (module, _vrrp_checksum and compute docstrings), vrrp.py, check_fixtures.py's _check_vrrp, ARCHITECTURE, CHANGELOG, the MANIFEST note, and the test docstrings in test_vrrp.py and test_checksum.py. "RFC 9568 drops the pseudo-header" becomes "leaves it out", since that RFC calls the change a clarification. The vendor claim is now cited per manual version: keepalived 2.2.8's keepalived.conf(5) names only "e.g. Cisco" as reading §5.2.8 as IPv6-only; Juniper, and the remark that tcpdump and Wireshark include the IPv4 pseudo-header, first appear in keepalived 2.3.0's. _vrrp_checksum's docstring is rewritten in PEP 257 form. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CEQCZGyuvpMyZwUbDucF3G
The comment over _IPV4_PROTOCOL_CLASSES/_IPV6_PROTOCOL_CLASSES said the v6 table holds its own entry only for AH (51). Since VRRP, it also holds VRRPv6 for 112, as _ip_protocol_class's docstring and _defaults.FAMILY_SPECIFIC_NUMBERS already say. Comment only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CEQCZGyuvpMyZwUbDucF3G
capture_fixtures_vrrp.sh runs `ethtool -K ... || true`, like every other capture script, so nothing checks that offload was turned off; the MANIFEST note and the script's header comment said it was. Both now say the script asks for it as best effort, and that what is guaranteed is check_fixtures.py verifying every IPv4 header checksum and VRRP checksum in the capture. Comment and docs only: the script's behaviour, vrrp.pcap and its staging copy are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CEQCZGyuvpMyZwUbDucF3G
The test added for the width mutant had none, a new pep257 D102. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CEQCZGyuvpMyZwUbDucF3G
3 tasks
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
VRRP model and decoding src/netprotocols/layer3/vrrp.py, src/netprotocols/_enums.py, src/netprotocols/__init__.py, src/netprotocols/layer3/__init__.py, src/netprotocols/_base.py, tests/conftest.py, tests/strategies.py, tests/test_contract.py, tests/test_fuzz.py, tests/test_vrrp.py, tests/test_enums.py, tests/test_docs.py, ARCHITECTURE.md, CHANGELOG.md, CONTRIBUTING.md, README.md, docs/CLAIMS.md |
Adds VRRP and VRRPv6 classes, version-specific decoding and serialization, accessors, validation, and public exports. Tests cover generated headers, truncation, and version-specific fields. |
Family dispatch and checksums src/netprotocols/_defaults.py, src/netprotocols/layer3/ip.py, src/netprotocols/registry.py, src/netprotocols/checksum.py, src/netprotocols/packet.py, tests/test_registry.py, tests/test_checksum.py, tests/test_vrrp.py, ARCHITECTURE.md, README.md |
Registers protocol 112 to use VRRP for IPv4 and VRRPv6 for IPv6. Adds VRRPv2 message checksums and VRRPv3 checksums that use the enclosing IP pseudo-header. |
Fixture capture and validation scripts/capture_fixtures_vrrp.sh, scripts/check_fixtures.py, tests/fixtures/MANIFEST.md, tests/test_vrrp.py, tests/test_corpus.py, CHANGELOG.md, README.md, docs/CLAIMS.md, pyproject.toml |
Adds a capture script for four VRRP scenarios and fixture checks for version, message length, and checksum. Updates corpus and coverage figures and adds tests for captured and modified frames. |
Priority:
Estimated code review effort: 4 (Complex) | ~45 minutes
Change: Feature
Sequence Diagram(s)
sequenceDiagram
participant IP as IPv4 or IPv6 layer
participant Registry as Protocol registry
participant Decoder as VRRP or VRRPv6
IP->>Registry: Look up protocol 112
Registry-->>IP: Return family-specific decoder
IP->>Decoder: Decode payload using family address width
Possibly related PRs
- EONRaider/NETProtocols#233: Adds the family-specific AH dispatch pattern that this change also uses for VRRP.
Suggested reviewers: claude
Merge Risk: 
13923
-
No actionable issue remains from this review; the change is ready for normal merge checks.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 42.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 139 functions across 22 files. (7 skipped… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title clearly and concisely describes the main change: VRRPv2 and VRRPv3 decoding with dispatch from IP protocol 112. Description check The description includes all required sections, explains the implementation and design decisions, lists code, fixtures, tests, and documentation, and records verification results for the required QA g… Linked Issues check Issue #181requirements are implemented.VRRPdecodes VRRPv2 and VRRPv3.VRRPv6decodes IPv6 VRRPv3 with 16-byte addresses and rejects v2. Protocol 112 dispatches through IPv4, IPv6, and IPv6 exte…Out of Scope Changes check The changes stay within issue #181. The capture script and fixture checker support the required VRRP fixture. The checksum helper refactor supports VRRP checksum integration. Tests, fuzz strategies, m…Full details: Description check
-
Explanation
The description includes all required sections, explains the implementation and design decisions, lists code, fixtures, tests, and documentation, and records verification results for the required QA gates.
Full details: Linked Issues check
-
Explanation
Issue
#181requirements are implemented.VRRPdecodes VRRPv2 and VRRPv3.VRRPv6decodes IPv6 VRRPv3 with 16-byte addresses and rejects v2. Protocol 112 dispatches through IPv4, IPv6, and IPv6 extension headers. Tests cover round trips, address widths, truncation, captured adverts, constructed IPv6 frames, registry wiring, checksum handling, and padding.CHANGELOG.mdincludes the change. The PR reports a passing test and verification ladder.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within issue
#181. The capture script and fixture checker support the required VRRP fixture. The checksum helper refactor supports VRRP checksum integration. Tests, fuzz strategies, mutation coverage, exports, registry documentation, architecture documentation, and changelog updates support implementation or verification. No unrelated change is demonstrated.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 42.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 139 functions across 22 files. (7 skipped: 7 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This was referenced Oct 25, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #181. This is the first item of Tier 2 (#172).
IP protocol 112 now dispatches to
netprotocols.VRRPfrom IPv4 and to its subclassVRRPv6from IPv6 and its extension headers. One class decodes both VRRPv2 (RFC 3768) and VRRPv3 (RFC 5798), branching on the version nibble.The branch was built by an implementer sub-agent and then reviewed by an independent sub-agent. A third agent fixed the findings and a fourth reviewed those fixes, per the repo's
stewardrule. The commit history shows each round.Design decisions worth your attention
VRRP/VRRPv6pair. The address list's element width (4 bytes or 16) depends on the carrying IP family, and nothing in the VRRP header says which.AH/AHv6precedent:VRRPv6is registered inip.proto.v6ahead of the inherited entry.FAMILY_SPECIFIC_NUMBERSis now{51, 112}.VRRPv6rejects v2, since RFC 3768 §1.2 is IPv4-only.verify()therefore returnsFalsefor an RFC 9568-form advert. Supporting both forms is tracked in VRRPv3 over IPv4: support the checksum without the IPv4 pseudo-header (RFC 9568) alongside keepalived's default #242, with the options set out there. The docs attribute each claim to its source and cite keepalived's manual by version.adver_word, the way ICMP keepsrest.Nonewhere that version lacks the field:auth_type/adver_intfor v2,max_adver_intfor v3.advertisement_intervalgives either version's interval in seconds.header_len. That is tested on padded corpus frames and by a fuzz property.TruncatedHeaderErrornamingcount_ip_addrsorauth_data.compute/verify/Packet.with_checksums().compute()'s UDP arm moved into a helper so its complexity doesn't grow. An independent differential test showed the move is behaviour-identical over 220k UDP datagrams.What's included
src/netprotocols/layer3/vrrp.py,IPProtocol.VRRP(112, "VRRP"), the two registry rows, a_vrrp_checksumarm, and exports.tests/fixtures/vrrp.pcap, 18 frames.scripts/check_fixtures.py: a new stdlib_check_vrrpverifies v2 and v3 checksums independently. It fails onvrrp-lengthandvrrp-version, and tamper tests cover each.tests/test_vrrp.py.TestVRRPChecksumintest_checksum.py, which is in mutmut scope.ALL_PROTOCOLS, round-trip strategies for both classes, an accessor-safety property, and a padding/trailer property.ipv6.disable=1.pyproject.tomlcoverage comment, re-measured; CHANGELOG under[Unreleased].Verification
uv run ruff checkanduv run ruff format --checkare cleanuv run mypyis clean (strict)uv run pytestpasses locally: 3,582 tests, CI's coverage command gives 99.97%, andvrrp.py/checksum.pyare at 100% line and branchCHANGELOG.mdhas an entry under## [Unreleased]scripts/check_fixtures.py tests/fixturespasses on all 158 framescheck_mutmut_survivors.pywith no new allowlist entries, and every mutant of_vrrp_checksumand_udp_checksumis killed.New protocol or dispatch change — also:
IPProtocol.VRRPcarries a display name in lockstepALL_PROTOCOLS/ROUND_TRIP_STRATEGIESNotes
Generated by Claude Code
Summary by CodeRabbit