RFR 15 8243099: Adding ADQ support to JDK

[Alan Bateman]

On 23/04/2020 20:11, Ivanov, Vladimir A wrote:
> 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'.
>
(Dropping core-libs-dev as net-dev is the more appropriate list for this 
area).

Thanks for the update.

The updated javadoc looks better but will need a few iterations. It 
would be good if it could start by explaining what the value of the 
socket option is, e.g. "The value of this socket option is an Integer 
representing the network device queue ...". I think it needs to be 
clearer on the types of sockets that support this option, does it 
support both stream-oriented and datagram-oriented sockets? Does it 
return a value when invoked on a ServerSocketChannel? Instead of 
@throws, the text will need to that attempting to set the socket option 
will cause UOE to be thrown. Once the javadoc is agreed then the CSR can 
be submitted and the code review can continue in parallel.

The implementation changes mostly look okay although 
IncomingNapiIdSupported0 should probably be renamed to start with lower 
case "i". Also someone might need to check that you can create an IPv4 
socket when a system is configured as an IPv6-only system.

Tests. I think the the new tests in ExtOptionTest will need closer 
examination as I can't tell how reliable they are. It might be that it 
needs a completely new test. The update to the NIO PrintSupportedOptions 
test define READ_ONLY_OPTS as Set<String>.

-Alan