Thanks a lot to Chris and Alan for detailed comments. The updated version of patch available at http://cr.openjdk.java.net/~sviswanathan/Vladimir/8243099/webrev.01/
Changes: 1. in native code the common pattern was used for the 'getsockopt' call. 2. condition to define SO_INCOMING_NAPI_ID was added. 3. the DatagarmSocket was added to the ExtOptionTest 4. testing on my side was extended to the subset 'test/jdk/java/net test/jdk/java/nio/channels test/jdk/javax/net test/jdk/jdk/net test/jdk/sun/net'. Results are same for the patched and non-patched builds on the RHEL7.7 OS. Tests test/jdk/java/net/SocketOption/AfterClose.java and test/jdk/java/nio/channels/etc/PrintSupportedOptions.java were updated to support read only properties. 5. description for the NAPI_ID was updated 6. the UnsupportedOperationException was added to the 'setOption' call for the 'SO_INCOMING_NAPI_ID'. Thanks, Vladimir -----Original Message----- From: Chris Hegarty <chris.hega...@oracle.com> Sent: Tuesday, April 21, 2020 7:29 AM To: Ivanov, Vladimir A <vladimir.a.iva...@intel.com>; core-libs-dev <core-libs-dev@openjdk.java.net>; net >> OpenJDK Network Dev list <net-...@openjdk.java.net> Subject: Re: RFR 15 8243099: Adding ADQ support to JDK Vladimir, > On 18 Apr 2020, at 00:13, Ivanov, Vladimir A <vladimir.a.iva...@intel.com> > wrote: > > Hello everyone, > I would like to add support for the Application Device Queue (ADQ) technology > to the JDK. > > The JBS entry and webrev were created for this improvement: > JBS: https://bugs.openjdk.java.net/browse/JDK-8243099 > Webrev: Here is an incomplete set of specific comments relating to the webrev. 1) I get a compilation error on my Linux Ubuntu 18.04 ( admittedly my gcc version may be more recent/strict than that of the one you built with ): $ gcc --version gcc (Ubuntu 8.3.0-6ubuntu1~18.10) 8.3.0 Copyright (C) 2018 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. Compilation error: /home/chhegar/repos/jdk/adq/jdk/open/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c: In function 'Java_jdk_net_LinuxSocketOptions_IncomingNapiIdSupported0': /home/chhegar/repos/jdk/adq/jdk/open/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c:231:72: error: passing argument 5 of 'getsockopt' makes pointer from integer without a cast [-Werror=int-conversion] 231 | rv = getsockopt(s, SOL_SOCKET, SO_INCOMING_NAPI_ID, (void *) &one, sizeof (one)); | ^~~~~~~~~~~~ | | | long unsigned int In file included from /home/chhegar/repos/jdk/adq/jdk/open/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c:25: /var/tmp/jib-chhegar/install/jpg/infra/builddeps/devkit-linux_x64/gcc9.2.0-OL6.4+1.0/devkit-linux_x64-gcc9.2.0-OL6.4+1.0.tar.gz/x86_64-linux-gnu-to-x86_64-linux-gnu/x86_64-linux-gnu/sysroot/usr/include/sys/socket.h:192:32: note: expected 'socklen_t * restrict' {aka 'unsigned int * restrict'} but argument is of type 'long unsigned int' 192 | socklen_t *__restrict __optlen) __THROW; | ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~ cc1: all warnings being treated as errors Simple fix: diff --git a/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c b/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c --- a/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c +++ b/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c @@ -224,11 +224,12 @@ ( JNIEnv *env, jobject unused) { int one = 1; int rv, s; + socklen_t sz = sizeof (one); s = socket(PF_INET, SOCK_STREAM, 0); if (s < 0) { return JNI_FALSE; } - rv = getsockopt(s, SOL_SOCKET, SO_INCOMING_NAPI_ID, (void *) &one, sizeof (one)); + rv = getsockopt(s, SOL_SOCKET, SO_INCOMING_NAPI_ID, (void *) &one, &sz); if (rv != 0 && errno == ENOPROTOOPT) { rv = JNI_FALSE; } else { 2) test/jdk/java/net/SocketOption/AfterClose.java fails This test iterates over all socket options reported to be supported by a particular socket, and asserts expected behaviour after the socket has been closed. Clearly, this new option results in unexpected after-close behaviour. One example failed scenario output: test AfterClose.closedBoundDatagramSocket(SO_INCOMING_NAPI_ID, null): failure java.lang.NullPointerException at AfterClose.closedBoundDatagramSocket(AfterClose.java:248) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:564) at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:85) at org.testng.internal.Invoker.invokeMethod(Invoker.java:639) at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:821) at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1131) at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:124) at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:108) at org.testng.TestRunner.privateRun(TestRunner.java:773) at org.testng.TestRunner.run(TestRunner.java:623) at org.testng.SuiteRunner.runTest(SuiteRunner.java:357) at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:352) at org.testng.SuiteRunner.privateRun(SuiteRunner.java:310) at org.testng.SuiteRunner.run(SuiteRunner.java:259) at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52) at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86) at org.testng.TestNG.runSuitesSequentially(TestNG.java:1185) at org.testng.TestNG.runSuitesLocally(TestNG.java:1110) at org.testng.TestNG.run(TestNG.java:1018) at com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:94) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:564) at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:298) at java.base/java.lang.Thread.run(Thread.java:832) 3) Interestingly, should this option be supported for datagram sockets too ( as the prior test output seems to suggest ). 4) test/jdk/java/nio/channels/etc/PrintSupportedOptions.java fails Example output: ----------System.err:(18/1162)---------- java.lang.InternalError: Unexpected option SO_INCOMING_NAPI_ID at jdk.net/jdk.net.ExtendedSocketOptions$1.setOption(ExtendedSocketOptions.java:250) at java.base/sun.nio.ch.Net.setSocketOption(Net.java:349) at java.base/sun.nio.ch.Net.setSocketOption(Net.java:335) at java.base/sun.nio.ch.SocketChannelImpl.setOption(SocketChannelImpl.java:241) at java.base/sun.nio.ch.SocketChannelImpl.setOption(SocketChannelImpl.java:67) at PrintSupportedOptions.test(PrintSupportedOptions.java:66) at PrintSupportedOptions.main(PrintSupportedOptions.java:49) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:564) at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:298) at java.base/java.lang.Thread.run(Thread.java:832) 5) Optionally define SO_INCOMING_NAPI_ID diff --git a/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c b/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c --- a/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c +++ b/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c @@ -33,7 +33,10 @@ #include "jni_util.h" #include "jdk_net_LinuxSocketOptions.h" -#define SO_INCOMING_NAPI_ID 56 +#ifndef SO_INCOMING_NAPI_ID +#define SO_INCOMING_NAPI_ID 56 +#endif + /* * Class: jdk_net_LinuxSocketOptions * Method: setQuickAck That’s all for now. -Chris.
