adding rsockets support into JDK

[Lu, Yingqi]

Hi Brian,

Good catches!

I quickly fixed A, B and D. Please check if they are correctly done.

For C, there is already a file named RdmaSocketDispatcher.java. Since LinuxRdmaSocketDispatcherImpl.java is under linux folder, I renamed it to RdmaSocketDispatcherImpl.java and fixed associated native function names. Please let me know if this is appropriate.

For E and F, I think those are still open items and the comments are left there intentionally. I will leave them to Chris ☺

Attached is a patch file as I do not have commit rights.

Thanks,
Lucy

From: nio-dev [mailto:nio-dev-bounces at openjdk.java.net] On Behalf Of Brian Burkhalter
Sent: Wednesday, January 30, 2019 5:47 PM
To: Chris Hegarty <chris.hegarty at oracle.com>
Cc: Viswanathan, Sandhya <sandhya.viswanathan at intel.com>; nio-dev at openjdk.java.net; Aundhe, Shirish <shirish.aundhe at intel.com>; Kaczmarek, Eric <eric.kaczmarek at intel.com>
Subject: Re: adding rsockets support into JDK

Some further minor observations below.

Thanks,

Brian

A) rdma_util_md.c:

 223 int rdma_supported() {
 224     int one = 1;
 225     int rv, s;
 226     s = rs_socket(PF_INET, SOCK_STREAM, 0);
 227     if (s < 0) {
 228         return JNI_FALSE;
 229     }
 230     return JNI_TRUE;
 231 }

Should there be “close(s);” before line 230 so as not to leave the FD open?

B) rdma_util_md.h:

RDMA_SendTo() is prototyped here but there is no implementation.

C) LinuxRdmaSocketDispatcherImpl:

Why not simply LinuxRdmaSocketDispatcher (without “Impl”) or even just RdmaSocketDispatcher?

D) LinuxRdmaSocketDispatcherImpl.c:

Contains commented out code.

E) test/IOExchanges:

Contains commented out “@Test”s but based on Chris’s message of Dec. 14 this is intentional:

 “For now, until Issue #2 is resolved, then just comment out the `@Test`
 from the twelve *_NBConn_* methods ( they currently fail, since they
 verify the yet-to-be-fixed Issue #2 ).”

F) test/NullBind:

Contains a TODO which I suppose is known as well.


On Jan 30, 2019, at 11:23 AM, Brian Burkhalter <brian.burkhalter at oracle.com<mailto:brian.burkhalter at oracle.com>> wrote:

Looks good. I think RdmaSocketOptions needs to be updated to match:

  40  * Defines socket options specific to RDMA-based TCP sockets and channels.


i.e., insert “socket” before “channels.” I could do that or it could be part of whatever other change is in the queue.

Thanks,

Brian


On Jan 30, 2019, at 7:02 AM, Chris Hegarty <chris.hegarty at oracle.com<mailto:chris.hegarty at oracle.com>> wrote:

I've taken into account both Brian and Alan's comments. The class-level
description now reads as follows:

/**
 * Factory methods for creating RDMA-based TCP sockets and socket channels.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20190131/5548fc30/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rsocket-branch.patch
Type: application/octet-stream
Size: 12563 bytes
Desc: rsocket-branch.patch
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20190131/5548fc30/rsocket-branch-0001.patch>