From b1de1da318631c6d29f6c04dea370f712078f443 Mon Sep 17 00:00:00 2001
From: Ting-Yu Wang <anivia@google.com>
Date: Thu, 7 Jan 2021 14:14:58 -0800
Subject: netstack: Refactor tcpip.Endpoint.Read

Read now takes a destination io.Writer, count, options. Keeping the method name
Read, in contrast to the Write method.

This enables:
* direct transfer of views under VV
* zero copy

It also eliminates the need for sentry to keep a slice of view because
userspace had requested a read that is smaller than the view returned, removing
the complexity there.

Read/Peek/ReadPacket are now consolidated together and some duplicate code is
removed.

PiperOrigin-RevId: 350636322
---
 .../syscalls/linux/socket_bind_to_device_distribution.cc | 16 +---------------
 test/syscalls/linux/socket_generic.cc                    |  9 +++++++++
 test/syscalls/linux/socket_inet_loopback.cc              | 16 +---------------
 3 files changed, 11 insertions(+), 30 deletions(-)

(limited to 'test')

diff --git a/test/syscalls/linux/socket_bind_to_device_distribution.cc b/test/syscalls/linux/socket_bind_to_device_distribution.cc
index 06419772f..f8a0a80f2 100644
--- a/test/syscalls/linux/socket_bind_to_device_distribution.cc
+++ b/test/syscalls/linux/socket_bind_to_device_distribution.cc
@@ -204,7 +204,7 @@ TEST_P(BindToDeviceDistributionTest, Tcp) {
         });
   }
 
-  for (int i = 0; i < kConnectAttempts; i++) {
+  for (int32_t i = 0; i < kConnectAttempts; i++) {
     const FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE(
         Socket(connector.family(), SOCK_STREAM, IPPROTO_TCP));
     ASSERT_THAT(
@@ -212,22 +212,8 @@ TEST_P(BindToDeviceDistributionTest, Tcp) {
                             connector.addr_len),
         SyscallSucceeds());
 
-    // Do two separate sends to ensure two segments are received. This is
-    // required for netstack where read is incorrectly assuming a whole
-    // segment is read when endpoint.Read() is called which is technically
-    // incorrect as the syscall that invoked endpoint.Read() may only
-    // consume it partially. This results in a case where a close() of
-    // such a socket does not trigger a RST in netstack due to the
-    // endpoint assuming that the endpoint has no unread data.
     EXPECT_THAT(RetryEINTR(send)(fd.get(), &i, sizeof(i), 0),
                 SyscallSucceedsWithValue(sizeof(i)));
-
-    // TODO(gvisor.dev/issue/1449): Remove this block once netstack correctly
-    //   generates a RST.
-    if (IsRunningOnGvisor()) {
-      EXPECT_THAT(RetryEINTR(send)(fd.get(), &i, sizeof(i), 0),
-                  SyscallSucceedsWithValue(sizeof(i)));
-    }
   }
 
   // Join threads to be sure that all connections have been counted.
diff --git a/test/syscalls/linux/socket_generic.cc b/test/syscalls/linux/socket_generic.cc
index a28ee2233..de0b8bb11 100644
--- a/test/syscalls/linux/socket_generic.cc
+++ b/test/syscalls/linux/socket_generic.cc
@@ -43,6 +43,15 @@ TEST_P(AllSocketPairTest, BasicReadWrite) {
   EXPECT_EQ(data, absl::string_view(buf, 3));
 }
 
+TEST_P(AllSocketPairTest, BasicReadWriteBadBuffer) {
+  auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair());
+  const std::string data = "abc";
+  ASSERT_THAT(WriteFd(sockets->first_fd(), data.c_str(), 3),
+              SyscallSucceedsWithValue(3));
+  ASSERT_THAT(ReadFd(sockets->second_fd(), nullptr, 3),
+              SyscallFailsWithErrno(EFAULT));
+}
+
 TEST_P(AllSocketPairTest, BasicSendRecv) {
   auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair());
   char sent_data[512];
diff --git a/test/syscalls/linux/socket_inet_loopback.cc b/test/syscalls/linux/socket_inet_loopback.cc
index 51b77ad85..a11147085 100644
--- a/test/syscalls/linux/socket_inet_loopback.cc
+++ b/test/syscalls/linux/socket_inet_loopback.cc
@@ -1507,7 +1507,7 @@ TEST_P(SocketInetReusePortTest, TcpPortReuseMultiThread_NoRandomSave) {
   }
 
   ScopedThread connecting_thread([&connector, &conn_addr]() {
-    for (int i = 0; i < kConnectAttempts; i++) {
+    for (int32_t i = 0; i < kConnectAttempts; i++) {
       const FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE(
           Socket(connector.family(), SOCK_STREAM, IPPROTO_TCP));
       ASSERT_THAT(
@@ -1515,22 +1515,8 @@ TEST_P(SocketInetReusePortTest, TcpPortReuseMultiThread_NoRandomSave) {
                               connector.addr_len),
           SyscallSucceeds());
 
-      // Do two separate sends to ensure two segments are received. This is
-      // required for netstack where read is incorrectly assuming a whole
-      // segment is read when endpoint.Read() is called which is technically
-      // incorrect as the syscall that invoked endpoint.Read() may only
-      // consume it partially. This results in a case where a close() of
-      // such a socket does not trigger a RST in netstack due to the
-      // endpoint assuming that the endpoint has no unread data.
       EXPECT_THAT(RetryEINTR(send)(fd.get(), &i, sizeof(i), 0),
                   SyscallSucceedsWithValue(sizeof(i)));
-
-      // TODO(gvisor.dev/issue/1449): Remove this block once netstack correctly
-      //   generates a RST.
-      if (IsRunningOnGvisor()) {
-        EXPECT_THAT(RetryEINTR(send)(fd.get(), &i, sizeof(i), 0),
-                    SyscallSucceedsWithValue(sizeof(i)));
-      }
     }
   });
 
-- 
cgit v1.2.3