adding rsockets support into JDK

[Alan Bateman]

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/20181028/d53563df/attachment.html>