Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23630 )
Change subject: [tests] Add IPv6 coverage to multiple tests ...................................................................... Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/23630/2//COMMIT_MSG Commit Message: PS2: At this point it's time to double check, I guess: what is ipv6-only mode, actually? Is it even possible to configure IP stack on Linux to disallow IPv4-on-IPv6 address mapping if there is an incoming connection from a IPv4 address? I'm not sure about that. >From the other side, I don't see setting IPV6_V6ONLY for sockets. Then how >IPv6-only mode is achieved then? One more question to clarify: what is 'dual' mode? Is that something about the default behavior of the IP stack on Linux where the IPv6 is enabled? http://gerrit.cloudera.org:8080/#/c/23630/2//COMMIT_MSG@16 PS2, Line 16: For client-test, the coverage is not added for 'dual' mode because of : the additional time it takes to run those tests. > IIRC, it was around 250 seconds for dual mode on my laptop. However, this c +1 for adding SKIP_IF_SLOW_NOT_ALLOWED into tests if they run for more than 3 seconds in DEBUG build on a relatively contemporary hardware. http://gerrit.cloudera.org:8080/#/c/23630/2//COMMIT_MSG@19 PS2, Line 19: - Some minor and miscellaneous changes to rename tests to get rid of : CLANG tidy warnings. Please separate these updates into its own changelist, and add the actual IPv6 updates on top of that changelist. http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/jwt-util-test.cc File src/kudu/util/jwt-util-test.cc: http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/jwt-util-test.cc@125 PS2, Line 125: "ipv4", "ipv6", "dual" Here and elsewhere: I guess it's time to introduce constants for these tokens and start using the new constants instead at least in tests? Probably, it makes sense doing so in its own changelist, maybe updating already existing tests that use so-called IP mode as well. After that, it could be possible to use the new constants in this patch as well? http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/net/net_util.cc File src/kudu/util/net/net_util.cc: http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/net/net_util.cc@524 PS2, Line 524: ToIpInterface(string* out) Given the code in this method, I think this name is very misleading. 'ToIpInterface' what? First: is this functionality is needed by only by the test scaffolding? If so, then I don't think this is the right place to place it. Second: please name methods to reflect their actual semantics. http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/net/net_util.cc@537 PS2, Line 537: *out = Substitute("[$0]", kWildcardIpV6Addr); > Binding to loopback for dual mode won't accept IPv4 requests. The idea is t This smells fishy. Is this for test testing harness? I'm not sure we should use non-loopback addresses for testing. It seems I didn't pay enough attention to this in http://gerrit.cloudera.org:8080/23411, but now I realize it's necessary to address that. http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/net/net_util.cc@859 PS2, Line 859: if (fam == AF_INET6 || fam == AF_UNSPEC) { : return kLoopbackIpV6Addr; : } I don't think this provides the functionality required by UNIQUE_LOOPBACK. The idea behind UNIQUE_LOOPBACK is to have a unique loopback IP address so it would be necessary to bother about port conflicts. Doesn't IPv6 stack on Linux provide similar functionality for the range of IPv4-mapped-on-IPv6 addresses, i.e. addresses in the form ::FFFF:<IPv4 127.0.0.0/8 address>? -- To view, visit http://gerrit.cloudera.org:8080/23630 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idc0df890c71e101964e7c7c84b5847963fab1451 Gerrit-Change-Number: 23630 Gerrit-PatchSet: 2 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: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Comment-Date: Tue, 18 Nov 2025 06:11:11 +0000 Gerrit-HasComments: Yes
