Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/23021 )
Change subject: KUDU-1457 [3/n] add IPv6 support for diagnostic socket ...................................................................... Patch Set 4: (2 comments) Overall looks solid, two questions. First pass from my end. Will do another tomorrow. http://gerrit.cloudera.org:8080/#/c/23021/4/src/kudu/util/net/diagnostic_socket-test.cc File src/kudu/util/net/diagnostic_socket-test.cc: http://gerrit.cloudera.org:8080/#/c/23021/4/src/kudu/util/net/diagnostic_socket-test.cc@133 PS4, Line 133: const auto compare_addresses = [&](DiagnosticSocket::TcpSocketInfo entry) { : if (family == AF_INET) { : // IPv4 comparison : return (src_addr.ipv4_addr().sin_addr.s_addr == entry.src_addr[0] && : INADDR_ANY == entry.dst_addr[0]); : } : // IPv6 comparison : return (memcmp(src_addr.ipv6_addr().sin6_addr.s6_addr, : entry.src_addr, sizeof(entry.src_addr)) == 0 && : memcmp(&in6addr_any, entry.dst_addr, : sizeof(entry.dst_addr)) == 0); : }; This lambda looks like functionality that could pop up elsewhere. I'm not sure whether it would make sense to add this as a method to TcpSocketInfo? As I see it is only used in this test once. Just a curiosity question. http://gerrit.cloudera.org:8080/#/c/23021/1/src/kudu/util/net/diagnostic_socket.h File src/kudu/util/net/diagnostic_socket.h: http://gerrit.cloudera.org:8080/#/c/23021/1/src/kudu/util/net/diagnostic_socket.h@64 PS1, Line 64: doc nit: maybe explicitly mention ipv6 in the todo as well Sockaddr class fields (supporting both IPv4 and IPv6). -- To view, visit http://gerrit.cloudera.org:8080/23021 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d78e241a8bb794465a613e7c0a11eea8f628849 Gerrit-Change-Number: 23021 Gerrit-PatchSet: 4 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, 09 Sep 2025 14:50:10 +0000 Gerrit-HasComments: Yes
