adding rsockets support into JDK

[Lu, Yingqi]

Hi All,

Here is the version #4 of the patch
http://cr.openjdk.java.net/~ylu/8195160.04/

In this version, I have changed:

1. Remove jdk.net.RdmaSocketImplFactory.
2. Remove the inheritance from SocketChannelImpl/ServerSocketChannelImpl, replace it with SocketChannel/ServerSocketChannel.
3. Remove unnecessary setters from SocketImpl.
4. Override accept method to the ServerSocket returned by jdk.net.openServerSocket.
5. Add a small comment to PollSelectorImpl.poll method
6. Clean up a little in Lib-jdk.net.gmk

I am still working on creating all the test cases. Do we need to replicate all the tests from both Socket/ServerSocket and SocketChannel/ServerSocketChannel, or we only need to pick up a subset of the cases?

Please review the patch and let me know your feedback and comments.

Thanks,
Lucy

>-----Original Message-----
>From: nio-dev [mailto:nio-dev-bounces at openjdk.java.net] On Behalf Of Lu,
>Yingqi
>Sent: Friday, May 11, 2018 9:38 AM
>To: Alan Bateman <Alan.Bateman at oracle.com>; nio-dev at openjdk.java.net
>Cc: Viswanathan, Sandhya <sandhya.viswanathan at intel.com>; Aundhe, Shirish
><shirish.aundhe at intel.com>; Kaczmarek, Eric <eric.kaczmarek at intel.com>
>Subject: RE: adding rsockets support into JDK
>
>Hi Alan,
>
>Thank you very much for your feedback!
>
>I agree that jdk.net.RdmaSocketImplFactory is not necessary anymore. I will
>remove it in next version.
>
>I will remove the inheritance from SocketChannelImpl/ServerSocketChannelImpl,
>replace them with SocketChannel/ServerSocketChannel. My initial thought was to
>avoid the code duplication, but I totally agree with you on your concern for
>future maintenance.
>
>I will also make other changes following your suggestions and start to add test
>cases. I will start from the examples of regular sockets and socket channels and
>add ones specific to RDMA.
>
>Thank you!!
>Lucy
>
>>-----Original Message-----
>>From: Alan Bateman [mailto:Alan.Bateman at oracle.com]
>>Sent: Friday, May 11, 2018 5:18 AM
>>To: Lu, Yingqi <yingqi.lu at intel.com>; nio-dev at openjdk.java.net
>>Cc: Viswanathan, Sandhya <sandhya.viswanathan at intel.com>; Aundhe,
>>Shirish <shirish.aundhe at intel.com>; Kaczmarek, Eric
>><eric.kaczmarek at intel.com>
>>Subject: Re: adding rsockets support into JDK
>>
>>On 04/05/2018 17:42, Lu, Yingqi wrote:
>>> Hi All,
>>>
>>> The version 3 of the patch is available now at
>>> http://cr.openjdk.java.net/~ylu/8195160.03/
>>>
>>> In this version, I have changed following items:
>>>
>>> 1. Added Sockets.openRdmaSelector() function
>>>
>>> 2. Replaced "changing system-wide SocketImplFactory" with returning
>>Socket(impl). I did the similar change to ServerSocket too. I also
>>moved RdmaSocketImplFactory from jdk.net to rdma.ch.
>>>
>>> 3. Reduced number the factory methods to 2, openRdmaSocket() and
>>openRdmaServerSocket().
>>>
>>> 4. Removed the changes to InetAddress.
>>>
>>> 5. Removed the methods changed to public in Socket/ServerSocket
>>>
>>> Please let me know your feedback and comments.
>>>
>>I looked through the current version.
>>
>>The API additions to jdk.net.Sockets API look good. The javadoc will
>>need to be fleshed out of course.
>>
>>Did you mean to include jdk.net.RdmaSocketImplFactory in the API? The
>>factory methods can create the RdmaSocketImpl directly so it may not be
>>needed.
>>
>>jdk.net.RdmaSocketOptions looks right. It will of course need javadoc
>>and some refactoring so that hardcoded values aren't in shared code.
>>
>>There are a few changes to the java.net API that will need consideration.
>>Having a protected constructor that takes a SocketImpl looks reasonable
>>on first glance it is consistent with the protected constructor in Socket.
>>
>>The addition of protected setters to SocketImpl may be okay too
>>although not strictly needed as the fd and address fields are protected
>>so they can be set by implementations already.
>>
>>You've changed ServerSocket accept to support the RdmaSocketImpl but I
>>don't think you need that. Instead, the ServerSocket returned by
>>openServerSocket can override the accept method. I think this means you
>>can drop the qualified export of jdk.net to java.base too.
>>
>>I'm a bit uncomfortable with extending SocketChannelImpl and
>>ServerSocketChannelImpl as proposed because it tightly ties the RMDA
>>implementation in jdk.net. It means we can't change anything in say
>>SocketChannelImpl without breaking the RDMA implementation. Given that
>>the RMDA implementations override almost everything then maybe they
>>should just extend SocketChannel directly and we'll need to the
>>duplication as needed.
>>
>>The extending of PollSelectorImpl is probably okay, just needs a
>>comment to poll method to explain what it is protected.
>>
>>libextnet also builds on Solaris so the updates to Lib-jdk.net.gmk will
>>need a few adjustments to ensure that it continues to build (you'll
>>need to add - lrdmacm to LIBS_linux rather than LIBS for example).
>>
>>Are you planning to develop tests for this? We also have the issue how
>>this is going to be tested and maintained over the long term.
>>
>>-Alan