Skip to content

Reject DATAGRAMs larger than the send buffer#2626

Open
zerox80 wants to merge 5 commits into
quinn-rs:mainfrom
zerox80:upstream-datagram-send-buffer-limit
Open

Reject DATAGRAMs larger than the send buffer#2626
zerox80 wants to merge 5 commits into
quinn-rs:mainfrom
zerox80:upstream-datagram-send-buffer-limit

Conversation

@zerox80

@zerox80 zerox80 commented Apr 27, 2026

Copy link
Copy Markdown

Follow-up to #2625.

A DATAGRAM larger than datagram_send_buffer_size cannot fit in the outgoing send buffer by itself. This returns TooLarge before trying either send mode, so the behavior is the same for drop = true and drop = false.

Tested with:

cargo fmt --check
cargo test -p quinn-proto --lib
cargo test -p quinn-proto datagram

@zerox80 zerox80 force-pushed the upstream-datagram-send-buffer-limit branch from 134613b to 8976994 Compare April 27, 2026 22:03
Comment thread quinn-proto/src/connection/datagrams.rs Outdated
@zerox80 zerox80 force-pushed the upstream-datagram-send-buffer-limit branch from 8976994 to c9ceb0c Compare April 28, 2026 06:04
@zerox80

zerox80 commented Apr 28, 2026

Copy link
Copy Markdown
Author

Updated this too. The size check now happens before the drop branch, so drop = false returns TooLarge for a datagram that cannot fit in the send buffer. I added a test for both send modes.

@zerox80

zerox80 commented May 24, 2026

Copy link
Copy Markdown
Author

What is the current state?

@zerox80 zerox80 force-pushed the upstream-datagram-send-buffer-limit branch from c9ceb0c to e93d77c Compare May 26, 2026 13:55

@djc djc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks okay to me, thanks.

Perhaps you'll want to backport it to the 0.11.x branch, too?

Comment thread quinn-proto/src/connection/datagrams.rs Outdated
Comment thread quinn-proto/src/connection/datagrams.rs Outdated
@zerox80 zerox80 force-pushed the upstream-datagram-send-buffer-limit branch from e93d77c to 7960a26 Compare June 1, 2026 20:27
@zerox80

zerox80 commented Jun 1, 2026

Copy link
Copy Markdown
Author

This looks okay to me, thanks.

Perhaps you'll want to backport it to the 0.11.x branch, too?

Good point, yes, this should also be backported to 0.11.x. I checked that the same send buffer logic is still present there. I’ll address the small nits here first, and once this lands on main I’ll open a separate backport PR against 0.11.x

@djc djc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please squash the changes from review into their originating commits.

Comment thread quinn-proto/src/connection/datagrams.rs
Comment thread quinn-proto/src/connection/datagrams.rs
@zerox80 zerox80 force-pushed the upstream-datagram-send-buffer-limit branch from 302f328 to f8135eb Compare June 2, 2026 12:15
@zerox80

zerox80 commented Jun 2, 2026

Copy link
Copy Markdown
Author

Done, thanks. I rewrote the stack so the review changes are squashed into their originating commits, added the send_buffer_size local binding as the first commit, and kept the has_send_buffer_space extraction as its own separate commit.

Tested with:
cargo fmt --check
cargo test -p quinn-proto --lib
cargo test -p quinn-proto datagram

@djc djc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for the iterations!

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