You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2020/04/14 22:20:23 UTC

[kudu] branch master updated: net: make Sockaddr more generic for other address families

This is an automated email from the ASF dual-hosted git repository.

todd pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 79079b6  net: make Sockaddr more generic for other address families
79079b6 is described below

commit 79079b6b64295b06c5a0f05ed200a55d89bccc47
Author: Todd Lipcon <to...@apache.org>
AuthorDate: Wed Apr 8 14:31:11 2020 -0700

    net: make Sockaddr more generic for other address families
    
    This paves the way for supporting other socket address types: namely
    IPv6 and Unix domain sockets. This patch doesn't add any new support,
    but increases the size of Sockaddr appropriately and making the methods
    more generic.
    
    The approach here is to use a union inside of the Sockaddr class which
    could store an address of any type. An alternative (taken by a long-ago
    attempt at unix sockets) would be to make Sockaddr an abstract base
    class with derived classes for each socket type. I went with this
    approach since all of the POSIX socket APIs expect this "tagged union"
    approach -- the same APIs are used whether we want to talk to a unix
    socket or an IPv4. The downside of this approach is a slightly larger
    Sockaddr object footprint, but we shouldn't have a lot of these on the
    heap.
    
    One functional change: the default constructor for Sockaddr now creates
    an "uninitialized" instance, whereas it used to create a wildcard
    address. That's now an explicit static method.
    
    Change-Id: Ifff57e13a0e9b86b191d776ead67371b1e2ed4e3
    Reviewed-on: http://gerrit.cloudera.org:8080/15690
    Tested-by: Kudu Jenkins
    Reviewed-by: Andrew Wong <aw...@cloudera.com>
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
 src/kudu/clock/builtin_ntp.cc      |   4 +-
 src/kudu/rpc/mt-rpc-test.cc        |   2 +-
 src/kudu/rpc/negotiation-test.cc   |   4 +-
 src/kudu/rpc/proxy.cc              |   2 +-
 src/kudu/rpc/rpc-test-base.h       |   6 +--
 src/kudu/rpc/rpc-test.cc           |  20 ++------
 src/kudu/server/webserver.cc       |   2 +-
 src/kudu/tserver/scanners.cc       |   1 +
 src/kudu/util/net/net_util-test.cc |  42 ++++++++++++++--
 src/kudu/util/net/net_util.cc      |   6 +--
 src/kudu/util/net/sockaddr.cc      |  95 ++++++++++++++++++++++++++---------
 src/kudu/util/net/sockaddr.h       | 100 ++++++++++++++++++++++++++++++-------
 src/kudu/util/net/socket.cc        |  16 +++---
 13 files changed, 216 insertions(+), 84 deletions(-)

