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

2020-06-29 Thread Andrew Haley
On 29/06/2020 17:06, Severin Gehwolf wrote: > It's a possibility. IMHO, it doesn't really make the code easier to > read, though. Some duplication for clarity seems OK to me in this case. > I'm not too fond of over-use of ifdef so I'd rather keep it at v5. > YMMV. OK. -- Andrew Haley (he/him) J

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

2020-06-29 Thread Severin Gehwolf
Hi Yyom, On Mon, 2020-06-29 at 21:18 +0530, Vyom Tiwari wrote: > Hi Severin, > > thanks for clarification, we can still simplify the > ExtendedOptionsImpl.c. Please have a look on below change and do let > me know if it makes sense. > > I moved the "#if defined(__linux__) || defined(MACOSX)" ins

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

2020-06-29 Thread Vyom Tiwari
Hi Severin, thanks for clarification, we can still simplify the ExtendedOptionsImpl.c. Please have a look on below change and do let me know if it makes sense. I moved the "#if defined(__linux__) || defined(MACOSX)" inside the corresponding methods and this way we will eliminate lot's of duplica

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

2020-06-29 Thread Severin Gehwolf
Hi Vyom, On Mon, 2020-06-29 at 17:11 +0530, Vyom Tiwari wrote: > Hi Severin, > > Latest code looks good Thanks! > I think in src/solaris/native/java/net/ExtendedOptionsImpl.c, you > don't need #ifdef blocks. In case of "flowOption" it was required > because flow is specific to Solaris. > > In

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

2020-06-29 Thread Vyom Tiwari
Hi Severin, Latest code looks good, I think in src/solaris/native/java/net/ExtendedOptionsImpl.c, you don't need #ifdef blocks. In case of "flowOption" it was required because flow is specific to Solaris. In case of KEEPALIVE we are using the POSIX api(setsockopt/getsockopt) which exists on all P

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

2020-06-29 Thread 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 explicit

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

2020-06-26 Thread Vyom Tiwari
Hi Severin, Overall code changes looks ok to me, I build & tested at my local REL it worked fine for me. 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.

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

2020-06-26 Thread Severin Gehwolf
Hi, On Thu, 2020-06-25 at 23:55 +, 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 g

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

2020-06-26 Thread Vyom Tiwari
/bernd.eckenfels.net > -- > *Von:* net-dev im Auftrag von Severin > Gehwolf > *Gesendet:* Thursday, June 25, 2020 3:39:38 PM > *An:* jdk8u-dev > *Cc:* net-dev > *Betreff:* [8u] RFR(m): 8194298: Add support for per Socket configuration > of T

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

2020-06-25 Thread Bernd Eckenfels
to the general implementation. Gruss Bernd -- http://bernd.eckenfels.net Von: net-dev im Auftrag von Severin Gehwolf Gesendet: Thursday, June 25, 2020 3:39:38 PM An: jdk8u-dev Cc: net-dev Betreff: [8u] RFR(m): 8194298: Add support for per Socket configuration of

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

2020-06-25 Thread Severin Gehwolf
Hi, Please review this OpenJDK 8u backport of JDK-8194298 which adds socket configuration options for TCP keepalive. It's one of the Oracle JDK parity patches. As with the OpenJDK 11u change, this includes Linux and Mac only changes. This backport is pretty much a rewrite as the JDK 11u codebase