-
Notifications
You must be signed in to change notification settings - Fork 131
feat: More ECN and DSCP stats #2505
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Inspired by @martinduke's [IETF presentation on ECN](https://datatracker.ietf.org/meeting/122/materials/slides-122-tsvwg-udp-ecn-00), add some more stats around what packet types we send and receive with different ECN codepoints. This will require additional glue code to ship these stats back as telemetry.
Failed Interop TestsQUIC Interop Runner, client vs. server, differences relative to 182afed. neqo-latest as client
neqo-latest as server
All resultsSucceeded Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
Unsupported Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
|
Benchmark resultsPerformance differences relative to 054b7cb. 1-conn/1-100mb-resp/mtu-1504 (aka. Download)/client: 💔 Performance has regressed.time: [735.04 ms 739.68 ms 744.37 ms] thrpt: [134.34 MiB/s 135.19 MiB/s 136.05 MiB/s] change: time: [+1.8860% +2.7051% +3.5675%] (p = 0.00 < 0.05) thrpt: [-3.4446% -2.6338% -1.8511%] 1-conn/10_000-parallel-1b-resp/mtu-1504 (aka. RPS)/client: No change in performance detected.time: [348.30 ms 349.82 ms 351.32 ms] thrpt: [28.464 Kelem/s 28.586 Kelem/s 28.711 Kelem/s] change: time: [-0.6378% +0.0565% +0.7257%] (p = 0.87 > 0.05) thrpt: [-0.7205% -0.0564% +0.6419%] 1-conn/1-1b-resp/mtu-1504 (aka. HPS)/client: No change in performance detected.time: [25.026 ms 25.187 ms 25.354 ms] thrpt: [39.442 elem/s 39.703 elem/s 39.959 elem/s] change: time: [-0.1797% +0.6585% +1.5540%] (p = 0.14 > 0.05) thrpt: [-1.5302% -0.6542% +0.1800%] 1-conn/1-100mb-req/mtu-1504 (aka. Upload)/client: Change within noise threshold.time: [1.8275 s 1.8447 s 1.8627 s] thrpt: [53.684 MiB/s 54.210 MiB/s 54.718 MiB/s] change: time: [-3.3122% -1.7298% -0.1833%] (p = 0.04 < 0.05) thrpt: [+0.1837% +1.7603% +3.4257%] decode 4096 bytes, mask ff: No change in performance detected.time: [12.086 µs 12.127 µs 12.174 µs] change: [-1.1286% -0.3225% +0.3639%] (p = 0.43 > 0.05) decode 1048576 bytes, mask ff: No change in performance detected.time: [3.0769 ms 3.0862 ms 3.0973 ms] change: [-0.6410% -0.1483% +0.3284%] (p = 0.55 > 0.05) decode 4096 bytes, mask 7f: No change in performance detected.time: [20.139 µs 20.187 µs 20.242 µs] change: [-0.3563% -0.0774% +0.2078%] (p = 0.60 > 0.05) decode 1048576 bytes, mask 7f: No change in performance detected.time: [5.2526 ms 5.2641 ms 5.2773 ms] change: [-0.3490% -0.0271% +0.3258%] (p = 0.89 > 0.05) decode 4096 bytes, mask 3f: No change in performance detected.time: [7.0100 µs 7.0319 µs 7.0603 µs] change: [-0.5338% -0.0053% +0.4403%] (p = 0.99 > 0.05) decode 1048576 bytes, mask 3f: No change in performance detected.time: [1.7922 ms 1.7979 ms 1.8049 ms] change: [-0.3569% +0.1104% +0.5890%] (p = 0.64 > 0.05) 1 streams of 1 bytes/multistream: 💔 Performance has regressed.time: [73.501 µs 74.577 µs 76.108 µs] change: [+1.5679% +3.1057% +5.2299%] (p = 0.00 < 0.05) 1000 streams of 1 bytes/multistream: 💔 Performance has regressed.time: [25.339 ms 25.378 ms 25.418 ms] change: [+1.5118% +1.7329% +1.9432%] (p = 0.00 < 0.05) 10000 streams of 1 bytes/multistream: 💔 Performance has regressed.time: [1.7058 s 1.7074 s 1.7090 s] change: [+1.2315% +1.3628% +1.4929%] (p = 0.00 < 0.05) 1 streams of 1000 bytes/multistream: Change within noise threshold.time: [75.461 µs 76.859 µs 78.683 µs] change: [+0.6846% +3.1506% +5.8967%] (p = 0.00 < 0.05) 100 streams of 1000 bytes/multistream: Change within noise threshold.time: [3.4201 ms 3.4269 ms 3.4341 ms] change: [+0.6923% +0.9885% +1.2704%] (p = 0.00 < 0.05) 1000 streams of 1000 bytes/multistream: Change within noise threshold.time: [143.86 ms 143.94 ms 144.01 ms] change: [+0.5644% +0.6399% +0.7129%] (p = 0.00 < 0.05) coalesce_acked_from_zero 1+1 entries: No change in performance detected.time: [94.655 ns 95.015 ns 95.375 ns] change: [-0.4691% +0.0932% +0.6824%] (p = 0.77 > 0.05) coalesce_acked_from_zero 3+1 entries: No change in performance detected.time: [112.71 ns 113.08 ns 113.47 ns] change: [-0.1562% +0.2874% +0.8083%] (p = 0.22 > 0.05) coalesce_acked_from_zero 10+1 entries: No change in performance detected.time: [112.38 ns 112.94 ns 113.60 ns] change: [-0.2043% +0.4505% +1.1414%] (p = 0.20 > 0.05) coalesce_acked_from_zero 1000+1 entries: Change within noise threshold.time: [94.130 ns 94.494 ns 94.882 ns] change: [+0.7154% +1.8366% +3.1210%] (p = 0.00 < 0.05) RxStreamOrderer::inbound_frame(): Change within noise threshold.time: [115.55 ms 115.60 ms 115.67 ms] change: [+0.4598% +0.5367% +0.6153%] (p = 0.00 < 0.05) SentPackets::take_ranges: No change in performance detected.time: [8.3562 µs 8.6462 µs 8.9254 µs] change: [-0.5633% +2.9601% +6.4513%] (p = 0.10 > 0.05) transfer/pacing-false/varying-seeds: 💚 Performance has improved.time: [35.299 ms 35.364 ms 35.429 ms] change: [-3.7113% -3.4502% -3.2045%] (p = 0.00 < 0.05) transfer/pacing-true/varying-seeds: 💚 Performance has improved.time: [36.183 ms 36.277 ms 36.371 ms] change: [-4.2521% -3.8515% -3.4731%] (p = 0.00 < 0.05) transfer/pacing-false/same-seed: 💚 Performance has improved.time: [35.344 ms 35.390 ms 35.439 ms] change: [-3.4056% -3.2326% -3.0433%] (p = 0.00 < 0.05) transfer/pacing-true/same-seed: 💚 Performance has improved.time: [37.052 ms 37.121 ms 37.192 ms] change: [-4.7178% -4.4772% -4.2185%] (p = 0.00 < 0.05) Client/server transfer resultsPerformance differences relative to 054b7cb. Transfer of 33554432 bytes over loopback, 30 runs. All unit-less numbers are in milliseconds.
|
This is the correct presentation: link. The key insight, IMO, is what ECT codepoints appear in the same connection. IIUC this will provide that, but in even more detail, by counting the number of packets in each. Another interesting metric is "how many packets go by before the first codepoint transition," which is an indicator whether this is just a result of handshake packets being Not-ECT or something else. |
Thanks, corrected above.
"First codepoint transition" as in "away from NotEct" or as in "away from whatever codepoint was used on the first packet"? |
What I did was the latter, but noting the packet count at each transition type would of course be the richest data set. NotEct to something else is generally a handshake->short header transition, ECT->Not is a validation failure, and ECT->CE probably tells you something about the path and your CCA, though I'm not sure what. It's just a suggestion; do it if you think it's valuable. |
Need to think about this, as our telemetry system (https://github.com/mozilla/glean/) makes it purposefully a bit challenging to gather variable-length data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Data exposure looks reasonable. That said, not sure how to gather the data with Glean.
I will watch Martin's talk recording to understand the greater picture.
Can we hold off here until then?
Sure. This is by no means urgent, and maybe we want to discuss a more principled approach to adding QUIC telemetry first. |
Co-authored-by: Max Inden <[email protected]> Signed-off-by: Lars Eggert <[email protected]>
Signed-off-by: Lars Eggert <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Watched Martin's talk now. Thank you for sharing the data.
I don't know which concrete engineering decisions we would support with the data exposed here. Do you have something concrete in mind, Lars?
Co-authored-by: Max Inden <[email protected]> Signed-off-by: Lars Eggert <[email protected]>
Co-authored-by: Max Inden <[email protected]> Signed-off-by: Lars Eggert <[email protected]>
Co-authored-by: Max Inden <[email protected]> Signed-off-by: Lars Eggert <[email protected]>
@@ -33,6 +41,7 @@ fn unknown_version() { | |||
unknown_version_packet.resize(MIN_INITIAL_PACKET_SIZE, 0x0); | |||
drop(client.process(Some(datagram(unknown_version_packet)), now())); | |||
assert_eq!(1, client.stats().dropped_rx); | |||
assert_dscp(&client.stats()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I generally strive for a single unit test to test a single property only.
Rational: When a unit test tests multiple properties, a test failure becomes difficult to interpret, i.e. one cannot immediately tell which property is genuinely broken or whether adjusting the test would inadvertently discard another important check. By keeping each test focused on a single property, one ensures that test failures clearly indicate which behavior needs attention, making it safer and more straightforward to update or refactor ones code.
With that in mind, I am not a fan of adding assert_dscp
to only-slightly-related tests, but instead I would prefer separate dedicated tests.
Feel free to ignore this. I don't feel strongly about it. It might not even be worth the effort here.
Inspired by @martinduke's IETF presentation on ECN, add some more stats around what packet types we send and receive with different ECN codepoints.
This will require additional glue code to ship these stats back as telemetry.
Debug output: