Re: RFR[8242885]: 'PlainDatagramSocketImpl doesn’t allow for the sending of IPv6 datagrams on macOS with sizes between 65508-65527 bytes'

[Patrick Concannon]

Hi Daniel,

Thanks for your reply.

I’ve made the changes you’ve requested and you can find them in the updated webrev below.

http://cr.openjdk.java.net/~pconcannon/8242885/webrevs/webrev.02/

Kind regards,
Patrick

> On 20 Jul 2020, at 15:09, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
> 
> Hi Patrick,
> 
> test/jdk/java/net/DatagramSocket/SetGetSendBufferSize.java
> 
>  95         if(Platform.isOSX() && IPSupport.hasIPv6()) {
>  96             try (var socket = supplier.open()){
>  97                 assertTrue(socket.getSendBufferSize() >= 65507, name);
>  98             }
>  99         }
> 
> I'd suggest:
> 
>         if (Platform.isOSX()) {
>             try (var socket = supplier.open()){
>                 assertTrue(socket.getSendBufferSize() >= 65507, name);
>                 if (IPSupport.hasIPv6() && !IPSupport.preferIPv4Stack()) {
>                     assertEquals(socket.getSendBufferSize(), 65527, name);
>                 }
>             }
>         }
> 
> test/jdk/java/net/DatagramSocket/SendReceiveMaxSize.java
> 
> The test has many issues, that you should be able to fix:
> 
> a. it tests for preferIPv6Addresses, but preferIPv6Addresses is
>   always false.
> b. to be able to send a datagram of size 65527, you need three
>   things:
>      a. the socket must be a dual socket.
>         this depends on two conditions: hasIPv6 && !preferIPv4Stack
>      b. the socket must be bound to an IPv6 address
>      c. the remote address must be IPv6
> 
> I believe the test should be reworked to correctly test all these
> things - and use the right combination of -Dprefer* to have a chance
> to test all these scenarios.
> 
> 
> 
> test/jdk/java/nio/channels/DatagramChannel/SendReceiveMaxSize.java
> 
> This looks better, but `assertEquals(sendBuf, receiveBuf);`
> probably doesn't do what you think it does. Please consider
> the answers to these questions:
> 
>    1. what is the state of sendBuf at the point where you
>       call assertEquals? What are its position and limit?
>    2. same question for receiveBuf?
>    3. what does assertEquals do to check that the two buffers
>       are equals?
> 
> hope this helps,
> 
> -- daniel
> 
> On 09/07/2020 16:25, Patrick Concannon wrote:
>> Hi,
>> Sorry for the delay.
>> Alan: I’ve changed `testInitialSendBufferSize` in the test `test/java/net/DatagramSocket/SetGetSendBufferSize.java` to now check the limit when the socket is IPv6 only. I’ve also updated the tests to be end-to-end (rather than use the loopback) as requested.
>> Vyom: I’ve updated the fix to use int constants now in place of values.
>> webrev: http://cr.openjdk.java.net/~pconcannon/8242885/webrevs/webrev.01/
>> Kind regards,
>> Patrick
>