IoHandle readv and writev

include/envoy/api/io_error.h

@@ -9,16 +9,6 @@
 namespace Envoy {
 namespace Api {
 
-class IoError;
-
-// IoErrorCode::Again is used frequently. Define it to be a distinguishable address to avoid
-// frequent memory allocation of IoError instance.
-// If this is used, IoCallResult has to be instantiated with a deleter that does not
-// deallocate memory for this error.
-// TODO: This is probably not the best way to avoid allocations in the case of
-// EAGAIN. This will be fixed as a part of #6037.
-#define ENVOY_ERROR_AGAIN reinterpret_cast<Api::IoError*>(0x01)
-
 /**
  * Base class for any I/O error.
  */
@@ -35,32 +25,13 @@ class IoError {
     InProgress,
     // Permission denied.
     Permission,
-    // Bad handle
-    BadHandle,
     // Other error codes cannot be mapped to any one above in getErrorCode().
     UnknownError
   };
   virtual ~IoError() {}
 
-  // Map platform specific error into IoErrorCode.
-  // Needed to hide errorCode() in case of ENVOY_ERROR_AGAIN.
-  static IoErrorCode getErrorCode(const IoError& err) {
-    if (&err == ENVOY_ERROR_AGAIN) {
-      return IoErrorCode::Again;
-    }
-    return err.errorCode();
-  }
-
-  static std::string getErrorDetails(const IoError& err) {
-    if (&err == ENVOY_ERROR_AGAIN) {
-      return "Try again later";
-    }
-    return err.errorDetails();
-  }
-
-protected:
-  virtual IoErrorCode errorCode() const PURE;
-  virtual std::string errorDetails() const PURE;
+  virtual IoErrorCode getErrorCode() const PURE;
+  virtual std::string getErrorDetails() const PURE;
 };
 
 using IoErrorDeleterType = void (*)(IoError*);
@@ -69,19 +40,39 @@ using IoErrorPtr = std::unique_ptr<IoError, IoErrorDeleterType>;
 /**
  * Basic type for return result which has a return code and error code defined
  * according to different implementations.
+ * If the call succeeds, ok() should return true and |rc_| is valid. Otherwise |err_|
+ * can be passed into IoError::getErrorCode() to extract the error. In this
+ * case, |rc_| is invalid.
  */
-template <typename T> struct IoCallResult {
-  IoCallResult(T rc, IoErrorPtr err) : rc_(rc), err_(std::move(err)) {}
+template <typename ReturnValue> struct IoCallResult {
+  IoCallResult(ReturnValue rc, IoErrorPtr err) : rc_(rc), err_(std::move(err)) {}
 
-  IoCallResult(IoCallResult<T>&& result) : rc_(result.rc_), err_(std::move(result.err_)) {}
+  IoCallResult(IoCallResult<ReturnValue>&& result)
+      : rc_(result.rc_), err_(std::move(result.err_)) {}
 
   virtual ~IoCallResult() {}
 
-  T rc_;
+  IoCallResult& operator=(IoCallResult&& result) {
+    rc_ = result.rc_;
+    err_ = std::move(result.err_);
+    return *this;
+  }
+
+  /**
+   * @return true if the call succeeds.
+   */
+  bool ok() const { return err_ == nullptr; }
+
+  // TODO(danzh): rename it to be more meaningful, i.e. return_value_.
+  ReturnValue rc_;
   IoErrorPtr err_;
 };
 
-using IoCallUintResult = IoCallResult<uint64_t>;
+using IoCallUint64Result = IoCallResult<uint64_t>;
+
+inline Api::IoCallUint64Result ioCallUint64ResultNoError() {
+  return IoCallUint64Result(0, IoErrorPtr(nullptr, [](IoError*) {}));
+}
 
 } // namespace Api
 } // namespace Envoy

include/envoy/buffer/BUILD

@@ -13,6 +13,7 @@ envoy_cc_library(
     hdrs = ["buffer.h"],
     deps = [
         "//include/envoy/api:os_sys_calls_interface",
+        "//include/envoy/network:io_handle_interface",
         "//source/common/common:byte_order_lib",
     ],
 )

include/envoy/buffer/buffer.h

@@ -8,6 +8,7 @@
 #include "envoy/api/os_sys_calls.h"
 #include "envoy/common/exception.h"
 #include "envoy/common/pure.h"
+#include "envoy/network/io_handle.h"
 
 #include "common/common/byte_order.h"
 
