Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fragmentation handling improvements #1547

Merged
merged 4 commits into from
May 6, 2023
Merged

Fragmentation handling improvements #1547

merged 4 commits into from
May 6, 2023

Conversation

Ralith
Copy link
Collaborator

@Ralith Ralith commented Apr 23, 2023

Fixes #664.

Some questions following from this work:

  • Should MTU discovery configuration be on the endpoint, rather than the connection? It's hard to imagine a case where a user might want to vary this per-connection, and usually the main concern is whether the common socket has DONTFRAG set. On the other hand, the reduction in complexity from moving it to the endpoint seems slight. I guess you could use a single endpoint to communicate both on a jumbo-frame-capable LAN and also the internet...
  • Setting IP_DONTFRAG might fail pre-Big Sur (version 11). As written, this PR will make endpoint creation fail if this occurs, probably with EINVAL. Are older macOS builds worth supporting?
  • Can we reduce the amount of work required from users to create a endpoint or connection with the recommended configuration for their platform?

@Ralith
Copy link
Collaborator Author

Ralith commented Apr 26, 2023

According to https://en.wikipedia.org/wiki/MacOS_version_history, macOS <11 is officially unsupported, at least.

@Ralith
Copy link
Collaborator Author

Ralith commented Apr 27, 2023

I've improved MTUD configuration ergonomics by adding an argument to quinn_proto::Connection::new which quinn uses to clobber the user-specified MTUD configuration on unsupported backends.

@Ralith Ralith force-pushed the frags branch 3 times, most recently from e046650 to 904e921 Compare May 1, 2023 23:44
@djc
Copy link
Member

djc commented May 2, 2023

Are older macOS builds worth supporting?

Let's default to no and see if people complain?

quinn-udp/src/fallback.rs Show resolved Hide resolved
quinn-udp/src/lib.rs Outdated Show resolved Hide resolved
quinn-udp/src/fallback.rs Outdated Show resolved Hide resolved
quinn-proto/src/endpoint.rs Outdated Show resolved Hide resolved
quinn-proto/src/endpoint.rs Outdated Show resolved Hide resolved
quinn-udp/src/unix.rs Show resolved Hide resolved
Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

Nice!

@djc djc merged commit a55fd51 into main May 6, 2023
@djc djc deleted the frags branch May 6, 2023 06:05
@complexspaces
Copy link

complexspaces commented May 9, 2023

FWIW, rustc and the standard library still supports all the way back to macOS 10.7. I think the approach of waiting to see is fine, but I figured it was worth mentioning since there are a lot of users who are still on the 10.x line for reasons outside their control and this change would preclude the use of the crate in builds with a deployment target set to even 10.15 (out of support for less then a year).

@djc
Copy link
Member

djc commented May 9, 2023

@complexspaces thanks for following up! Is Quinn in use (perhaps indirectly) at 1P?

since there are a lot of users who are still on the 10.x line for reasons outside their control

Can you quality "a lot of users" a bit more? Presumably this is the 1P population, can you disclose a rough percentage?

builds with a deployment target set to even 10.15 (out of support for less then a year).

In my mind that's still likely to be a pretty small population... For context, 10.15 (Catalina) is the version released in Oct 2019, so about 2.5 years ago at this point.

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.

Set IP Don't Fragment bit on all outgoing UDP datagrams
3 participants