feat(transport): pass ECN CE marks to CC #1689

mxinden · 2024-02-27T13:45:09Z

Pass ECN CE marks received through FRAME_TYPE_ACK_ECN frames to the congestion controler.

mxinden · 2024-03-05T08:59:53Z

Rebasing onto latest #1678 now.

Edit: ✔️

The remaining bits from mozilla#1495 The remaining todo item after this PR is to actually act on incoming CE marks, i.e., trigger a congestion control action. See mozilla#1689

Signed-off-by: Lars Eggert <[email protected]>

…more-ecn

Signed-off-by: Lars Eggert <[email protected]>

…more-ecn

ci(interop): run ecn test

Co-authored-by: Max Inden <[email protected]> Signed-off-by: Lars Eggert <[email protected]>

If this makes ngtcp2 happy, refactor into separate PR.

Signed-off-by: Lars Eggert <[email protected]>

Pass ECN CE marks received through FRAME_TYPE_ACK_ECN frames to the congestion controler.

* feat: Send and process ACK-ECN The remaining bits from #1495 The remaining todo item after this PR is to actually act on incoming CE marks, i.e., trigger a congestion control action. See #1689 * Modifier * Fix botched merge * Fix merge * Rework * Add more tests that hopefully cover all cases. * WIP * Tests passing * More tests * Minimize diff * ci(interop): run ecn test * Update neqo-transport/src/connection/tests/ecn.rs Co-authored-by: Max Inden <[email protected]> Signed-off-by: Lars Eggert <[email protected]> * Update neqo-transport/src/qlog.rs Co-authored-by: Max Inden <[email protected]> Signed-off-by: Lars Eggert <[email protected]> * Fix qns * Reduce number of CONNECTION_CLOSE frames If this makes ngtcp2 happy, refactor into separate PR. * Update neqo-transport/src/ecn.rs Co-authored-by: Max Inden <[email protected]> Signed-off-by: Lars Eggert <[email protected]> * Update neqo-transport/src/ecn.rs Co-authored-by: Max Inden <[email protected]> Signed-off-by: Lars Eggert <[email protected]> * Update neqo-transport/src/ecn.rs Co-authored-by: Max Inden <[email protected]> Signed-off-by: Lars Eggert <[email protected]> * Fixes * Update neqo-transport/src/connection/tests/ecn.rs Co-authored-by: Martin Thomson <[email protected]> Signed-off-by: Lars Eggert <[email protected]> * Update neqo-transport/src/connection/tests/ecn.rs Co-authored-by: Martin Thomson <[email protected]> Signed-off-by: Lars Eggert <[email protected]> * Update neqo-transport/src/connection/mod.rs Co-authored-by: Martin Thomson <[email protected]> Signed-off-by: Lars Eggert <[email protected]> * Update neqo-transport/src/frame.rs Co-authored-by: Martin Thomson <[email protected]> Signed-off-by: Lars Eggert <[email protected]> * Update neqo-transport/src/frame.rs Co-authored-by: Martin Thomson <[email protected]> Signed-off-by: Lars Eggert <[email protected]> * Update neqo-transport/src/ecn.rs Co-authored-by: Martin Thomson <[email protected]> Signed-off-by: Lars Eggert <[email protected]> * Some fixups from code review * More code review fixups * Check whether migration happened as expected. Minor other tweaks. * Consolidate all the `nodata` tests --------- Signed-off-by: Lars Eggert <[email protected]> Co-authored-by: Max Inden <[email protected]> Co-authored-by: Martin Thomson <[email protected]>

mxinden · 2024-04-16T10:51:45Z

Resolving the merge conflicts now.

neqo-transport/src/path.rs

codecov · 2024-04-16T11:19:28Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.25%. Comparing base (4fc4d16) to head (7b49ef3).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1689      +/-   ##
==========================================
+ Coverage   93.23%   93.25%   +0.01%     
==========================================
  Files         110      110              
  Lines       35749    35793      +44     
==========================================
+ Hits        33329    33377      +48     
+ Misses       2420     2416       -4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2024-04-16T13:45:49Z

Benchmark results

Performance differences relative to 4fc4d16.

