Patch for adding SO_REUSEPORT socket option

[Chris Hegarty]

> From: net-dev [mailto:net-dev-bounces at openjdk.java.net] On Behalf Of Lu, Yingqi
> Sent: Thursday, February 11, 2016 9:47 AM
> To: Alan Bateman <Alan.Bateman at oracle.com>; Volker Simonis <volker.simonis at gmail.com>; Michael McMahon <michael.x.mcmahon at oracle.com>
> Cc: Kaczmarek, Eric <eric.kaczmarek at intel.com>; Viswanathan, Sandhya <sandhya.viswanathan at intel.com>; Kharbas, Kishor <kishor.kharbas at intel.com>; Aundhe, Shirish <shirish.aundhe at intel.com>; net-dev at openjdk.java.net
> Subject: RE: Patch for adding SO_REUSEPORT socket option
>
> Hi Alan,
>
> Here is the most recent modification of the patch. The link is available here:
> http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.11/

I know this has gone through several rounds of review already.
I do not want to stall progress, I am reasonably happy with the
changes, but I have a few comments ( that can be addressed later,
if needed ).

  - MulticastSocket.java: I would add a javadoc link to
    {@linkplain StandardSocketOptions.SO_REUSEPORT SO_REUSEPORT}

  - Maybe the socket impl set/getOption(..) methods should check
    supportedOptions().contains(name) first.  There seems to be
    many places where SO_REUSEPORT is special-cased, that could
    be replaced with a general supported options check, no?

  - How useful is it to add SO_REUSEPORT to non-listening stream
    orientated sockets ?

-Chris.



> Please let us know if everything is all right.
>
> Again, thank you very much for your help!
>
> Thanks,
> Lucy
>
>
> -----Original Message-----
> From: Alan Bateman [mailto:Alan.Bateman at oracle.com]
> Sent: Wednesday, February 10, 2016 10:38 AM
> To: Lu, Yingqi <yingqi.lu at intel.com>; Volker Simonis <volker.simonis at gmail.com>; Michael McMahon <michael.x.mcmahon at oracle.com>
> Cc: net-dev at openjdk.java.net; Viswanathan, Sandhya <sandhya.viswanathan at intel.com>; 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 05/02/2016 17:27, Lu, Yingqi wrote:
>> Hi Alan,
>>
>> Here is the new modification of the patch. The link is available here:
>> http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.10/
>>
>> In this version, we make supportedOptions() in AbstractPlainSocketImpl and AbstractPlainDatagramSocketImpl return an immutable set of the socket options. In addition, we corrected couple formatting issues.
>>
>> Please let us know your feedback and comments.
>>
> I've looked through the latest revision. Just a couple of small things:
>
> In MulticastSocket then a small typo (in two places) where you have "SO_REUSPORT" instead of "SO_REUSEPORT". Also it links to
> setOption(int,Object) then I assume it should be setOption(SocketOption,Object).
>
> In AbstractPlainSocketImpl and AbstractPlainDatagramSocketImpl then supportedOptions looks technically correct but there is no need to create an unmodifiableSet on each call to supportedOptions. Instead you can simply do:
>
> Set<SocketOption<?>> options = socketOptions; if (options == null) {
>       if (isReusePortAvailable()) {
>           options = new HashSet<>();
>           options.addAll(super.supportedOptions());
>           options.add(StandardSocketOptions.SO_REUSEPORT);
>           options = Collections.unmodifiableSet(options);
>       } else {
>           options = super.supportedOptions();
>       }
>       socketOptions = options;
> }
> return options;
>
> I don't see any other issues at this time and I'm happy to sponsor and help you get this in.
>
> -Alan.
>
>
>