PING: Re: RFR: JDK-8184770: JDWP support for IPv6

[serguei.spitsyn at oracle.com]

Hi Alex,

Thank you for the update!
It looks good to me.

Thanks,
Serguei


On 5/13/19 14:06, Alex Menkov wrote:
> Hi Chris, Serguei,
>
> Updated webrev:
> http://cr.openjdk.java.net/~amenkov/IPv6/webrev.05/
> CSR (approved):
> https://bugs.openjdk.java.net/browse/JDK-8223104
>
> Changes (vs. webrev.04):
>
> - setsockopt(IPV6_V6ONLY) was moved from Win-specific code to shared 
> setOptionsCommon function (in socketTransport.c)
>   the value by default is "true" on Windows and is "false" on Unix, 
> but it can be overridden, so lets set it explicitly;
> - a comment why the result of setsockopt(IPV6_V6ONLY) is ignored is 
> added;
> - added some comments as per Serguei request.
>
> About scopes support - I thought that the functionality is not 
> important for debugger, but I can implement it (I'd prefer to do it by 
> separate RFE).
>
> --alex
>
> On 05/11/2019 03:39, Chris Hegarty wrote:
>>
>>
>>> On 7 May 2019, at 19:40, serguei.spitsyn at oracle.com wrote:
>>>
>>> Hi guys,
>>>
>>> We need a couple of partial reviews for this enhancement:
>>>
>>>   - From the net-dev to check IPv6-addresses related part.
>>>     It does not need to be a thorough review.
>>>     We need another pair of eyes to check for obvious typos or misses.
>>>     Included Chris H. to mailing list in hope for some assistance.
>>>     (Chris, we need some help to find one of the IPv6 experts.)
>>>
>>>   - From the serviceability-dev to check if compatibility
>>>     is not broken and nothing is missed.
>>>     This includes part of code that is not directly involved
>>>     into manipulations with the IPv4/IPv6 addresses.
>>>     The related spec update is tracked by a sub-task:
>>>       https://bugs.openjdk.java.net/browse/JDK-8221707
>>>
>>>
>>> Related CSR:
>>> https://bugs.openjdk.java.net/browse/JDK-8223104
>>>
>>> The latest webrev:
>>> http://cr.openjdk.java.net/~amenkov/IPv6/webrev.04/
>>
>>
>> src/jdk.jdwp.agent/windows/native/libdt_socket/socket_md.c
>>
>>     223     if (domain == AF_INET6) {
>>     224         int off = 0;
>>     225         // make the socket a dual mode socket
>>     226         setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, (char 
>> *)&off, sizeof(off));
>>     227     }
>>     228   }
>>
>>    This code is fine, but maybe you want to expand the comment to
>>    mention that the setsockopt may fail if IPv4 is not supported. And
>>    that’s OK.
>>
>>    I cannot find a similar setting of IPV6_V6ONLY for the unix
>>    equivalent. Why not, or where can it be found?
>>
>> src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c
>>
>>    There is a lot of native code here, I skimmed over it, seems
>>    reasonable. There is an obvious lack of any reference to IPv6
>>    scopes. Can the address given to bind be an IPv6 link-local?
>>    If so, then the scope will need to be parsed / set appropriately.
>>    ( seems that the new test skips all scoped addresses? )
>>
>> -Chris.
>>