fix: cherry-pick of #3573 for Neqo v0.26.1#3574
Conversation
The `scone` connection parameter (off by default since mozilla#3492) only gated advertisement of the SCONE transport parameter. The Initial- packet SCONE indication (`0xc8 0x13`) was still emitted unconditionally, so every default-config client was signaling SCONE on the wire without actually negotiating it. This is suspected — not yet confirmed — to be the cause of a Firefox 150 regression where Facebook's edge reacts to the indicator and Bitdefender's HTTP/3 inspection then trips on the altered server-side frames (bug 2034178). The QUIC handshake still completes, so the H3->H2 fallback never kicks in. Gate both the build-time reservation and the pad-time indicator on `scone_enabled()`; when disabled, pad Initials with zeros as before SCONE was introduced.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v0.26 #3574 +/- ##
=======================================
Coverage 94.53% 94.53%
=======================================
Files 127 127
Lines 39627 39632 +5
Branches 39627 39632 +5
=======================================
+ Hits 37462 37468 +6
+ Misses 1328 1327 -1
Partials 837 837
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Merging this PR will degrade performance by 5.71%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | coalesce_acked_from_zero 10+1 entries |
3.1 µs | 3.2 µs | -3.68% |
| ❌ | Simulation | coalesce_acked_from_zero 1000+1 entries |
2.6 µs | 2.7 µs | -4.34% |
| ❌ | Simulation | coalesce_acked_from_zero 3+1 entries |
3.1 µs | 3.2 µs | -3.68% |
| ❌ | Simulation | coalesce_acked_from_zero 1+1 entries |
2.9 µs | 3.1 µs | -5.71% |
Comparing mxinden:v0.26-disable-scone-padding (0eb6238) with main (faa1437)2
Footnotes
-
27 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
v0.26(7c0b85f) during the generation of this report, somain(faa1437) was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
Benchmark resultsSignificant performance differences relative to 7c0b85f. streams/walltime/1000-streams/each-1000-bytes: 💚 Performance has improved by -2.7700%. time: [44.342 ms 44.424 ms 44.538 ms]
change: [-2.9709% -2.7700% -2.5557] (p = 0.00 < 0.05)
Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severeAll resultstransfer/1-conn/1-100mb-resp (aka. Download)/mtu-1504: No change in performance detected. time: [207.86 ms 208.27 ms 208.79 ms]
thrpt: [478.96 MiB/s 480.13 MiB/s 481.10 MiB/s]
change:
time: [-0.3399% -0.0789% +0.2138] (p = 0.58 > 0.05)
thrpt: [-0.2134% +0.0790% +0.3410]
No change in performance detected.
Found 4 outliers among 100 measurements (4.00%)
2 (2.00%) high mild
2 (2.00%) high severetransfer/1-conn/10_000-parallel-1b-resp (aka. RPS)/mtu-1504: No change in performance detected. time: [283.61 ms 285.57 ms 287.58 ms]
thrpt: [34.773 Kelem/s 35.017 Kelem/s 35.260 Kelem/s]
change:
time: [-0.7936% +0.1327% +1.0635] (p = 0.79 > 0.05)
thrpt: [-1.0523% -0.1326% +0.8000]
No change in performance detected.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildtransfer/1-conn/1-1b-resp (aka. HPS)/mtu-1504: Change within noise threshold. time: [38.801 ms 38.981 ms 39.178 ms]
thrpt: [25.525 B/s 25.653 B/s 25.772 B/s]
change:
time: [+0.1492% +0.8027% +1.4976] (p = 0.02 < 0.05)
thrpt: [-1.4755% -0.7963% -0.1490]
Change within noise threshold.
Found 14 outliers among 100 measurements (14.00%)
4 (4.00%) high mild
10 (10.00%) high severetransfer/1-conn/1-100mb-req (aka. Upload)/mtu-1504: Change within noise threshold. time: [208.34 ms 208.68 ms 209.03 ms]
thrpt: [478.40 MiB/s 479.21 MiB/s 479.98 MiB/s]
change:
time: [-0.6873% -0.4142% -0.1606] (p = 0.00 < 0.05)
thrpt: [+0.1608% +0.4159% +0.6920]
Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severestreams/walltime/1-streams/each-1000-bytes: Change within noise threshold. time: [585.77 µs 587.79 µs 590.12 µs]
change: [-1.2614% -0.7057% -0.1433] (p = 0.01 < 0.05)
Change within noise threshold.
Found 10 outliers among 100 measurements (10.00%)
2 (2.00%) high mild
8 (8.00%) high severestreams/walltime/1000-streams/each-1-bytes: Change within noise threshold. time: [12.315 ms 12.332 ms 12.348 ms]
change: [-1.2941% -1.0751% -0.8687] (p = 0.00 < 0.05)
Change within noise threshold.streams/walltime/1000-streams/each-1000-bytes: 💚 Performance has improved by -2.7700%. time: [44.342 ms 44.424 ms 44.538 ms]
change: [-2.9709% -2.7700% -2.5557] (p = 0.00 < 0.05)
Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severetransfer/walltime/pacing-false/varying-seeds: Change within noise threshold. time: [80.013 ms 80.071 ms 80.129 ms]
change: [-2.2837% -2.0832% -1.9319] (p = 0.00 < 0.05)
Change within noise threshold.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildtransfer/walltime/pacing-true/varying-seeds: Change within noise threshold. time: [81.388 ms 81.440 ms 81.494 ms]
change: [-2.1614% -2.0366% -1.9273] (p = 0.00 < 0.05)
Change within noise threshold.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildtransfer/walltime/pacing-false/same-seed: Change within noise threshold. time: [79.778 ms 79.827 ms 79.878 ms]
change: [-2.2498% -2.0836% -1.9537] (p = 0.00 < 0.05)
Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high mildtransfer/walltime/pacing-true/same-seed: Change within noise threshold. time: [81.514 ms 81.577 ms 81.647 ms]
change: [-2.3417% -2.1562% -2.0092] (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 severeDownload data for |
There was a problem hiding this comment.
Pull request overview
This PR cherry-picks the fix from #3573 onto the v0.26 release branch to ensure the SCONE Initial padding indicator is only emitted when the scone connection parameter is enabled, avoiding unintended on-the-wire signaling in default configurations (relevant to Firefox Beta uplift for the reported regression).
Changes:
- Gate Initial SCONE padding reservation and SCONE indication emission on
conn_params.scone_enabled(), and pad with zeros when SCONE is disabled. - Update the
corrupted_initialhandshake test to align with zero-padding behavior when SCONE is disabled. - Bump workspace version to
0.26.1and updateCargo.lockpackage versions accordingly.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
neqo-transport/src/connection/mod.rs |
Only reserve/emit SCONE Initial indication when scone_enabled(); otherwise pad Initials with zeros. |
neqo-transport/src/connection/tests/handshake.rs |
Adjust test logic for finding a corruption target now that default padding is zeros. |
Cargo.toml |
Workspace version bump to 0.26.1. |
Cargo.lock |
Lockfile updated to reflect 0.26.1 workspace crate versions. |
Client/server transfer resultsPerformance differences relative to 7c0b85f. 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 |
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
|
Cherry-pick of #3573 for Neqo v0.26.1 to be landed into Firefox 151 (currently Firefox Beta).
Note that this pull request targets the
v0.26Git branch, branched off of thev0.26.1Git tag.