refactor: Use nss-rs instead of neqo-crypto#3399
Draft
Not-Nik wants to merge 1 commit intomozilla:mainfrom
Draft
refactor: Use nss-rs instead of neqo-crypto#3399Not-Nik wants to merge 1 commit intomozilla:mainfrom
Not-Nik wants to merge 1 commit intomozilla:mainfrom
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3399 +/- ##
==========================================
+ Coverage 94.24% 94.58% +0.33%
==========================================
Files 125 110 -15
Lines 37973 36103 -1870
Branches 37973 36103 -1870
==========================================
- Hits 35787 34147 -1640
+ Misses 1349 1256 -93
+ Partials 837 700 -137
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Member
|
Code coverage here is completely off. We'll need a fresh baseline for the project after such a large change. |
Contributor
Failed Interop TestsQUIC Interop Runner, client vs. server, differences relative to
All resultsSucceeded Interop TestsQUIC Interop Runner, client vs. server neqo-pr as client
neqo-pr as server
Unsupported Interop TestsQUIC Interop Runner, client vs. server neqo-pr as client
neqo-pr as server
|
Contributor
Client/server transfer resultsPerformance differences relative to 378c365. Transfer of 33554432 bytes over loopback, min. 100 runs. All unit-less numbers are in milliseconds.
Table above only shows statistically significant changes. See all results below. All resultsTransfer of 33554432 bytes over loopback, min. 100 runs. All unit-less numbers are in milliseconds.
Download data for |
Unable to generate the performance reportThere was an internal error while processing the run's data. We're working on fixing the issue. Feel free to contact us on Discord or at support@codspeed.io if the issue persists. |
Contributor
Benchmark resultsNo significant performance differences relative to 378c365. All resultstransfer/1-conn/1-100mb-resp (aka. Download)/mtu-1504: Change within noise threshold. time: [203.92 ms 204.41 ms 205.02 ms]
thrpt: [487.77 MiB/s 489.22 MiB/s 490.39 MiB/s]
change:
time: [+0.2747% +0.6120% +0.9708] (p = 0.00 < 0.05)
thrpt: [-0.9615% -0.6083% -0.2739]
Change within noise threshold.
Found 5 outliers among 100 measurements (5.00%)
2 (2.00%) high mild
3 (3.00%) high severetransfer/1-conn/10_000-parallel-1b-resp (aka. RPS)/mtu-1504: Change within noise threshold. time: [281.51 ms 283.37 ms 285.26 ms]
thrpt: [35.055 Kelem/s 35.289 Kelem/s 35.522 Kelem/s]
change:
time: [-1.8038% -0.9423% -0.0097] (p = 0.04 < 0.05)
thrpt: [+0.0097% +0.9513% +1.8369]
Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high mildtransfer/1-conn/1-1b-resp (aka. HPS)/mtu-1504: No change in performance detected. time: [38.528 ms 38.681 ms 38.851 ms]
thrpt: [25.739 B/s 25.853 B/s 25.955 B/s]
change:
time: [-0.1086% +0.4310% +0.9414] (p = 0.11 > 0.05)
thrpt: [-0.9326% -0.4292% +0.1088]
No change in performance detected.
Found 6 outliers among 100 measurements (6.00%)
2 (2.00%) high mild
4 (4.00%) high severetransfer/1-conn/1-100mb-req (aka. Upload)/mtu-1504: No change in performance detected. time: [205.18 ms 205.53 ms 205.90 ms]
thrpt: [485.67 MiB/s 486.54 MiB/s 487.38 MiB/s]
change:
time: [-0.0217% +0.2048% +0.4307] (p = 0.08 > 0.05)
thrpt: [-0.4288% -0.2044% +0.0217]
No change in performance detected.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high milddecode 4096 bytes, mask ff: No change in performance detected. time: [4.5166 µs 4.5244 µs 4.5331 µs]
change: [-0.2087% +0.2873% +0.7851] (p = 0.27 > 0.05)
No change in performance detected.
Found 7 outliers among 100 measurements (7.00%)
6 (6.00%) high mild
1 (1.00%) high severedecode 1048576 bytes, mask ff: No change in performance detected. time: [1.1578 ms 1.1592 ms 1.1605 ms]
change: [-0.6124% -0.2143% +0.1619] (p = 0.31 > 0.05)
No change in performance detected.
Found 14 outliers among 100 measurements (14.00%)
10 (10.00%) low severe
3 (3.00%) high mild
1 (1.00%) high severedecode 4096 bytes, mask 7f: No change in performance detected. time: [5.7923 µs 5.8027 µs 5.8142 µs]
change: [-0.6476% -0.1688% +0.2310] (p = 0.48 > 0.05)
No change in performance detected.
Found 4 outliers among 100 measurements (4.00%)
2 (2.00%) high mild
2 (2.00%) high severedecode 1048576 bytes, mask 7f: Change within noise threshold. time: [1.4873 ms 1.4897 ms 1.4923 ms]
change: [+0.0315% +0.2688% +0.5056] (p = 0.03 < 0.05)
Change within noise threshold.decode 4096 bytes, mask 3f: No change in performance detected. time: [5.5363 µs 5.5446 µs 5.5532 µs]
change: [-0.0751% +0.4011% +0.9663] (p = 0.17 > 0.05)
No change in performance detected.
Found 8 outliers among 100 measurements (8.00%)
4 (4.00%) high mild
4 (4.00%) high severedecode 1048576 bytes, mask 3f: No change in performance detected. time: [1.4149 ms 1.4170 ms 1.4192 ms]
change: [-0.2799% -0.0503% +0.1715] (p = 0.66 > 0.05)
No change in performance detected.streams/simulated/1-streams/each-1000-bytes: No change in performance detected. time: [129.68 ms 129.68 ms 129.68 ms]
thrpt: [7.5304 KiB/s 7.5306 KiB/s 7.5308 KiB/s]
change:
time: [-0.0026% +0.0009% +0.0042] (p = 0.62 > 0.05)
thrpt: [-0.0042% -0.0009% +0.0026]
No change in performance detected.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildstreams/simulated/1000-streams/each-1-bytes: No change in performance detected. time: [2.5362 s 2.5365 s 2.5368 s]
thrpt: [394.20 B/s 394.24 B/s 394.29 B/s]
change:
time: [-0.0129% +0.0023% +0.0175] (p = 0.77 > 0.05)
thrpt: [-0.0175% -0.0023% +0.0129]
No change in performance detected.streams/simulated/1000-streams/each-1000-bytes: Change within noise threshold. time: [6.5830 s 6.5893 s 6.5969 s]
thrpt: [148.03 KiB/s 148.20 KiB/s 148.35 KiB/s]
change:
time: [-0.4098% -0.2050% -0.0198] (p = 0.04 < 0.05)
thrpt: [+0.0198% +0.2054% +0.4115]
Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high severestreams/walltime/1-streams/each-1000-bytes: No change in performance detected. time: [589.18 µs 590.57 µs 592.33 µs]
change: [-0.5324% -0.0906% +0.3481] (p = 0.69 > 0.05)
No change in performance detected.
Found 4 outliers among 100 measurements (4.00%)
4 (4.00%) high severestreams/walltime/1000-streams/each-1-bytes: Change within noise threshold. time: [12.485 ms 12.527 ms 12.595 ms]
change: [+0.5561% +0.9617% +1.5064] (p = 0.00 < 0.05)
Change within noise threshold.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high severestreams/walltime/1000-streams/each-1000-bytes: No change in performance detected. time: [44.999 ms 45.040 ms 45.081 ms]
change: [-0.0311% +0.1449% +0.3092] (p = 0.10 > 0.05)
No change in performance detected.coalesce_acked_from_zero 1+1 entries: Change within noise threshold. time: [91.990 ns 92.233 ns 92.466 ns]
change: [-1.2158% -0.8043% -0.3958] (p = 0.00 < 0.05)
Change within noise threshold.
Found 7 outliers among 100 measurements (7.00%)
6 (6.00%) high mild
1 (1.00%) high severecoalesce_acked_from_zero 3+1 entries: Change within noise threshold. time: [110.11 ns 110.45 ns 110.82 ns]
change: [-1.0080% -0.5584% -0.0705] (p = 0.02 < 0.05)
Change within noise threshold.
Found 13 outliers among 100 measurements (13.00%)
13 (13.00%) high severecoalesce_acked_from_zero 10+1 entries: No change in performance detected. time: [109.43 ns 109.67 ns 109.98 ns]
change: [-1.1063% -0.4802% +0.1685] (p = 0.16 > 0.05)
No change in performance detected.
Found 14 outliers among 100 measurements (14.00%)
5 (5.00%) low severe
1 (1.00%) low mild
2 (2.00%) high mild
6 (6.00%) high severecoalesce_acked_from_zero 1000+1 entries: No change in performance detected. time: [95.009 ns 95.162 ns 95.329 ns]
change: [-0.9533% -0.3388% +0.3978] (p = 0.33 > 0.05)
No change in performance detected.
Found 9 outliers among 100 measurements (9.00%)
5 (5.00%) high mild
4 (4.00%) high severeRxStreamOrderer::inbound_frame(): Change within noise threshold. time: [108.61 ms 108.71 ms 108.82 ms]
change: [+0.2848% +0.3976% +0.5196] (p = 0.00 < 0.05)
Change within noise threshold.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildsent::Packets::take_ranges: No change in performance detected. time: [4.4718 µs 4.5653 µs 4.6514 µs]
change: [-4.6647% -0.9189% +2.6874] (p = 0.65 > 0.05)
No change in performance detected.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildtransfer/simulated/pacing-false/varying-seeds: No change in performance detected. time: [23.941 s 23.941 s 23.941 s]
thrpt: [171.09 KiB/s 171.09 KiB/s 171.09 KiB/s]
change:
time: [+0.0000% +0.0000% +0.0000] (p = NaN > 0.05)
thrpt: [+0.0000% +0.0000% +0.0000]
No change in performance detected.transfer/simulated/pacing-true/varying-seeds: No change in performance detected. time: [23.676 s 23.676 s 23.676 s]
thrpt: [173.01 KiB/s 173.01 KiB/s 173.01 KiB/s]
change:
time: [+0.0000% +0.0000% +0.0000] (p = NaN > 0.05)
thrpt: [+0.0000% +0.0000% +0.0000]
No change in performance detected.transfer/simulated/pacing-false/same-seed: No change in performance detected. time: [23.941 s 23.941 s 23.941 s]
thrpt: [171.09 KiB/s 171.09 KiB/s 171.09 KiB/s]
change:
time: [+0.0000% +0.0000% +0.0000] (p = NaN > 0.05)
thrpt: [+0.0000% +0.0000% +0.0000]
No change in performance detected.transfer/simulated/pacing-true/same-seed: No change in performance detected. time: [23.676 s 23.676 s 23.676 s]
thrpt: [173.01 KiB/s 173.01 KiB/s 173.01 KiB/s]
change:
time: [+0.0000% +0.0000% +0.0000] (p = NaN > 0.05)
thrpt: [+0.0000% +0.0000% +0.0000]
No change in performance detected.transfer/walltime/pacing-false/varying-seeds: Change within noise threshold. time: [23.054 ms 23.080 ms 23.119 ms]
change: [-1.3643% -1.2209% -1.0580] (p = 0.00 < 0.05)
Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severetransfer/walltime/pacing-true/varying-seeds: Change within noise threshold. time: [23.006 ms 23.040 ms 23.093 ms]
change: [-3.4280% -3.2537% -2.9893] (p = 0.00 < 0.05)
Change within noise threshold.
Found 3 outliers among 100 measurements (3.00%)
2 (2.00%) high mild
1 (1.00%) high severetransfer/walltime/pacing-false/same-seed: Change within noise threshold. time: [23.108 ms 23.134 ms 23.172 ms]
change: [-3.0134% -2.8728% -2.7062] (p = 0.00 < 0.05)
Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) low mild
1 (1.00%) high severetransfer/walltime/pacing-true/same-seed: Change within noise threshold. time: [23.344 ms 23.365 ms 23.390 ms]
change: [-1.9197% -1.6899% -1.5031] (p = 0.00 < 0.05)
Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severeDownload data for |
larseggert
reviewed
Feb 23, 2026
| description: "Path to file containing the minimum NSS version (relative to workspace root)" | ||
| required: false | ||
| default: "neqo-crypto/min_version.txt" | ||
| default: "nss-rs/min_version.txt" |
Collaborator
There was a problem hiding this comment.
Should this move into nss-rs? How would you handle the case where two conumsers of nss-rs have different requirements for min versions?
Member
There was a problem hiding this comment.
nss-rs is going to be the thing that has the NSS dependency, so it should be part of that crate, yes.
Comment on lines
+64
to
+70
| nss_rs_dir=$(cargo metadata --format-version=1 -q \ | ||
| | jq -r ".packages[] | ||
| | select(.name==\"nss-rs\") | ||
| | .manifest_path" \ | ||
| | xargs dirname) | ||
| cat "$nss_rs_dir/min_version.txt" > versions.txt | ||
| curl https://raw.githubusercontent.com/mozilla/nss-rs/refs/heads/main/min_version.txt >> versions.txt |
Collaborator
There was a problem hiding this comment.
This is repeated several times. Can this be factored out?
|
|
||
| /// The AEAD used for Retry is fixed, so use thread local storage. | ||
| fn make_aead(version: Version) -> Aead { | ||
| fn make_aead(version: Version) -> RecordProtection { |
Collaborator
There was a problem hiding this comment.
Why the name change from Aead to RecordProtection?
Member
There was a problem hiding this comment.
That's on me. There are two AEADs in nss-rs that use different interfaces in NSS (one is hooked into TLS record protection). I think we want to eliminate that one eventually, but we agreed to rename it for now.
Collaborator
There was a problem hiding this comment.
How about
use nss_rs::RecordProtection as Aead;Would minimize the diff...
Member
There was a problem hiding this comment.
I don't mind the renaming.
| required-git-spec = "rev" | ||
| allow-git = [ | ||
| "https://github.com/mozilla/nss-rs.git", | ||
| ] |
Collaborator
There was a problem hiding this comment.
I guess this is temporary - add a TODO about reverting it.
larseggert
reviewed
Feb 24, 2026
| http = { version = "0.2.9", default-features = false } | ||
| libc = { version = "0.2", default-features = false } | ||
| log = { version = "0.4", default-features = false } | ||
| nss-rs = { git = "https://github.com/mozilla/nss-rs", rev = "0b9edb5a11e7f3d26638ec4b190947d30611cc18" } |
Collaborator
There was a problem hiding this comment.
Suggested change
| nss-rs = { git = "https://github.com/mozilla/nss-rs", rev = "0b9edb5a11e7f3d26638ec4b190947d30611cc18" } | |
| neqo-crypto = { package = "nss-rs", git = "https://github.com/mozilla/nss-rs", rev = "0b9edb5a11e7f3d26638ec4b190947d30611cc18" } |
If you rename like this, can we avoid a bunch of neqo-crypto -> nss-rs changes?
But we might actually want those for clarity. What do others think?
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
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.
Replaces neqo-crypto with nss-rs. nss-rs is the new unified NSS binding for Rust. It merges functionality of nss-gk-api and neqo-crypto, which were originally based on the same code, but diverged into separate libraries. The new crate is located in its own repository at mozilla/nss-rs. Other Firefox components like mls-platform-api and authenticator-rs will also transition to nss-rs in the future.
Most of the API of both nss-gk-api and neqo-crypto was preserved except for minor changes. Since both libraries declared an
Aeadtype, the neqo-crypto version is now namedRecordProtection(mozilla/nss-rs#14), but both are exposed at the old path (i.e.nss_rs::RecordProtection,nss_rs::aead::Aead).P.S.: This is still a draft, because some of neqo's CI jobs can't run on forks and I am unsure if they will pass here.