RFR 15 8243099: Adding ADQ support to JDK
Ivanov, Vladimir A
vladimir.a.ivanov at intel.com
Thu Apr 23 19:11:55 UTC 2020
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.hegarty at oracle.com>
Sent: Tuesday, April 21, 2020 7:29 AM
To: Ivanov, Vladimir A <vladimir.a.ivanov at intel.com>; core-libs-dev <core-libs-dev at openjdk.java.net>; net >> OpenJDK Network Dev list <net-dev at 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.ivanov at 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.
More information about the core-libs-dev
mailing list