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> 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 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, 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 > > >
