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 <[email protected]> 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:[email protected]] On Behalf Of Michael
> McMahon
> Sent: Monday, November 23, 2015 2:54 AM
> To: Volker Simonis <[email protected]>; Alan Bateman
> <[email protected]>
> Cc: Kharbas, Kishor <[email protected]>; [email protected]
> 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-reuseadd
>> 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 <[email protected]>
>> 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
>
