post-quantum key exchange writeup #2281
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
Benchmark resultsInstruction countsSignificant differencesThere are no significant instruction count differences Other differencesClick to expand
Wall-timeSignificant differencesThere are no significant wall-time differences Other differencesClick to expand
Additional informationCheckout details:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2281 +/- ##
=======================================
Coverage 94.82% 94.82%
=======================================
Files 104 104
Lines 24100 24100
=======================================
Hits 22853 22853
Misses 1247 1247 ☔ View full report in Codecov by Sentry. |
79a4507 to
f937994
Compare
78c0430 to
7eca367
Compare
|
This is ready for review now. The code for the non-optimised case is on jbp-pqkx-comparative-benchmarks Manual documentation run is https://github.com/rustls/rustls/actions/runs/12372917287/job/34532172546 (deploy step is expected to fail, but prior step contains the website generation, and internal/external link checking.) |
djc
approved these changes
Dec 17, 2024
There was a problem hiding this comment.
Nice work!
I feel like the article kind of buries the lede ("rustls can do PQC handshakes faster than OpenSSL handles regular ones"), and I wonder if it might be worth restructuring to put that conclusion up front (maybe also in the title), and then back it up with the full story?
cpu
approved these changes
Dec 17, 2024
There was a problem hiding this comment.
I'm not sure if I'm missing something obvious, or looking at an older rendering, but in the part of the writeup that says "Instead, we can do:" and has the second diagram I feel like it's not super clear what the difference is between that diagram, and the one before it. Could we maybe use some colour here? Something like:
- Diagram 1:
[ML-KEM share (green)][X25519 share (yellow)] [X25519 share (orange)] - Diagram 2:
[ML-KEM share (green)][X25519 share (yellow)] [X25519 share (yellow)]
7eca367 to
ac0ee3b
Compare
Have just tried this.
Yeah I think I'm trying to tell two stories. Maybe this could be clearer as two completely separate pages?
|
I think both of those might be too thin on their own so combining them could still make sense, just consider more clearly separating these two topics and presenting them in optimal order (which I suggest would be to have the OpenSSL comparison come first). |
FWIW I was thinking the same 👍 |
ac0ee3b to
644cc89
Compare
644cc89 to
956cf0e
Compare
OK, I think that reads a lot better. WDYT? |
cpu
approved these changes
Dec 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Rendered