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.bate...@oracle.com] Sent: Monday, December 07, 2015 7:04 AM To: Lu, Yingqi <yingqi...@intel.com> Cc: net-dev@openjdk.java.net; Kharbas, Kishor <kishor.khar...@intel.com>; Kaczmarek, Eric <eric.kaczma...@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.