@@ -165,12 +166,12 @@ class Instance {
 
   /**
    * Read from a file descriptor directly into the buffer.
-   * @param fd supplies the descriptor to read from.
+   * @param io_handle supplies the io handle to read from.
    * @param max_length supplies the maximum length to read.
-   * @return a Api::SysCallIntResult with rc_ = the number of bytes read if successful, or rc_ = -1
-   *   for failure. If the call is successful, errno_ shouldn't be used.
+   * @return a IoCallUint64Result with err_ = nullptr and rc_ = the number of bytes
+   * read if successful, or err_ = some IoError for failure. If call failed, rc_ shouldn't be used.
    */
-  virtual Api::SysCallIntResult read(int fd, uint64_t max_length) PURE;
+  virtual Api::IoCallUint64Result read(Network::IoHandle& io_handle, uint64_t max_length) PURE;
 
   /**
    * Reserve space in the buffer.
@@ -198,11 +199,12 @@ class Instance {
 
   /**
    * Write the buffer out to a file descriptor.
-   * @param fd supplies the descriptor to write to.
-   * @return a Api::SysCallIntResult with rc_ = the number of bytes written if successful, or rc_ =
-   * -1 for failure. If the call is successful, errno_ shouldn't be used.
+   * @param io_handle supplies the io_handle to write to.
+   * @return a IoCallUint64Result with err_ = nullptr and rc_ = the number of bytes
+   * written if successful, or err_ = some IoError for failure. If call failed, rc_ shouldn't be
+   * used.
    */
-  virtual Api::SysCallIntResult write(int fd) PURE;
+  virtual Api::IoCallUint64Result write(Network::IoHandle& io_handle) PURE;
 
   /**
    * Copy an integer out of the buffer.

include/envoy/network/io_handle.h

@@ -1,12 +1,17 @@
 #pragma once
 
-#include <memory>
-
 #include "envoy/api/io_error.h"
 #include "envoy/common/pure.h"
 
 namespace Envoy {
+namespace Buffer {
+struct RawSlice;
+} // namespace Buffer
+
 namespace Network {
+namespace Address {
+class Instance;
+} // namespace Address
 
 /**
  * IoHandle: an abstract interface for all I/O operations
@@ -26,12 +31,32 @@ class IoHandle {
   /**
    * Clean up IoHandle resources
    */
-  virtual Api::IoCallUintResult close() PURE;
+  virtual Api::IoCallUint64Result close() PURE;
 
   /**
    * Return true if close() hasn't been called.
    */
   virtual bool isOpen() const PURE;
+
+  /**
+   * Read data into given slices.
+   * @param max_length supplies the maximum length to read.
+   * @param slices points to the output location.
+   * @param num_slice indicates the number of slices |slices| contains.
+   * @return a Api::IoCallUint64Result with err_ = an Api::IoError instance or
+   * err_ = nullptr and rc_ = the bytes read for success.
+   */
+  virtual Api::IoCallUint64Result readv(uint64_t max_length, Buffer::RawSlice* slices,
+                                        uint64_t num_slice) PURE;
+
+  /**
+   * Write the data in slices out.
+   * @param slices points to the location of data to be written.
+   * @param num_slice indicates number of slices |slices| contains.
+   * @return a Api::IoCallUint64Result with err_ = an Api::IoError instance or
+   * err_ = nullptr and rc_ = the bytes written for success.
+   */
+  virtual Api::IoCallUint64Result writev(const Buffer::RawSlice* slices, uint64_t num_slice) PURE;
 };
 
 typedef std::unique_ptr<IoHandle> IoHandlePtr;

source/common/buffer/BUILD

