RFR[8239355]: '(dc) Initial value of SO_SNDBUF should allow sending large datagrams (macOS)'
Hi, Could someone please review my fix for JDK-8239355 '(dc) Initial value of SO_SNDBUF should allow sending large datagrams (macOS)' ? By default, macOS imposes a size of 9216on Datagrams which limits applications that don't set the SO_SNDBUF option - legacy DatagramSocket sets the value to 65507 at creation time. This fix updates DatagramChannel so that the SO_SNDBUF is set to a minimum value of 65527 for IPv6 sockets and 65507 for IPv4 sockets on macOS. bug: https://bugs.openjdk.java.net/browse/JDK-8239355 webrev: http://cr.openjdk.java.net/~pconcannon/8239355/webrevs/webrev.00/index.html Kind regards, Patrick
8240754: Instrument FlowTest.java to provide more debug traces.
Hi, Please find below a changeset that adds some more debugging output to FlowTest.java for better test failure diagnosis. 8240754: Instrument FlowTest.java to provide more debug traces. https://bugs.openjdk.java.net/browse/JDK-8240754 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8240754/webrev.00/ best regards, -- daniel
Re: RFR[8239355]: '(dc) Initial value of SO_SNDBUF should allow sending large datagrams (macOS)'
On 09/03/2020 12:39, Patrick Concannon wrote: Hi, Could someone please review my fix for JDK-8239355 '(dc) Initial value of SO_SNDBUF should allow sending large datagrams (macOS)' ? By default, macOS imposes a size of 9216on Datagrams which limits applications that don't set the SO_SNDBUF option - legacy DatagramSocket sets the value to 65507 at creation time. This fix updates DatagramChannel so that the SO_SNDBUF is set to a minimum value of 65527 for IPv6 sockets and 65507 for IPv4 sockets on macOS. bug: https://bugs.openjdk.java.net/browse/JDK-8239355 webrev: http://cr.openjdk.java.net/~pconcannon/8239355/webrevs/webrev.00/index.html This is a change to the DatagramChannel implementation. I guess I assumed there would be a DatagramChannel test that checked that getOption(SO_SNDBUF) returned a minimum value for both the IPv4 and IPv6 cases. Testing send is a good idea but I'm concerned it is testing DatagramSocket implementation details that are a bit questionable. It's mostly SOCKET_SNDBUF = IPV4_SENDBUF that I'm concerned about because PlainDatagramSocketImpl isn't quite right (it shouldn't set SO_RCVBUF, it shouldn't set SO_SNDBUF if the default is larger, and it's not quite right for the IPv6 case). Will the test need further work when DatagramSocket is changed to delegate to a socket adaptor? Maybe we should change PlainDatagramSocketImpl at the same time? -Alan
Re: 8240754: Instrument FlowTest.java to provide more debug traces.
Daniel, > On 9 Mar 2020, at 16:53, Daniel Fuchs wrote: > > Hi, > > Please find below a changeset that adds some more debugging > output to FlowTest.java for better test failure diagnosis. > > 8240754: Instrument FlowTest.java to provide more debug traces. > https://bugs.openjdk.java.net/browse/JDK-8240754 > > webrev: > http://cr.openjdk.java.net/~dfuchs/webrev_8240754/webrev.00/ Looks ok to me. -Chris.
Re: RFR[8239355]: '(dc) Initial value of SO_SNDBUF should allow sending large datagrams (macOS)'
Hi Alan, On 09/03/2020 17:01, Alan Bateman wrote: On 09/03/2020 12:39, Patrick Concannon wrote: bug: https://bugs.openjdk.java.net/browse/JDK-8239355 webrev: http://cr.openjdk.java.net/~pconcannon/8239355/webrevs/webrev.00/index.html This is a change to the DatagramChannel implementation. I guess I assumed there would be a DatagramChannel test that checked that getOption(SO_SNDBUF) returned a minimum value for both the IPv4 and IPv6 cases. +1. It's so obvious I didn't think of it when Patrick & I discussed the test. Testing send is a good idea but I'm concerned it is testing DatagramSocket implementation details that are a bit questionable. It's mostly SOCKET_SNDBUF = IPV4_SENDBUF that I'm concerned about because PlainDatagramSocketImpl isn't quite right (it shouldn't set SO_RCVBUF, it shouldn't set SO_SNDBUF if the default is larger, and it's not quite right for the IPv6 case). Yes - these are known limitations/issues with the current DatagramSocket implementation on macOS. Will the test need further work when DatagramSocket is changed to delegate to a socket adaptor? Yes the test will need to change, and I believe that's OK. Patrick already has already a change for that in the JEP 373 branch. Maybe we should change PlainDatagramSocketImpl at the same time? I would advise against it. I think we should leave the legacy stack unchanged. best regards, -- daniel -Alan
Re: RFR[8239355]: '(dc) Initial value of SO_SNDBUF should allow sending large datagrams (macOS)'
On 09/03/2020 18:08, Daniel Fuchs wrote: Hi Alan, On 09/03/2020 17:01, Alan Bateman wrote: On 09/03/2020 12:39, Patrick Concannon wrote: bug: https://bugs.openjdk.java.net/browse/JDK-8239355 webrev: http://cr.openjdk.java.net/~pconcannon/8239355/webrevs/webrev.00/index.html This is a change to the DatagramChannel implementation. I guess I assumed there would be a DatagramChannel test that checked that getOption(SO_SNDBUF) returned a minimum value for both the IPv4 and IPv6 cases. +1. It's so obvious I didn't think of it when Patrick & I discussed the test. Okay, I think that would be useful to add as a macOS specific test. Also we can test DatagramChannel sending datagrams of maximum size on all platforms (doesn't need to be macOS specific). We probably don't need a DatagramSocket test for this change as it doesn't touch that code (and the code in PDSI isn't quite right). -Alan
