Skip to content

fix(connlib): correctly compute the GSO batch size#8754

Merged
thomaseizinger merged 2 commits into
mainfrom
fix/limit-gso-batch-size
Apr 12, 2025
Merged

fix(connlib): correctly compute the GSO batch size#8754
thomaseizinger merged 2 commits into
mainfrom
fix/limit-gso-batch-size

Conversation

@thomaseizinger
Copy link
Copy Markdown
Member

@thomaseizinger thomaseizinger commented Apr 11, 2025

We are currently naively chunking our buffer into segment_size * max_gso_segments(). max_gso_segments is by default 64. Assuming we processed several IP packets, this would quickly balloon to a size that the kernel cannot handle. For example, during an iperf3 run, we receive a lot of packets at maximum MTU size (1280). With the overhead that we are adding to the packet, this results in a UDP payload size of 1320.

1320 x 64 = 84480

That is way too large for the kernel to handle and it will fail the sendmsg call with EMSGSIZE. Unfortunately, this error wasn't surfaced because quinn_udp handles it internally because it can also happen as a result of MTU probes.

We've already patched quinn_udp in the past to move the handling of more quinn-specific errors to the infallible send function. The same is being done for this error in quinn-rs/quinn#2199.

Resolves: #8699

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 11, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
firezone ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 12, 2025 8:31am

@thomaseizinger thomaseizinger force-pushed the fix/limit-gso-batch-size branch from 0b96cd7 to fa73c6d Compare April 11, 2025 13:37
@thomaseizinger thomaseizinger changed the title fix(connlib): limit the UDP GSO batch size fix(connlib): correctly compute the GSO batch size Apr 11, 2025
@thomaseizinger
Copy link
Copy Markdown
Member Author

We are back.

image

@thomaseizinger thomaseizinger marked this pull request as ready for review April 11, 2025 13:53
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread rust/socket-factory/src/lib.rs
Comment thread website/src/components/Changelog/Headless.tsx Outdated
@thomaseizinger
Copy link
Copy Markdown
Member Author

All good, this is ready to merge. The problems I was facing locally ended up being something else.

@thomaseizinger thomaseizinger force-pushed the fix/limit-gso-batch-size branch from 2159add to 6fd2a82 Compare April 12, 2025 08:29
@thomaseizinger thomaseizinger added this pull request to the merge queue Apr 12, 2025
Merged via the queue into main with commit 132487c Apr 12, 2025
114 checks passed
@thomaseizinger thomaseizinger deleted the fix/limit-gso-batch-size branch April 12, 2025 13:28
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.

TCP performance is especially latency-sensitive

3 participants