network: remove remaining socket apis from address (#11295)

Move remaining network stack/socket layer interactions from addresses to sockets. Signed-off-by: Florin Coras <[email protected]>
envoyproxy · May 28, 2020 · 8713125 · 8713125
1 parent 2fb3921
commit 8713125
Show file tree

Hide file tree

Showing 23 changed files with 284 additions and 178 deletions.
diff --git a/include/envoy/network/address.h b/include/envoy/network/address.h
@@ -98,6 +98,23 @@ class Ip {
   virtual IpVersion version() const PURE;
 };
 
+/**
+ * Interface for a generic Pipe address
+ */
+class Pipe {
+public:
+  virtual ~Pipe() = default;
+  /**
+   * @return abstract namespace flag
+   */
+  virtual bool abstractNamespace() const PURE;
+
+  /**
+   * @return pipe mode
+   */
+  virtual mode_t mode() const PURE;
+};
+
 enum class Type { Ip, Pipe };
 enum class SocketType { Stream, Datagram };
 
@@ -138,27 +155,24 @@ class Instance {
   virtual const std::string& logicalName() const PURE;
 
   /**
-   * Bind a socket to this address. The socket should have been created with a call to socket() on
-   * an Instance of the same address family.
-   * @param fd supplies the platform socket handle.
-   * @return a Api::SysCallIntResult with rc_ = 0 for success and rc_ = -1 for failure. If the call
-   *   is successful, errno_ shouldn't be used.
+   * @return the IP address information IFF type() == Type::Ip, otherwise nullptr.
    */
-  virtual Api::SysCallIntResult bind(os_fd_t fd) const PURE;
+  virtual const Ip* ip() const PURE;
 
   /**
-   * Connect a socket to this address. The socket should have been created with a call to socket()
-   * on this object.
-   * @param fd supplies the platform socket handle.
-   * @return a Api::SysCallIntResult with rc_ = 0 for success and rc_ = -1 for failure. If the call
-   *   is successful, errno_ shouldn't be used.
+   * @return the pipe address information IFF type() == Type::Pipe, otherwise nullptr.
    */
-  virtual Api::SysCallIntResult connect(os_fd_t fd) const PURE;
+  virtual const Pipe* pipe() const PURE;
 
   /**
-   * @return the IP address information IFF type() == Type::Ip, otherwise nullptr.
+   * @return the underlying structure wherein the address is stored
    */
-  virtual const Ip* ip() const PURE;
+  virtual const sockaddr* sockAddr() const PURE;
+
+  /**
+   * @return length of the address container
+   */
+  virtual socklen_t sockAddrLen() const PURE;
 
   /**
    * @return the type of address.

diff --git a/include/envoy/network/socket.h b/include/envoy/network/socket.h
@@ -88,6 +88,31 @@ class Socket {
    */
   virtual bool isOpen() const PURE;
 
+  /**
+   * Bind a socket to this address. The socket should have been created with a call to socket()
+   * @param address address to bind the socket to.
+   * @return a Api::SysCallIntResult with rc_ = 0 for success and rc_ = -1 for failure. If the call
+   *   is successful, errno_ shouldn't be used.
+   */
+  virtual Api::SysCallIntResult bind(const Address::InstanceConstSharedPtr address) PURE;
+
+  /**
+   * Listen on bound socket.
+   * @param backlog maximum number of pending connections for listener
+   * @return a Api::SysCallIntResult with rc_ = 0 for success and rc_ = -1 for failure. If the call
+   *   is successful, errno_ shouldn't be used.
+   */
+  virtual Api::SysCallIntResult listen(int backlog) PURE;
+
+  /**
+   * Connect a socket to this address. The socket should have been created with a call to socket()
+   * on this object.
+   * @param address remote address to connect to.
+   * @return a Api::SysCallIntResult with rc_ = 0 for success and rc_ = -1 for failure. If the call
+   *   is successful, errno_ shouldn't be used.
+   */
+  virtual Api::SysCallIntResult connect(const Address::InstanceConstSharedPtr address) PURE;
+
   /**
    * Visitor class for setting socket options.
    */

diff --git a/source/common/network/address_impl.cc b/source/common/network/address_impl.cc
@@ -148,15 +148,6 @@ bool Ipv4Instance::operator==(const Instance& rhs) const {
           (ip_.port() == rhs_casted->ip_.port()));
 }
 
-Api::SysCallIntResult Ipv4Instance::bind(os_fd_t fd) const {
-  return Api::OsSysCallsSingleton::get().bind(
-      fd, reinterpret_cast<const sockaddr*>(&ip_.ipv4_.address_), sizeof(ip_.ipv4_.address_));
-}
-
-Api::SysCallIntResult Ipv4Instance::connect(os_fd_t fd) const {
-  return Api::OsSysCallsSingleton::get().connect(fd, sockAddr(), sockAddrLen());
-}
-
 std::string Ipv4Instance::sockaddrToString(const sockaddr_in& addr) {
   static constexpr size_t BufferSize = 16; // enough space to hold an IPv4 address in string form
   char str[BufferSize];
@@ -236,15 +227,6 @@ bool Ipv6Instance::operator==(const Instance& rhs) const {
           (ip_.port() == rhs_casted->ip_.port()));
 }
 
-Api::SysCallIntResult Ipv6Instance::bind(os_fd_t fd) const {
-  return Api::OsSysCallsSingleton::get().bind(
-      fd, reinterpret_cast<const sockaddr*>(&ip_.ipv6_.address_), sizeof(ip_.ipv6_.address_));
-}
-
-Api::SysCallIntResult Ipv6Instance::connect(os_fd_t fd) const {
-  return Api::OsSysCallsSingleton::get().connect(fd, sockAddr(), sockAddrLen());
-}
-
 PipeInstance::PipeInstance(const sockaddr_un* address, socklen_t ss_len, mode_t mode)
     : InstanceBase(Type::Pipe) {
   if (address->sun_path[0] == '\0') {
@@ -253,31 +235,31 @@ PipeInstance::PipeInstance(const sockaddr_un* address, socklen_t ss_len, mode_t
 #endif
     RELEASE_ASSERT(static_cast<unsigned int>(ss_len) >= offsetof(struct sockaddr_un, sun_path) + 1,
                    "");
-    abstract_namespace_ = true;
-    address_length_ = ss_len - offsetof(struct sockaddr_un, sun_path);
+    pipe_.abstract_namespace_ = true;
+    pipe_.address_length_ = ss_len - offsetof(struct sockaddr_un, sun_path);
   }
-  address_ = *address;
-  if (abstract_namespace_) {
+  pipe_.address_ = *address;
+  if (pipe_.abstract_namespace_) {
     if (mode != 0) {
       throw EnvoyException("Cannot set mode for Abstract AF_UNIX sockets");
     }
     // Replace all null characters with '@' in friendly_name_.
-    friendly_name_ =
-        friendlyNameFromAbstractPath(absl::string_view(address_.sun_path, address_length_));
+    friendly_name_ = friendlyNameFromAbstractPath(
+        absl::string_view(pipe_.address_.sun_path, pipe_.address_length_));
   } else {
     friendly_name_ = address->sun_path;
   }
-  this->mode = mode;
+  pipe_.mode_ = mode;
 }
 
 PipeInstance::PipeInstance(const std::string& pipe_path, mode_t mode) : InstanceBase(Type::Pipe) {
-  if (pipe_path.size() >= sizeof(address_.sun_path)) {
+  if (pipe_path.size() >= sizeof(pipe_.address_.sun_path)) {
     throw EnvoyException(
         fmt::format("Path \"{}\" exceeds maximum UNIX domain socket path size of {}.", pipe_path,
-                    sizeof(address_.sun_path)));
+                    sizeof(pipe_.address_.sun_path)));
   }
-  memset(&address_, 0, sizeof(address_));
-  address_.sun_family = AF_UNIX;
+  memset(&pipe_.address_, 0, sizeof(pipe_.address_));
+  pipe_.address_.sun_family = AF_UNIX;
   if (pipe_path[0] == '@') {
     // This indicates an abstract namespace.
     // In this case, null bytes in the name have no special significance, and so we copy all
@@ -290,48 +272,27 @@ PipeInstance::PipeInstance(const std::string& pipe_path, mode_t mode) : Instance
     if (mode != 0) {
       throw EnvoyException("Cannot set mode for Abstract AF_UNIX sockets");
     }
-    abstract_namespace_ = true;
-    address_length_ = pipe_path.size();
-    memcpy(&address_.sun_path[0], pipe_path.data(), pipe_path.size());
-    address_.sun_path[0] = '\0';
-    address_.sun_path[pipe_path.size()] = '\0';
-    friendly_name_ =
-        friendlyNameFromAbstractPath(absl::string_view(address_.sun_path, address_length_));
+    pipe_.abstract_namespace_ = true;
+    pipe_.address_length_ = pipe_path.size();
+    memcpy(&pipe_.address_.sun_path[0], pipe_path.data(), pipe_path.size());
+    pipe_.address_.sun_path[0] = '\0';
+    pipe_.address_.sun_path[pipe_path.size()] = '\0';
+    friendly_name_ = friendlyNameFromAbstractPath(
+        absl::string_view(pipe_.address_.sun_path, pipe_.address_length_));
   } else {
     // Throw an error if the pipe path has an embedded null character.
     if (pipe_path.size() != strlen(pipe_path.c_str())) {
       throw EnvoyException("UNIX domain socket pathname contains embedded null characters");
     }
-    StringUtil::strlcpy(&address_.sun_path[0], pipe_path.c_str(), sizeof(address_.sun_path));
-    friendly_name_ = address_.sun_path;
+    StringUtil::strlcpy(&pipe_.address_.sun_path[0], pipe_path.c_str(),
+                        sizeof(pipe_.address_.sun_path));
+    friendly_name_ = pipe_.address_.sun_path;
   }
-  this->mode = mode;
+  pipe_.mode_ = mode;
 }
 
 bool PipeInstance::operator==(const Instance& rhs) const { return asString() == rhs.asString(); }
 
-Api::SysCallIntResult PipeInstance::bind(os_fd_t fd) const {
-  if (!abstract_namespace_) {
-    // Try to unlink an existing filesystem object at the requested path. Ignore
-    // errors -- it's fine if the path doesn't exist, and if it exists but can't
-    // be unlinked then `::bind()` will generate a reasonable errno.
-    unlink(address_.sun_path);
-  }
-  auto& os_syscalls = Api::OsSysCallsSingleton::get();
-  auto bind_result = os_syscalls.bind(fd, sockAddr(), sockAddrLen());
-  if (mode != 0 && !abstract_namespace_ && bind_result.rc_ == 0) {
-    auto set_permissions = os_syscalls.chmod(address_.sun_path, mode);
-    if (set_permissions.rc_ != 0) {
-      throw EnvoyException(absl::StrCat("Failed to create socket with mode ", mode));
-    }
-  }
-  return bind_result;
-}
-
-Api::SysCallIntResult PipeInstance::connect(os_fd_t fd) const {
-  return Api::OsSysCallsSingleton::get().connect(fd, sockAddr(), sockAddrLen());
-}
-
 } // namespace Address
 } // namespace Network
 } // namespace Envoy
