-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Socket.DontFragment can't be set on IPv6 DualMode sockets #102700
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
Conversation
src/libraries/System.Net.Sockets/tests/FunctionalTests/ArgumentValidationTests.cs
Outdated
Show resolved
Hide resolved
@@ -1157,5 +1157,15 @@ public void CancelConnectAsync_NullEventArgs_Throws_ArgumentNull() | |||
{ | |||
Assert.Throws<ArgumentNullException>(() => Socket.CancelConnectAsync(null)); | |||
} | |||
|
|||
// MacOS doesn't support setting don't-fragment (DF) bit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is supported actually since MacOSX 11.0 (Big Sur)+. We should be more specific with the filter condition. Can you check on what pipelines did it fail originally? FYI our other DontFragment
test only filters out iOS+tvOS+Catalyst, but not sure if it's correct:
runtime/src/libraries/System.Net.Sockets/tests/FunctionalTests/UdpClientTest.cs
Lines 268 to 275 in 2f08fcb
[Fact] | |
[ActiveIssue("https://github.com/dotnet/runtime/issues/51392", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)] | |
public void DontFragment_Roundtrips() | |
{ | |
using (var udpClient = new UdpClient()) | |
{ | |
Assert.False(udpClient.DontFragment); | |
udpClient.DontFragment = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The failing configurations are :
net9.0-osx-Debug-x64-coreclr_release-OSX.1200.Amd64.Open
net9.0-osx-Debug-x64-Mono_Minijit_Debug-OSX.1200.Amd64.Open
System.Net.Sockets.SocketException : Invalid argument
at System.Net.Sockets.Socket.UpdateStatusAfterSocketErrorAndThrowException(SocketError error, Boolean disconnectOnFailure, String callerName) in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs:line 3737
at System.Net.Sockets.Socket.UpdateStatusAfterSocketOptionErrorAndThrowException(SocketError error, String callerName) in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs:line 3728
at System.Net.Sockets.Socket.SetSocketOption(SocketOptionLevel optionLevel, SocketOptionName optionName, Int32 optionValue, Boolean silent) in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs:line 3466
at System.Net.Sockets.Socket.SetSocketOption(SocketOptionLevel optionLevel, SocketOptionName optionName, Int32 optionValue) in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs:line 1966
at System.Net.Sockets.Socket.set_DontFragment(Boolean value) in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs:line 653
at System.Net.Sockets.Tests.ArgumentValidation.CanSetDontFragment_OnIPV6Address_DualModeSocket() in /_/src/libraries/System.Net.Sockets/tests/FunctionalTests/ArgumentValidationTests.cs:line 1166
at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args) in /_/src/coreclr/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.CoreCLR.cs:line 36
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr) in /_/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.cs:line 57
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is weird since those machines are running on 12.7.4. Regardless, I think this PR changes things for the better so I think we can merge this and open a tracking issue for the MacOS problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
macOS did not support DF for long time at all. It does not surprise me that it does not work on dual mode socket. FreeBSD (and others) may be on the same boat e.g. it may be better to make this to running only on platfoprms where we know it works instead of excluding macOS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wfurt any objection to merging this?
I think it OK. small suggestion for the test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
thanks
Fixes #76410