Oh, I got it now. You want SO_REUSEPORT to be disabled by default for both ServerSocketChannelImpl and SocketChannelImpl.
I am actually OK either way. Alan/Michael, what is your take on this? If we all think "default off" is the correct/better way to go, we can just simply remove that block and leave it to the user. Please let me know! Thanks, Lucy -----Original Message----- From: Volker Simonis [mailto:volker.simo...@gmail.com] Sent: Monday, December 07, 2015 10:38 AM To: Lu, Yingqi <yingqi...@intel.com> Cc: Alan Bateman <alan.bate...@oracle.com>; Michael McMahon <michael.x.mcma...@oracle.com>; Kaczmarek, Eric <eric.kaczma...@intel.com>; net-dev@openjdk.java.net; Kharbas, Kishor <kishor.khar...@intel.com> Subject: Re: Patch for adding SO_REUSEPORT socket option On Mon, Dec 7, 2015 at 7:02 PM, Lu, Yingqi <yingqi...@intel.com> wrote: > Hi Volker, > > Thank you very much for your feedback. > > 1. For the following code, I will do it as platform specific as you suggested. > 37 /* Defines SO_REUSEPORT */ > 38 #ifndef SO_REUSEPORT > 39 #define SO_REUSEPORT 15 > 40 #endif > > 2. I will remove the setReusePort and getReusePort methods from > Socket/ServerSocket/DiagramSocket. > > 3. I will remove SO_REUSEPORT from following, since they do not really have > performance impact here. > src/java.management/share/classes/sun/management/jdp > src/jdk.jdwp.agent/unix/native/libdt_socket > > 4. I will follow the example of IPv6 check for reuseportSupported(), adding > caching (check once) as well as relocate the native check to libnet. > > 5. Regarding to your following comment, I think set SO_REUSEPORT is not for > every socket. The "support or not check" has already been done before going > to this native function. If not supported, it won’t go here. Please let me > know if my understanding is correct. > - why do we unconditionally set SO_REUSEPORT for every socket created in > 'Java_sun_nio_ch_Net_socket0()'? I don't think that's right. 'Java_sun_nio_ch_Net_socket0()' has an argument 'reuse'. This argument is checked and only if it is true, we set the SO_REUSEADDR option. But there's no such check for SO_REUSEPORT. We unconditionally set it always, no difference if 'reuse' was true or false (so we also set it for every SocketChannelImpl for example, not only for ServerSocketChannelImpl as with SO_REUSEADDR). Notice that I don't want you to put the new code inside the "if (reuse)" block. I think we simply shouldn't implicitly set SO_REUSEPORT. We should leave that up to the user, if he thinks it is necessary. Or am I missing something here? > > Thanks, > Lucy > > -----Original Message----- > From: Volker Simonis [mailto:volker.simo...@gmail.com] > Sent: Monday, December 07, 2015 9:51 AM > To: Lu, Yingqi <yingqi...@intel.com>; Alan Bateman > <alan.bate...@oracle.com>; Michael McMahon > <michael.x.mcma...@oracle.com> > Cc: Kaczmarek, Eric <eric.kaczma...@intel.com>; > net-dev@openjdk.java.net; Kharbas, Kishor <kishor.khar...@intel.com> > Subject: Re: Patch for adding SO_REUSEPORT socket option > > Hi Lucy, > > thanks for updating the change and sorry for the late review. It looks pretty > good now. Please find my comments below: > > genSocketOptionRegistry.c > net_util_md.h > nio_util.h > socket_md.h > ==================== > > I'm a little concerned about the following code: > > 37 /* Defines SO_REUSEPORT */ > 38 #ifndef SO_REUSEPORT > 39 #define SO_REUSEPORT 15 > 40 #endif > > because the value of SO_REUSPORT is platform dependent. It's 15 on Linux, but > 0x100e on Solaris and 0x0200 on MacOS X, AIX and HPUX. So we should make this > platform dependent otherwise this code will break if the JDK will be compiled > lets say on Solaris 10 which doesn't have SO_REUSEPORT but later runs on > Solaris 11 where it will set the socket option '15' which is undefined there. > > Net.c > ==== > - you can call initialize 'reuseportsupported' by calling > 'reuseportSupported0()' from Java_sun_nio_ch_Net_initIDs() which is called > exactly once during class initialization (so no need for the > 'reuseportsupported_set' guard). In Java_sun_nio_ch_Net_reuseportSupported()' > you can then simply return 'reuseportsupported'. > > - why do we unconditionally set SO_REUSEPORT for every socket created in > 'Java_sun_nio_ch_Net_socket0()'? I don't think that's right. > > src/java.management/share/classes/sun/management/jdp > src/jdk.jdwp.agent/unix/native/libdt_socket > =============================== > - I don't think we should change these socket options. I think there's not > much to win but we can get problems on platforms where SO_REUSEPORT has a > different semantics. > > DatagramSocket.java > ================ > > - that's probably another clean-up change, but do we really still need to > support pre-1.4 DatagramSocketImpl? > > > And as always, the copyright of all the files you've touched has to be > updated to 2015 (or 2016 :) > > Regards, > Volker > > On Wed, Dec 2, 2015 at 6:08 PM, Lu, Yingqi <yingqi...@intel.com> wrote: >> Hi All, >> >> >> >> A quick check here. Does anyone get a chance to try the most recent patch? >> Any feedback and comments? >> >> >> >> Thanks, >> >> Lucy >> >> >> >> From: Lu, Yingqi >> Sent: Monday, November 30, 2015 11:25 PM >> To: Lu, Yingqi <yingqi...@intel.com>; Volker Simonis >> <volker.simo...@gmail.com> >> >> >> Cc: Kaczmarek, Eric <eric.kaczma...@intel.com>; >> net-dev@openjdk.java.net; Kharbas, Kishor <kishor.khar...@intel.com> >> Subject: RE: Patch for adding SO_REUSEPORT socket option >> >> >> >> Hi All, >> >> >> >> Here is the most recent version of the patch. >> http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.05/ >> >> >> >> We already tested patch on Linux 3.4.110 kernel and 3.18 kernel with >> regression tests from java.net and java.nio, and a simple program for >> reuseport. We also tested the same thing on Windows platform. Results >> are as expected. >> >> >> >> For the simple program, we tested Sockets.setOption, >> setOption/getOption and set/getReusePort methods from java.net. >> >> >> >> Please review this version and let us know your feedback. >> >> >> >> Thanks very much for your help! >> >> Lucy >> >> >> >> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of >> Lu, Yingqi >> Sent: Wednesday, November 25, 2015 5:12 PM >> To: Volker Simonis <volker.simo...@gmail.com> >> Cc: Kaczmarek, Eric <eric.kaczma...@intel.com>; >> net-dev@openjdk.java.net; Kharbas, Kishor <kishor.khar...@intel.com> >> Subject: RE: Patch for adding SO_REUSEPORT socket option >> >> >> >> Here is an update. >> >> >> >> Changes are already completed locally. All the tests passed on an old >> Linux kernel 3.4.110 which does not have SO_REUSEPORT. Same tests are >> done on Linux 4.2 kernel before. >> >> >> >> Here are the quick information on the current implementation: >> >> >> >> 1. For multicast socket, SO_REUSEPORT will be set by default if >> supported. We use Net.reuseportSupported method to check before >> calling setReusePort(). If not supported, will silently continue without >> setting it. >> >> 2. For socket, serversocket and datagramsocket, we check if >> SO_REUSEPORT is supported before calling setOption/getOption and >> setReusePort/ getReusePort methods. If not supported, UOE exception >> will be thrown. >> >> 3. We modified a bug in the OptionsTest.java file. >> >> >> >> We will test Windows environment to see if it behaves the expected way. >> However, we need help to test other OSes from the community J >> >> >> >> Due to the Thanksgiving holiday schedule, we will send updated webrev >> package on Monday. >> >> >> >> Thank you all for your help and happy thanksgiving! >> >> Lucy >> >> >> >> >> >> From: Lu, Yingqi >> Sent: Wednesday, November 25, 2015 1:23 PM >> To: 'Volker Simonis' <volker.simo...@gmail.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, 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> 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] >> 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-reuse >>>> a d 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 >>>
