trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jpe...@apache.org
Subject [trafficserver] branch master updated: TS-4903: EISCONN errors when using TCP Fast Open.
Date Thu, 29 Sep 2016 15:16:32 GMT
This is an automated email from the ASF dual-hosted git repository.

jpeach pushed a commit to branch master
in repository https://git-dual.apache.org/repos/asf/trafficserver.git

The following commit(s) were added to refs/heads/master by this push:
       new  bde5e69   TS-4903: EISCONN errors when using TCP Fast Open.
bde5e69 is described below

commit bde5e69a5b3d776187fa05feaea76f529e813058
Author: James Peach <jpeach@apache.org>
AuthorDate: Wed Sep 28 14:38:49 2016 -0700

    TS-4903: EISCONN errors when using TCP Fast Open.
    
    The write VIO is reset when we reuse a VC, so we need to explicitly
    track whether we have connected a TCP Fast Open socket. In the
    previous patch we eschewed using the is_connected state of the
    Connection, but that is explicitly what we want so we should just
    use it and allow the Connection to be not connected until the Fast
    Open connects it.
---
 iocore/net/BIO_fastopen.cc          |  4 ++--
 iocore/net/Net.cc                   |  2 +-
 iocore/net/P_Net.h                  |  2 +-
 iocore/net/UnixConnection.cc        | 12 ++++++------
 iocore/net/UnixNetVConnection.cc    | 11 +++++++++--
 proxy/config/metrics.config.default |  7 +------
 6 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/iocore/net/BIO_fastopen.cc b/iocore/net/BIO_fastopen.cc
index 55be47c..77e9378 100644
--- a/iocore/net/BIO_fastopen.cc
+++ b/iocore/net/BIO_fastopen.cc
@@ -71,8 +71,8 @@ fastopen_bwrite(BIO *bio, const char *in, int insz)
     NET_INCREMENT_DYN_STAT(net_fastopen_attempts_stat);
 
     err = socketManager.sendto(bio->num, (void *)in, insz, MSG_FASTOPEN, dst, ats_ip_size(dst));
-    if (err < 0) {
-      NET_INCREMENT_DYN_STAT(net_fastopen_failures_stat);
+    if (err >= 0) {
+      NET_INCREMENT_DYN_STAT(net_fastopen_successes_stat);
     }
 
     bio->ptr = NULL;
diff --git a/iocore/net/Net.cc b/iocore/net/Net.cc
index 46d0739..c8b9ee5 100644
--- a/iocore/net/Net.cc
+++ b/iocore/net/Net.cc
@@ -71,7 +71,7 @@ register_net_stats()
     {"proxy.process.net.read_bytes", net_read_bytes_stat},
     {"proxy.process.net.write_bytes", net_write_bytes_stat},
     {"proxy.process.net.fastopen_out.attempts", net_fastopen_attempts_stat},
-    {"proxy.process.net.fastopen_out.failures", net_fastopen_failures_stat},
+    {"proxy.process.net.fastopen_out.successes", net_fastopen_successes_stat},
     {"proxy.process.socks.connections_successful", socks_connections_successful_stat},
     {"proxy.process.socks.connections_unsuccessful", socks_connections_unsuccessful_stat},
   };
diff --git a/iocore/net/P_Net.h b/iocore/net/P_Net.h
index a807b06..c2c2fd9 100644
--- a/iocore/net/P_Net.h
+++ b/iocore/net/P_Net.h
@@ -54,7 +54,7 @@ enum Net_Stats {
   keep_alive_queue_timeout_count_stat,
   default_inactivity_timeout_stat,
   net_fastopen_attempts_stat,
-  net_fastopen_failures_stat,
+  net_fastopen_successes_stat,
   Net_Stat_Count
 };
 
diff --git a/iocore/net/UnixConnection.cc b/iocore/net/UnixConnection.cc
index 78860e8..af44e2f 100644
--- a/iocore/net/UnixConnection.cc
+++ b/iocore/net/UnixConnection.cc
@@ -316,8 +316,9 @@ Connection::connect(sockaddr const *target, NetVCOptions const &opt)
 
   int res;
 