drain a timer quickly time: [311.70 ns 320.07 ns 327.85 ns]
change: [-29.637% -28.071% -26.115%] (p = 0.00 < 0.05)
💚 Performance has improved.
coalesce_acked_from_zero 1+1 entries
time: [196.40 ns 196.93 ns 197.50 ns]
change: [-0.3707% +0.1741% +0.9002%] (p = 0.63 > 0.05)
No change in performance detected.
coalesce_acked_from_zero 3+1 entries
time: [239.34 ns 239.94 ns 240.57 ns]
change: [+0.1652% +0.6963% +1.2449%] (p = 0.01 < 0.05)
Change within noise threshold.
coalesce_acked_from_zero 10+1 entries
time: [237.66 ns 238.38 ns 239.33 ns]
change: [+0.0186% +0.4597% +0.9051%] (p = 0.05 < 0.05)
Change within noise threshold.
coalesce_acked_from_zero 1000+1 entries
time: [218.68 ns 228.35 ns 249.53 ns]
change: [-0.6325% +1.3512% +4.5165%] (p = 0.50 > 0.05)
No change in performance detected.
RxStreamOrderer::inbound_frame()
time: [118.30 ms 118.36 ms 118.42 ms]
change: [-1.2687% -1.0632% -0.9252%] (p = 0.00 < 0.05)
Change within noise threshold.
transfer/Run multiple transfers with varying seeds
time: [119.61 ms 119.89 ms 120.16 ms]
thrpt: [33.290 MiB/s 33.365 MiB/s 33.441 MiB/s]
change:
time: [-4.1230% -3.8101% -3.4954%] (p = 0.00 < 0.05)
thrpt: [+3.6220% +3.9610% +4.3003%]
💚 Performance has improved.
transfer/Run multiple transfers with the same seed
time: [119.51 ms 119.74 ms 119.97 ms]
thrpt: [33.341 MiB/s 33.405 MiB/s 33.471 MiB/s]
change:
time: [-4.5950% -4.3607% -4.1248%] (p = 0.00 < 0.05)
thrpt: [+4.3022% +4.5595% +4.8163%]
💚 Performance has improved.
1-conn/1-100mb-resp (aka. Download)/client
time: [1.1133 s 1.1185 s 1.1234 s]
thrpt: [89.019 MiB/s 89.402 MiB/s 89.822 MiB/s]
change:
time: [+1.9214% +2.5435% +3.1538%] (p = 0.00 < 0.05)
thrpt: [-3.0574% -2.4804% -1.8852%]
💔 Performance has regressed.
1-conn/10_000-parallel-1b-resp (aka. RPS)/client
time: [424.41 ms 426.75 ms 429.12 ms]
thrpt: [23.303 Kelem/s 23.433 Kelem/s 23.562 Kelem/s]
change:
time: [-1.3144% -0.5468% +0.2237%] (p = 0.16 > 0.05)
thrpt: [-0.2232% +0.5498% +1.3319%]
No change in performance detected.
1-conn/1-1b-resp (aka. HPS)/client
time: [50.283 ms 50.572 ms 50.877 ms]
thrpt: [19.655 elem/s 19.774 elem/s 19.887 elem/s]
change:
time: [-0.9437% -0.1329% +0.7065%] (p = 0.76 > 0.05)
thrpt: [-0.7015% +0.1331% +0.9527%]
No change in performance detected.

Client/server transfer results

Transfer of 134217728 bytes over loopback.

Client	Server	CC	Pacing	Mean [ms]	Min [ms]	Max [ms]	Relative
msquic	msquic			820.7 ± 305.3	437.1	1296.1	1.00
neqo	msquic	reno	on	1056.9 ± 230.2	834.9	1367.3	1.00
neqo	msquic	reno		936.6 ± 217.9	696.7	1296.5	1.00
neqo	msquic	cubic	on	1015.7 ± 282.7	756.3	1562.0	1.00
neqo	msquic	cubic		832.2 ± 125.6	728.4	1136.2	1.00
msquic	neqo	reno	on	4415.6 ± 171.9	4155.2	4695.8	1.00
msquic	neqo	reno		4435.6 ± 219.2	4117.6	4814.3	1.00
msquic	neqo	cubic	on	4494.2 ± 172.6	4233.4	4700.5	1.00
msquic	neqo	cubic		4502.8 ± 288.5	4157.0	5126.7	1.00
neqo	neqo	reno	on	3575.0 ± 303.6	3250.0	4265.4	1.00
neqo	neqo	reno		3442.9 ± 157.6	3166.6	3769.3	1.00
neqo	neqo	cubic	on	4165.2 ± 611.1	2671.3	4929.8	1.00
neqo	neqo	cubic		4088.5 ± 420.0	3143.0	4464.1	1.00

⬇️ Download logs

mxinden · 2024-04-19T07:56:26Z

@larseggert I addressed the outstanding comment. Mind taking another look?

mxinden mentioned this pull request Feb 29, 2024

feat: Send and process ACK-ECN #1678

Merged

mxinden force-pushed the ecn-cc branch 2 times, most recently from 5fe6803 to 7dd116b Compare March 1, 2024 11:46

mxinden force-pushed the ecn-cc branch 2 times, most recently from 37e6db6 to 0f3772b Compare March 5, 2024 16:24

