Skip to content

Add IgmpSlice for zero-copy IGMP parsing#148

Open
jeff-moon wants to merge 1 commit intoJulianSchmid:masterfrom
jeff-moon:igmp_slice
Open

Add IgmpSlice for zero-copy IGMP parsing#148
jeff-moon wants to merge 1 commit intoJulianSchmid:masterfrom
jeff-moon:igmp_slice

Conversation

@jeff-moon
Copy link
Copy Markdown

@jeff-moon jeff-moon commented May 6, 2026

Adds zero-copy slice types for IGMP packets covering v1, v2, and v3:

  • IgmpSlice: wraps raw IGMP packet bytes with field accessors
  • ReportGroupRecordV3Slice: zero-copy access to IGMPv3 group records
  • ReportGroupRecordV3SliceIter: iterates group records in v3 reports

Summary by CodeRabbit

  • New Features
    • Added IGMPv3 group record slice support for efficient parsing of group records with source addresses and auxiliary data.
    • Added IGMP slice reader enabling safe parsing of IGMP headers and payloads across IGMPv1, v2, and v3 message types without copying data.

Adds zero-copy slice types for IGMP packets covering v1, v2, and v3:
- IgmpSlice: wraps raw IGMP packet bytes with field accessors
- ReportGroupRecordV3Slice: zero-copy access to IGMPv3 group records
- ReportGroupRecordV3SliceIter: iterates group records in v3 reports
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 2026

📝 Walkthrough

Walkthrough

This PR introduces zero-copy slice-based parsing for IGMPv3 group records and general IGMP packets. Two new slice types (ReportGroupRecordV3Slice and IgmpSlice) are added with constructors performing length validation and accessors for header fields and payloads, then publicly re-exported from the transport module.

Changes

IGMP v3 Slice Parsing

Layer / File(s) Summary
Core Data Structures
etherparse/src/transport/igmp/report_group_record_v3_slice.rs
New ReportGroupRecordV3Slice<'a> struct provides zero-copy access to a single IGMPv3 group record with from_slice() validation, accessors for record type, source count, multicast address, source addresses, and auxiliary data. New ReportGroupRecordV3SliceIter<'a> enables iteration over multiple records.
IGMP Packet Slicing
etherparse/src/transport/igmp_slice.rs
New IgmpSlice<'a> struct provides zero-copy parsing of full IGMP packets with from_slice() validation and accessors for IGMP type, response codes, checksums, and payloads, supporting IGMPv1/v2/v3 variants.
Module Wiring & Export
etherparse/src/transport/igmp/mod.rs, etherparse/src/transport/mod.rs
New report_group_record_v3_slice module declared and publicly re-exported; igmp_slice module added to transport layer and re-exported publicly.
Tests & Validation
etherparse/src/transport/igmp/report_group_record_v3_slice.rs, etherparse/src/transport/igmp_slice.rs
Comprehensive unit tests cover boundary conditions, zero/non-zero sources, auxiliary data, truncation errors, field accessors, iterator behavior, and formatting for both slice types.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

  • JulianSchmid/etherparse#142: Extends IGMP slice-level parsing by adding report group record v3 slicing and overall IGMP packet slicing, building directly on IGMP header and type definitions.

Suggested reviewers

  • JulianSchmid

Poem

🐰 Hop hop, the slices now align,
Zero-copy parsing—a design so fine,
IGMPv3 records dance without a clone,
Through byte arrays swift, the packets have flown!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add IgmpSlice for zero-copy IGMP parsing' directly and accurately summarizes the main change: introducing a new IgmpSlice type for zero-copy IGMP packet parsing.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jeff-moon
Copy link
Copy Markdown
Author

Thank you for the work you did on the previous IGMP PR @JulianSchmid! I've updated my IGMP slice parsing to conform to the changes you've made. Please let me know if there's any changes you would like me to make here!

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
etherparse/src/transport/igmp_slice.rs (1)

230-258: 💤 Low value

Minor: redundant slicing in test.

bytes is already [u8; 16], so &bytes[..16] is equivalent to &bytes. Not worth fixing on its own — flagging only in case you touch this test for other reasons.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@etherparse/src/transport/igmp_slice.rs` around lines 230 - 258, In the
from_slice_v3_query test, remove the redundant slice expression when calling
IgmpSlice::from_slice by passing the byte array directly (replace &bytes[..16]
with &bytes) to simplify the test; update the call site in the
from_slice_v3_query test that invokes IgmpSlice::from_slice and keep all other
assertions and uses of bytes unchanged.
etherparse/src/transport/igmp/report_group_record_v3_slice.rs (1)

148-152: 💤 Low value

Doc nit: clarify what slice() returns vs. the constructor input.

Worth noting in the doc that slice() is exactly record_len bytes (the full record), not the original slice passed to from_slice. This contrasts with IgmpSlice::slice(), which exposes the full input including trailing payload — small clarification will help users avoid confusion.

📝 Suggested doc tweak
-    /// Returns the full slice of this group record.
+    /// Returns the bytes of this group record (header + sources + aux data).
+    ///
+    /// Note: this is exactly the validated record length, not any
+    /// trailing bytes from the original input passed to
+    /// [`ReportGroupRecordV3Slice::from_slice`] (those are returned
+    /// separately as the second element of the tuple).
     #[inline]
     pub fn slice(&self) -> &'a [u8] {
         self.slice
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@etherparse/src/transport/igmp/report_group_record_v3_slice.rs` around lines
148 - 152, Update the doc comment for the method slice() on
ReportGroupRecordV3Slice to explicitly state that it returns exactly the
record_len bytes of the group record (the full record slice stored in the
struct) rather than the original input slice passed to from_slice; mention the
contrast with IgmpSlice::slice() which exposes the entire input including
trailing payload so callers understand slice() here is trimmed to the record
length.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@etherparse/src/transport/igmp_slice.rs`:
- Around line 230-258: In the from_slice_v3_query test, remove the redundant
slice expression when calling IgmpSlice::from_slice by passing the byte array
directly (replace &bytes[..16] with &bytes) to simplify the test; update the
call site in the from_slice_v3_query test that invokes IgmpSlice::from_slice and
keep all other assertions and uses of bytes unchanged.

In `@etherparse/src/transport/igmp/report_group_record_v3_slice.rs`:
- Around line 148-152: Update the doc comment for the method slice() on
ReportGroupRecordV3Slice to explicitly state that it returns exactly the
record_len bytes of the group record (the full record slice stored in the
struct) rather than the original input slice passed to from_slice; mention the
contrast with IgmpSlice::slice() which exposes the entire input including
trailing payload so callers understand slice() here is trimmed to the record
length.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b14abde8-90f2-450f-a770-9f0f791ace9e

📥 Commits

Reviewing files that changed from the base of the PR and between 7cb4b6f and 8371b85.

📒 Files selected for processing (4)
  • etherparse/src/transport/igmp/mod.rs
  • etherparse/src/transport/igmp/report_group_record_v3_slice.rs
  • etherparse/src/transport/igmp_slice.rs
  • etherparse/src/transport/mod.rs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant