trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jpe...@apache.org
Subject git commit: [TS-1538] SSL accept performance regression
Date Wed, 16 Jan 2013 03:13:49 GMT
Updated Branches:
  refs/heads/master 461de9227 -> ccc4b37b9


[TS-1538] SSL accept performance regression

We need to hold a Continuation mutex across the IO to force the SSL
handshake. Trampoline through a special purpose trampoline so that
we don't have to bottleneck on the acceptor.


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/ccc4b37b
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/ccc4b37b
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/ccc4b37b

Branch: refs/heads/master
Commit: ccc4b37b91f25c2e985f195b77369ce20e2e8e81
Parents: 461de92
Author: James Peach <jpeach@apache.org>
Authored: Mon Jan 7 21:10:17 2013 -0800
Committer: James Peach <jpeach@apache.org>
Committed: Tue Jan 15 19:13:05 2013 -0800

----------------------------------------------------------------------
 CHANGES                              |    3 +
 iocore/net/P_SSLNextProtocolAccept.h |    2 +
 iocore/net/SSLNextProtocolAccept.cc  |   79 +++++++++++++++++++++--------
 proxy/http/HttpAccept.h              |    2 +-
 4 files changed, 63 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ccc4b37b/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index cca0e10..977530f 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 3.3.1
 
+
+  *) [TS-1538] SSL accept performance regression
+
   *) [TS-977] RecCore usage cleanup
 
   *) [TS-1574] [TS-1577] when read_from_writer, we should not do range acceleration.

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ccc4b37b/iocore/net/P_SSLNextProtocolAccept.h
----------------------------------------------------------------------
diff --git a/iocore/net/P_SSLNextProtocolAccept.h b/iocore/net/P_SSLNextProtocolAccept.h
index 7c44bbc..c3ee575 100644
--- a/iocore/net/P_SSLNextProtocolAccept.h
+++ b/iocore/net/P_SSLNextProtocolAccept.h
@@ -56,6 +56,8 @@ private:
   MIOBuffer * buffer; // XXX do we really need this?
   Continuation * endpoint;
   SSLNextProtocolSet protoset;
+
+friend struct SSLNextProtocolTrampoline;
 };
 
 #endif /* P_SSLNextProtocolAccept_H_ */

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ccc4b37b/iocore/net/SSLNextProtocolAccept.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLNextProtocolAccept.cc b/iocore/net/SSLNextProtocolAccept.cc
index 3cb577a..a03da15 100644
--- a/iocore/net/SSLNextProtocolAccept.cc
+++ b/iocore/net/SSLNextProtocolAccept.cc
@@ -26,11 +26,13 @@
 static void
 send_plugin_event(Continuation * plugin, int event, void * edata)
 {
-  if (likely(plugin)) {
+  if (plugin->mutex) {
     EThread * thread(this_ethread());
     MUTEX_TAKE_LOCK(plugin->mutex, thread);
     plugin->handleEvent(event, edata);
     MUTEX_UNTAKE_LOCK(plugin->mutex, thread);
+  } else {
+    plugin->handleEvent(event, edata);
   }
 }
 
@@ -56,16 +58,64 @@ ssl_netvc_cast(int event, void * edata)
   }
 }
 
