Alexey Serbin has posted comments on this change. ( )

Change subject: KUDU-1900: add loopback check and test

Patch Set 5:

File src/kudu/integration-tests/
PS5, Line 335:  struct ifaddrs *ifap;
             :   if (getifaddrs(&ifap) > -1) {
             :     SCOPED_CLEANUP({
             :       freeifaddrs(ifap);
             :     });
             :     for (struct ifaddrs *ifa = ifap; ifa; ifa = ifa->ifa_next) {
             :       if (ifa->ifa_addr == nullptr || ifa->ifa_netmask == nullptr
             :           || ifa->ifa_addr->sa_family != AF_INET)
             :         continue;
             :       struct sockaddr_in *addr_in = reinterpret_cast<struct 
             :       if 
((NetworkByteOrder::FromHost32(addr_in->sin_addr.s_addr) >> 24) != 127) {
             :         char s[INET_ADDRSTRLEN];
             :         inet_ntop(AF_INET, &(addr_in->sin_addr), s, 
             :         FLAGS_local_ip_for_outbound_sockets = string(s, 
             :         // Found and assigned an external IP.
             :         return true;
             :       }
             :     }
             :   }
> Yes, but it will be less efficient, because GetLocalNetworks will loop thro
I would not be too much concerned with efficiency in this particular case 
because it's just a test.  However, if you feel strong about this, maybe think 
about making it more generic and moving into net_util.{h,cc}.
PS5, Line 375: encrypted
> So, that's something I would like to ask you back :) When we set --rpc_encr
Effectively, the application of the --rpc_encrypt_loopback_connections=true 
flag is gated by the -rpc_encryption=disabled setting (see 
PS5, Line 406: true
> hm... yes, there is something wrong here. Token should not be passed here
Something is fishy here: if in case of encrypted loopback connection between 
the client and the server the authz token is not present (as I can see it from 
line 403), then it's a bit surprising that the token is present in this case.  
At least, it would be nice to add some comment explaining why that's the case.

And it seems I know why:

Probably, that's a bug.  Let's just add a comment and move further -- I think 
it's worth addressing that issue in a separate changelist.

To view, visit
To unsubscribe, visit

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3483a9729ddeeb7901e3738532a45b49e713208f
Gerrit-Change-Number: 12474
Gerrit-PatchSet: 5
Gerrit-Owner: Greg Solovyev <>
Gerrit-Reviewer: Adar Dembo <>
Gerrit-Reviewer: Alexey Serbin <>
Gerrit-Reviewer: Greg Solovyev <>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 14 Feb 2019 19:48:22 +0000
Gerrit-HasComments: Yes

Reply via email to