You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jp...@apache.org on 2013/01/16 04:13:49 UTC
git commit: [TS-1538] SSL accept performance regression
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 <jp...@apache.org>
Authored: Mon Jan 7 21:10:17 2013 -0800
Committer: James Peach <jp...@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);