Skip to content

Reject datagrams larger than the peer's max_datagram_frame_size before they reach the queue - #4025

Closed
jesup wants to merge 1 commit into
users/jesup/hook_up_queuefrom
users/jesup/max_datagram_frame_size
Closed

jesup wants to merge 1 commit into
users/jesup/hook_up_queuefrom
users/jesup/max_datagram_frame_size

Conversation

@jesup

@jesup jesup commented Sep 25, 2026

Copy link
Copy Markdown
Member

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The boundary still permits frames exceeding the peer’s encoded-frame limit and performs rejection after allocating the oversized buffer.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity

Open (2)
What changed in this PR

Adds synchronous peer-size validation for HTTP/3 datagrams before queueing.

Changes:

  • Validates encoded WebTransport and CONNECT-UDP datagram sizes.
  • Adds boundary tests for accepted and rejected payloads.
  • Documents transport-layer sizing assumptions.
File Description
neqo-transport/​src/​connection/​mod.rs Documents queue size-check responsibilities.
neqo-http3/​src/​features/​extended_connect/​session.rs Rejects oversized encoded datagrams.
neqo-http3/​src/​features/​extended_connect/​tests/​webtransport/​datagrams.rs Tests WebTransport size boundaries.
neqo-http3/​tests/​connect_udp.rs Tests CONNECT-UDP prefix-inclusive sizing.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread neqo-transport/src/connection/mod.rs Outdated
Comment thread neqo-http3/src/features/extended_connect/session.rs Outdated
@codecov

codecov Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.08%. Comparing base (a372889) to head (e4665bf).

Additional details and impacted files
@@                      Coverage Diff                      @@
##           users/jesup/hook_up_queue    #4025      +/-   ##
=============================================================
- Coverage                      97.10%   97.08%   -0.03%     
=============================================================
  Files                            114      114              
  Lines                          41061    41067       +6     
  Branches                       41061    41067       +6     
=============================================================
- Hits                           39873    39869       -4     
- Misses                          1174     1184      +10     
  Partials                          14       14              
Flag Coverage Δ
linux 97.14% <100.00%> (-0.02%) ⬇️
macos 95.27% <100.00%> (-0.02%) ⬇️
windows 95.37% <100.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
neqo-common 99.31% <ø> (ø)
neqo-http3 95.35% <100.00%> (+<0.01%) ⬆️
neqo-qpack 96.97% <ø> (ø)
neqo-transport 97.90% <100.00%> (-0.05%) ⬇️
neqo-udp 95.37% <ø> (ø)
mtu 89.13% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codspeed

codspeed Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 99 untouched benchmarks


Comparing users/jesup/max_datagram_frame_size (e4665bf) with users/jesup/hook_up_queue (a372889)

Open in CodSpeed

@jesup
jesup force-pushed the users/jesup/max_datagram_frame_size branch from 6a222ee to 4b80085 Compare September 25, 2026 12:35
…queue_datagram

An encoded datagram exceeding the peer's max_datagram_frame_size used to be
accepted into the queue and only fail (or silently never fit) at packet-build
time. enqueue_datagram now returns TooMuchData, as the legacy add_datagram
did, and send_datagram propagates it.
@jesup
jesup force-pushed the users/jesup/hook_up_queue branch from 6c1685e to a372889 Compare September 25, 2026 15:05
@jesup
jesup force-pushed the users/jesup/max_datagram_frame_size branch from 4b80085 to e4665bf Compare September 25, 2026 15:05
@github-actions

Copy link
Copy Markdown
Contributor

Failed Interop Tests

QUIC Interop Runner, client vs. server, differences relative to users/jesup/hook_up_queue at a372889.

neqo-pr as clientneqo-pr as server
neqo-pr vs. go-x-net: BP BA
neqo-pr vs. haproxy: ⚠️M BP BA
neqo-pr vs. kwik: Z
neqo-pr vs. lsquic: L1 C1
neqo-pr vs. msquic: Z A L1 ⚠️L2 C1
neqo-pr vs. mvfst: A ⚠️L1
neqo-pr vs. neqo: Z A
neqo-pr vs. nginx: BP BA
neqo-pr vs. ngtcp2: run cancelled after 20 min
neqo-pr vs. picoquic: Z A 🚀BP
neqo-pr vs. quic-go: A
neqo-pr vs. quiche: BP BA
neqo-pr vs. s2n-quic: CM
neqo-pr vs. tquic: S BP BA
neqo-pr vs. xquic: S R Z A L1 C1
aioquic vs. neqo-pr: CM
go-x-net vs. neqo-pr: CM
kwik vs. neqo-pr: BP BA CM
msquic vs. neqo-pr: CM
mvfst vs. neqo-pr: Z L1 C1 CM
neqo vs. neqo-pr: Z A
openssl vs. neqo-pr: LR M A CM
quic-go vs. neqo-pr: CM
quiche vs. neqo-pr: CM
quinn vs. neqo-pr: ⚠️C1 V2 CM
s2n-quic vs. neqo-pr: ⚠️B CM
tquic vs. neqo-pr: ⚠️C1 CM
xquic vs. neqo-pr: M CM
All results

