Hi Volker,

Thanks very much for letting me know. I actually took the related regression 
tests and they all passed. I think you tested on an old kernel which does not 
have SO_REUSEPORT enabled. In this case, it should not set the flag.

Let me double check. I will get back to you as soon as I can.

Thanks,
Lucy

-----Original Message-----
From: Volker Simonis [mailto:volker.simo...@gmail.com] 
Sent: Wednesday, November 25, 2015 10:46 AM
To: Lu, Yingqi <yingqi...@intel.com>
Cc: Michael McMahon <michael.x.mcma...@oracle.com>; Alan Bateman 
<alan.bate...@oracle.com>; Kharbas, Kishor <kishor.khar...@intel.com>; 
net-dev@openjdk.java.net; Kaczmarek, Eric <eric.kaczma...@intel.com>
Subject: Re: Patch for adding SO_REUSEPORT socket option

Hi Lucy,

I took a brief look at your changes but there seems to be something not right. 
I can't understand for example why you unconditionally try to set SO_REUSEPORT 
on all sockets in Java_sun_nio_ch_Net_socket0() ?

Also which your changes applied, simple regression tests like 
test/java/net/SocketOption/OptionsTest.java start to fail even on
Linux/x86_64:

java.net.SocketException: Protocol not available
        at java.net.PlainDatagramSocketImpl.socketSetOption0(Native Method)
        at 
java.net.PlainDatagramSocketImpl.socketSetOption(PlainDatagramSocketImpl.java:85)
        at 
java.net.AbstractPlainDatagramSocketImpl.setOption(AbstractPlainDatagramSocketImpl.java:314)
        at java.net.DatagramSocket.setReusePort(DatagramSocket.java:1145)
        at java.net.MulticastSocket.<init>(MulticastSocket.java:180)
        at java.net.MulticastSocket.<init>(MulticastSocket.java:142)
        at OptionsTest.doMcSocketTests(OptionsTest.java:131)
        at OptionsTest.main(OptionsTest.java:247)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:520)
        at 
com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:92)
        at java.lang.Thread.run(Thread.java:747)


Can you please make sure that your changes at least don't break the regression 
tests?

Thank you and best regards,
Volker

