Check return results for setting socket buffer sizes in QUIC

...and

More aggressively validate that setsockopt(buffersize) worked on Windows.

Add histograms to monitor the impact of this change.

r=rch,wtc
BUG=355222

Review URL: https://codereview.chromium.org/196393003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@258859 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 91 insertions, 11 deletions

diff --git a/net/base/net_error_list.h b/net/base/net_error_list.h
index b2ddd4a..b6659c4 100644
--- a/net/base/net_error_list.h
+++ b/net/base/net_error_list.h
@@ -313,6 +313,12 @@ NET_ERROR(CT_NO_SCTS_VERIFIED_OK, -158)
 // The SSL server sent us a fatal unrecognized_name alert.
 NET_ERROR(SSL_UNRECOGNIZED_NAME_ALERT, -159)
 
+// Failed to increase the socket's receive buffer size as requested
+NET_ERROR(SOCKET_SET_RECEIVE_BUFFER_SIZE_ERROR, -160)
+
+// Failed to increase the socket's send buffer size as requested.
+NET_ERROR(SOCKET_SET_SEND_BUFFER_SIZE_ERROR, -161)
+
 // Certificate error codes
 //
 // The values of certificate error codes must be consecutive.
diff --git a/net/quic/quic_stream_factory.cc b/net/quic/quic_stream_factory.cc
index 2d94951..ee16ca8 100644
--- a/net/quic/quic_stream_factory.cc
+++ b/net/quic/quic_stream_factory.cc
@@ -41,8 +41,20 @@ namespace net {
 
 namespace {
 
+enum CreateSessionFailure {
+  CREATION_ERROR_CONNECTING_SOCKET,
+  CREATION_ERROR_SETTING_RECEIVE_BUFFER,
+  CREATION_ERROR_SETTING_SEND_BUFFER,
+  CREATION_ERROR_MAX
+};
+
 const uint64 kBrokenAlternateProtocolDelaySecs = 300;
 
+void HistogramCreateSessionFailure(enum CreateSessionFailure error) {
+  UMA_HISTOGRAM_ENUMERATION("Net.QuicSession.CreationError", error,
+                            CREATION_ERROR_MAX);
+}
+
 }  // namespace
 
 QuicStreamFactory::IpAliasKey::IpAliasKey() {}
@@ -671,8 +683,10 @@ int QuicStreamFactory::CreateSession(
           base::Bind(&PortSuggester::SuggestPort, port_suggester),
           net_log.net_log(), net_log.source()));
   int rv = socket->Connect(addr);
