-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Rework ExclusiveAddressUse and ReuseAddress on non-Windows platforms #11509
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -233,5 +233,65 @@ private static Socket CreateBoundUdpSocket(out int localPort) | |
| localPort = (receiveSocket.LocalEndPoint as IPEndPoint).Port; | ||
| return receiveSocket; | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(null, null, null, true)] | ||
| [InlineData(null, null, false, true)] | ||
| [InlineData(null, false, false, true)] | ||
| [InlineData(null, true, false, true)] | ||
| [InlineData(null, true, true, false)] | ||
| [InlineData(true, null, null, true)] | ||
| [InlineData(true, null, false, true)] | ||
| [InlineData(true, null, true, true)] | ||
| [InlineData(true, false, null, true)] | ||
| [InlineData(true, false, false, true)] | ||
| [InlineData(true, false, true, true)] | ||
| public void ReuseAddress(bool? exclusiveAddressUse, bool? firstSocketReuseAddress, bool? secondSocketReuseAddress, bool expectFailure) | ||
| { | ||
| using (Socket a = new Socket(AddressFamily.InterNetwork, SocketType.Dgram, ProtocolType.Udp)) | ||
| { | ||
| if (exclusiveAddressUse.HasValue) | ||
| { | ||
| a.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ExclusiveAddressUse, exclusiveAddressUse.Value); | ||
| } | ||
| if (firstSocketReuseAddress.HasValue) | ||
| { | ||
| a.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ReuseAddress, firstSocketReuseAddress.Value); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have existing tests verifying the roundtripping behavior for SocketOptionName.ReuseAddress with GetSocketOption? |
||
| } | ||
|
|
||
| a.Bind(new IPEndPoint(IPAddress.Loopback, 0)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any need to test both IPv4 and IPv6, or is just IPv4 sufficient for this purpose? |
||
|
|
||
| using (Socket b = new Socket(AddressFamily.InterNetwork, SocketType.Dgram, ProtocolType.Udp)) | ||
| { | ||
| if (secondSocketReuseAddress.HasValue) | ||
| { | ||
| b.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ReuseAddress, secondSocketReuseAddress.Value); | ||
| } | ||
|
|
||
| if (expectFailure) | ||
| { | ||
| Assert.ThrowsAny<SocketException>(() => b.Bind(a.LocalEndPoint)); | ||
| } | ||
| else | ||
| { | ||
| b.Bind(a.LocalEndPoint); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| [Theory] | ||
| [PlatformSpecific(PlatformID.Windows)] | ||
| [InlineData(false, null, null, true)] | ||
| [InlineData(false, null, false, true)] | ||
| [InlineData(false, false, null, true)] | ||
| [InlineData(false, false, false, true)] | ||
| [InlineData(false, true, null, true)] | ||
| [InlineData(false, true, false, true)] | ||
| [InlineData(false, true, true, false)] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the expected behavior of these cases on Unix? Should we verify those failures via a test that's Unix-only? |
||
| public void ReuseAddress_Windows(bool? exclusiveAddressUse, bool? firstSocketReuseAddress, bool? secondSocketReuseAddress, bool expectFailure) | ||
| { | ||
| ReuseAddress(exclusiveAddressUse, firstSocketReuseAddress, secondSocketReuseAddress, expectFailure); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -88,7 +88,7 @@ public void ConnectedAvailable_NullClient() | |
| [OuterLoop] // TODO: Issue #11345 | ||
| [Fact] | ||
| [PlatformSpecific(PlatformID.Windows)] | ||
| public void ExclusiveAddressUse_NullClient() | ||
| public void ExclusiveAddressUse_NullClient_Windows() | ||
| { | ||
| using (TcpClient client = new TcpClient()) | ||
| { | ||
|
|
@@ -100,13 +100,33 @@ public void ExclusiveAddressUse_NullClient() | |
|
|
||
| [OuterLoop] // TODO: Issue #11345 | ||
| [Fact] | ||
| [PlatformSpecific(PlatformID.Windows)] | ||
| public void Roundtrip_ExclusiveAddressUse_GetEqualsSet() | ||
| [PlatformSpecific(~PlatformID.Windows)] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typically we've used PlatformID.AnyUnix for this rather than ~Windows. |
||
| public void ExclusiveAddressUse_NullClient_NonWindows() | ||
| { | ||
| using (TcpClient client = new TcpClient()) | ||
| { | ||
| client.Client = null; | ||
|
|
||
| Assert.True(client.ExclusiveAddressUse); | ||
| } | ||
| } | ||
|
|
||
| [Fact] | ||
| public void Roundtrip_ExclusiveAddressUse_GetEqualsSet_True() | ||
| { | ||
| using (TcpClient client = new TcpClient()) | ||
| { | ||
| client.ExclusiveAddressUse = true; | ||
| Assert.True(client.ExclusiveAddressUse); | ||
| } | ||
| } | ||
|
|
||
| [Fact] | ||
| [PlatformSpecific(PlatformID.Windows)] | ||
| public void Roundtrip_ExclusiveAddressUse_GetEqualsSet_False() | ||
| { | ||
| using (TcpClient client = new TcpClient()) | ||
| { | ||
| client.ExclusiveAddressUse = false; | ||
| Assert.False(client.ExclusiveAddressUse); | ||
| } | ||
|
|
@@ -115,14 +135,13 @@ public void Roundtrip_ExclusiveAddressUse_GetEqualsSet() | |
| [OuterLoop] // TODO: Issue #11345 | ||
| [Fact] | ||
| [PlatformSpecific(PlatformID.AnyUnix)] | ||
| public void ExclusiveAddressUse_NotSupported() | ||
| public void ExclusiveAddressUse_Set_False_NotSupported() | ||
| { | ||
| using (TcpClient client = new TcpClient()) | ||
| { | ||
| Assert.Throws<SocketException>(() => client.ExclusiveAddressUse); | ||
| Assert.Throws<SocketException>(() => | ||
| { | ||
| client.ExclusiveAddressUse = true; | ||
| client.ExclusiveAddressUse = false; | ||
| }); | ||
| } | ||
| } | ||
|
|
||
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.
As pointed out in the design discussion, this could be confusing: what if on *NIX I only want to set
SO_REUSEPORTbut notSO_REUSEADDR?Is that a a valid *NIX scenario? If yes, is there a way to achieve that somehow?
Uh oh!
There was an error while loading. Please reload this page.
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.
I'm not sure if that's a valid scenario. It's not one anyone's asking for right now. 😃
The premise of this change is that we have an already-established semantic for
ReuseAddressin managed code, and there's more value in this semantic being portable than having the managed socket option names map 1:1 with native socket options. I agree that breaking that 1:1 mapping may be confusing for folks who are specifically targeting Unix socket options, but keeping the 1:1 mapping is also confusing, to those who are trying to get consistent behavior across platforms.Uh oh!
There was an error while loading. Please reload this page.
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.
Well I've done a bit of research and found that SO_REUSEPORT seems to be a bit exotic yet useful in extreme load scenarios (i.e. at Google: https://lwn.net/Articles/542629/).
My guess is that no one would ask for this in the near future, since it hasn't been available in .NET Framework so far. However can it hurt to add an option to allow to possibly tweak high performance socket code in special conditions, when you know you're running on *NIX?
EDIT: Here is the original motivation for this feature (source: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c617f398edd4db2b8567a28e899a88f8f574798d):
Nginx uses it as well: https://www.nginx.com/blog/socket-sharding-nginx-release-1-9-1/
And Apache does so too: https://httpd.apache.org/docs/trunk/en/mod/mpm_common.html#listencoresbucketsratio
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.
Thanks @christianhuening for the investigation!
Maybe properly supporting this and documenting/adding a new property/method would be a better long-term plan.
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.
Once Socket has its public Handle property added back, this could be done via a P/Invoke that's only invoked when on Unix. That could serve as a stop-gap until such time as it was determined to be important to add .NET APIs for.