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

[Arthur Eubanks]

Even though I showed our internal patches in this area earlier, I haven't
really looked into exactly what this patch does (since it was written by
someone else).
I can confirm that it fixes jshell in an IPv6 only environment though.

*From: *<serguei.spitsyn at oracle.com>
*Date: *Tue, May 7, 2019 at 11:42 AM
*To: *Alex Menkov, <serviceability-dev at openjdk.java.net>, net-dev, Chris
Hegarty

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/
>
>
> Thanks,
> Serguei
>
>
> On 5/6/19 3:27 PM, serguei.spitsyn at oracle.com wrote:
>
> Hi Alex,
>
> It looks great and very solid in general!
>
> Some minor comments are below.
>
>
> http://cr.openjdk.java.net/~amenkov/IPv6/webrev.04/src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c.frames.html
>
>  263  * Result must be release with dbgsysFreeAddrInfo.
>
>   A typo: "must be release" => "must be released"
>
>   80 typedef struct {  81     struct in6_addr subnet;  82     struct in6_addr netmask;
>   83 } AllowedPeerInfo;
>  . . .
>  431 parseAllowedPeersInternal(char *buffer) {
>  . . . 483 }
>  . . .
>  524 isPeerAllowed(struct sockaddr_storage *peer) { 525     struct in6_addr tmp; 526     struct in6_addr *addr6; 527     if (peer->ss_family == AF_INET) { 528         convertIPv4ToIPv6((struct sockaddr *)peer, &tmp); 529         addr6 = &tmp; 530     } else { 531         addr6 = &(((struct sockaddr_in6 *)peer)->sin6_addr); 532     } 533  534     for (int i = 0; i < _peers_cnt; ++i) { 535         if (isAddressInSubnet(addr6, &(_peers[i].subnet), &(_peers[i].netmask))) {
>  536             return 1;
>  537         }
>  538     }
>
>  The allowed pears are converted into and used/checked in the IPv6 format.
>  Some short comments about it in all three places above would be helpful.
>  I'd consider to do the same in parseAllowedAddr() before this fragment:
>
>  367     if (addrInfo->ai_family == AF_INET6) { 368         memcpy(result, &(((struct sockaddr_in6 *)(addrInfo->ai_addr))->sin6_addr), sizeof(*result)); 369         *isIPv4 = 0; 370     } else {    // IPv4 address 371         struct in6_addr addr6; 372         convertIPv4ToIPv6(addrInfo->ai_addr, &addr6); 373         memcpy(result, &addr6, sizeof(*result)); 374         *isIPv4 = 1;
>  375     }
>
>
>  and in parseAllowedMask() before the line:
>
>  420             result->s6_addr[i] = (char)(0xFF << (8 - prefixLen));
>
>
>  This fragment needs a comment:
>  402     if (isIPv4) { 403         prefixLen += 96;
>  404     }
>
>
>
> We have to find out at least one more reviewer for this fix!
>
> Thanks,
> Serguei
>
>
> On 5/3/19 18:14, serguei.spitsyn at oracle.com wrote:
>
> Hi Alex,
>
> Thank you for creating the CSR!
> I've added myself as a reviewer and corrected a couple of places.
> Feel free to change my changes if necessary. :)
> It would be nice to get one more CSR review if possible, so I've added the
> net-dev mailing list.
>
> I hope to finish the webrev review soon.
>
> Thanks,
> Serguei
>
> On 5/1/19 10:54 AM, Alex Menkov wrote:
>
> Hi all,
>
> I created CSR for the feature:
> https://bugs.openjdk.java.net/browse/JDK-8223104
>
> The latest webrev:
> http://cr.openjdk.java.net/~amenkov/IPv6/webrev.04/
>
> --alex
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190507/9385f9ab/attachment-0001.html>