Succeeded Interop Tests

QUIC Interop Runner, client vs. server

neqo-pr as client

neqo-pr as server

Unsupported Interop Tests

QUIC Interop Runner, client vs. server

neqo-pr as client

neqo-pr as server

@github-actions

Copy link
Copy Markdown
Contributor

Client/server transfer results

Performance differences relative to a372889.

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

Client vs. server Mean±σ Min–Max Median±MAD MiB/s±σ ΔMedian
google-neqo-cubic 70.3 ± 0.3 69.4 – 71.0 70.3 ± 0.3 455.2 ± 2.1 💔 +2.8 (+4.2%)

Table above only shows statistically significant changes. See all results below.

All results

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

Client vs. server Mean±σ Min–Max Median±MAD MiB/s±σ ΔMedian
google-google 132.2 ± 0.4 131.4 – 133.3 132.2 ± 0.3 242.0 ± 0.7
google-neqo-cubic 70.3 ± 0.3 69.4 – 71.0 70.3 ± 0.3 455.2 ± 2.1 💔 +2.8 (+4.2%)
neqo-google-cubic ⚠️ 228.7 ± 47.5 176.3 – 475.8 220.2 ± 24.6 139.9 ± 29.1 -0.3 (-0.1%)
neqo-neqo-cubic 19.2 ± 0.1 18.8 – 19.5 19.1 ± 0.2 1670.9 ± 11.9 -0.0 (-0.0%)
neqo-neqo-cubic-nopacing 18.9 ± 0.1 18.6 – 19.3 18.9 ± 0.1 1694.0 ± 11.8 +0.0 (+0.2%)
neqo-neqo-newreno 19.7 ± 0.2 19.4 – 20.1 19.7 ± 0.2 1623.9 ± 12.5 +0.1 (+0.4%)
neqo-neqo-newreno-nopacing 18.9 ± 0.1 18.5 – 19.2 18.9 ± 0.1 1693.0 ± 12.8 -0.1 (-0.4%)
neqo-quiche-cubic 33.2 ± 0.3 32.6 – 33.7 33.2 ± 0.3 964.8 ± 7.6 +0.1 (+0.2%)
neqo-s2n-cubic 37.8 ± 0.2 37.3 – 38.5 37.8 ± 0.2 846.7 ± 4.4 -0.2 (-0.5%)
quiche-neqo-cubic ⚠️ 38.3 ± 2.8 37.0 – 65.5 38.0 ± 0.3 834.5 ± 60.6 -0.2 (-0.5%)
quiche-quiche 39.5 ± 0.2 39.0 – 40.2 39.4 ± 0.2 810.9 ± 4.5
s2n-neqo-cubic 112.4 ± 0.3 111.2 – 113.2 112.4 ± 0.3 284.6 ± 0.8 +0.1 (+0.1%)
s2n-s2n ⚠️ 161.7 ± 15.9 135.4 – 257.1 159.5 ± 1.0 197.9 ± 19.4

Download data for profiler.firefox.com or download performance comparison data.

@github-actions

Copy link
Copy Markdown
Contributor

Benchmark results

No significant performance differences relative to a372889.

All results
transfer/1-conn/1-100mb-req (aka. Upload)/mtu-1500
       time:   [48.731 ms 48.880 ms 49.091 ms]
       thrpt:  [1.9893 GiB/s 1.9979 GiB/s 2.0040 GiB/s]
Found 5 outliers among 100 measurements (5.00%)
2 (2.00%) high mild
3 (3.00%) high severe
transfer/1-conn/1-100mb-resp (aka. Download)/mtu-1500
       time:   [50.005 ms 50.069 ms 50.137 ms]
       thrpt:  [1.9478 GiB/s 1.9504 GiB/s 1.9529 GiB/s]
Found 4 outliers among 100 measurements (4.00%)
4 (4.00%) high mild
transfer/1-conn/1-1b-resp (aka. HPS)/mtu-1500
       time:   [2.9340 ms 2.9351 ms 2.9363 ms]
       thrpt:  [340.57   B/s 340.70   B/s 340.84   B/s]
