RFR:8194298 Add support for per Socket configuration of TCP keepalive

[Langer, Christoph]

Hi,

As I had written previously, I’m still unclear whether the naming of the MacOS implementation files is the best choice.

Currently it is:
src/jdk.net/macosx/classes/jdk/net/UnixSocketOptions.java
src/jdk.net/macosx/native/libextnet/UnixSocketOptions.c

Maybe it  should rather be:
src/jdk.net/macosx/classes/jdk/net/MacOSXSocketOptions.java
src/jdk.net/macosx/native/libextnet/MacOSXSocketOptions.c

Since for Linux we have:
src/jdk.net/linux/classes/jdk/net/LinuxSocketOptions.java
src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c

I’m asking because I’m planning to add some AIX options and will have to choose a name for this implementation eventually.

@Alan: What do you think?

Best regards
Christoph


From: net-dev [mailto:net-dev-bounces at openjdk.java.net] On Behalf Of vyom tewari
Sent: Montag, 14. Mai 2018 17:31
To: Alan Bateman <Alan.Bateman at oracle.com>; OpenJDK Network Dev list <net-dev at openjdk.java.net>
Subject: Re: RFR:8194298 Add support for per Socket configuration of TCP keepalive


Hi All,

Please find the latest webrev(http://cr.openjdk.java.net/~vtewari/8194298/webrev0.5/index.html). Only change with the previous wrev(04) is i removed "socket type" as Alan suggested and used the default  constructor (Set<SocketOption<?>> options = new HashSet<>();) in ExtendedSocketOptions.java

Thanks,

Vyom

On Sunday 13 May 2018 12:37 PM, Alan Bateman wrote:
On 12/05/2018 10:21, vyom tewari wrote:

:

Thanks for review, please find the updated webrev(http://cr.openjdk.java.net/~vtewari/8194298/webrev0.4/index.html<http://cr.openjdk.java.net/%7Evtewari/8194298/webrev0.4/index.html>).
I've skimmed through this webrev.

The spec for the new options mostly look good but all three include "The exact semantics of this socket option are socket type and system dependent". I assume "socket type" should be dropped from this as these are TCP options. Maybe you can borrow text from the TCP_NODELAY option where it has the statement "The socket option is specific to stream-oriented sockets using the TCP/IP protocol".

The changes to the NetworkChannel implementations look okay. The changes to the NIO tests looks okay too but would be good if you could keep the formatting consistent with the existing code if you can.

Ivan had good comments so I didn't spent as much time on that code and it seems very reviewed.

-Alan.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/net-dev/attachments/20180515/77d6a9f0/attachment.html>