Hi Alan, Here is a new version of the patch: http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.08/
In this version, based on your comment, we have changed following items: 1. Remove the dependency of DatagramSocketImpl and MulticastSocket with AbstractPlainSocketImpl. 2. Made addReusePortOption() to be both thread safe and package private. Please let us know your feedback and comments. Thanks, Lucy -----Original Message----- From: Alan Bateman [mailto:alan.bate...@oracle.com] Sent: Friday, January 15, 2016 4:29 AM To: Lu, Yingqi <yingqi...@intel.com> Cc: net-dev@openjdk.java.net; Viswanathan, Sandhya <sandhya.viswanat...@intel.com>; Kharbas, Kishor <kishor.khar...@intel.com>; Aundhe, Shirish <shirish.aun...@intel.com>; Kaczmarek, Eric <eric.kaczma...@intel.com> Subject: Re: Patch for adding SO_REUSEPORT socket option On 14/01/2016 18:28, Lu, Yingqi wrote: > Hi Alan/Michael/Volker, > > Here is the link to the version 7 of the patch. > http://cr.openjdk.java.net/~mcberg/jdk/6432031/webrev.07/ > > Please note: after the code has been uploaded, I found out I forgot to remove > extra check of ENOPROTOOPT in the net_util_md.c. Since it took us couple days > to upload the code each time, I think it might be a good idea to mark this as > an open item for now. I will change it in next version for sure together with > anything you found in the current version. > > I think this looks quite good but there is still one architectural issue that doesn't look quite right to me. The issue is that DatagramSocketImpl and MulticastSocket shouldn't be dependent on the built-in AbstractPlainSocketImpl. If you develop your own DatagramSocketImpl implementation then you'll quickly see the issue. It looks like you have it right in SocketImpl, it's just DatagramSocketImpl and MulticastSocket where the dependency seems to exist. One other comment is that I don't think SocketImpl.addAdditionalOptions is ready to be exposed in the API (you've added it as a protected method as it will become part of the Java SE API). Clearly there is a need for impls to have a way to expand the set of socket options but it needs further consideration on what that API should be. The simplest is to just drop protected and make this a package-private method. One other point on addAdditionalOptions is that it mutates the set of socket options and so is not thread safe. We'll need to come up with a way to do this in a safe way. One final comment is on the Unix version of PlainSocketImpl and PlainDatagramSocketImpl. I'm wondering if these changes are really needed. This may be something Michael may have suggestions on. -Alan