diff --git a/src/kudu/clock/builtin_ntp.cc b/src/kudu/clock/builtin_ntp.cc
index 9b309cf..52688cb 100644
--- a/src/kudu/clock/builtin_ntp.cc
+++ b/src/kudu/clock/builtin_ntp.cc
@@ -899,7 +899,6 @@ Status BuiltInNtp::SendRequests() {
 Status BuiltInNtp::SendPoll(ServerState* s) {
   CHECK_NE(-1, socket_.GetFd());
   const Sockaddr& addr = s->cur_addr();
-  sockaddr_in si_other = addr.addr();
 
   unique_ptr<PendingRequest> pr(new PendingRequest);
   pr->server = s;
@@ -914,8 +913,7 @@ Status BuiltInNtp::SendPoll(ServerState* s) {
     sched_yield();
     pr->send_time_mono_us = GetMonoTimeMicrosRaw();
     return sendto(socket_.GetFd(), &pr->request, sizeof(pr->request),
-                  /*flags=*/0, reinterpret_cast<sockaddr*>(&si_other),
-                  sizeof(si_other));
+                  /*flags=*/0, addr.addr(), addr.addrlen());
   };
   ssize_t rc = -1;
   RETRY_ON_EINTR(rc, sender());
diff --git a/src/kudu/rpc/mt-rpc-test.cc b/src/kudu/rpc/mt-rpc-test.cc
index e7fde11..1207333 100644
--- a/src/kudu/rpc/mt-rpc-test.cc
+++ b/src/kudu/rpc/mt-rpc-test.cc
@@ -209,7 +209,7 @@ TEST_F(MultiThreadedRpcTest, TestBlowOutServiceQueue) {
   CHECK_OK(bld.Build(&server_messenger_));
 
   shared_ptr<AcceptorPool> pool;
-  ASSERT_OK(server_messenger_->AddAcceptorPool(Sockaddr(), &pool));
+  ASSERT_OK(server_messenger_->AddAcceptorPool(Sockaddr::Wildcard(), &pool));
   ASSERT_OK(pool->Start(kMaxConcurrency));
   Sockaddr server_addr = pool->bind_address();
 
diff --git a/src/kudu/rpc/negotiation-test.cc b/src/kudu/rpc/negotiation-test.cc
index 90044ac..99da1ca 100644
--- a/src/kudu/rpc/negotiation-test.cc
+++ b/src/kudu/rpc/negotiation-test.cc
@@ -233,7 +233,7 @@ TEST_P(TestNegotiation, TestNegotiation) {
   // Create the listening socket, client socket, and server socket.
   Socket listening_socket;
   ASSERT_OK(listening_socket.Init(0));
-  ASSERT_OK(listening_socket.BindAndListen(Sockaddr(), 1));
+  ASSERT_OK(listening_socket.BindAndListen(Sockaddr::Wildcard(), 1));
   Sockaddr server_addr;
   ASSERT_OK(listening_socket.GetSocketAddress(&server_addr));
 
@@ -1005,7 +1005,7 @@ static void RunNegotiationTest(const SocketCallable& server_runner,
                                const SocketCallable& client_runner) {
   Socket server_sock;
   CHECK_OK(server_sock.Init(0));
-  ASSERT_OK(server_sock.BindAndListen(Sockaddr(), 1));
+  ASSERT_OK(server_sock.BindAndListen(Sockaddr::Wildcard(), 1));
   Sockaddr server_bind_addr;
   ASSERT_OK(server_sock.GetSocketAddress(&server_bind_addr));
   thread server(RunAcceptingDelegator, &server_sock, server_runner);
diff --git a/src/kudu/rpc/proxy.cc b/src/kudu/rpc/proxy.cc
index 61eb997..8bd45fb 100644
--- a/src/kudu/rpc/proxy.cc
+++ b/src/kudu/rpc/proxy.cc
@@ -52,7 +52,7 @@ Proxy::Proxy(std::shared_ptr<Messenger> messenger,
       is_started_(false) {
   CHECK(messenger_ != nullptr);
   DCHECK(!service_name_.empty()) << "Proxy service name must not be blank";
-
+  DCHECK(remote.is_initialized());
   // By default, we set the real user to the currently logged-in user.
   // Effective user and password remain blank.
   string real_user;
diff --git a/src/kudu/rpc/rpc-test-base.h b/src/kudu/rpc/rpc-test-base.h
index c7c33f6..1710e9c 100644
--- a/src/kudu/rpc/rpc-test-base.h
+++ b/src/kudu/rpc/rpc-test-base.h
@@ -603,8 +603,8 @@ class RpcTestBase : public KuduTest {
 
   // Start a simple socket listening on a local port, returning the address.
   // This isn't an RPC server -- just a plain socket which can be helpful for testing.
-  Status StartFakeServer(Socket *listen_sock, Sockaddr *listen_addr) {
-    Sockaddr bind_addr;
+  static Status StartFakeServer(Socket *listen_sock, Sockaddr *listen_addr) {
+    Sockaddr bind_addr = Sockaddr::Wildcard();
     bind_addr.set_port(0);
     RETURN_NOT_OK(listen_sock->Init(0));
     RETURN_NOT_OK(listen_sock->BindAndListen(bind_addr, 1));
@@ -640,7 +640,7 @@ class RpcTestBase : public KuduTest {
       server_messenger_ = messenger;
     }
     std::shared_ptr<AcceptorPool> pool;
-    RETURN_NOT_OK(server_messenger_->AddAcceptorPool(Sockaddr(), &pool));
+    RETURN_NOT_OK(server_messenger_->AddAcceptorPool(Sockaddr::Wildcard(), &pool));
     RETURN_NOT_OK(pool->Start(2));
     *server_addr = pool->bind_address();
     mem_tracker_ = MemTracker::CreateTracker(-1, "result_tracker");
diff --git a/src/kudu/rpc/rpc-test.cc b/src/kudu/rpc/rpc-test.cc
index 52aede8..92c9671 100644
--- a/src/kudu/rpc/rpc-test.cc
+++ b/src/kudu/rpc/rpc-test.cc
@@ -93,20 +93,6 @@ class TestRpc : public RpcTestBase, public ::testing::WithParamInterface<bool> {
 // This is used to run all parameterized tests with and without SSL.
 INSTANTIATE_TEST_CASE_P(OptionalSSL, TestRpc, testing::Values(false, true));
 
-TEST_F(TestRpc, TestSockaddr) {
-  Sockaddr addr1, addr2;
-  addr1.set_port(1000);
-  addr2.set_port(2000);
-  // port is ignored when comparing Sockaddr objects
-  ASSERT_FALSE(addr1 < addr2);
-  ASSERT_FALSE(addr2 < addr1);
-  ASSERT_EQ(1000, addr1.port());
-  ASSERT_EQ(2000, addr2.port());
-  ASSERT_EQ(string("0.0.0.0:1000"), addr1.ToString());
-  ASSERT_EQ(string("0.0.0.0:2000"), addr2.ToString());
-  Sockaddr addr3(addr1);
-  ASSERT_EQ(string("0.0.0.0:1000"), addr3.ToString());
-}
 
 TEST_P(TestRpc, TestMessengerCreateDestroy) {
   shared_ptr<Messenger> messenger;
@@ -125,7 +111,7 @@ TEST_P(TestRpc, TestAcceptorPoolStartStop) {
     shared_ptr<Messenger> messenger;
     ASSERT_OK(CreateMessenger("TestAcceptorPoolStartStop", &messenger, 1, GetParam()));
     shared_ptr<AcceptorPool> pool;
-    ASSERT_OK(messenger->AddAcceptorPool(Sockaddr(), &pool));
+    ASSERT_OK(messenger->AddAcceptorPool(Sockaddr::Wildcard(), &pool));
     Sockaddr bound_addr;
     ASSERT_OK(pool->GetBoundAddress(&bound_addr));
     ASSERT_NE(0, bound_addr.port());
@@ -322,7 +308,7 @@ TEST_P(TestRpc, TestCallWithBadPasswordProtectedKey) {
 TEST_P(TestRpc, TestCallToBadServer) {
   shared_ptr<Messenger> client_messenger;
   ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, GetParam()));
-  Sockaddr addr;
+  Sockaddr addr = Sockaddr::Wildcard();
   addr.set_port(0);
   Proxy p(client_messenger, addr, addr.host(),
           GenericCalculatorService::static_service_name());
@@ -1189,7 +1175,7 @@ static void DestroyMessengerCallback(shared_ptr<Messenger>* messenger,
 TEST_P(TestRpc, TestRpcCallbackDestroysMessenger) {
   shared_ptr<Messenger> client_messenger;
   ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, GetParam()));
-  Sockaddr bad_addr;
+  Sockaddr bad_addr = Sockaddr::Wildcard();
   CountDownLatch latch(1);
 
   AddRequestPB req;
diff --git a/src/kudu/server/webserver.cc b/src/kudu/server/webserver.cc
index 0f75752..aeae119 100644
--- a/src/kudu/server/webserver.cc
+++ b/src/kudu/server/webserver.cc
@@ -350,7 +350,7 @@ Status Webserver::Start() {
   signal(SIGCHLD, sig_chld);
 
   if (context_ == nullptr) {
-    Sockaddr addr;
+    Sockaddr addr = Sockaddr::Wildcard();
     addr.set_port(opts_.port);
     TryRunLsof(addr);
     string err_msg = Substitute("Webserver: could not start on address $0", http_address_);
diff --git a/src/kudu/tserver/scanners.cc b/src/kudu/tserver/scanners.cc
index 8d90567..71cf9b4 100644
--- a/src/kudu/tserver/scanners.cc
+++ b/src/kudu/tserver/scanners.cc
@@ -40,6 +40,7 @@
 #include "kudu/tablet/tablet_metrics.h"
 #include "kudu/tserver/scanner_metrics.h"
 #include "kudu/util/flag_tags.h"
+#include "kudu/util/locks.h"
 #include "kudu/util/logging.h"
 #include "kudu/util/metrics.h"
 #include "kudu/util/status.h"
diff --git a/src/kudu/util/net/net_util-test.cc b/src/kudu/util/net/net_util-test.cc
index 5422c00..d0ffd8d 100644
--- a/src/kudu/util/net/net_util-test.cc
+++ b/src/kudu/util/net/net_util-test.cc
@@ -40,10 +40,10 @@ namespace kudu {
 
 class NetUtilTest : public KuduTest {
  protected:
-  Status DoParseBindAddresses(const string& input, string* result) {
+  static Status DoParseBindAddresses(const string& input, string* result) {
     vector<Sockaddr> addrs;
     RETURN_NOT_OK(ParseAddressList(input, kDefaultPort, &addrs));
-    std::sort(addrs.begin(), addrs.end());
+    std::sort(addrs.begin(), addrs.end(), Sockaddr::BytewiseLess);
 
     vector<string> addr_strs;
     for (const Sockaddr& addr : addrs) {
@@ -188,7 +188,7 @@ TEST_F(NetUtilTest, TestLsof) {
   Socket s;
   ASSERT_OK(s.Init(0));
 
-  Sockaddr addr; // wildcard
+  Sockaddr addr = Sockaddr::Wildcard(); // wildcard
   ASSERT_OK(s.BindAndListen(addr, 1));
 
   ASSERT_OK(s.GetSocketAddress(&addr));
@@ -213,4 +213,40 @@ TEST_F(NetUtilTest, TestGetRandomPort) {
   LOG(INFO) << "Random port is " << port;
 }
 
+TEST_F(NetUtilTest, TestSockaddr) {
+  auto addr1 = Sockaddr::Wildcard();
+  addr1.set_port(1000);
+  auto addr2 = Sockaddr::Wildcard();
+  addr2.set_port(2000);
+  ASSERT_EQ(1000, addr1.port());
+  ASSERT_EQ(2000, addr2.port());
+  ASSERT_EQ(string("0.0.0.0:1000"), addr1.ToString());
+  ASSERT_EQ(string("0.0.0.0:2000"), addr2.ToString());
+  Sockaddr addr3(addr1);
+  ASSERT_EQ(string("0.0.0.0:1000"), addr3.ToString());
+}
+
+TEST_F(NetUtilTest, TestSockaddrEquality) {
+  Sockaddr uninitialized_1;
+  Sockaddr uninitialized_2;
+  ASSERT_TRUE(uninitialized_1 == uninitialized_2);
+
+  Sockaddr wildcard = Sockaddr::Wildcard();
+  ASSERT_FALSE(wildcard == uninitialized_1);
+  ASSERT_FALSE(uninitialized_1 == wildcard);
+
+  Sockaddr wildcard_2 = Sockaddr::Wildcard();
+  ASSERT_TRUE(wildcard == wildcard_2);
+  ASSERT_TRUE(wildcard_2 == wildcard);
+
+  Sockaddr ip_port;
+  ASSERT_OK(ip_port.ParseString("127.0.0.1:12345", 0));
+  ASSERT_FALSE(ip_port == uninitialized_1);
+  ASSERT_FALSE(ip_port == wildcard);
+  ASSERT_TRUE(ip_port == ip_port);
+
+  Sockaddr copy = ip_port;
+  ASSERT_TRUE(ip_port == copy);
+}
+
 } // namespace kudu
diff --git a/src/kudu/util/net/net_util.cc b/src/kudu/util/net/net_util.cc
index f3a78b9..6f5325d 100644
--- a/src/kudu/util/net/net_util.cc
+++ b/src/kudu/util/net/net_util.cc
@@ -283,7 +283,7 @@ Network::Network(uint32_t addr, uint32_t netmask)
   : addr_(addr), netmask_(netmask) {}
 
 bool Network::WithinNetwork(const Sockaddr& addr) const {
-  return ((addr.addr().sin_addr.s_addr & netmask_) ==
+  return ((addr.ipv4_addr().sin_addr.s_addr & netmask_) ==
           (addr_ & netmask_));
 }
 
@@ -302,7 +302,7 @@ Status Network::ParseCIDRString(const string& addr) {
 
   // Netmask in network byte order
   uint32_t netmask = NetworkByteOrder::FromHost32(~(0xffffffff >> bits));
-  addr_ = sockaddr.addr().sin_addr.s_addr;
+  addr_ = sockaddr.ipv4_addr().sin_addr.s_addr;
   netmask_ = netmask;
   return Status::OK();
 }
@@ -389,7 +389,7 @@ Status GetLocalNetworks(std::vector<Network>* net) {
     if (ifa->ifa_addr->sa_family == AF_INET) {
       Sockaddr addr(*reinterpret_cast<struct sockaddr_in*>(ifa->ifa_addr));
       Sockaddr netmask(*reinterpret_cast<struct sockaddr_in*>(ifa->ifa_netmask));
-      Network network(addr.addr().sin_addr.s_addr, netmask.addr().sin_addr.s_addr);
+      Network network(addr.ipv4_addr().sin_addr.s_addr, netmask.ipv4_addr().sin_addr.s_addr);
       net->push_back(network);
     }
   }
diff --git a/src/kudu/util/net/sockaddr.cc b/src/kudu/util/net/sockaddr.cc
index 8e445f9..45957df 100644
--- a/src/kudu/util/net/sockaddr.cc
+++ b/src/kudu/util/net/sockaddr.cc
@@ -23,13 +23,18 @@
 
 #include <cerrno>
 #include <cstring>
+#include <limits>
 #include <string>
 
-#include "kudu/gutil/hash/builtin_type_hash.h"
+#include <glog/logging.h>
+
+#include "kudu/gutil/dynamic_annotations.h"
+#include "kudu/gutil/hash/string_hash.h"
 #include "kudu/gutil/port.h"
 #include "kudu/gutil/strings/join.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/net/net_util.h"
+#include "kudu/util/slice.h"
 #include "kudu/util/stopwatch.h"
 
 using std::string;
@@ -42,59 +47,102 @@ namespace kudu {
 /// Sockaddr
 ///
 Sockaddr::Sockaddr() {
-  memset(&addr_, 0, sizeof(addr_));
-  addr_.sin_family = AF_INET;
-  addr_.sin_addr.s_addr = INADDR_ANY;
+  set_length(0);
+}
+
+Sockaddr::Sockaddr(const Sockaddr& other) noexcept {
+  *this = other;
+}
+
+Sockaddr::~Sockaddr() {
+  ASAN_UNPOISON_MEMORY_REGION(&storage_, sizeof(storage_));
+}
+
+Sockaddr Sockaddr::Wildcard() {
+  struct sockaddr_in addr;
+  memset(&addr, 0, sizeof(addr));
+  addr.sin_family = AF_INET;
+  addr.sin_addr.s_addr = INADDR_ANY;
+  return Sockaddr(addr);
+}
+
+Sockaddr& Sockaddr::operator=(const Sockaddr& other) noexcept {
+  if (&other == this) return *this;
+  set_length(other.len_);
+  memcpy(&storage_, &other.storage_, len_);
+  return *this;
 }
 
 Sockaddr::Sockaddr(const struct sockaddr_in& addr) {
-  memcpy(&addr_, &addr, sizeof(struct sockaddr_in));
+  DCHECK_EQ(AF_INET, addr.sin_family);
+  set_length(sizeof(addr));
+  memcpy(&storage_.in, &addr, sizeof(struct sockaddr_in));
 }
 
 Status Sockaddr::ParseString(const std::string& s, uint16_t default_port) {
   HostPort hp;
   RETURN_NOT_OK(hp.ParseString(s, default_port));
 
-  if (inet_pton(AF_INET, hp.host().c_str(), &addr_.sin_addr) != 1) {
+  struct in_addr addr;
+  if (inet_pton(AF_INET, hp.host().c_str(), &addr) != 1) {
     return Status::InvalidArgument("Invalid IP address", hp.host());
   }
+  set_length(sizeof(struct sockaddr_in));
+  storage_.in.sin_family = AF_INET;
+  storage_.in.sin_addr = addr;
   set_port(hp.port());
   return Status::OK();
 }
 
 Sockaddr& Sockaddr::operator=(const struct sockaddr_in &addr) {
-  memcpy(&addr_, &addr, sizeof(struct sockaddr_in));
+  set_length(sizeof(addr));
+  memcpy(&storage_, &addr, sizeof(addr));
+  DCHECK_EQ(family(), AF_INET);
   return *this;
 }
 
 bool Sockaddr::operator==(const Sockaddr& other) const {
-  return memcmp(&other.addr_, &addr_, sizeof(addr_)) == 0;
+  return BytewiseCompare(*this, other) == 0;
+}
+int Sockaddr::BytewiseCompare(const Sockaddr& a, const Sockaddr& b) {
+  Slice a_slice(reinterpret_cast<const uint8_t*>(&a.storage_), a.len_);
+  Slice b_slice(reinterpret_cast<const uint8_t*>(&b.storage_), b.len_);
+  return a_slice.compare(b_slice);
 }
 
-bool Sockaddr::operator<(const Sockaddr &rhs) const {
-  return addr_.sin_addr.s_addr < rhs.addr_.sin_addr.s_addr;
+void Sockaddr::set_length(socklen_t len) {
+  DCHECK(len == 0 || len >= sizeof(sa_family_t));
+  DCHECK_LE(len, sizeof(storage_));
+  len_ = len;
+  ASAN_UNPOISON_MEMORY_REGION(&storage_, len_);
+  ASAN_POISON_MEMORY_REGION(reinterpret_cast<uint8_t*>(&storage_) + len_,
+                            sizeof(storage_) - len_);
 }
 
 uint32_t Sockaddr::HashCode() const {
-  uint32_t hash = Hash32NumWithSeed(addr_.sin_addr.s_addr, 0);
-  hash = Hash32NumWithSeed(addr_.sin_port, hash);
-  return hash;
+  return HashStringThoroughly(reinterpret_cast<const char*>(&storage_), addrlen());
 }
 
 void Sockaddr::set_port(int port) {
-  addr_.sin_port = htons(port);
+  DCHECK_EQ(family(), AF_INET);
+  DCHECK_GE(port, 0);
+  DCHECK_LE(port, std::numeric_limits<uint16_t>::max());
+  storage_.in.sin_port = htons(port);
 }
 
 int Sockaddr::port() const {
-  return ntohs(addr_.sin_port);
+  DCHECK_EQ(family(), AF_INET);
+  return ntohs(storage_.in.sin_port);
 }
 
 std::string Sockaddr::host() const {
-  return HostPort::AddrToString(addr_.sin_addr.s_addr);
+  DCHECK_EQ(family(), AF_INET);
+  return HostPort::AddrToString(storage_.in.sin_addr.s_addr);
 }
 
-const struct sockaddr_in& Sockaddr::addr() const {
-  return addr_;
+const struct sockaddr_in& Sockaddr::ipv4_addr() const {
+  DCHECK_EQ(family(), AF_INET);
+  return storage_.in;
 }
 
 std::string Sockaddr::ToString() const {
@@ -102,23 +150,24 @@ std::string Sockaddr::ToString() const {
 }
 
 bool Sockaddr::IsWildcard() const {
-  return addr_.sin_addr.s_addr == 0;
+  DCHECK_EQ(family(), AF_INET);
+  return storage_.in.sin_addr.s_addr == 0;
 }
 
 bool Sockaddr::IsAnyLocalAddress() const {
-  return HostPort::IsLoopback(addr_.sin_addr.s_addr);
+  DCHECK_EQ(family(), AF_INET);
+  return HostPort::IsLoopback(storage_.in.sin_addr.s_addr);
 }
 
 Status Sockaddr::LookupHostname(string* hostname) const {
   char host[NI_MAXHOST];
   int flags = 0;
 
+  auto* addr = reinterpret_cast<const sockaddr*>(&storage_);
   int rc = 0;
   LOG_SLOW_EXECUTION(WARNING, 200,
                      Substitute("DNS reverse-lookup for $0", ToString())) {
-    rc = getnameinfo((struct sockaddr *) &addr_, sizeof(sockaddr_in),
-                     host, NI_MAXHOST,
-                     nullptr, 0, flags);
+    rc = getnameinfo(addr, addrlen(), host, NI_MAXHOST, nullptr, 0, flags);
   }
   if (PREDICT_FALSE(rc != 0)) {
     if (rc == EAI_SYSTEM) {
diff --git a/src/kudu/util/net/sockaddr.h b/src/kudu/util/net/sockaddr.h
index f61cc9d..99370dc 100644
--- a/src/kudu/util/net/sockaddr.h
+++ b/src/kudu/util/net/sockaddr.h
@@ -18,51 +18,100 @@
 #define KUDU_UTIL_NET_SOCKADDR_H
 
 #include <netinet/in.h>
+#include <sys/socket.h>
+#include <sys/un.h>
 
 #include <cstdint>
 #include <string>
 #include <type_traits>
 #include <vector>
 
+#include <glog/logging.h>
+
 #include "kudu/util/status.h"
 
 namespace kudu {
 
-///
 /// Represents a sockaddr.
 ///
-/// Currently only IPv4 is implemented.  When IPv6 and UNIX domain are
-/// implemented, this should become an abstract base class and those should be
-/// multiple implementations.
-///
+/// Typically this wraps a sockaddr_in, but in the future will be extended to support
+/// IPv6 and Unix sockets.
 class Sockaddr {
  public:
+  // Create an uninitialized socket. This instance must be assigned to before usage.
   Sockaddr();
+  ~Sockaddr();
+
+  // Copy constructor.
+  Sockaddr(const Sockaddr& other) noexcept;
+
+  // Construct from an IPv4 socket address.
   explicit Sockaddr(const struct sockaddr_in &addr);
 
-  // Parse a string IP address of the form "A.B.C.D:port", storing the result
-  // in this Sockaddr object. If no ':port' is specified, uses 'default_port'.
-  // Note that this function will not handle resolving hostnames.
-  //
-  // Returns a bad Status if the input is malformed.
-  Status ParseString(const std::string& s, uint16_t default_port);
+  // Return the IPv4 wildcard address.
+  static Sockaddr Wildcard();
 
+  // Assignment operators.
+  Sockaddr& operator=(const Sockaddr& other) noexcept;
   Sockaddr& operator=(const struct sockaddr_in &addr);
 
+  // Compare two addresses for equality. To be equal, the addresses must have the same
+  // family and have the same bytewise representation. Two uninitialized addresses
+  // are equal to each other but not to any other address.
   bool operator==(const Sockaddr& other) const;
+  uint32_t HashCode() const;
 
-  // Compare the endpoints of two sockaddrs.
-  // The port number is ignored in this comparison.
-  bool operator<(const Sockaddr &rhs) const;
+  // Compare two addresses bytewise.
+  //
+  // Returns a negative, zero, or positive integer to indicate if 'a' is less than,
+  // equal to, or greater than 'b'.
+  //
+  // Addresses of different families (or uninitialized addresses) can be safely compared.
+  // The comparison result has no semantic meaning but is deterministic.
+  static int BytewiseCompare(const Sockaddr& a, const Sockaddr& b);
 
-  uint32_t HashCode() const;
+  static bool BytewiseLess(const Sockaddr& a, const Sockaddr& b) {
+    return BytewiseCompare(a, b) < 0;
+  }
+
+  bool is_initialized() const {
+    return len_ != 0;
+  }
+
+  // Parse a string IPv4 address of the form "A.B.C.D:port", storing the result
+  // in this Sockaddr object. If no ':port' is specified, uses 'default_port'.
+  // Note that this function will not handle resolving hostnames.
+  //
+  // Returns a bad Status if the input is malformed.
+  Status ParseString(const std::string& s, uint16_t default_port);
 
   // Returns the dotted-decimal string '1.2.3.4' of the host component of this address.
   std::string host() const;
 
+  // Set the IP port for this address.
+  // REQUIRES: is an IPv4 address.
   void set_port(int port);
+
+  // Get the IP port for this address.
+  // REQUIRES: is an IPv4 address.
   int port() const;
-  const struct sockaddr_in& addr() const;
+
+
+  const struct sockaddr* addr() const {
+    return reinterpret_cast<const sockaddr*>(&storage_);
+  }
+
+  const struct sockaddr_in& ipv4_addr() const;
+
+  socklen_t addrlen() const {
+    DCHECK(is_initialized());
+    return len_;
+  }
+
+  sa_family_t family() const {
+    DCHECK(is_initialized());
+    return storage_.generic.ss_family;
+  }
 
   // Returns the stringified address in '1.2.3.4:<port>' format.
   std::string ToString() const;
@@ -82,7 +131,24 @@ class Sockaddr {
 
   // the default auto-generated copy constructor is fine here
  private:
-  struct sockaddr_in addr_;
+  // Set the length of the internal storage to 'len' and adjust ASAN poisoning
+  // appropriately.
+  void set_length(socklen_t len);
+
+  // The length of valid bytes in storage_.
+  //
+  // For an uninitialized socket, this will be 0. Otherwise, this is guaranteed
+  // to be at least sizeof(sa_family_t).
+  //
+  // For some address types (ipv4, ipv6) this is fixed to the size of the appropriate
+  // struct. For other types (unix) this is variable-length depending on the length of
+  // the path.
+  socklen_t len_ = 0;
+  // Internal storage. This is a tagged union based on 'generic.ss_family'.
+  union {
+    struct sockaddr_storage generic;
+    struct sockaddr_in in;
+  } storage_;
 };
 
 } // namespace kudu
diff --git a/src/kudu/util/net/socket.cc b/src/kudu/util/net/socket.cc
index 018b16c..b0ce530 100644
--- a/src/kudu/util/net/socket.cc
+++ b/src/kudu/util/net/socket.cc
@@ -310,10 +310,8 @@ bool Socket::IsLoopbackConnection() const {
 }
 
 Status Socket::Bind(const Sockaddr& bind_addr) {
-  struct sockaddr_in addr = bind_addr.addr();
-
   DCHECK_GE(fd_, 0);
-  if (PREDICT_FALSE(::bind(fd_, (struct sockaddr*) &addr, sizeof(addr)))) {
+  if (PREDICT_FALSE(::bind(fd_, bind_addr.addr(), bind_addr.addrlen()))) {
     int err = errno;
     Status s = Status::NetworkError(
         strings::Substitute("error binding socket to $0: $1",
@@ -384,12 +382,9 @@ Status Socket::Connect(const Sockaddr &remote) {
     RETURN_NOT_OK(BindForOutgoingConnection());
   }
 
-  struct sockaddr_in addr;
-  memcpy(&addr, &remote.addr(), sizeof(sockaddr_in));
   DCHECK_GE(fd_, 0);
   int ret;
-  RETRY_ON_EINTR(ret, ::connect(
-      fd_, reinterpret_cast<const struct sockaddr*>(&addr), sizeof(addr)));
+  RETRY_ON_EINTR(ret, ::connect(fd_, remote.addr(), remote.addrlen()));
   if (ret < 0) {
     int err = errno;
     return Status::NetworkError("connect(2) error", ErrnoToString(err), err);
@@ -517,13 +512,14 @@ Status Socket::Recv(uint8_t *buf, int32_t amt, int32_t *nread) {
   RETRY_ON_EINTR(res, recv(fd_, buf, amt, 0));
   if (res <= 0) {
     Sockaddr remote;
-    GetPeerAddress(&remote);
+    Status get_addr_status = GetPeerAddress(&remote);
+    string remote_str = get_addr_status.ok() ? remote.ToString() : "unknown peer";
     if (res == 0) {
-      string error_message = Substitute("recv got EOF from $0", remote.ToString());
+      string error_message = Substitute("recv got EOF from $0", remote_str);
       return Status::NetworkError(error_message, Slice(), ESHUTDOWN);
     }
     int err = errno;
-    string error_message = Substitute("recv error from $0", remote.ToString());
+    string error_message = Substitute("recv error from $0", remote_str);
     return Status::NetworkError(error_message, ErrnoToString(err), err);
   }
   *nread = res;