@@ -24,7 +24,6 @@ envoy_cc_library(
     hdrs = ["buffer_impl.h"],
     deps = [
         "//include/envoy/buffer:buffer_interface",
-        "//source/common/api:os_sys_calls_lib",
         "//source/common/common:non_copyable",
         "//source/common/common:stack_array",
         "//source/common/event:libevent_lib",

source/common/buffer/buffer_impl.cc

@@ -3,7 +3,6 @@
 #include <cstdint>
 #include <string>
 
-#include "common/api/os_sys_calls_impl.h"
 #include "common/common/assert.h"
 #include "common/common/stack_array.h"
 
@@ -124,44 +123,30 @@ void OwnedImpl::move(Instance& rhs, uint64_t length) {
   static_cast<LibEventInstance&>(rhs).postProcess();
 }
 
-Api::SysCallIntResult OwnedImpl::read(int fd, uint64_t max_length) {
+Api::IoCallUint64Result OwnedImpl::read(Network::IoHandle& io_handle, uint64_t max_length) {
   if (max_length == 0) {
-    return {0, 0};
+    return Api::ioCallUint64ResultNoError();
   }
   constexpr uint64_t MaxSlices = 2;
   RawSlice slices[MaxSlices];
   const uint64_t num_slices = reserve(max_length, slices, MaxSlices);
-  STACK_ARRAY(iov, iovec, num_slices);
-  uint64_t num_slices_to_read = 0;
-  uint64_t num_bytes_to_read = 0;
-  for (; num_slices_to_read < num_slices && num_bytes_to_read < max_length; num_slices_to_read++) {
-    iov[num_slices_to_read].iov_base = slices[num_slices_to_read].mem_;
-    const size_t slice_length = std::min(slices[num_slices_to_read].len_,
-                                         static_cast<size_t>(max_length - num_bytes_to_read));
-    iov[num_slices_to_read].iov_len = slice_length;
-    num_bytes_to_read += slice_length;
-  }
-  ASSERT(num_slices_to_read <= MaxSlices);
-  ASSERT(num_bytes_to_read <= max_length);
-  auto& os_syscalls = Api::OsSysCallsSingleton::get();
-  const Api::SysCallSizeResult result =
-      os_syscalls.readv(fd, iov.begin(), static_cast<int>(num_slices_to_read));
-  if (result.rc_ < 0) {
-    return {static_cast<int>(result.rc_), result.errno_};
-  }
-  uint64_t num_slices_to_commit = 0;
-  uint64_t bytes_to_commit = result.rc_;
-  ASSERT(bytes_to_commit <= max_length);
-  while (bytes_to_commit != 0) {
-    slices[num_slices_to_commit].len_ =
-        std::min(slices[num_slices_to_commit].len_, static_cast<size_t>(bytes_to_commit));
-    ASSERT(bytes_to_commit >= slices[num_slices_to_commit].len_);
-    bytes_to_commit -= slices[num_slices_to_commit].len_;
-    num_slices_to_commit++;
+  Api::IoCallUint64Result result = io_handle.readv(max_length, slices, num_slices);
+  if (result.ok()) {
+    // Read succeeded.
+    uint64_t num_slices_to_commit = 0;
+    uint64_t bytes_to_commit = result.rc_;
+    ASSERT(bytes_to_commit <= max_length);
+    while (bytes_to_commit != 0) {
+      slices[num_slices_to_commit].len_ =
+          std::min(slices[num_slices_to_commit].len_, static_cast<size_t>(bytes_to_commit));
+      ASSERT(bytes_to_commit >= slices[num_slices_to_commit].len_);
+      bytes_to_commit -= slices[num_slices_to_commit].len_;
+      num_slices_to_commit++;
+    }
+    ASSERT(num_slices_to_commit <= num_slices);
+    commit(slices, num_slices_to_commit);
   }
-  ASSERT(num_slices_to_commit <= num_slices);
-  commit(slices, num_slices_to_commit);
-  return {static_cast<int>(result.rc_), result.errno_};
+  return result;
 }
 
 uint64_t OwnedImpl::reserve(uint64_t length, RawSlice* iovecs, uint64_t num_iovecs) {
@@ -184,28 +169,15 @@ ssize_t OwnedImpl::search(const void* data, uint64_t size, size_t start) const {
   return result_ptr.pos;
 }
 
-Api::SysCallIntResult OwnedImpl::write(int fd) {
+Api::IoCallUint64Result OwnedImpl::write(Network::IoHandle& io_handle) {
   constexpr uint64_t MaxSlices = 16;
   RawSlice slices[MaxSlices];
   const uint64_t num_slices = std::min(getRawSlices(slices, MaxSlices), MaxSlices);
-  STACK_ARRAY(iov, iovec, num_slices);
-  uint64_t num_slices_to_write = 0;
-  for (uint64_t i = 0; i < num_slices; i++) {
-    if (slices[i].mem_ != nullptr && slices[i].len_ != 0) {
-      iov[num_slices_to_write].iov_base = slices[i].mem_;
-      iov[num_slices_to_write].iov_len = slices[i].len_;
-      num_slices_to_write++;
-    }
-  }
-  if (num_slices_to_write == 0) {
-    return {0, 0};
-  }
-  auto& os_syscalls = Api::OsSysCallsSingleton::get();
-  const Api::SysCallSizeResult result = os_syscalls.writev(fd, iov.begin(), num_slices_to_write);
-  if (result.rc_ > 0) {
+  Api::IoCallUint64Result result = io_handle.writev(slices, num_slices);
+  if (result.ok() && result.rc_ > 0) {
     drain(static_cast<uint64_t>(result.rc_));
   }
-  return {static_cast<int>(result.rc_), result.errno_};
+  return result;
 }
 
 OwnedImpl::OwnedImpl() : buffer_(evbuffer_new()) {}

source/common/buffer/buffer_impl.h

@@ -4,6 +4,7 @@
 #include <string>
 
 #include "envoy/buffer/buffer.h"
+#include "envoy/network/io_handle.h"
 
 #include "common/common/non_copyable.h"
 #include "common/event/libevent.h"
@@ -82,10 +83,10 @@ class OwnedImpl : public LibEventInstance {
   void* linearize(uint32_t size) override;
   void move(Instance& rhs) override;
   void move(Instance& rhs, uint64_t length) override;
-  Api::SysCallIntResult read(int fd, uint64_t max_length) override;
+  Api::IoCallUint64Result read(Network::IoHandle& io_handle, uint64_t max_length) override;
   uint64_t reserve(uint64_t length, RawSlice* iovecs, uint64_t num_iovecs) override;
   ssize_t search(const void* data, uint64_t size, size_t start) const override;
-  Api::SysCallIntResult write(int fd) override;
+  Api::IoCallUint64Result write(Network::IoHandle& io_handle) override;
   void postProcess() override {}
   std::string toString() const override;
 

source/common/buffer/watermark_buffer.cc

@@ -50,8 +50,8 @@ void WatermarkBuffer::move(Instance& rhs, uint64_t length) {
   checkHighWatermark();
 }
 
-Api::SysCallIntResult WatermarkBuffer::read(int fd, uint64_t max_length) {
-  Api::SysCallIntResult result = OwnedImpl::read(fd, max_length);
+Api::IoCallUint64Result WatermarkBuffer::read(Network::IoHandle& io_handle, uint64_t max_length) {
+  Api::IoCallUint64Result result = OwnedImpl::read(io_handle, max_length);
   checkHighWatermark();
   return result;
 }
@@ -62,8 +62,8 @@ uint64_t WatermarkBuffer::reserve(uint64_t length, RawSlice* iovecs, uint64_t nu
   return bytes_reserved;
 }
 
-Api::SysCallIntResult WatermarkBuffer::write(int fd) {
-  Api::SysCallIntResult result = OwnedImpl::write(fd);
+Api::IoCallUint64Result WatermarkBuffer::write(Network::IoHandle& io_handle) {
+  Api::IoCallUint64Result result = OwnedImpl::write(io_handle);
   checkLowWatermark();
   return result;
 }

source/common/buffer/watermark_buffer.h

@@ -30,9 +30,9 @@ class WatermarkBuffer : public OwnedImpl {
   void drain(uint64_t size) override;
   void move(Instance& rhs) override;
   void move(Instance& rhs, uint64_t length) override;
-  Api::SysCallIntResult read(int fd, uint64_t max_length) override;
+  Api::IoCallUint64Result read(Network::IoHandle& io_handle, uint64_t max_length) override;
   uint64_t reserve(uint64_t length, RawSlice* iovecs, uint64_t num_iovecs) override;
-  Api::SysCallIntResult write(int fd) override;
+  Api::IoCallUint64Result write(Network::IoHandle& io_handle) override;
   void postProcess() override { checkLowWatermark(); }
 
   void setWatermarks(uint32_t watermark) { setWatermarks(watermark / 2, watermark); }

source/common/network/BUILD

@@ -102,14 +102,27 @@ envoy_cc_library(
     ],
 )
 
+envoy_cc_library(
+    name = "io_socket_error_lib",
+    srcs = ["io_socket_error_impl.cc"],
+    hdrs = ["io_socket_error_impl.h"],
+    deps = [
+        "//include/envoy/api:io_error_interface",
+        "//source/common/common:assert_lib",
+    ],
+)
+
 envoy_cc_library(
     name = "io_socket_handle_lib",
     srcs = ["io_socket_handle_impl.cc"],
     hdrs = ["io_socket_handle_impl.h"],
     deps = [
+        ":io_socket_error_lib",
+        "//include/envoy/buffer:buffer_interface",
         "//include/envoy/network:io_handle_interface",
         "//source/common/api:os_sys_calls_lib",
         "//source/common/common:assert_lib",
+        "//source/common/common:stack_array",
     ],
 )