Patch for adding SO_REUSEPORT socket option

[Lu, Yingqi]

Hi Alan,

Happy New Year!!

Thanks very much for your time reviewing the patch. I will make the changes regarding your comments and send them back here soon.

Thanks,
Lucy

-----Original Message-----
From: Alan Bateman [mailto:Alan.Bateman at oracle.com] 
Sent: Friday, January 01, 2016 3:18 AM
To: Lu, Yingqi <yingqi.lu at intel.com>
Cc: Viswanathan, Sandhya <sandhya.viswanathan at intel.com>; net-dev at openjdk.java.net; Kharbas, Kishor <kishor.kharbas at intel.com>; Aundhe, Shirish <shirish.aundhe at intel.com>; Kaczmarek, Eric <eric.kaczmarek at intel.com>
Subject: Re: Patch for adding SO_REUSEPORT socket option

On 21/12/2015 17:53, Lu, Yingqi wrote:
> Hi All,
>
> Sorry for the late reply. Here is the link to the most recent 
> reversion of the patch (version #6). 
> http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.06/
>
> In this version, we have done following modifications based on the feedback we received. Please review and let us know if there is anything missing. Thank you very much!
>
>
I went through the latest webrev and it is look quite good.

One thing that doesn't seem quite right is SocketImpl::isReusePortAvailable. Whether SO_REUSEPORT is supported or not depends on the concrete implementation of the SocketImpl. So I think this needs to be moved down to AbstractPlainSocketImpl.

In net_util_md.h then SO_REUSEPORT is defined as 0x0200 on "unknown" 
platforms. I wonder if it would be better to not set it and instead have reuseport_supported return JNI_FALSE when SO_REUSEPORT is not defined.

StandardSocketOptions.SO_REUSEPORT is referenced in shared code
(sun/nio/ch/*Impl.java) so shouldn't it be defined to something on all platforms? I ask because genSocketOptionRegistry.c open emits a definition on non-Windows builds. Maybe it needs to emit it with a value of 0 on Windows. Also like net_util_md.c then maybe it should be emitted as 0 rather than 0x0200 when the value is not known.

The javadoc looks good, a minor nit is that SocketOptions.SO_REUSEPORT needs @since 9.

A few other minor nits in passing:

-
src/java.base/share/classes/java/net/AbstractPlainDatagramSocketImpl.java indentation doesn't look right.

- sun.nio.ch.Net.checkedReusePort - shouldn't be a need to initialize the volatile boolean to its default value (there is a patch in progress to remove many of these).

- SdpSupport.convert0 - I assume this should only use getsockopt when the socket option is supported. I think this means it will need #ifdef SO_REUSEPORT.

- A place uses have "//SO.." when "// SO..." would be more consistent.

I think that's it.

-Alan