Found 9 outliers among 100 measurements (9.00%)
6 (6.00%) high mild
3 (3.00%) high severe
transfer/1-conn/10_000-parallel-1b-resp (aka. RPS)/mtu-1500
       time:   [67.247 ms 67.516 ms 67.787 ms]
       thrpt:  [147.52 Kelem/s 148.11 Kelem/s 148.70 Kelem/s]
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild
streams-flow-controlled/walltime/1-streams/each-4194304-bytes
       time:   [10.344 ms 10.348 ms 10.353 ms]
       thrpt:  [386.36 MiB/s 386.54 MiB/s 386.71 MiB/s]
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild
streams-flow-controlled/walltime/10-streams/each-1048576-bytes
       time:   [28.414 ms 28.427 ms 28.440 ms]
       thrpt:  [351.62 MiB/s 351.78 MiB/s 351.93 MiB/s]
streams/walltime/1-streams/each-1000-bytes
       time:   [131.80 µs 132.37 µs 133.10 µs]
       thrpt:  [7.1649 MiB/s 7.2044 MiB/s 7.2356 MiB/s]
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high mild
streams/walltime/1000-streams/each-1-bytes
       time:   [3.8402 ms 3.8521 ms 3.8637 ms]
       thrpt:  [252.75 KiB/s 253.51 KiB/s 254.30 KiB/s]
streams/walltime/1000-streams/each-1000-bytes
       time:   [12.494 ms 12.503 ms 12.512 ms]
       thrpt:  [76.223 MiB/s 76.278 MiB/s 76.333 MiB/s]
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) low mild
1 (1.00%) high mild
transfer/walltime/pacing-false/same-seed
       time:   [2.0463 ms 2.0471 ms 2.0481 ms]
       thrpt:  [1.9073 GiB/s 1.9082 GiB/s 1.9089 GiB/s]
transfer/walltime/pacing-false/varying-seeds
       time:   [2.0303 ms 2.0314 ms 2.0327 ms]
       thrpt:  [1.9217 GiB/s 1.9229 GiB/s 1.9240 GiB/s]
transfer/walltime/pacing-true/same-seed
       time:   [2.0895 ms 2.0911 ms 2.0931 ms]
       thrpt:  [1.8663 GiB/s 1.8680 GiB/s 1.8695 GiB/s]
transfer/walltime/pacing-true/varying-seeds
       time:   [2.0558 ms 2.0568 ms 2.0579 ms]
       thrpt:  [1.8982 GiB/s 1.8992 GiB/s 1.9001 GiB/s]

Instructions per cycle

Criterion reported no significant timing changes.

All benchmarks
Benchmark IPC before IPC after ΔIPC
streams/walltime/1000-streams/each-1000-bytes 3.01 2.98 -0.9%
streams-flow-controlled/walltime/1-streams/each-4194304-bytes 3.00 3.01 +0.5%
transfer/1-conn/10_000-parallel-1b-resp (aka. RPS)/mtu-1500 3.19 3.20 +0.5%
transfer/walltime/pacing-true/varying-seeds 2.89 2.88 -0.4%
transfer/walltime/pacing-false/varying-seeds 2.88 2.89 +0.4%
transfer/1-conn/1-100mb-resp (aka. Download)/mtu-1500 3.22 3.20 -0.4%
streams/walltime/1000-streams/each-1-bytes 3.27 3.28 +0.3%
streams-flow-controlled/walltime/10-streams/each-1048576-bytes 3.16 3.15 -0.3%
streams/walltime/1-streams/each-1000-bytes 2.55 2.55 -0.3%
transfer/1-conn/1-100mb-req (aka. Upload)/mtu-1500 3.17 3.17 +0.1%
transfer/1-conn/1-1b-resp (aka. HPS)/mtu-1500 2.24 2.24 -0.1%
transfer/walltime/pacing-false/same-seed 2.89 2.89 -0.0%
transfer/walltime/pacing-true/same-seed 2.90 2.90 +0.0%
Profiles for profiler.firefox.com (62)

Download data for profiler.firefox.com or download performance comparison data.

@jesup

jesup commented Sep 29, 2026

Copy link
Copy Markdown
Member Author

Landed as part of #4003 (squash-merged in 9d7e35b).

@jesup jesup closed this Sep 29, 2026
@jesup
jesup deleted the users/jesup/max_datagram_frame_size branch September 30, 2026 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants