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.
