Patch for adding SO_REUSEPORT socket option

[Lu, Yingqi]

Michael,

Thank you for confirming this. I will drop test/java/net/Socket/setReusePort/Basic.java as well since that was added for testing the setReusePort and getReusePort ☺

Thanks,
Lucy

From: Michael McMahon [mailto:michael.x.mcmahon at oracle.com]
Sent: Monday, December 07, 2015 9:13 AM
To: Lu, Yingqi <yingqi.lu at intel.com>; Alan Bateman <Alan.Bateman at oracle.com>
Cc: Kaczmarek, Eric <eric.kaczmarek at intel.com>; Kharbas, Kishor <kishor.kharbas at intel.com>; net-dev at openjdk.java.net
Subject: Re: Patch for adding SO_REUSEPORT socket option

Yes, since the functionality is basically duplicated across the two mechanisms
and since setOption/getOption already exists, the only new part of the API
is the SO_REUSEPORT option itself in StandardSocketOptions.

- Michael

On 07/12/15 17:04, Lu, Yingqi wrote:
Hi Alan and Michael,

Thank you very much for your detailed feedback. Before I start modify the code, I want to make sure I understand correctly. Regarding the following comment, do you mean that we should remove the setReusePort and getReusePort methods? If yes, we can easily remove them, and keep all the setOption/getOption methods the same way for SO_REUSEPORT.
I agree with Michael, we should decide whether it make sense to add specific methods to Socket/ServerSocket/DatagramSocket. They aren't strictly needed and given that the socket option is essentially optional then setOption/getOption should be fine.

I will work on this today. Hopefully can send out an update tomorrow.

Thanks,
Lucy

From: Alan Bateman [mailto:Alan.Bateman at oracle.com]
Sent: Monday, December 07, 2015 7:04 AM
To: Lu, Yingqi <yingqi.lu at intel.com><mailto:yingqi.lu at intel.com>
Cc: net-dev at openjdk.java.net<mailto:net-dev at openjdk.java.net>; Kharbas, Kishor <kishor.kharbas at intel.com><mailto:kishor.kharbas at intel.com>; Kaczmarek, Eric <eric.kaczmarek at intel.com><mailto:eric.kaczmarek at intel.com>
Subject: Re: Patch for adding SO_REUSEPORT socket option


On 02/12/2015 17:08, Lu, Yingqi wrote:
Hi All,

A quick check here. Does anyone get a chance to try the most recent patch? Any feedback and comments?

Thanks,
Lucy

I looked through the latest webrev (webrev.05) and it's looking quite good.

I agree with Michael, we should decide whether it make sense to add specific methods to Socket/ServerSocket/DatagramSocket. They aren't strictly needed and given that the socket option is essentially optional then setOption/getOption should be fine.

The updated wording in StandardSocketOptions looks okay but one suggestion is to include the word "usually" as it can't mandate the behavior on all platforms where it is supported - for example "the socket option will usually allow ...". A minor point here is that they needs @since 9.

The updates to the NIO implementation classes mostly look okay, except sun.nio.ch.Net where I have a number of comments:

1. In Net.java then you'll see where it caches several capabilities, that could be used to cache whether this socket option is supported or not, no need to do it in native code (Net.c).

2. I see reuseportSupported() is public so that it can be in several places (including code in java.net) but would be nice to avoid that.

3. For #1 and #3 then maybe the simplest is to add a native function in libnet like we have for IPv6 and that would allow the net and nio code to use the same implementation that is consistent with the existing code.

4. Minor comment on Net.c is that you use UNUSED whereas the existing code uses "this".

I wonder if it might be simpler to leave the debugger agent out of this, I don't see a big reason to change it.

For the tests then it would be good to avoid using sun.net.ch.Net.reuseportSupported(). Instead I think it should check that supportedOptions returns the right value for the platform.

I assume tests such as MulticastSendReceiveTests don't need to set this option.

-Alan.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/net-dev/attachments/20151207/808b8b97/attachment-0001.html>