quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.55k stars 360 forks source link

Match GSO segment size to the first datagram, not the MTU #1837

Closed Ralith closed 1 month ago

Ralith commented 1 month ago

Fixes #1832.

~Still working on~ how to handle the case where a datagram is queued but won't fit with the current segment size. We need to either detect this in advance and finish the GSO batch before beginning another packet, or gracefully abandon packet assembly after failing to fit any frames. ~Currently leaning towards the latter.~ This will need careful testing.

~Also still TODO: End the GSO batch early if the amount of padding in the current packet would be excessive.~

Ralith commented 1 month ago

On second thought, maybe judging whether a datagram fits into a certain segment size in advance of beginning packet assembly isn't so bad. We already have the logic to make a conservative estimate in max_datagram_size, and (as of the preceding PR) drop datagrams that exceed that limit. Refactoring packet assembly to be safe to abandon is interesting too, and might allow us to simplify or even abandon some can_send logic in the long run, but it's a lot more churn and we can pursue it separately if desired.

Ralith commented 1 month ago

This is now feature-complete, but still needs testing.

Ralith commented 1 month ago

Went ahead and banged out the tests, finding and fixing a number of bugs in the process.

djc commented 1 month ago

Good stuff!