set may_fragment based on whether setting IP_DONTFRAG fails with ENOPROTOOPT.

[jieliangma]

fix issue

[djc]

Possible argument against this PR: iOS 13 (the last version of iOS that didn't support this, assuming iOS releases got support at the same time as macOS) has been out of security support since Sep 2020.

@jieliangma why has this device not been updated to a later OS? It looks like it ought to support iOS 16 (the current latest version) just fine.

[Ralith]

The reason we hesitated to add support for older macOS/iOS versions up front in #1547 is that platforms which may fragment packets must report this accurately in may_fragment(), so that PMTU discovery is correctly disabled. Because PMTU discovery is a desirable performance improvement in general, we must ensure that we do so only on these legacy platforms, without interfering with performance on modern Unixes.

In short, this PR can't be merged without adding dynamic iOS version detection logic to may_fragment(). For robustness, we should also only ignore the specific expected error, and only in environments specifically expected to produce it.

[djc]

In short, this PR can't be merged without adding dynamic iOS version detection logic to may_fragment(). For robustness, we should also only ignore the specific expected error, and only in environments specifically expected to produce it.

If we try to set IP_DONTFRAG in init(), I suppose we can store the may_fragment bit as state after that, so that we don't need to interrogate the socket for every call to may_fragment()?

[Ralith]

Good point, that's also a lot easier and less fragile than version checks. may_fragment should then probably be a method on UdpSocketState.

[Ralith]

Thanks for the revision! Please split the cosmetic refactoring commits out from the semantic changes so they can be easily reviewed.

[jieliangma]

Thanks for the revision! Please split the cosmetic refactoring commits out from the semantic changes so they can be easily reviewed.

done

[djc]

In short, this PR can't be merged without adding dynamic iOS version detection logic to may_fragment(). For robustness, we should also only ignore the specific expected error, and only in environments specifically expected to produce it.

It seems okay to me to set may_fragment based on whether setting IP_DONTFRAG fails with NOPROTOOPT, without separate implementations for FreeBSD or newer versions of the Apple OSes, do you see any reason?

[Ralith]

Yeah, that sounds good to me, since we're no longer relying on an independent judgement in may_fragment to line up.

[djc]

@jieliangma #1629 refactored part of this code a bit, should make the changes here easier but will require some rebasing or rework.

[djc]

Going to merge this and fix it up a little. Thanks @jieliangma!