diff --git a/source/common/network/address_impl.h b/source/common/network/address_impl.h
@@ -8,7 +8,6 @@
 
 #include "envoy/common/platform.h"
 #include "envoy/network/address.h"
-#include "envoy/network/io_handle.h"
 
 namespace Envoy {
 namespace Network {
@@ -38,9 +37,6 @@ class InstanceBase : public Instance {
   const std::string& logicalName() const override { return asString(); }
   Type type() const override { return type_; }
 
-  virtual const sockaddr* sockAddr() const PURE;
-  virtual socklen_t sockAddrLen() const PURE;
-
 protected:
   InstanceBase(Type type) : type_(type) {}
 
@@ -78,11 +74,8 @@ class Ipv4Instance : public InstanceBase {
 
   // Network::Address::Instance
   bool operator==(const Instance& rhs) const override;
-  Api::SysCallIntResult bind(os_fd_t fd) const override;
-  Api::SysCallIntResult connect(os_fd_t fd) const override;
   const Ip* ip() const override { return &ip_; }
-
-  // Network::Address::InstanceBase
+  const Pipe* pipe() const override { return nullptr; }
   const sockaddr* sockAddr() const override {
     return reinterpret_cast<const sockaddr*>(&ip_.ipv4_.address_);
   }
@@ -151,11 +144,8 @@ class Ipv6Instance : public InstanceBase {
 
   // Network::Address::Instance
   bool operator==(const Instance& rhs) const override;
-  Api::SysCallIntResult bind(os_fd_t fd) const override;
-  Api::SysCallIntResult connect(os_fd_t fd) const override;
   const Ip* ip() const override { return &ip_; }
-
-  // Network::Address::InstanceBase
+  const Pipe* pipe() const override { return nullptr; }
   const sockaddr* sockAddr() const override {
     return reinterpret_cast<const sockaddr*>(&ip_.ipv6_.address_);
   }
@@ -214,25 +204,32 @@ class PipeInstance : public InstanceBase {
 
   // Network::Address::Instance
   bool operator==(const Instance& rhs) const override;
-  Api::SysCallIntResult bind(os_fd_t fd) const override;
-  Api::SysCallIntResult connect(os_fd_t fd) const override;
   const Ip* ip() const override { return nullptr; }
-
-  // Network::Address::InstanceBase
-  const sockaddr* sockAddr() const override { return reinterpret_cast<const sockaddr*>(&address_); }
+  const Pipe* pipe() const override { return &pipe_; }
+  const sockaddr* sockAddr() const override {
+    return reinterpret_cast<const sockaddr*>(&pipe_.address_);
+  }
   socklen_t sockAddrLen() const override {
-    if (abstract_namespace_) {
-      return offsetof(struct sockaddr_un, sun_path) + address_length_;
+    if (pipe_.abstract_namespace_) {
+      return offsetof(struct sockaddr_un, sun_path) + pipe_.address_length_;
     }
-    return sizeof(address_);
+    return sizeof(pipe_.address_);
   }
 
 private:
-  sockaddr_un address_;
-  // For abstract namespaces.
-  bool abstract_namespace_{false};
-  uint32_t address_length_{0};
-  mode_t mode{0};
+  struct PipeHelper : public Pipe {
+
+    bool abstractNamespace() const override { return abstract_namespace_; }
+    mode_t mode() const override { return mode_; }
+
+    sockaddr_un address_;
+    // For abstract namespaces.
+    bool abstract_namespace_{false};
+    uint32_t address_length_{0};
+    mode_t mode_{0};
+  };
+
+  PipeHelper pipe_;
 };
 
 } // namespace Address

diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc
@@ -734,16 +734,18 @@ ClientConnectionImpl::ClientConnectionImpl(
       file_event_->activate(Event::FileReadyType::Write);
       return;
     }
-    const Network::Address::Instance* source_to_use = source_address.get();
+
+    const Network::Address::InstanceConstSharedPtr* source = &source_address;
+
     if (socket_->localAddress()) {
-      source_to_use = socket_->localAddress().get();
+      source = &socket_->localAddress();
     }
 
-    if (source_to_use != nullptr) {
-      const Api::SysCallIntResult result = source_to_use->bind(ioHandle().fd());
+    if (*source != nullptr) {
+      Api::SysCallIntResult result = socket_->bind(*source);
       if (result.rc_ < 0) {
         // TODO(lizan): consider add this error into transportFailureReason.
-        ENVOY_LOG_MISC(debug, "Bind failure. Failed to bind to {}: {}", source_to_use->asString(),
+        ENVOY_LOG_MISC(debug, "Bind failure. Failed to bind to {}: {}", source->get()->asString(),
                        strerror(result.errno_));
         bind_error_ = true;
         // Set a special error state to ensure asynchronous close to give the owner of the
@@ -759,7 +761,7 @@ ClientConnectionImpl::ClientConnectionImpl(
 
 void ClientConnectionImpl::connect() {
   ENVOY_CONN_LOG(debug, "connecting to {}", *this, socket_->remoteAddress()->asString());
-  const Api::SysCallIntResult result = socket_->remoteAddress()->connect(ioHandle().fd());
+  const Api::SysCallIntResult result = socket_->connect(socket_->remoteAddress());
   if (result.rc_ == 0) {
     // write will become ready.
     ASSERT(connecting_);
@@ -780,6 +782,7 @@ void ClientConnectionImpl::connect() {
 
   // The local address can only be retrieved for IP connections. Other
   // types, such as UDS, don't have a notion of a local address.
+  // TODO(fcoras) move to SocketImpl?
   if (socket_->remoteAddress()->type() == Address::Type::Ip) {
     socket_->setLocalAddress(SocketInterface::addressFromFd(ioHandle().fd()));
   }