Skip to content

[feature] Fix #1095 Implement full IPv4 header and fragmentation#1108

Draft
aaronddavies wants to merge 8 commits into
smoltcp-rs:mainfrom
aaronddavies:adavies/1095-implement-full-ipv4-header
Draft

[feature] Fix #1095 Implement full IPv4 header and fragmentation#1108
aaronddavies wants to merge 8 commits into
smoltcp-rs:mainfrom
aaronddavies:adavies/1095-implement-full-ipv4-header

Conversation

@aaronddavies

@aaronddavies aaronddavies commented Dec 8, 2025

Copy link
Copy Markdown
Contributor

Fixes #1095.

Changes to operational code:

  • Add the missing IPv4 fixed length fields. They are now a part of the Ipv4Repr struct so that they are no longer dropped when message traffic goes through smoltcp. (TODO - this is being updated to pass the bytes to raw sockets only, and not augment the Repr.)
  • Add support for options. This implemented as a fixed size byte array to the IPv4Repr struct. The header length field determines how many bytes of the options array are to be included when emitting from the struct. Bench tests showed that a the raw array had better performance than using Option<[u8]>. (TODO - see above.)
  • Filter options across fragments for outgoing packets.
    1. Options are persisted across fragments if the copy bit in the type octet is set.
    2. Option lengths are preserved according to the length octet following the type octet, with the exception of padding octets.
  • Capture the options from an incoming packet. The options of the first incoming fragment of a packet stream are captured as the representative set of options for the final assembled packet, which is to be delivered to the sockets. NOTE: An options capture error in this process will cause the entire packet to be dropped.

Changes to unit tests:

  • Update all test IPv4 repr definitions to include the init of the rest of the header fields. This boilerplate is a significant portion of the line changes in this PR. (TODO - see above, this will no longer be necessary.)
  • Fix unit tests that don't require fragmentation to not specify fragmentation in their test packet headers
  • Use a config-if block for choosing the right IpvX struct repr (TODO - see above, this will no longer be necessary.)
  • Fix some places to only import related modules if features require them
  • Add unit tests for the new changes listed above

Reference:

  1. RFC 791
  2. IPv4 Parameters - Options

Bench tests before & after

current HEAD main (39cd44e)
test wire::bench_emit_ipv4 ... bench:           8.27 ns/iter (+/- 0.52)
test wire::bench_emit_ipv6 ... bench:           1.11 ns/iter (+/- 0.01)
test wire::bench_emit_tcp  ... bench:          29.89 ns/iter (+/- 0.20)
test wire::bench_emit_udp  ... bench:          25.04 ns/iter (+/- 0.18)


this branch (1bce638)
test wire::bench_emit_ipv4 ... bench:           9.44 ns/iter (+/- 0.54)
test wire::bench_emit_ipv6 ... bench:           1.11 ns/iter (+/- 0.01)
test wire::bench_emit_tcp  ... bench:          29.90 ns/iter (+/- 0.40)
test wire::bench_emit_udp  ... bench:          25.06 ns/iter (+/- 0.44)

@codecov

codecov Bot commented Dec 8, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.70312% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.62%. Comparing base (08c7be0) to head (0025017).

Files with missing lines Patch % Lines
src/wire/icmpv4.rs 0.00% 16 Missing ⚠️
src/iface/interface/ipv4.rs 92.00% 2 Missing ⚠️
src/wire/ipv4.rs 98.70% 2 Missing ⚠️
src/iface/fragmentation.rs 99.34% 1 Missing ⚠️
src/iface/interface/mod.rs 90.90% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1108      +/-   ##
==========================================
+ Coverage   81.28%   81.62%   +0.34%     
==========================================
  Files          81       81              
  Lines       24838    25306     +468     
==========================================
+ Hits        20189    20656     +467     
- Misses       4649     4650       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@aaronddavies aaronddavies marked this pull request as ready for review December 13, 2025 15:37
@Dirbaio

Dirbaio commented Jan 30, 2026

Copy link
Copy Markdown
Member

Thanks for the PR!

I'm concerned adding all these fields and options to Ipv4Repr will increase memory usage. Ipv4Repr intentionally includes only the fields smoltcp cares about when emitting packets. Do you think it's possible to fix this by avoiding roundtrips through Ipv4Repr? ie avoiding parsing+emitting in the first place, just copying bytes.

Fragment packet data in multiples of 8 octets according to RFC 791. The fragment offset for packet fragments is measured in units of 8 octets, therefore payloads must be fragmented in multiples of 8. This adds logic in two places: 1) where the first fragment is sent immediately, and 2) where each successive fragment is sent from the queue.

This is a nice bug fix. could you send a PR with just this change?

@aaronddavies

Copy link
Copy Markdown
Contributor Author

@Dirbaio thank you for the review and feedback! That's a good point and I will get the bugfix into a separate PR.

I will also look at carrying around the original bytes without augmenting the repr.

@aaronddavies

Copy link
Copy Markdown
Contributor Author

Fragment packet data in multiples of 8 octets according to RFC 791. The fragment offset for packet fragments is measured in units of 8 octets, therefore payloads must be fragmented in multiples of 8. This adds logic in two places: 1) where the first fragment is sent immediately, and 2) where each successive fragment is sent from the queue.

This is a nice bug fix. could you send a PR with just this change?

Opened #1116.

@aaronddavies aaronddavies changed the title [feature] Fix #1095 #1104 Implement full IPv4 header and fragmentation [feature] Fix #1095 Implement full IPv4 header and fragmentation Feb 10, 2026
@aaronddavies aaronddavies force-pushed the adavies/1095-implement-full-ipv4-header branch 3 times, most recently from 9cfc3a3 to 0025017 Compare February 10, 2026 06:19
@aaronddavies aaronddavies marked this pull request as draft February 11, 2026 13:45
@aaronddavies aaronddavies force-pushed the adavies/1095-implement-full-ipv4-header branch from 0025017 to 1d5392d Compare February 11, 2026 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Sending a raw IP packet drops some fields from original IP header

2 participants