Conversation
There was a problem hiding this comment.
Pull request overview
Updates neqo-udp’s UDP send error handling to avoid treating ENOBUFS as a fatal send error, aligning send behavior with transient NIC/stack buffer exhaustion on some platforms.
Changes:
- Treat
ENOBUFSfromUdpSocketState::try_send()asio::ErrorKind::WouldBlockinstead of a hard error. - Add
is_enobufs()helper (unix-gated) and update unit tests to account for the new predicate.
You can also share your feedback on Copilot code review. Take the survey.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3482 +/- ##
==========================================
- Coverage 94.30% 94.17% -0.13%
==========================================
Files 127 131 +4
Lines 38711 39053 +342
Branches 38711 39053 +342
==========================================
+ Hits 36505 36777 +272
- Misses 1365 1427 +62
- Partials 841 849 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
|
04de82b to
345f3cf
Compare
Based on a discussion with @ianswett yesterday, don't treat `ENOBUFS` as a fatal error when sending UDP packets.
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
|
Benchmark resultsNo significant performance differences relative to 2cd5820. All resultstransfer/1-conn/1-100mb-resp (aka. Download)/mtu-1504: Change within noise threshold. time: [199.79 ms 200.23 ms 200.74 ms]
thrpt: [498.15 MiB/s 499.43 MiB/s 500.52 MiB/s]
change:
time: [-1.3960% -1.0768% -0.7316] (p = 0.00 < 0.05)
thrpt: [+0.7370% +1.0885% +1.4157]
Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severetransfer/1-conn/10_000-parallel-1b-resp (aka. RPS)/mtu-1504: Change within noise threshold. time: [286.19 ms 287.93 ms 289.71 ms]
thrpt: [34.517 Kelem/s 34.730 Kelem/s 34.942 Kelem/s]
change:
time: [+0.7749% +1.7085% +2.6814] (p = 0.00 < 0.05)
thrpt: [-2.6113% -1.6798% -0.7689]
Change within noise threshold.
Found 3 outliers among 100 measurements (3.00%)
3 (3.00%) high mildtransfer/1-conn/1-1b-resp (aka. HPS)/mtu-1504: No change in performance detected. time: [38.518 ms 38.650 ms 38.798 ms]
thrpt: [25.774 B/s 25.873 B/s 25.962 B/s]
change:
time: [-0.0674% +0.3627% +0.8649] (p = 0.13 > 0.05)
thrpt: [-0.8575% -0.3613% +0.0674]
No change in performance detected.
Found 5 outliers among 100 measurements (5.00%)
1 (1.00%) high mild
4 (4.00%) high severetransfer/1-conn/1-100mb-req (aka. Upload)/mtu-1504: No change in performance detected. time: [203.70 ms 204.11 ms 204.54 ms]
thrpt: [488.91 MiB/s 489.94 MiB/s 490.91 MiB/s]
change:
time: [-0.4962% -0.1651% +0.1384] (p = 0.32 > 0.05)
thrpt: [-0.1383% +0.1654% +0.4987]
No change in performance detected.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severestreams/walltime/1-streams/each-1000-bytes: No change in performance detected. time: [583.76 µs 585.79 µs 588.16 µs]
change: [-0.2605% +0.2787% +0.8354] (p = 0.31 > 0.05)
No change in performance detected.
Found 10 outliers among 100 measurements (10.00%)
1 (1.00%) high mild
9 (9.00%) high severestreams/walltime/1000-streams/each-1-bytes: No change in performance detected. time: [12.479 ms 12.510 ms 12.554 ms]
change: [-0.0931% +0.2694% +0.6763] (p = 0.18 > 0.05)
No change in performance detected.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high severestreams/walltime/1000-streams/each-1000-bytes: Change within noise threshold. time: [45.341 ms 45.388 ms 45.436 ms]
change: [+0.8729% +1.0265% +1.1562] (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/varying-seeds: No change in performance detected. time: [80.282 ms 80.337 ms 80.395 ms]
change: [-0.3641% -0.1638% -0.0158] (p = 0.06 > 0.05)
No change in performance detected.
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high mildtransfer/walltime/pacing-true/varying-seeds: Change within noise threshold. time: [79.728 ms 79.794 ms 79.868 ms]
change: [-0.2832% -0.1503% -0.0198] (p = 0.02 < 0.05)
Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high severetransfer/walltime/pacing-false/same-seed: Change within noise threshold. time: [80.094 ms 80.152 ms 80.217 ms]
change: [+0.3772% +0.6063% +0.7907] (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/same-seed: No change in performance detected. time: [79.841 ms 79.886 ms 79.932 ms]
change: [-0.2220% -0.0203% +0.1439] (p = 0.84 > 0.05)
No change in performance detected.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildDownload data for |
Client/server transfer resultsPerformance differences relative to 2cd5820. 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 |
|
No objections! |
|
@mxinden quinn-rs/quinn#2293 got closed - what now? |
|
@larseggert let's try to get to the root of the discussion. I prepared quinn-rs/quinn#2594. Can you help out? |
Based on a discussion with @ianswett yesterday, don't treat
ENOBUFSas a fatal error when sending UDP packets.