larseggert and others added 24 commits March 15, 2024 10:14

feat: Send and process ACK-ECN

69b0d86

The remaining bits from mozilla#1495 The remaining todo item after this PR is to actually act on incoming CE marks, i.e., trigger a congestion control action. See mozilla#1689

Modifier

3ee0b93

Merge branch 'main' into feat-more-ecn

cd3d937

Signed-off-by: Lars Eggert <[email protected]>

Merge branch 'feat-more-ecn' of github.com:larseggert/neqo into feat-…

7a11060

…more-ecn

Fix botched merge

16d73db

Merge branch 'main' into feat-more-ecn

cf90ee6

Signed-off-by: Lars Eggert <[email protected]>

Fix merge

afcf94f

Merge remote-tracking branch 'origin/main' into feat-more-ecn

f702c11

Merge branch 'main' into feat-more-ecn

f9e26cb

Rework

800b31c

Merge branch 'main' into feat-more-ecn

2290f0b

Merge branch 'feat-more-ecn' of github.com:larseggert/neqo into feat-…

ea2501c

…more-ecn

Add more tests that hopefully cover all cases.

1532ba7

WIP

8146ce2

Tests passing

67c2899

More tests

2041bfd

Minimize diff

9d79b59

Merge branch 'main' into feat-more-ecn

4056b8f

ci(interop): run ecn test

8bf6124

Merge pull request mozilla#22 from mxinden/qns-ecn

e7a2834

ci(interop): run ecn test

Update neqo-transport/src/connection/tests/ecn.rs

3a4febe

Co-authored-by: Max Inden <[email protected]> Signed-off-by: Lars Eggert <[email protected]>

Update neqo-transport/src/qlog.rs

e423d8a

Co-authored-by: Max Inden <[email protected]> Signed-off-by: Lars Eggert <[email protected]>

Fix qns

3c69dbe

Merge branch 'main' into feat-more-ecn

79306bc

larseggert added 3 commits April 9, 2024 14:02

Reduce number of CONNECTION_CLOSE frames

1999874

If this makes ngtcp2 happy, refactor into separate PR.

Merge branch 'main' into feat-more-ecn

0c8c709

Merge branch 'main' into feat-more-ecn

95af14b

Signed-off-by: Lars Eggert <[email protected]>

mxinden force-pushed the ecn-cc branch from 0f3772b to 40345b1 Compare April 11, 2024 10:42

feat(transport): pass ECN CE marks to CC

48c2c75

Pass ECN CE marks received through FRAME_TYPE_ACK_ECN frames to the congestion controler.

mxinden force-pushed the ecn-cc branch from 40345b1 to 48c2c75 Compare April 11, 2024 11:09

larseggert approved these changes Apr 11, 2024

View reviewed changes

Merge branch 'main' of https://github.com/mozilla/neqo into ecn-cc

5857502

mxinden commented Apr 16, 2024

View reviewed changes

neqo-transport/src/path.rs Outdated Show resolved Hide resolved

mxinden marked this pull request as ready for review April 16, 2024 11:15

mxinden requested review from KershawChang and martinthomson as code owners April 16, 2024 11:15

mxinden added 2 commits April 19, 2024 09:51

Update ACK delay on ECN CE mark

13fd396

Merge branch 'main' of https://github.com/mozilla/neqo into ecn-cc

7b49ef3

larseggert approved these changes Apr 23, 2024

View reviewed changes

larseggert added this pull request to the merge queue Apr 23, 2024

Merged via the queue into mozilla:main with commit 17c45d8 Apr 23, 2024
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(transport): pass ECN CE marks to CC #1689

feat(transport): pass ECN CE marks to CC #1689

mxinden commented Feb 27, 2024 •

edited

Loading

mxinden commented Mar 5, 2024 •

edited

Loading

mxinden commented Apr 16, 2024

codecov bot commented Apr 16, 2024 •

edited

Loading

github-actions bot commented Apr 16, 2024 •

edited

Loading

mxinden commented Apr 19, 2024

feat(transport): pass ECN CE marks to CC #1689

feat(transport): pass ECN CE marks to CC #1689

Conversation

mxinden commented Feb 27, 2024 • edited Loading

mxinden commented Mar 5, 2024 • edited Loading

mxinden commented Apr 16, 2024

codecov bot commented Apr 16, 2024 • edited Loading

Codecov Report

github-actions bot commented Apr 16, 2024 • edited Loading

Benchmark results

Client/server transfer results

mxinden commented Apr 19, 2024

mxinden commented Feb 27, 2024 •

edited

Loading

mxinden commented Mar 5, 2024 •

edited

Loading

codecov bot commented Apr 16, 2024 •

edited

Loading

github-actions bot commented Apr 16, 2024 •

edited

Loading