neqo icon indicating copy to clipboard operation
neqo copied to clipboard
verified publisher icon mozilla

fix: Drop of Initial space only after sending 1st Handshake

Open larseggert opened this issue 1 year ago • 6 comments

This is what RFC9000 says, and it helps with servers that expect ACKs in the Initial packet number space to happen. It also eliminates a difference in behavior we had when a client received an Initial packet containing an ACK and a CRYPTO frame vs. a CRYPTO frame and an ACK, where that CRYPTO frame caused the Initial packet number space to be dropped.

Broken out of #1998

larseggert avatar Sep 16 '24 10:09 larseggert

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.35%. Comparing base (c6d5502) to head (41463d9).

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2118      +/-   ##
==========================================
- Coverage   95.38%   95.35%   -0.03%     
==========================================
  Files         112      112              
  Lines       36593    36590       -3     
==========================================
- Hits        34903    34892      -11     
- Misses       1690     1698       +8

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Sep 16 '24 10:09 codecov[bot]

Failed Interop Tests

QUIC Interop Runner, client vs. server, differences relative to 28c0ea7eef34b86f67b9f9ef6da005e4fde1cf1a.

neqo-latest as client

neqo-latest as server

All results

Succeeded Interop Tests

QUIC Interop Runner, client vs. server

neqo-latest as client

neqo-latest as server

Unsupported Interop Tests

QUIC Interop Runner, client vs. server

neqo-latest as client

neqo-latest as server

github-actions[bot] avatar Sep 16 '24 10:09 github-actions[bot]

Benchmark results

Performance differences relative to 28c0ea7eef34b86f67b9f9ef6da005e4fde1cf1a.

1-conn/1-100mb-resp/mtu-1504 (aka. Download)/client: Change within noise threshold.
       time:   [717.15 ms 721.13 ms 725.14 ms]
       thrpt:  [137.90 MiB/s 138.67 MiB/s 139.44 MiB/s]
change:
       time:   [+0.2777% +1.0363% +1.8708%] (p = 0.01 -1.0257% -0.2769%]
1-conn/10_000-parallel-1b-resp/mtu-1504 (aka. RPS)/client: Change within noise threshold.
       time:   [354.79 ms 356.24 ms 357.69 ms]
       thrpt:  [27.957 Kelem/s 28.071 Kelem/s 28.185 Kelem/s]
change:
       time:   [+0.2043% +0.8996% +1.6029%] (p = 0.01 -0.8915% -0.2039%]

Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild
1-conn/1-1b-resp/mtu-1504 (aka. HPS)/client: No change in performance detected.
       time:   [25.644 ms 25.785 ms 25.932 ms]
       thrpt:  [38.563  elem/s 38.782  elem/s 38.996  elem/s]
change:
       time:   [-1.3376% -0.4958% +0.3864%] (p = 0.25 > 0.05)
       thrpt:  [-0.3849% +0.4982% +1.3557%]

Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild
1-conn/1-100mb-req/mtu-1504 (aka. Upload)/client: :green_heart: Performance has improved.
       time:   [1.8538 s 1.8755 s 1.8996 s]
       thrpt:  [52.643 MiB/s 53.318 MiB/s 53.944 MiB/s]
change:
       time:   [-17.120% -15.619% -14.129%] (p = 0.00 +18.510% +20.656%]

Found 7 outliers among 100 measurements (7.00%)
2 (2.00%) high mild
5 (5.00%) high severe
decode 4096 bytes, mask ff: No change in performance detected.
       time:   [11.981 µs 12.011 µs 12.048 µs]
       change: [-0.2327% +0.5173% +1.6320%] (p = 0.46 > 0.05)

Found 15 outliers among 100 measurements (15.00%)
3 (3.00%) low severe
2 (2.00%) low mild
3 (3.00%) high mild
7 (7.00%) high severe
decode 1048576 bytes, mask ff: No change in performance detected.
       time:   [2.9568 ms 2.9666 ms 2.9780 ms]
       change: [-0.1050% +0.3380% +0.8060%] (p = 0.15 > 0.05)

Found 10 outliers among 100 measurements (10.00%)
10 (10.00%) high severe
decode 4096 bytes, mask 7f: No change in performance detected.
       time:   [20.029 µs 20.088 µs 20.151 µs]
       change: [-0.4507% +0.0630% +0.6376%] (p = 0.84 > 0.05)

