feat(phy): add TCP segmentation support#1148
Conversation
It is possible for packets larger than the current capture limits to be sent with the segmentation offload feature.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1148 +/- ##
==========================================
- Coverage 81.48% 81.48% -0.01%
==========================================
Files 81 81
Lines 25007 25014 +7
==========================================
+ Hits 20378 20383 +5
- Misses 4629 4631 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| crate::wire::IpVersion::Ipv6 => segmentation_caps.tcpv6, | ||
| } | ||
| .map(|buf_size| { | ||
| #[cfg(feature = "medium-ethernet")] |
There was a problem hiding this comment.
checking feature = "medium-ethernet" here is not enough, you have to check the actual medium in the phy capabilities.
It's possible to enable medium-ethernet and medium-ip at the same time, so even if medium-ethernet is enabled you may have an IP medium phy.
| #[cfg(feature = "packetmeta-id")] | ||
| pub id: u32, | ||
| #[cfg(feature = "segmentation-offload")] | ||
| pub segmentation_offload_size: Option<NonZeroU16>, |
There was a problem hiding this comment.
Can you document what this does?
| ip_mtu - ip_repr.header_len() - TCP_HEADER_LEN | ||
| }) | ||
| .unwrap_or(segment_size) | ||
| }; |
There was a problem hiding this comment.
can you add tests for the logic here? e.g. that if segmentation offload is enabled oversized packets are emitted up to the limit supported by the phy, with the right MSS value in the meta.
The Windows and Linux documentations for the same feature were used as references. It was more difficult to find information on how devices support the feature, so the interface is mostly based on how virtio-net devices expect to be driven. The code was tested on the Hermit kernel with a virtio-net device, but only with IPv4, as Hermit does not support IPv6.
Design rationale
packetmeta-id.TcpPackets to set the partial checksum in our case) functional for sufficiently small unsegmented packets. On the other hand, it can also be argued that causing a failure even for smaller packets can help uncover the error case with the larger packets earlier in the development of the device drivers.Comparison to #830
ChecksumCapabilities. This PR does not do so for the following reasons:SegmentationCapabilitiesrather than the TCP specific nameTsoCapabilities. I believe the current structure can be expanded to support UDP, though I currently do not have a prototype for that.set_metacalls seems correct to me but because it is not segmentation offload specific and I was not able to test its necessity because of the lack of IPv6 support on our test platform, I did not include it in this PR.