Re: RFR(s): Improving performance of Windows socket connect on the loopback adapter
On 16/07/2020 05:37, Bernd Eckenfels wrote: Hello Nikola, Can you explain why timeouts play a role here at all? Normally when connecting to a non existing socket it should immediately respond with a TCP RST and that should not cause a retry or delay. Nikola mentioned that he is in contact with someone working on Windows networking and it would be useful to know why this isn't changed in Windows and it would avoid a workaround in the JDK or anywhere else that attempt to establish a connection to the loopback. We initially started to see issues on Windows 10, Michael has more on this in JDK-8234823 [1]. I should say that I have no objection to Nikola's proposal, I think it's great to know that there is an ioctl to change this. I suspect we might have to do the equivalent in the JDWP socket transport although probably low priority as the shared memory transport is the default there. -Alan [1] https://bugs.openjdk.java.net/browse/JDK-8234823
Re: RFR(s): Improving performance of Windows socket connect on the loopback adapter
On 16/07/2020 01:02, Nikola Grcevski wrote: : Please find the webrev with this improvement here: http://cr.openjdk.java.net/~adityam/nikola/fast_connect_loopback/ Just a few comments on the first iteration. Can we move the function prototype to the Windows net_util_md.h? That would avoid needing to update the shared net_util.h and would avoid unused code in the Unix implementation. Net.connect0 is used for both TCP and UDP sockets. It already uses getsockopt to query the socket type so I think we can do that unconditionally. That would allow you to use the ioctl when the type == SOCK_STREAM and the target is the loopback. I think we need to find a better name for IN6_IS_ADDR_LOOPBACK_IP4, maybe we could find something that has "MAPPED" in the name so that it's a bit more consistent with other macros. -Alan.
RE: RFR(s): Improving performance of Windows socket connect on the loopback adapter
Hi Bernd and Alan, I had initially the same question about the socket connects, especially since it works so much better on Unix’s. The reason why Windows can’t change the implementation is app compatibility. They have implemented an ECN (Explicit Congestion Notification) related mitigation in the stack for handling RSTs sent in response to SYNs. They tried to exempt loopback from this mitigation because ECN doesn’t really apply. However, this resulted in various apps breaking, because those apps have made assumptions that loopback connect failure will actually take longer on Windows. It appears that this timeout has become an undocumented contract in various Windows apps and a change of behavior breaks multiple of them :(. The team responsible for the network stack attempted to change the default behaviour, but were unable to after receiving pushback from various app owners. I'm working on the suggested changes to the webrev. If it makes sense, I can include a comment in the code itself on the history of the timeout? Thanks! Nikola From: Alan Bateman Sent: July 16, 2020 2:59 AM To: Bernd Eckenfels ; net-dev@openjdk.java.net; Nikola Grcevski Subject: Re: RFR(s): Improving performance of Windows socket connect on the loopback adapter On 16/07/2020 05:37, Bernd Eckenfels wrote: Hello Nikola, Can you explain why timeouts play a role here at all? Normally when connecting to a non existing socket it should immediately respond with a TCP RST and that should not cause a retry or delay. Nikola mentioned that he is in contact with someone working on Windows networking and it would be useful to know why this isn't changed in Windows and it would avoid a workaround in the JDK or anywhere else that attempt to establish a connection to the loopback. We initially started to see issues on Windows 10, Michael has more on this in JDK-8234823 [1]. I should say that I have no objection to Nikola's proposal, I think it's great to know that there is an ioctl to change this. I suspect we might have to do the equivalent in the JDWP socket transport although probably low priority as the shared memory transport is the default there. -Alan [1] https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.openjdk.java.net%2Fbrowse%2FJDK-8234823&data=02%7C01%7CNikola.Grcevski%40microsoft.com%7Cdc732e9b27df4c8d6a6c08d829560ced%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637304796845600565&sdata=QemEEB2koJBLaeOtPZqJDzOmhsz7GV2CYqzOG6sEqVc%3D&reserved=0
IPV6 literal string handling in tests
Hello net-dev
I have been investigating an intermittent error with the following tests in
jdk11u and
identified fixes that could be back-ported:
tests/jdk/sun/net/www/protocal/httponly.java
tests/jdk/sun/net/www/protocal/nocache.java
The error was a malformed string being passed to the URI/URL constructor
(that parses the given string) in the format "http://0:0:0:0:1%1:65500/path";,
the root cause being that InetAddress.getHostAddress in some cases returns
an IPV6 literal string (e.g. "0:0:0:0:1%1"), this specific constructor
expects IPV6 literals to be enclosed in brackets e.g.
"http://[0:0:0:0:1%1]:65500/path
The issue has been addressed in tip with following change
https://hg.openjdk.java.net/jdk/jdk/rev/5302477c8285 in response
to https://bugs.openjdk.java.net/browse/JDK-8224761
Has there been discussion about back-porting the fix to the tests to jdk11u?
I looked for other tests with the pattern that can result in malformed URLs
and only found one case in:
tests/jdk/sun/net/www/http/KeepAliveCache/B5045306.java
This has also been fixed in tip with a different change
http://hg.openjdk.java.net/jdk/jdk/rev/3131927311ee
in response to https://bugs.openjdk.java.net/browse/JDK-8230858
However these two changes achieve the same results using two different
solutions;
the fix for B5045306.java and 12 other tests in tip use the following code
pattern when
using the URI/URL constructor that takes a string to parse:
String hostAddr = InetAddress.getLocalHost().getHostAddress();
if (hostAddr.indexof(':') > -1) hostAddr = "[" + hostAddr + "]";
Whereas the fix for httponly.java and nocache.java use the test utility class
test/lib/jdk/tests/lib/net/URIBuilder.java, which essentially boils down to
using
the URI/URL constructor that constructs a hierarchical URI from the given
components
Regardless of whether the fixes are back-ported to jdk11u, is there a shared
view in
this mailing list that we pick one pattern on tip for consistency, I'm happy to
make
the changes if there is agreement
Thanks in advance
Mat Carter
Microsoft
RE: RFR(s): Improving performance of Windows socket connect on the loopback adapter
Thanks for reviewing the patch Alan. I applied your suggestions in this new webrev here: http://cr.openjdk.java.net/~adityam/nikola/fast_connect_loopback_1/ I renamed the macro to IN6_IS_ADDR_MAPPED_IP4_LOOPBACK, following how IN6_IS_ADDR_ANY is named in the same file. I hope the new name works, but if there's something more suitable I don't mind changing it again. I re-tested the patch on the jtreg tier1, net, nio and rmi suites. If there are any other tests I should run, or if you have a suggestion for new tests I should write please let me know. Best, Nikola -Original Message- From: Alan Bateman Sent: July 16, 2020 4:47 AM To: Nikola Grcevski ; net-dev@openjdk.java.net Subject: Re: RFR(s): Improving performance of Windows socket connect on the loopback adapter On 16/07/2020 01:02, Nikola Grcevski wrote: > : > > Please find the webrev with this improvement here: > > https://nam06.safelinks.protection.outlook.com/?url=http:%2F%2Fcr.open > jdk.java.net%2F~adityam%2Fnikola%2Ffast_connect_loopback%2F&data=0 > 2%7C01%7CNikola.Grcevski%40microsoft.com%7C3a6d8732ac1643b5b88c08d8296 > 526de%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637304861703337536& > amp;sdata=ZOAsgwuLxmwD3lUOl1kdRJMuhyX4qkp1UdvAlcB08io%3D&reserved= > 0 > Just a few comments on the first iteration. Can we move the function prototype to the Windows net_util_md.h? That would avoid needing to update the shared net_util.h and would avoid unused code in the Unix implementation. Net.connect0 is used for both TCP and UDP sockets. It already uses getsockopt to query the socket type so I think we can do that unconditionally. That would allow you to use the ioctl when the type == SOCK_STREAM and the target is the loopback. I think we need to find a better name for IN6_IS_ADDR_LOOPBACK_IP4, maybe we could find something that has "MAPPED" in the name so that it's a bit more consistent with other macros. -Alan.
