Yes, it should work! I already located the issues. Good catch! I will submit an update as soon as possible.
Thanks, Lucy From: Volker Simonis [mailto:volker.simo...@gmail.com] Sent: Wednesday, November 25, 2015 12:17 PM 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 Yes, I indeed tested on an old kernel - but that should still work after your change! On Wednesday, November 25, 2015, Lu, Yingqi <yingqi...@intel.com<mailto:yingqi...@intel.com>> wrote: 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<javascript:;>] Sent: Wednesday, November 25, 2015 10:46 AM To: Lu, Yingqi <yingqi...@intel.com<javascript:;>> Cc: Michael McMahon <michael.x.mcma...@oracle.com<javascript:;>>; Alan Bateman <alan.bate...@oracle.com<javascript:;>>; Kharbas, Kishor <kishor.khar...@intel.com<javascript:;>>; net-dev@openjdk.java.net<javascript:;>; Kaczmarek, Eric <eric.kaczma...@intel.com<javascript:;>> 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<javascript:;>> 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<http://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<javascript:;>] On > Behalf Of > Michael McMahon > Sent: Monday, November 23, 2015 2:54 AM > To: Volker Simonis <volker.simo...@gmail.com<javascript:;>>; Alan Bateman > <alan.bate...@oracle.com<javascript:;>> > Cc: Kharbas, Kishor <kishor.khar...@intel.com<javascript:;>>; > net-dev@openjdk.java.net<javascript:;> > 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<javascript:;>> 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<http://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 >
