[noq-udp] Retransmit failing GSO packets and split batches#746
[noq-udp] Retransmit failing GSO packets and split batches#746kixelated wants to merge 3 commits into
Conversation
|
Alternatively: quinn-rs/quinn#2724 |
|
The Initial ending up in a GSO batch surprised me, but I guess that's a PQC thing where you could end up with two packets for the Initial. |
flub
left a comment
flub
left a comment
There was a problem hiding this comment.
Yeah, I agree that this is desirable. It's not even that complex.
Once interesting consequence from this is that it maybe no longer pays off for the caller to try and figure out what batch sizes etc are needed. It can just pick a default batch size and always throw it in (if it's congestion control and pacing is fine with that). The socket will than send it. Though there are the other platforms to worry about and I wouldn't be in a hurry to adopt that strategy. But an interesting thought.
| let mut pos = 0; | ||
| while pos < transmit.contents.len() { | ||
| let remaining = transmit.contents.len() - pos; | ||
| let batch_end = pos + next_batch_bytes(remaining, max_bytes, segment_size); |
There was a problem hiding this comment.
I don't really like these two helpers that take so many usize args, it is too easy to get confused. I'm tempted to go all out and build a TransmitSplitter that you can construct with the original transmit and then keep calling TransmitSplitter.next_transmit(&UdpSocketState) and TransmitSplitter.advance() if the transmit did send successfully.
Though the current code isn't terrible. And I could be convinced that it is fine too if you really don't like that for some reason.
2 participants
IDK if quinn-rs/quinn#2653 will ever get merged.
The main benefit is not adding ~500ms or whatever to the QUIC connection time on devices where GSO immediately fails, apparently Android. This should also fix disabling GSO if the batch size gets too large somehow (but I don't know how to trigger that).