feat(s2n-quic): add jumbo frame support #1630
Conversation
This commit adds unit tests for jumbo frame mtus, and refactors some of the current quic sim testing setup to try and bit a bit more straightforward.
This commit adds an example that shows how to configure an io provider to use jumbo frames with a larger buffer. I was running into permissions notes so it might be worth adding a readme describing how to actually use the example or if there are specific permissions issues with ... buffer sizing?
Previously buffers were too small to support probing for jumbo frames, because the write contexts weren't large enough to write the big packets into. This commit sizes the buffers according to the desired max_mtu.
This commit adds a readme for the jumbo frame example. It also adds an mtu event subscriber that will print out MTU updates to the console.
|
I'll work on figuring on the CI errors, and re-request review after I've gotten those squashed. |
* format with stable toolchain, not nightly * fix spelling mistakes * fix queue implementation (compile time switch problem)
| MtuUpdated { path_id: 0, mtu: 1472, cause: ProbeAcknowledged } | ||
| MtuUpdated { path_id: 0, mtu: 5222, cause: ProbeAcknowledged } | ||
| MtuUpdated { path_id: 0, mtu: 7097, cause: ProbeAcknowledged } | ||
| MtuUpdated { path_id: 0, mtu: 8035, cause: ProbeAcknowledged } | ||
| MtuUpdated { path_id: 0, mtu: 8504, cause: ProbeAcknowledged } | ||
| MtuUpdated { path_id: 0, mtu: 8738, cause: ProbeAcknowledged } | ||
| MtuUpdated { path_id: 0, mtu: 8855, cause: ProbeAcknowledged } | ||
| MtuUpdated { path_id: 0, mtu: 8914, cause: ProbeAcknowledged } | ||
| MtuUpdated { path_id: 0, mtu: 8943, cause: ProbeAcknowledged } |
There was a problem hiding this comment.
Not blocking this PR but I'm wondering if this is the best strategy... That's quite a few probes that we're sending. For networks that you know do support jumbo frames, like EC2, it might be more optimal to try and jump to the highest packet size and do a "reverse" binary search from there.
There was a problem hiding this comment.
Attempting to summarize offline conversation here.
Starting the probe at the lower end and going upwards was chosen to keep the probing overhead low on networks when
- users were sending small packet &&
- users enabled jumbo frames, but they were not supported on the network
That case performs particularly poorly because
- For each acked packet, the congestion window grows by packet_size
- so the loss of a 9,000 byte probe slows down the growth of the congestion window
That being said, users that are sure of their network topology and MTU would gain some benefit by immediately probing for the maximum mtu, so we should figure out a way to unblock them, although that work is outside the scope of this PR.
* updated netbench msrv to 1.62 to allow transitive dependency (time) to build. * add forgotten license information to example * correct naming error for jumbo example * set client io provider + refactor tests
* remove first and second person references * remove exclamation points * remove most adjectives and adverbs
| @@ -0,0 +1,101 @@ | |||
| # Jumbo Frame Support | |||
There was a problem hiding this comment.
Had some offline discussion that the language is a bit too informal in this readme for public facing AWS stuff, updating in next commit to remove first and second person references, exclamation points, and most adjectives/adverbs.
Co-authored-by: Wesley Rosenblum <[email protected]>
Description of changes:
jumbo frame example
unit tests for jumbo frames using quic sim
buffer sizing based on mtu
update netbench msrv to 1.62
Call-outs:
the netbench MSRV increase is not particularly relevant to this feature work, but CI is currently failing because a transitive dependency (time) bumped their MSRV, and CI will continue to fail without us bumping the netbench msrv. Netbench is a development dependency, and therefore we can increase it's MSRV with little risk .
The increased buffer sizing is necessary, because otherwise the MTU controller will fail to write the packets because there isn't enough room in the event context.
It feels odd for the event-recorder macro to live in a random test module. But I don't know that there is a broad enough use case to add it so the event section either.
Also, the event recorder can't be used for any events that have references/lifetimes, but it still seems sufficiently useful.
Should we be concerned about the increase in buffer space? I'm not sure how much of quic's memory footprint is comprised of the tx/rx buffers, but this results in about
x6buffer sizing when someone uses jumbo frames.I'm planning on following up this PR with one to include jumbo frame configuration for our netbench and interop tooling.
Testing:
Unit tests that confirm a jumbo frame can be used.
Also the example sort of functions as a test, since we can see that we are negotiating a larger MTU with the tokio IO provider.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.