On Tue, Nov 24, 2015 at 6:04 PM, Lu, Yingqi <yingqi...@intel.com> wrote:
> Hi Michael/Alan/Volker,
>
> Following your suggestions, here is the most recent version (Version 
> 4) of the patch. 
> http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.04/
>
> In this version, we have done following changes:
>
> 1. Add reuseportSupported() method in sun.nio.ch.Net and its implementation 
> in Net.c. Only add SO_REUSEPORT to the option set when it is supported. In 
> all the tests, we use supportedOptions method to test if SO_REUSEPORT is 
> supported or not.
>
> 2. We dropped NetworkChannels from the Javadoc. We removed Linux specific 
> wordings in Javadoc for SO_REUSEPORT.
>
> 3. We expand the feature to all UNIX based OSes. However, we do not have all 
> the OSes to test. Please test and let us know if there is anything missing in 
> either compilation or run time.
>
> Please review the patch and let us know your feedback. Thank you very much 
> for your help!
>
> Thanks,
> Lucy
>
> -----Original Message-----
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of 
> Michael McMahon
> Sent: Monday, November 23, 2015 2:54 AM
> To: Volker Simonis <volker.simo...@gmail.com>; Alan Bateman 
> <alan.bate...@oracle.com>
> Cc: Kharbas, Kishor <kishor.khar...@intel.com>; 
> net-dev@openjdk.java.net
> Subject: Re: Patch for adding SO_REUSEPORT socket option
>
> I agree we should enable the option on all platforms.
> We can add the code to do that and run the tests.
>
> On the existing use of SO_REUSEPORT on AIX and Mac it appears that is set to 
> emulate expected behavior on other platforms when SO_REUSEADDR is set for 
> datagram sockets.
> The expectation is that ports can be reused for datagram sockets and the JCK 
> tests this. So, I guess we have to leave this behavior by default, except if 
> SO_REUSEPORT is explicitly disabled maybe. Though this code hasn't been 
> forward ported to JDK 9 yet.
>
> For reference, SO_REUSEPORT on Linux is documented here 
> http://man7.org/linux/man-pages/man7/socket.7.html
>
> - Michael
>
> On 23/11/15 09:13, Volker Simonis wrote:
>> Hi Lucy,
>>
>> in general I support the addition of SO_REUSEPORT to the set of 
>> standard socket options. However for me the problem is not that this 
>> new option is not supported on all platforms, but instead that it has 
>> such different semantics on different platforms. If you look at the 
>> code, you'll see that we already implicitly set SO_REUSEPORT on Mac 
>> and AIX for datagram sockets for which we set SO_REUSEADDR. So maybe 
>> we have to rethink this, once  SO_REUSEPORT becomes available as a 
>> standard socket option.
>>
>> I like the new wording you've posted for JavaDoc of SO_REUSEPORT, but 
>> I think the sentence:
>>
>> * Although SO_REUSEADDR option already enables similar
>> * functionality, SO_REUSEPORT prevents port hijacking and
>> * distributes the involving datagrams evenly across all of the
>> * receiving threads.
>>
>> refers to a Linux-specific implementation detail which shouldn't be 
>> mentioned in the general documentation. You already have the sentence 
>> "The exact semantics of this socket option are socket type and system 
>> dependent" which should let everybody think twice before using this 
>> option. I'm also not sure about the link to the Linux article but I 
>> again think it is inappropriate in a general API documentation 
>> (otherwise we would have to add links for every platform which 
>> supports SO_REUSEPORT).
>>
>> As far as I can see (and please correct me if I'm wrong) you actually 
>> only add the new option for Linux platforms. But this socket option 
>> is also supported on Solaris (>= 11), MacOS X, AIX. So could you 
>> please enable it on the other platforms as well.
>>
>> Finally I want to mention the good stackoverflow article at 
>> http://stackoverflow.com/questions/14388706/socket-options-so-reusead
>> d r-and-so-reuseport-how-do-they-differ-do-they-mean-t
>> which covers the topic SO_REUSEADDR vs. SO_REUSEPORT quite well. And 
>> I've collected the man-page entries for SO_REUSEADDR and SO_REUSEPORT 
>> for the systems I have  (unfortunately, I couldn't find an updated 
>> Linux man-page which mentions SO_REUSEPORT):
>>
>> Linux
>> =====
>>
>>         SO_REUSEADDR
>>                Indicates that the rules used in validating addresses
>>                supplied in a bind(2) call should allow reuse of local
>>                addresses.  For AF_INET sockets this means that a socket
>>                may bind, except when there is an active listening
>>                socket bound to the address.  When the listening socket
>>                is bound to INADDR_ANY with a specific port then it is
>>                not possi- ble to bind to this port for any local
>>                address.  Argument is an integer boolean flag.
>>
>>         Linux will only allow port reuse with the SO_REUSEADDR option
>>         when this option was set both in the previous program that
>>         performed a bind(2) to the port and in the program that wants
>>         to reuse the port.  This differs from some implementations
>>         (e.g., FreeBSD) where only the later program needs to set the
>>         SO_REUSEADDR option.  Typically this difference is invisi- ble,
>>         since, for example, a server program is designed to always set
>>         this option.
>>
>> MacOS X
>> =======
>>             SO_REUSEADDR    enables local address reuse
>>             SO_REUSEPORT    enables duplicate address and port bindings
>>
>>       SO_REUSEADDR indicates that the rules used in validating
>>       addresses supplied in a bind(2) call should allow reuse of local
>>       addresses.
>>
>>       SO_REUSEPORT allows completely duplicate bindings by multiple
>>       processes if they all set SO_REUSEPORT before bind- ing the port.
>>       This option permits multiple instances of a program to each
>>       receive UDP/IP multicast or broadcast datagrams destined for the
>>       bound port.
>>
>> Solaris
>> =======
>>
>>       SO_REUSEADDR          enable/disable local address reuse
>>
>>
>>       SO_REUSEPORT          enable/disable local  port  reuse  for
>>                             PF_INET/PF_INET6 socket
>>
>>       The SO_REUSEADDR/SO_REUSEPORT options indi- cate that the rules
>>       used in validating addresses and ports supplied in a
>>       bind(3SOCKET) call should allow reuse of local addresses or
>>       ports.
>>
>> AIX
>> ===
>>
>>               SO_REUSEADDR
>>                     Specifies that the rules used in validating
>>                     addresses supplied by a bind subroutine should
>>                     allow reuse of a local port. A particular IP
>>                     address can only be bound once to the same
>>                     port. This option enables or disables reuse of
>>                     local ports.
>>
>>                     SO_REUSEADDR allows an application to explicitly
>>                     deny subsequent bind subroutine to the port/address
>>                     of the socket with SO_REUSEADDR set. This allows an
>>                     application to block other applications from
>>                     binding with the bind subroutine.
>>
>>                SO_REUSEPORT
>>                     Specifies that the rules used in validating
>>                     addresses supplied by a bind subroutine should
>>                     allow reuse of a local port/address
>>                     combination. Each binding of the port/address
>>                     combination must specify the SO_REUSEPORT socket
>>                     option. This option enables or disables the reuse
>>                     of local port/address combinations.
>>
>> HPUX
>> ====
>>
>>             SO_REUSEADDR
>>                (int; boolean; AF_INET sockets only) If enabled, allows
>>                a local address to be reused in subsequent calls to
>>                bind().  Default: disallowed.
>>
>>             SO_REUSEPORT
>>                (int; boolean; AF_INET sockets only) If enabled, allows
>>                a local address and port to be reused in subsequent
>>                calls to bind().  Default: disallowed.
>>
>>        Setting the SO_REUSEADDR option allows the local socket address
>>        to be reused in subsequent calls to bind().  This permits
>>        multiple SOCK_STREAM sockets to be bound to the same local
>>        address, as long as all existing sockets with the desired local
>>        address are in a connected state before bind() is called for a
>>        new socket.  For SOCK_DGRAM sockets, SO_REUSEADDR allows
>>        multiple sockets to receive UDP multicast datagrams addressed to
>>        the bound port number.  For all SOCK_DGRAM sockets bound to the
>>        same local address, SO_REUSEADDR must be set before calling
>>        bind().
>>
>>        Setting the SO_REUSEPORT option allows multiple SOCK_DGRAM
>>        sockets to share the same address and port.  Each one of those
>>        sockets, including the first one to use that port, must specify
>>        this option before calling bind().
>>
>> Regards,
>> Volker
>>
>>
>> On Mon, Nov 23, 2015 at 9:00 AM, Alan Bateman <alan.bate...@oracle.com> 
>> wrote:
>>>
>>> On 23/11/2015 04:12, Lu, Yingqi wrote:
>>>
>>> Hi Alan,
>>>
>>>
>>>
>>> One more question please J I want to make sure I understand 
>>> correctly on your following suggestion. In order to use 
>>> supportedOptions method to test SO_REUSEPORT, I will need to first 
>>> write a native function to check if SO_REUSEPORT is supported. Then, 
>>> in the defaultOptions method, I do a conditional add for 
>>> StandardSocketOptions.SO_REUSEPORT
>>> if it is supported on the platform? Is this a preferred way to implement? 
>>> Please let me know!
>>>
>>>
>>> Yes as supportedOptions() shouldn't return SO_REUSEPORT in the set 
>>> when it's not supported. It might be simplest to put that code in 
>>> sun.nio.ch.Net, maybe isReusePortSupported or some such method. In 
>>> the implementation
>>> (Net.c) then you can return true or false depending on the platform 
>>> and maybe kernel version.
>>>
>>> -Alan
>

Reply via email to