[8u] RFR(m): 8194298: Add support for per Socket configuration of TCP keepalive

[Severin Gehwolf]

Hi Vyom!

On Fri, 2020-06-26 at 15:55 +0530, Vyom Tiwari wrote:
> Hi Severin,
> 
> Overall code changes looks ok to me, I build & tested at my local REL
> it worked fine for me.

Thanks for the review!

> Below are few minor comments.
> 
> 1-> Net.java
>     1.1-> I think you don't need to explicitly convert value to integer and then pass it. You can avoid the local int variable creation as follows.
>       ExtendedOptionsImpl.setTcpKeepAliveIntvl(fd, (Integer)value);
>    1.2-> In getSocketOption you don't need to explicitly cast it to Integer.
>              return ExtendedOptionsImpl.getTcpKeepAliveIntvl(fd);
> 2-> PlainSocketImpl.java
>     1.1 -> In setOption(SocketOption<T> name, T value) you can avoid the local int variable.

Should all be fixed.

> 3-> Any specific reason for two set of functions "setTcpKeepAliveTime0 and setTcpKeepAliveTime" for all three socket options ?

Not really, other than keeping the backport aligned to the JDK 11u
patch. I've updated it in the latest webrev:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8194298/jdk8/05/webrev/

Thanks,
Severin


> Thanks,
> Vyom
> 
> On Fri, Jun 26, 2020 at 1:08 PM Severin Gehwolf <sgehwolf at redhat.com> wrote:
> > Hi,
> > 
> > On Thu, 2020-06-25 at 23:55 +0000, Bernd Eckenfels wrote:
> > > This would be a great addition.
> > 
> > Thanks.
> > 
> > > I do not understand why it does not support the options available for
> > > Windows. Especially given the fact that it actually implements 6
> > > native methods to print "Unsupported".
> > > 
> > > But I guess that's less a question to the backport and more to the
> > > general implementation.
> > 
> > Yes, indeed. This should be brought up for discussion in JDK head first
> > and implemented there before we can consider a backport.
> > 
> > Thanks,
> > Severin
> > 
> > 
> 
>