adding rsockets support into JDK

[Lu, Yingqi]

Hi Alan,

I updated the patch version 15. In the update, I addressed your following comments first as they are part of the APIs. Brain helped generate a specdiff and attached it to the CSR at https://bugs.openjdk.java.net/browse/JDK-8205186. The CSR is updated to “proposed” status. In addition, Brain helped trunk the change for “Add protected constructor java.net.ServerSocket(SocketImpl)” (https://bugs.openjdk.java.net/browse/JDK-8213217). This was a blocking issue for this work.

The RdmaSocketOptions API mostly looks okay. The only thing that seems to be missed is some hints as to the valid ranges for the RMDA send/receive queue size. Does setOption(RMDA_RQSIZE, 0) or setOption(RMDA_RQSIZE, -1) make sense for example. It's okay to specify the upper limit as implementation specific, it's the 0 and negative values that aren't clear.

In RdmaSocketOptions class description then I assume "The rsocket implementaiton is Linux only" can be dropped or converted to an API note.

I am working on the rest of your comments and will post version 16 of patch early next week.

Thanks,
Lucy

From: Alan Bateman [mailto:Alan.Bateman at oracle.com]
Sent: Sunday, October 28, 2018 2:10 AM
To: Lu, Yingqi <yingqi.lu at intel.com>; Brian Burkhalter <brian.burkhalter at oracle.com>
Cc: nio-dev at openjdk.java.net; 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 26/10/2018 22:48, Lu, Yingqi wrote:
Hi Brian,

Here is the version 15 of the patch: http://cr.openjdk.java.net/~ylu/8195160.15<http://cr.openjdk.java.net/%7Eylu/8195160.15>

I went through the current patch.

One thing that isn't quite right is that Socket/ServerSocket have been updated so that the supportedOptions API returns the RDMA options. This is something that been pushed down to the implementation of the SocketImpl. When supportedOptions is invoked it will delegate to the socket impl and that should add the RDMA socket options.

I also see that SocketImpl setSocket/setServerSocket have been updated to special case the RDMA SocketImp. This shouldn't be needed so I think we need to understand the setSocket/setServerSocket usage to see how this can be restructured. If we can fix that then the RDMA SocketImpl should be just an implementation and the need for the shared secrets mechanism should go away.

sun.net.ch.Net defines static methods, it should be final. RdmaNet work the same way, it doesn't need to extend Net.

The RdmaSocketOptions API mostly looks okay. The only thing that seems to be missed is some hints as to the valid ranges for the RMDA send/receive queue size. Does setOption(RMDA_RQSIZE, 0) or setOption(RMDA_RQSIZE, -1) make sense for example. It's okay to specify the upper limit as implementation specific, it's the 0 and negative values that aren't clear.

In RdmaSocketOptions class description then I assume "The rsocket implementaiton is Linux only" can be dropped or converted to an API note.

One implementation nit is that it looks like loadRdmaFuncs will be called multiple times. I assume this should have a guard so that it only executes once.

We touched on IPv6 addresses in some of the early discussions here - can you summarize what the support is?

That's all I have for now.

-Alan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20181103/463e5be7/attachment-0001.html>