server: default send_tls13_tickets 4 -> 2 #2187
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ctz
approved these changes
Oct 30, 2024
Member
ctz
left a comment
There was a problem hiding this comment.
We could also remove the now-unnecessary setting of this in rustls/examples/internal/bench_impl.rs?
Benchmark resultsInstruction countsSignificant differencesClick to expand
Other differencesClick to expand
Wall-timeSignificant differencesClick to expand
Other differencesClick to expand
Additional informationCheckout details:
|
This change aligns the Rustls default for a `ServerConfig::builder()`'s `send_tls13_tickets` value to match BoringSSL/OpenSSL's default of 2. Previously we used 4 but this manifests as a performance gap in server-side resumption cost when comparing default configurations between TLS libraries due to the high cost of creating new tickets. We believe a default of 2 strikes a better balance in general, and avoids folks repeatedly tripping into false assumptions during benchmarking. Anyone that has a workload that prefers up-front server-side cost in favour of reduced client-side latency for batched connections can tweak the value to 4 or higher manually.
bdff699 to
cf7954a
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2187 +/- ##
==========================================
- Coverage 94.65% 94.65% -0.01%
==========================================
Files 102 102
Lines 23749 23749
==========================================
- Hits 22480 22479 -1
- Misses 1269 1270 +1 ☔ View full report in Codecov by Sentry. |
Member
I don't disagree but I want to challenge this. I understand that, all other things being equal, it would be better to match others so that people benchmarking will be less likely to confuse themselves, but I also believe it is more important that we deliver good software to downstream (server) users than that we don't confuse benchmark readers. As such, doesn't this make rustls worse at resuming sessions in practice? Do we send new tickets in resumed sessions? Why did we pick 4 in the first place? Preferring upfront server-side cost (how much cost are we talking about in practice?) in favor of reduced client-side latency seems like a pretty good trade-off for lots of people... I don't feel like the trade-off is all that crisply articulated in this PR's description. |
Member
Yes, but I think in quite limited circumstances; see below.
Yes,
For reference, that commit was d780790 though I didn't record why I specifically chose 4 the default changed from 1 to 4, it was in the context of making TLS1.3 tickets non-reusable. I think I chose the number 4 on the basis of a typical limit of concurrent connections made by a HTTP1 client to a given host (the actual number varies wildly by browser, with RFC2616 specifying 2 but (eg) Chrome doing 6. However that measure is not relevant for HTTP2, nor HTTP3.
Seems the benchmarks here are saying the extra 2 tickets cost us 8%/6% (client/server) of a normal resumption, or ~0%/2% of a full handshake.
The case where 2 is worse than 4 is pretty specific, I think. For the sake of argument, say the number is N:
To reiterate, this only limits the number of resumed handshakes a client can make in parallel at any given point. In serial is fine: each time a handshake completes, it replenishes the client's cache by Footnotes
|
djc
approved these changes
Oct 31, 2024
Member
djc
left a comment
There was a problem hiding this comment.
Okay, I agree that this makes sense then!
Member
Author
|
@ctz Thanks for adding that extra analysis. @djc Thank you for pushing back 👍 My PR description wasn't doing this change justice. I've lifted some of Ctz's thoughts into the PR description, and tried to reframe it in terms of being a better default in general. The focus shouldn't have been on the benchmarking foibles. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change aligns the Rustls default for a
ServerConfig::builder()'ssend_tls13_ticketsvalue to match BoringSSL/OpenSSL's default of 2.Previously we used 4 but this prefers a more limited use-case where the client wants to be able to resume 4 handshakes in parallel in its initial post-full-handshake flight of connections. The overhead of optimizing for this scenario vs using a smaller number of tickets by default also manifests as a performance cost in server-side resumption: the extra 2 tickets cost us 8%/6% (client/server) of a normal resumption, or ~0%/2% of a full handshake. This also manifests when comparing default configurations between TLS libraries due to the high cost of creating new tickets:
We believe a default of 2 strikes a better balance in general - recall each resumed handshake will also replenish the client's tickets. It also avoids folks repeatedly tripping into this during benchmarking (e.g. #1751 (comment), #2167), and aligns us with other parts of the TLS ecosystem on a sensible default. Anyone that has a workload that prefers the old behaviour can adjust the value to 4 or higher manually but we expect this to be niche and not the norm.