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: *<[email protected]> *Date: *Tue, May 7, 2019 at 11:42 AM *To: *Alex Menkov, <[email protected]>, 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, [email protected] 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, [email protected] 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 > > > > >