-  if (target != NULL)
+  if (target != NULL) {
     this->setRemote(target);
+  }
 
   // apply dynamic options with this.addr initialized
   apply_options(opt);
@@ -325,10 +326,6 @@ Connection::connect(sockaddr const *target, NetVCOptions const &opt)
   cleaner<Connection> cleanup(this, &Connection::_cleanup); // mark for close until
we succeed.
 
   if (opt.f_tcp_fastopen && !opt.f_blocking_connect) {
-    ProxyMutex *mutex = this_ethread()->mutex.get();
-
-    NET_INCREMENT_DYN_STAT(net_fastopen_attempts_stat);
-
     // TCP Fast Open is (effectively) a non-blocking connect, so set the
     // return value we would see in that case.
     errno = EINPROGRESS;
@@ -353,7 +350,10 @@ Connection::connect(sockaddr const *target, NetVCOptions const &opt)
   }
 
   cleanup.reset();
-  is_connected = true;
+
+  // Only mark this connection as connected if we successfully called connect(2). When we
+  // do the TCP Fast Open later, we need to track this accurately.
+  is_connected = !(opt.f_tcp_fastopen && !opt.f_blocking_connect);
   return 0;
 }
 
diff --git a/iocore/net/UnixNetVConnection.cc b/iocore/net/UnixNetVConnection.cc
index 96302b9..7c18e27 100644
--- a/iocore/net/UnixNetVConnection.cc
+++ b/iocore/net/UnixNetVConnection.cc
@@ -996,7 +996,7 @@ UnixNetVConnection::load_buffer_and_write(int64_t towrite, MIOBufferAccessor
&bu
     // correctly disabled support in the socket option configuration.
     ink_assert(MSG_FASTOPEN != 0 || this->options.f_tcp_fastopen == false);
 
-    if (this->options.f_tcp_fastopen && this->write.vio.ndone == 0) {
+    if (!this->con.is_connected && this->options.f_tcp_fastopen) {
       struct msghdr msg;
 
       ink_zero(msg);
@@ -1005,9 +1005,16 @@ UnixNetVConnection::load_buffer_and_write(int64_t towrite, MIOBufferAccessor
&bu
       msg.msg_iov     = &tiovec[0];
       msg.msg_iovlen  = niov;
 
+      NET_INCREMENT_DYN_STAT(net_fastopen_attempts_stat);
+
       r = socketManager.sendmsg(con.fd, &msg, MSG_FASTOPEN);
       if (r < 0) {
-        NET_INCREMENT_DYN_STAT(net_fastopen_failures_stat);
+        if (r == -EINPROGRESS || r == -EWOULDBLOCK) {
+          this->con.is_connected = true;
+        }
+      } else {
+        NET_INCREMENT_DYN_STAT(net_fastopen_successes_stat);
+        this->con.is_connected = true;
       }
 
     } else {
diff --git a/proxy/config/metrics.config.default b/proxy/config/metrics.config.default
index a02e2fa..fb07dd4 100644
--- a/proxy/config/metrics.config.default
+++ b/proxy/config/metrics.config.default
@@ -47,7 +47,7 @@
 --
 -- integer(NAME, FUNC), counter(NAME, FUNC), float(NAME, FUNC)
 --    These global functions register a metric of the corresponding name. Each
---    registered metric will be periodically recalculated by evaluating the 
+--    registered metric will be periodically recalculated by evaluating the
 --    given function.
 --
 --    The scope of a metric is derived from the name. 'proxy.cluster.*' metrics
@@ -1563,8 +1563,3 @@ integer 'proxy.cluster.log.bytes_received_from_network_avg_10s' [[
 counter 'proxy.process.ssl.total_success_handshake_count' [[
   return proxy.process.ssl.total_success_handshake_count_in
 ]]
-
-counter 'proxy.process.net.fastopen_out.success' [[
-  return proxy.process.net.fastopen_out.attempts -
-    proxy.process.net.fastopen_out.failures
-]]

-- 
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <commits@trafficserver.apache.org>'].

Mime
View raw message