Just wanted to report that from the AIX side everything looks clean
and ready to go.
Regards,
Volker
On Wed, Feb 17, 2016 at 5:56 PM, Seán Coffey <sean.cof...@oracle.com> wrote:
> One comment on the supportability side from me :
>
> java/net/AbstractPlainDatagramSocketImpl.java
>
>> + case SO_REUSEPORT:
>> + if (o == null || !(o instanceof Boolean)) {
>> + throw new SocketException("bad argument for
>> SO_REUSEPORT");
>> + }
>> + if
>> (!supportedOptions().contains(StandardSocketOptions.SO_REUSEPORT)) {
>> + throw new UnsupportedOperationException("unsupported
>> option");
>
> Can we improve the message ? e.g "SO_REUSEPORT is an unsupported option" -
> line occurs twice.
>
> Ditto for java/net/AbstractPlainSocketImpl.java and
> java/net/PlainDatagramSocketImpl.java and java/net/PlainSocketImpl.java
> (windows & unix variants)
>
>
> java/net/DualStackPlainDatagramSocketImpl.java,
> java/net/DualStackPlainSocketImpl.java, java/net/PlainSocketImpl.java,
> java/net/TwoStacksPlainDatagramSocketImpl.java and
> java/net/TwoStacksPlainSocketImpl.java also need tweaking on that message.
>
> Thanks,
> Sean.
>
>
> On 17/02/16 11:43, Chris Hegarty wrote:
>>
>>
>>> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Lu,
>>> Yingqi
>>> Sent: Thursday, February 11, 2016 9:47 AM
>>> To: Alan Bateman <alan.bate...@oracle.com>; Volker Simonis
>>> <volker.simo...@gmail.com>; Michael McMahon <michael.x.mcma...@oracle.com>
>>> Cc: Kaczmarek, Eric <eric.kaczma...@intel.com>; Viswanathan, Sandhya
>>> <sandhya.viswanat...@intel.com>; Kharbas, Kishor <kishor.khar...@intel.com>;
>>> Aundhe, Shirish <shirish.aun...@intel.com>; net-dev@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.bate...@oracle.com]
>>> Sent: Wednesday, February 10, 2016 10:38 AM
>>> To: Lu, Yingqi <yingqi...@intel.com>; Volker Simonis
>>> <volker.simo...@gmail.com>; Michael McMahon <michael.x.mcma...@oracle.com>
>>> Cc: net-dev@openjdk.java.net; Viswanathan, Sandhya
>>> <sandhya.viswanat...@intel.com>; Kharbas, Kishor <kishor.khar...@intel.com>;
>>> Aundhe, Shirish <shirish.aun...@intel.com>; Kaczmarek, Eric
>>> <eric.kaczma...@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.
>>>
>>>
>>>
>
