Repository navigation
Conversation
zerox80
requested review from
KershawChang,
larseggert,
martinthomson,
mxinden and
omansfeld
as code owners
October 13, 2026 21:59
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Applying the peer limit leaves the pacer using its previous MTU until later ACK or loss processing.
1 open finding
What changed in this PR
Caps ordinary QUIC datagrams by the peer-advertised UDP payload limit while preserving PMTU discovery state.
Changes:
- Applies the peer limit to effective PLPMTU.
- Adds IPv4/IPv6, directional, PMTUD, congestion-control, and pacing tests.
| File | Description |
|---|---|
neqo-transport/src/pmtud.rs |
Caps effective PLPMTU and adds unit coverage. |
neqo-transport/src/sender.rs |
Tests congestion-control and pacer behavior. |
neqo-transport/src/connection/tests/pmtud.rs |
Adds end-to-end payload-limit tests. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
118
to
119
| pub const fn set_peer_max_udp_payload(&mut self, peer_max_udp_payload: usize) { | ||
| self.peer_max_udp_payload = Some(peer_max_udp_payload); |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4081 +/- ##
==========================================
- Coverage 97.15% 96.97% -0.18%
==========================================
Files 113 118 +5
Lines 41200 41803 +603
Branches 41200 41803 +603
==========================================
+ Hits 40028 40540 +512
- Misses 1158 1239 +81
- Partials 14 24 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Contributor
Merging this PR will regress 2 benchmarks
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | decode 10 STREAM frames |
3.8 µs | 3.9 µs | -2.74% |
| ❌ | Simulation | simulated/pacing-false/varying-seeds |
73.2 ms | 75 ms | -2.41% |
| ⚡ | WallTime | walltime/pacing-true/varying-seeds |
2.1 ms | 2 ms | +6.83% |
| ⚡ | WallTime | walltime/pacing-true/same-seed |
2.1 ms | 2 ms | +4.46% |
| ⚡ | WallTime | walltime/pacing-false/same-seed |
2 ms | 2 ms | +3.63% |
| ⚡ | WallTime | walltime/pacing-false/varying-seeds |
2 ms | 2 ms | +2.79% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing zerox80:codex/peer-udp-payload-limit (9a6a1d7) with main (ad89cf0)
Contributor
Contributor
Client/server transfer resultsPerformance differences relative to ad7d629. 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 |
Contributor
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
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a peer advertises
max_udp_payload_size = 1200, Neqo still uses the default 1252-byte IPv4 or 1232-byte IPv6 payload size for ordinary datagrams. The parameter currently limits upward PMTU probes only, so a receiver enforcing its advertised limit can discard stream-data datagrams even with PMTUD disabled.Cap
Pmtud::plpmtu()by the known peer limit, so normal packet construction, batching and congestion control use the effective payload size. Apply peer-limit updates through the path and sender so the pacer uses the effective packet size immediately, including when a new path inherits the limit. Keep the discovered path MTU and probe search table intact.The connection tests cover IPv4/IPv6, both senders and PMTUD enabled/disabled. Symmetric limits cover eight sending cases; one-sided limits cover another sixteen. Only traffic towards the peer advertising 1200 bytes is capped. The asymmetric test checks actual GSO segment lengths, prevents oversized probes towards the limited peer, and exercises a larger probe followed by a smaller final datagram in the opposite direction. Both directions deliver stream data.
The PMTUD unit test covers limits below, equal to and above the default, including restarts and the raise timer. A sender test checks the congestion-window floor, bytes in flight across send/ACK, and pacer MTU updates before the first ACK for IPv4/IPv6 and NewReno/Cubic with each of Classic/HyStart/SEARCH slow start.
Reference: RFC 9000, Sections 14 and 18.2.
Local validation (performed by the PR author):
cargo test --locked -p neqo-transport peer_max_udp_payload).cargo +nightly fmt --all -- --check: passed.cargo hack clippywith the repository's feature-powerset options and-D warnings: all 45 combinations passed.--deny rustdoc::broken_intra_doc_links --deny warnings: passed.cargo hack testwith the repository's feature-powerset options: all 38 combinations passed.