Found 20 outliers among 100 measurements (20.00%)
2 (2.00%) low severe
2 (2.00%) low mild
1 (1.00%) high mild
15 (15.00%) high severe
decode 1048576 bytes, mask 7f: No change in performance detected.
       time:   [4.7937 ms 4.8053 ms 4.8185 ms]
       change: [-0.4014% -0.0420% +0.3191%] (p = 0.82 > 0.05)

Found 13 outliers among 100 measurements (13.00%)
13 (13.00%) high severe
decode 4096 bytes, mask 3f: No change in performance detected.
       time:   [6.3146 µs 6.3329 µs 6.3576 µs]
       change: [-0.7632% -0.1409% +0.4603%] (p = 0.67 > 0.05)

Found 17 outliers among 100 measurements (17.00%)
5 (5.00%) low severe
2 (2.00%) low mild
5 (5.00%) high mild
5 (5.00%) high severe
decode 1048576 bytes, mask 3f: No change in performance detected.
       time:   [2.1486 ms 2.1554 ms 2.1626 ms]
       change: [-0.5305% -0.0597% +0.3969%] (p = 0.83 > 0.05)

Found 8 outliers among 100 measurements (8.00%)
1 (1.00%) low mild
7 (7.00%) high severe
1 streams of 1 bytes/multistream: Change within noise threshold.
       time:   [72.907 µs 73.121 µs 73.333 µs]
       change: [+0.5476% +0.9594% +1.4076%] (p = 0.00
1000 streams of 1 bytes/multistream: :green_heart: Performance has improved.
       time:   [24.821 ms 24.859 ms 24.896 ms]
       change: [-2.1188% -1.8956% -1.7045%] (p = 0.00
10000 streams of 1 bytes/multistream: Change within noise threshold.
       time:   [1.6763 s 1.6781 s 1.6800 s]
       change: [-1.2588% -1.0991% -0.9441%] (p = 0.00 Found 26 outliers among 100 measurements (26.00%)
8 (8.00%) low severe
8 (8.00%) low mild
1 (1.00%) high mild
9 (9.00%) high severe
1 streams of 1000 bytes/multistream: Change within noise threshold.
       time:   [75.954 µs 76.172 µs 76.392 µs]
       change: [+0.9681% +2.5195% +3.5187%] (p = 0.00 Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild
100 streams of 1000 bytes/multistream: No change in performance detected.
       time:   [3.3889 ms 3.3962 ms 3.4038 ms]
       change: [-0.0227% +0.2802% +0.5822%] (p = 0.07 > 0.05)
1000 streams of 1000 bytes/multistream: Change within noise threshold.
       time:   [142.83 ms 142.92 ms 143.02 ms]
       change: [-0.6357% -0.5563% -0.4681%] (p = 0.00 Found 5 outliers among 100 measurements (5.00%)
4 (4.00%) high mild
1 (1.00%) high severe
coalesce_acked_from_zero 1+1 entries: No change in performance detected.
       time:   [94.718 ns 95.078 ns 95.448 ns]
       change: [-0.4957% -0.0399% +0.4302%] (p = 0.87 > 0.05)

Found 16 outliers among 100 measurements (16.00%)
1 (1.00%) low mild
11 (11.00%) high mild
4 (4.00%) high severe
coalesce_acked_from_zero 3+1 entries: No change in performance detected.
       time:   [112.77 ns 113.12 ns 113.50 ns]
       change: [-0.3658% +0.1499% +0.6298%] (p = 0.58 > 0.05)

Found 15 outliers among 100 measurements (15.00%)
1 (1.00%) low mild
7 (7.00%) high mild
7 (7.00%) high severe
coalesce_acked_from_zero 10+1 entries: No change in performance detected.
       time:   [111.96 ns 112.29 ns 112.73 ns]
       change: [-0.3660% +0.1958% +0.7956%] (p = 0.54 > 0.05)

Found 19 outliers among 100 measurements (19.00%)
5 (5.00%) low severe
3 (3.00%) low mild
3 (3.00%) high mild
8 (8.00%) high severe
coalesce_acked_from_zero 1000+1 entries: No change in performance detected.
       time:   [93.153 ns 93.548 ns 93.984 ns]
       change: [-1.1197% +0.3088% +2.0144%] (p = 0.72 > 0.05)

Found 9 outliers among 100 measurements (9.00%)
3 (3.00%) high mild
6 (6.00%) high severe
RxStreamOrderer::inbound_frame(): Change within noise threshold.
       time:   [116.86 ms 116.91 ms 116.95 ms]
       change: [-0.1339% -0.0779% -0.0220%] (p = 0.01 Found 15 outliers among 100 measurements (15.00%)
5 (5.00%) low mild
10 (10.00%) high mild
SentPackets::take_ranges: No change in performance detected.
       time:   [8.4148 µs 8.6268 µs 8.8229 µs]
       change: [-2.5826% -0.0041% +2.5079%] (p = 0.99 > 0.05)

Found 19 outliers among 100 measurements (19.00%)
3 (3.00%) low severe
12 (12.00%) low mild
3 (3.00%) high mild
1 (1.00%) high severe
transfer/pacing-false/varying-seeds: Change within noise threshold.
       time:   [35.981 ms 36.047 ms 36.112 ms]
       change: [+0.3241% +0.5793% +0.8392%] (p = 0.00 Found 3 outliers among 100 measurements (3.00%)
1 (1.00%) low mild
2 (2.00%) high mild
transfer/pacing-true/varying-seeds: No change in performance detected.
       time:   [36.634 ms 36.734 ms 36.834 ms]
       change: [-0.2367% +0.1628% +0.5625%] (p = 0.43 > 0.05)
transfer/pacing-false/same-seed: Change within noise threshold.
       time:   [35.976 ms 36.030 ms 36.085 ms]
       change: [+1.2257% +1.4111% +1.5995%] (p = 0.00
transfer/pacing-true/same-seed: Change within noise threshold.
       time:   [37.751 ms 37.817 ms 37.883 ms]
       change: [+1.6029% +1.8689% +2.1037%] (p = 0.00 Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild

Client/server transfer results

Performance differences relative to 28c0ea7eef34b86f67b9f9ef6da005e4fde1cf1a.

Transfer of 33554432 bytes over loopback, 30 runs. All unit-less numbers are in milliseconds.

Client Server CC Pacing Mean ± σ Min Max MiB/s ± σ Δ main Δ main
neqo neqo reno on 324.3 ± 32.6 294.6 460.9 98.7 ± 1.0 2.2 0.7%
neqo neqo reno 358.8 ± 116.7 297.8 801.6 89.2 ± 0.3 -9.0 -2.4%
neqo neqo cubic on 330.4 ± 44.1 294.7 532.5 96.8 ± 0.7 -9.8 -2.9%
neqo neqo cubic 323.2 ± 33.9 299.2 478.3 99.0 ± 0.9 -16.4 -4.8%
google neqo reno on 761.1 ± 82.7 567.8 857.4 42.0 ± 0.4 -13.0 -1.7%
google neqo reno 757.6 ± 91.4 563.9 994.2 42.2 ± 0.4 -14.6 -1.9%
google neqo cubic on 764.2 ± 91.3 560.9 954.9 41.9 ± 0.4 -3.9 -0.5%
google neqo cubic 757.1 ± 84.4 558.4 902.4 42.3 ± 0.4 -8.5 -1.1%
google google 577.0 ± 42.5 549.7 784.9 55.5 ± 0.8 -4.9 -0.8%
neqo msquic reno on 269.6 ± 34.0 244.1 422.6 118.7 ± 0.9 -6.7 -2.4%
neqo msquic reno 271.9 ± 39.7 242.8 425.1 117.7 ± 0.8 -3.5 -1.3%
neqo msquic cubic on 263.3 ± 13.9 242.2 295.5 121.6 ± 2.3 -11.2 -4.1%
neqo msquic cubic 262.3 ± 17.0 239.8 316.7 122.0 ± 1.9 -10.3 -3.8%
msquic msquic 180.6 ± 28.4 147.6 256.7 177.2 ± 1.1 -4.2 -2.3%

:arrow_down: Download logs

github-actions[bot] avatar Sep 16 '24 12:09 github-actions[bot]

Firefox builds for this PR

The following builds are available for testing. Crossed-out builds did not succeed.

github-actions[bot] avatar Sep 16 '24 12:09 github-actions[bot]

That is the section in question.

larseggert avatar Sep 19 '24 12:09 larseggert

@martinthomson I'd appreciate a review, since the code I am touching is pretty complex.

larseggert avatar Sep 20 '24 07:09 larseggert

@larseggert what are the next steps here?

mxinden avatar Apr 07 '25 13:04 mxinden

I want to double-check that everything is working as intended here.

larseggert avatar Apr 07 '25 17:04 larseggert