+// SSLNextProtocolTrampoline is the receiver of the I/O event generated when we perform a
0-length read on the new SSL
+// connection. The 0-length read forces the SSL handshake, which allows us to bind an endpoint
that is selected by the
+// NPN extension. The Continuation that receives the read event *must* have a mutex, but
we don't want to take a global
+// lock across the handshake, so we make a trampoline to bounce the event from the SSL acceptor
to the ultimate session
+// acceptor.
+struct SSLNextProtocolTrampoline : public Continuation
+{
+  explicit
+  SSLNextProtocolTrampoline(const SSLNextProtocolAccept * npn)
+    : Continuation(new_ProxyMutex()), npnParent(npn)
+  {
+    SET_HANDLER(&SSLNextProtocolTrampoline::ioCompletionEvent);
+  }
+
+  int ioCompletionEvent(int event, void * edata)
+  {
+    VIO * vio;
+    Continuation * plugin;
+    SSLNetVConnection * netvc;
+
+    switch (event) {
+    case VC_EVENT_INACTIVITY_TIMEOUT:
+    case VC_EVENT_READ_COMPLETE:
+    case VC_EVENT_ERROR:
+      vio = static_cast<VIO *>(edata);
+      break;
+    default:
+      return EVENT_ERROR;
+    }
+
+    netvc = dynamic_cast<SSLNetVConnection *>(vio->vc_server);
+    ink_assert(netvc != NULL);
+
+    plugin = netvc->endpoint();
+    if (plugin) {
+      send_plugin_event(plugin, NET_EVENT_ACCEPT, netvc);
+    } else if (npnParent->endpoint) {
+      // Route to the default endpoint
+      send_plugin_event(npnParent->endpoint, NET_EVENT_ACCEPT, netvc);
+    } else {
+      // No handler, what should we do? Best to just kill the VC while we can.
+      netvc->do_io(VIO::CLOSE);
+    }
+
+    delete this;
+    return EVENT_CONT;
+  }
+
+  const SSLNextProtocolAccept * npnParent;
+};
+
 int
 SSLNextProtocolAccept::mainEvent(int event, void * edata)
 {
   SSLNetVConnection * netvc = ssl_netvc_cast(event, edata);
-  Continuation * plugin;
 
   Debug("ssl", "[SSLNextProtocolAccept:mainEvent] event %d netvc %p", event, netvc);
 
-  MUTEX_LOCK(lock, this->mutex, this_ethread());
-
   switch (event) {
   case NET_EVENT_ACCEPT:
     ink_release_assert(netvc != NULL);
@@ -74,25 +124,10 @@ SSLNextProtocolAccept::mainEvent(int event, void * edata)
     // the endpoint that there is an accept to handle until the read completes
     // and we know which protocol was negotiated.
     netvc->registerNextProtocolSet(&this->protoset);
-    netvc->do_io(VIO::READ, this, 0, this->buffer, 0);
-    return EVENT_CONT;
-  case VC_EVENT_READ_COMPLETE:
-    ink_release_assert(netvc != NULL);
-    plugin = netvc->endpoint();
-    if (plugin) {
-      send_plugin_event(plugin, NET_EVENT_ACCEPT, netvc);
-    } else if (this->endpoint) {
-      // Route to the default endpoint
-      send_plugin_event(this->endpoint, NET_EVENT_ACCEPT, netvc);
-    } else {
-      // No handler, what should we do? Best to just kill the VC while we can.
-      netvc->do_io(VIO::CLOSE);
-    }
+    netvc->do_io(VIO::READ, NEW(new SSLNextProtocolTrampoline(this)), 0, this->buffer,
0);
     return EVENT_CONT;
-  case VC_EVENT_INACTIVITY_TIMEOUT:
-  case VC_EVENT_ERROR:
-    netvc->do_io(VIO::CLOSE);
   default:
+    netvc->do_io(VIO::CLOSE);
     return EVENT_DONE;
   }
 }
@@ -112,7 +147,7 @@ SSLNextProtocolAccept::unregisterEndpoint(
 }
 
 SSLNextProtocolAccept::SSLNextProtocolAccept(Continuation * ep)
-    : Continuation(new_ProxyMutex()), buffer(new_empty_MIOBuffer()), endpoint(ep)
+    : Continuation(NULL), buffer(new_empty_MIOBuffer()), endpoint(ep)
 {
   SET_HANDLER(&SSLNextProtocolAccept::mainEvent);
 }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ccc4b37b/proxy/http/HttpAccept.h
----------------------------------------------------------------------
diff --git a/proxy/http/HttpAccept.h b/proxy/http/HttpAccept.h
index ed4786f..e268067 100644
--- a/proxy/http/HttpAccept.h
+++ b/proxy/http/HttpAccept.h
@@ -170,7 +170,7 @@ public:
   typedef detail::HttpAcceptOptions Options;
 
   HttpAccept(Options const& opt = DEFAULT_OPTIONS)
-    : Continuation(new_ProxyMutex())
+    : Continuation(NULL)
     , detail::HttpAcceptOptions(opt) // copy these.
   {
     SET_HANDLER(&HttpAccept::mainEvent);


Mime
View raw message