-  if (rv != OK)
+  if (rv != OK) {
+    HistogramCreateSessionFailure(CREATION_ERROR_CONNECTING_SOCKET);
     return rv;
+  }
   UMA_HISTOGRAM_COUNTS("Net.QuicEphemeralPortsSuggested",
                        port_suggester->call_count());
   if (enable_port_selection) {
@@ -686,11 +700,17 @@ int QuicStreamFactory::CreateSession(
   // does not consume "too much" memory.  If we see bursty packet loss, we may
   // revisit this setting and test for its impact.
   const int32 kSocketBufferSize(TcpReceiver::kReceiveWindowTCP);
-  socket->SetReceiveBufferSize(kSocketBufferSize);
+  if (!socket->SetReceiveBufferSize(kSocketBufferSize)) {
+    HistogramCreateSessionFailure(CREATION_ERROR_SETTING_RECEIVE_BUFFER);
+    return ERR_SOCKET_SET_RECEIVE_BUFFER_SIZE_ERROR;
+  }
   // Set a buffer large enough to contain the initial CWND's worth of packet
   // to work around the problem with CHLO packets being sent out with the
   // wrong encryption level, when the send buffer is full.
-  socket->SetSendBufferSize(kMaxPacketSize * 20); // Support 20 packets.
+  if (!socket->SetSendBufferSize(kMaxPacketSize * 20)) {
+    HistogramCreateSessionFailure(CREATION_ERROR_SETTING_SEND_BUFFER);
+    return ERR_SOCKET_SET_SEND_BUFFER_SIZE_ERROR;
+  }
 
   scoped_ptr<QuicDefaultPacketWriter> writer(
       new QuicDefaultPacketWriter(socket.get()));
diff --git a/net/udp/udp_socket_win.cc b/net/udp/udp_socket_win.cc
index beaf400..5c8bff4 100644
--- a/net/udp/udp_socket_win.cc
+++ b/net/udp/udp_socket_win.cc
@@ -390,18 +390,42 @@ int UDPSocketWin::CreateSocket(int addr_family) {
 
 bool UDPSocketWin::SetReceiveBufferSize(int32 size) {
   DCHECK(CalledOnValidThread());
-  int rv = setsockopt(socket_, SOL_SOCKET, SO_RCVBUF,
-                      reinterpret_cast<const char*>(&size), sizeof(size));
-  DCHECK(!rv) << "Could not set socket receive buffer size: " << errno;
-  return rv == 0;
+  setsockopt(socket_, SOL_SOCKET, SO_RCVBUF,
+              reinterpret_cast<const char*>(&size), sizeof(size));
+  // If the setsockopt fails, but the buffer is big enough, we will return
+  // success. It is not worth testing the return value as we still need to check
+  // via getsockopt anyway according to Windows documentation.
+  int32 actual_size = 0;
+  int option_size = sizeof(actual_size);
+  int rv = getsockopt(socket_, SOL_SOCKET, SO_RCVBUF,
+                      reinterpret_cast<char*>(&actual_size), &option_size);
+  if (rv != 0)
+    return false;
+  if (actual_size < size) {
+    UMA_HISTOGRAM_CUSTOM_COUNTS("Net.SocketReceiveBufferUnchangeable",
+                                actual_size, 1000, 1000000, 50);
+  }
+  return actual_size >= size;
 }
 
 bool UDPSocketWin::SetSendBufferSize(int32 size) {
   DCHECK(CalledOnValidThread());
-  int rv = setsockopt(socket_, SOL_SOCKET, SO_SNDBUF,
-                      reinterpret_cast<const char*>(&size), sizeof(size));
-  DCHECK(!rv) << "Could not set socket send buffer size: " << errno;
-  return rv == 0;
+  setsockopt(socket_, SOL_SOCKET, SO_SNDBUF,
+             reinterpret_cast<const char*>(&size), sizeof(size));
+  // If the setsockopt fails, but the buffer is big enough, we will return
+  // success. It is not worth testing the return value as we still need to check
+  // via getsockopt anyway according to Windows documentation.
+  int32 actual_size = 0;
+  int option_size = sizeof(actual_size);
+  int rv = getsockopt(socket_, SOL_SOCKET, SO_SNDBUF,
+                      reinterpret_cast<char*>(&actual_size), &option_size);
+  if (rv != 0)
+    return false;
+  if (actual_size < size) {
+    UMA_HISTOGRAM_CUSTOM_COUNTS("Net.SocketUnchangeableSendBuffer",
+                                actual_size, 1000, 1000000, 50);
+  }
+  return actual_size >= size;
 }
 
 void UDPSocketWin::AllowAddressReuse() {
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index 44e1cd0..abd6511 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -12144,6 +12144,14 @@ other types of suffix sets.
   </summary>
 </histogram>
 
+<histogram name="Net.QuicSession.CreationError" enum="QuicSessionErrorCodes">
+  <owner>jar@chromium.org</owner>
+  <summary>
+    Count of errors during attempts to create a QUIC session (before even using
+    the session).
+  </summary>
+</histogram>
+
 <histogram name="Net.QuicSession.FinalTcpCwnd">
   <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
   <summary>
@@ -12406,6 +12414,22 @@ other types of suffix sets.
   </summary>
 </histogram>
 
+<histogram name="Net.SocketUnchangeableReceiveBuffer" units="Bytes">
+  <owner>jar@chromium.org</owner>
+  <summary>
+    The size of a socket's receive buffer when the attempt to change it via
+    setsockopt failed.
+  </summary>
+</histogram>
+
+<histogram name="Net.SocketUnchangeableSendBuffer" units="Bytes">
+  <owner>jar@chromium.org</owner>
+  <summary>
+    The size of a socket's send buffer when the attempt to change it via
+    setsockopt failed.
+  </summary>
+</histogram>
+
 <histogram name="Net.SOCKSSocketIdleTimeBeforeNextUse_ReusedSocket">
   <obsolete>
     see SocketIdleTimeBeforeNextUse_ReusedSocket_SOCK
@@ -37181,6 +37205,12 @@ other types of suffix sets.
   <int value="6" label="CANCELLED"/>
 </enum>
 
+<enum name="QuicSessionErrorCodes" type="int">
+  <int value="0" label="CONNECTING_SOCKET"/>
+  <int value="1" label="SETTING_RECEIVE_BUFFER"/>
+  <int value="2" label="SETTING_SEND_BUFFER"/>
+</enum>
+
 <enum name="RapporDiscardReason" type="int">
   <int value="0" label="Upload Success"/>
   <int value="1" label="Upload Rejected"/>