quinn
quinn copied to clipboard
Should `quinn-udp` automatically chunk `Transmit::contents`?
As part of implementing GSO in our application, we ran into a bug where we submitted Transmits that had a way too large contents slice. I went about the implementation a bit too naive unfortunately: https://github.com/firezone/firezone/blob/649c03e2908c797664ded06ef93234312e694cb7/rust/socket-factory/src/lib.rs#L308-L326
Can you spot the bug? Depending on what segment_size is, we may end up with a slice that is much larger than the allowed 65535 bytes by the kernel. A typical MTU of 1200-1500 can easily hit this in a bandwidth heavy application. max_gso_segments defaults to 64 so any segment_size over 1023 with 64 segments will result in a Transmit that will actually be rejected by the kernel.
Here is the fix: https://github.com/firezone/firezone/pull/8754
In order to surface this correctly to the user, I opened https://github.com/quinn-rs/quinn/pull/2199 but I think we should go further. In fact, I think I shouldn't need to write the above linked code at all. quinn-udp already has the knowledge of how many gso segments are currently allowed. My suggestion would be that quinn-udp should implement the linked code fragment and slice the passed Transmit::contents into chunks that are acceptable for the kernel.
From an API design perspective, I think this is much better as it reduces the "chatter" we need to have with UdpSocketState: Instead of having to ask it for the max_gso_segments, we can just pass it a possibly massive slice and quinn-udp takes care of:
- Ensuring that we don't do GSO if we shouldn't, i.e. effectively making many
Transmits out of the one that was passed - Ensuring that we only pass a slice of at most 65535 bytes if the kernel supports GSO.
Thoughts?