Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/22984 )
Change subject: KUDU-1457 [1/n] add IPv6 support to net utils ...................................................................... Patch Set 13: (4 comments) http://gerrit.cloudera.org:8080/#/c/22984/1/src/kudu/util/net/sockaddr.cc File src/kudu/util/net/sockaddr.cc: http://gerrit.cloudera.org:8080/#/c/22984/1/src/kudu/util/net/sockaddr.cc@225 PS1, Line 225: if (storage_.un.sun_path[0] == '\0') { : return UnixAddressType::kAbst > > When you say "from outside" what does that mean? Is sockaddr_in6 coming f I have added explicit zeroing out of the two fields. http://gerrit.cloudera.org:8080/#/c/22984/13/src/kudu/util/net/sockaddr.cc File src/kudu/util/net/sockaddr.cc: http://gerrit.cloudera.org:8080/#/c/22984/13/src/kudu/util/net/sockaddr.cc@109 PS13, Line 109: Sockaddr::Sockaddr(const struct sockaddr_in6& addr) : > As of code in PS13, Sockaddr::operator=(const struct sockaddr_in6&) and Soc Not required here, instead doing it in these callees would make sense: Sockaddr& Sockaddr::operator=(const Sockaddr& other) Sockaddr::Sockaddr(const struct sockaddr& addr, socklen_t len) So, in total I see four places if we include Sockaddr::ParseFromNumericHostPort and Sockaddr::operator=(const struct sockaddr_in6& addr). http://gerrit.cloudera.org:8080/#/c/22984/11/src/kudu/util/net/socket-test.cc File src/kudu/util/net/socket-test.cc: http://gerrit.cloudera.org:8080/#/c/22984/11/src/kudu/util/net/socket-test.cc@103 PS11, Line 103: ASSERT_OK(s_in6.ParseString(kIpV6Addr, kPort)); : ASSERT_OK(s_un.ParseUnixDomainPath(kPath)); : > > Yes, I have verified HashCode() and BytewiseCompare() and both them do ca Length of kPath is 34 bytes which is enough to overwrite the fields of in6 structure including sin6_flowinfo and sin6_scope_id. ParseUnixDomainPath/ParseString takes care of first stamping over zeroing out two the two fields and then zeroing out. So we are good there. http://gerrit.cloudera.org:8080/#/c/22984/13/src/kudu/util/net/socket-test.cc File src/kudu/util/net/socket-test.cc: http://gerrit.cloudera.org:8080/#/c/22984/13/src/kudu/util/net/socket-test.cc@288 PS13, Line 288: // IPv6. : DoTestServerConnect("::1", AF_INET6); : // IPv4-mapped IPv6 address. : DoTestServerConnect("::ffff:127.0.0.1", AF_INET6); > I've seen test nodes where IPv6 stack was explicitly disabled, and no IPv6 I have responded to similar comment here: https://gerrit.cloudera.org/#/c/23021/1/src/kudu/util/net/diagnostic_socket-test.cc@202 I don't mind if the tests fail for some time until we have a robust detection mechanism for such scenarios. I can add a comment for now if that is ok with you? If configuring IPv6 on nodes is not a significant task, that is also fine by me. -- To view, visit http://gerrit.cloudera.org:8080/22984 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I22c773ffb2ff44b9cd765b546d6724ec5543586f Gerrit-Change-Number: 22984 Gerrit-PatchSet: 13 Gerrit-Owner: Ashwani Raina <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Comment-Date: Thu, 28 Aug 2025 15:51:38 +0000 Gerrit-HasComments: Yes
