You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by us...@apache.org on 2013/10/08 22:19:25 UTC
git commit: TS-2268 Add support for opening protocol traffic sockets
through the traffic_manager.
Updated Branches:
refs/heads/master 7ba121c9a -> 3c0c835c1
TS-2268 Add support for opening protocol traffic sockets through the
traffic_manager.
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/3c0c835c
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/3c0c835c
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/3c0c835c
Branch: refs/heads/master
Commit: 3c0c835c1b06cb5c76ae8dba5add9a8ffc07495d
Parents: 7ba121c
Author: Uri Shachar <us...@apache.org>
Authored: Tue Oct 8 23:17:08 2013 +0300
Committer: Uri Shachar <us...@apache.org>
Committed: Tue Oct 8 23:17:08 2013 +0300
----------------------------------------------------------------------
CHANGES | 3 +++
lib/records/I_RecHttp.h | 8 +++++++-
lib/records/RecHttp.cc | 5 +++++
proxy/InkAPI.cc | 19 ++++++++++++++++++-
proxy/api/ts/experimental.h | 16 ++++++++++++++++
proxy/http/HttpProxyServerMain.cc | 2 +-
6 files changed, 50 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3c0c835c/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index c2c52f3..fa566c0 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,9 @@
-*- coding: utf-8 -*-
Changes with Apache Traffic Server 4.1.0
+ *) [TS-2268] Add support for opening protocol traffic sockets through the
+ traffic_manager. Added TSPluginDescriptorAccept into expiremental API.
+
*) [TS-2266] Add a "make rat" Makefile target, to create a RAT report. This
is used for verifying licensing compliance on the entire source tree.
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3c0c835c/lib/records/I_RecHttp.h
----------------------------------------------------------------------
diff --git a/lib/records/I_RecHttp.h b/lib/records/I_RecHttp.h
index 4bd3de0..241d870 100644
--- a/lib/records/I_RecHttp.h
+++ b/lib/records/I_RecHttp.h
@@ -94,7 +94,8 @@ public:
TRANSPORT_DEFAULT = 0, ///< Default (normal HTTP).
TRANSPORT_COMPRESSED, ///< Compressed HTTP.
TRANSPORT_BLIND_TUNNEL, ///< Blind tunnel (no processing).
- TRANSPORT_SSL ///< SSL connection.
+ TRANSPORT_SSL, ///< SSL connection.
+ TRANSPORT_PLUGIN /// < Protocol plugin connection
};
int m_fd; ///< Pre-opened file descriptor if present.
@@ -134,6 +135,9 @@ public:
/// Check for SSL port.
bool isSSL() const;
+ /// Check for SSL port.
+ bool isPlugin() const;
+
/// Process options text.
/// @a opts should not contain any whitespace, only the option string.
/// This object's internal state is updated as specified by @a opts.
@@ -258,6 +262,7 @@ public:
static char const* const OPT_TRANSPARENT_FULL; ///< Full transparency.
static char const* const OPT_TRANSPARENT_PASSTHROUGH; ///< Pass-through non-HTTP.
static char const* const OPT_SSL; ///< SSL (experimental)
+ static char const* const OPT_PLUGIN; ///< Protocol Plugin handle (experimental)
static char const* const OPT_BLIND_TUNNEL; ///< Blind tunnel.
static char const* const OPT_COMPRESSED; ///< Compressed.
static char const* const OPT_HOST_RES_PREFIX; ///< Set DNS family preference.
@@ -270,6 +275,7 @@ protected:
};
inline bool HttpProxyPort::isSSL() const { return TRANSPORT_SSL == m_type; }
+inline bool HttpProxyPort::isPlugin() const { return TRANSPORT_PLUGIN == m_type; }
inline IpAddr&
HttpProxyPort::outboundIp(uint16_t family) {
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3c0c835c/lib/records/RecHttp.cc
----------------------------------------------------------------------
diff --git a/lib/records/RecHttp.cc b/lib/records/RecHttp.cc
index 13a20a6..e9ad2b5 100644
--- a/lib/records/RecHttp.cc
+++ b/lib/records/RecHttp.cc
@@ -77,6 +77,7 @@ char const* const HttpProxyPort::OPT_TRANSPARENT_OUTBOUND = "tr-out";
char const* const HttpProxyPort::OPT_TRANSPARENT_FULL = "tr-full";
char const* const HttpProxyPort::OPT_TRANSPARENT_PASSTHROUGH = "tr-pass";
char const* const HttpProxyPort::OPT_SSL = "ssl";
+char const* const HttpProxyPort::OPT_PLUGIN = "plugin";
char const* const HttpProxyPort::OPT_BLIND_TUNNEL = "blind";
char const* const HttpProxyPort::OPT_COMPRESSED = "compressed";
@@ -264,6 +265,8 @@ HttpProxyPort::processOptions(char const* opts) {
} else if (0 == strcasecmp(OPT_SSL, item)) {
m_type = TRANSPORT_SSL;
m_inbound_transparent_p = m_outbound_transparent_p = false;
+ } else if (0 == strcasecmp(OPT_PLUGIN, item)) {
+ m_type = TRANSPORT_PLUGIN;
} else if (0 == strcasecmp(OPT_TRANSPARENT_INBOUND, item)) {
# if TS_USE_TPROXY
m_inbound_transparent_p = true;
@@ -399,6 +402,8 @@ HttpProxyPort::print(char* out, size_t n) {
zret += snprintf(out+zret, n-zret, ":%s", OPT_BLIND_TUNNEL);
else if (TRANSPORT_SSL == m_type)
zret += snprintf(out+zret, n-zret, ":%s", OPT_SSL);
+ else if (TRANSPORT_PLUGIN == m_type)
+ zret += snprintf(out+zret, n-zret, ":%s", OPT_PLUGIN);
else if (TRANSPORT_COMPRESSED == m_type)
zret += snprintf(out+zret, n-zret, ":%s", OPT_COMPRESSED);
if (zret >= n) return n;
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3c0c835c/proxy/InkAPI.cc
----------------------------------------------------------------------
diff --git a/proxy/InkAPI.cc b/proxy/InkAPI.cc
index 1418826..f51f4c8 100644
--- a/proxy/InkAPI.cc
+++ b/proxy/InkAPI.cc
@@ -8204,7 +8204,7 @@ TSPortDescriptorParse(const char * descriptor)
TSReturnCode
TSPortDescriptorAccept(TSPortDescriptor descp, TSCont contp)
{
- Action * action;
+ Action * action = NULL;
HttpProxyPort * port = (HttpProxyPort *)descp;
NetProcessor::AcceptOptions net(make_net_accept_options(*port, 0 /* nthreads */));
@@ -8217,6 +8217,23 @@ TSPortDescriptorAccept(TSPortDescriptor descp, TSCont contp)
return action ? TS_SUCCESS : TS_ERROR;
}
+TSReturnCode
+TSPluginDescriptorAccept(TSCont contp)
+{
+ Action * action = NULL;
+
+ HttpProxyPort::Group& proxy_ports = HttpProxyPort::global();
+ for ( int i = 0 , n = proxy_ports.length() ; i < n ; ++i ) {
+ HttpProxyPort& port = proxy_ports[i];
+ if (port.isPlugin()) {
+ NetProcessor::AcceptOptions net(make_net_accept_options(port, 0 /* nthreads */));
+ action = netProcessor.main_accept((INKContInternal *)contp, port.m_fd, net);
+ }
+ }
+ return action ? TS_SUCCESS : TS_ERROR;
+}
+
+
int
TSHttpTxnBackgroundFillStarted(TSHttpTxn txnp)
{
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3c0c835c/proxy/api/ts/experimental.h
----------------------------------------------------------------------
diff --git a/proxy/api/ts/experimental.h b/proxy/api/ts/experimental.h
index f13edab..75037a2 100644
--- a/proxy/api/ts/experimental.h
+++ b/proxy/api/ts/experimental.h
@@ -219,6 +219,22 @@ extern "C"
****************************************************************************/
tsapi int TSHttpTxnLookingUpTypeGet(TSHttpTxn txnp);
+ /**
+ Attempt to attach the contp continuation to sockets that have already been
+ opened by the traffic manager and defined as belonging to plugins (based on
+ records.config configuration). If a connection is successfully accepted,
+ the TS_EVENT_NET_ACCEPT is delivered to the continuation. The event
+ data will be a valid TSVConn bound to the accepted connection.
+ In order to configure such a socket, add the "plugin" keyword to a port
+ in proxy.config.http.server_ports like "8082:plugin"
+ Transparency/IP settings can also be defined, but a port cannot have
+ both the "ssl" or "plugin" keywords configured.
+
+ Need to update records.config comments on proxy.config.http.server_ports
+ when this option is promoted from experimental.
+ */
+ tsapi TSReturnCode TSPluginDescriptorAccept(TSCont contp);
+
/**
Opens a network connection to the host specified by the 'to' sockaddr
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3c0c835c/proxy/http/HttpProxyServerMain.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpProxyServerMain.cc b/proxy/http/HttpProxyServerMain.cc
index cae18bd..ad02779 100644
--- a/proxy/http/HttpProxyServerMain.cc
+++ b/proxy/http/HttpProxyServerMain.cc
@@ -233,7 +233,7 @@ start_HttpProxyServer()
if (port.isSSL()) {
if (NULL == sslNetProcessor.main_accept(acceptor._accept, port.m_fd, acceptor._net_opt))
return;
- } else {
+ } else if (! port.isPlugin()) {
if (NULL == netProcessor.main_accept(acceptor._accept, port.m_fd, acceptor._net_opt))
return;
}
Re: git commit: TS-2268 Add support for opening protocol traffic
sockets through the traffic_manager.
Posted by James Peach <jp...@apache.org>.
On Oct 18, 2013, at 6:08 AM, Uri Shachar <us...@hotmail.com> wrote:
> On Thu, 17 Oct 2013 15:49:19 -0700 James Peach wrote:
>> On Oct 17, 2013, at 3:23 PM, Uri Shachar <us...@hotmail.com> wrote:
>>> On Thu, 17 Oct 2013 11:45:41 -0700 James Peach wrote:
>>>> On Oct 13, 2013, at 2:15 PM, Uri Shachar <us...@hotmail.com> wrote:
>>>>> On Wed, 9 Oct 2013 10:41:42 -0700 James Peach wrote:
>>>>>> On Oct 8, 2013, at 1:19 PM, ushachar@apache.org wrote:
>>>>>>
>>>>>>> Updated Branches:
>>>>>>> refs/heads/master 7ba121c9a -> 3c0c835c1
>>>>>>>
>>>>>>>
>>>>>>> TS-2268 Add support for opening protocol traffic sockets through the
>>>>>>> traffic_manager.
>>>>>>
>> There's some interesting features that you are talking about here. I'd like to bring the discussion back to the specific API. Does this API help us get there? What can we do to address the concerns that I raised about it?
>
> I think we've reached an agreement on most of your concerns - the only item you raised which is still somewhat open is allowing protocol plugins to sit after the SSL engine. I feel that implementing this properly (with full support for plugin chaining, maybe even converting the SSL engine into a protocol plugin) is beyond the scope of this change, and it is not required to make the new API useful.
> (I'm all for it - just not in this context).
>
> As discussed - I'm going to change TSPluginDescriptorAccept(cont) into TSNamedAccept(STRING, cont) and add a "name" tag to the port descriptors so you'd configure 8085:plugin:name=myplugin:tr-full in proxy.config.http.server_ports (I want to separate the name from the "plugin accept" functionality to allow for future extensions).
API-wise that sounds fine. Perhaps TSNamedAccept is a uncomfortably close to TSNetAcceptNamedProtocol. How about TSNetAcceptNamedDescriptor()?
Implementation-wise, is this still going to be implemented as a TransportType? I think that it should be a separate property on HttpProxyPort so that you can accept on SSL and other transport types.
J
Re: git commit: TS-2268 Add support for opening protocol traffic
sockets through the traffic_manager.
Posted by James Peach <jp...@apache.org>.
On Oct 18, 2013, at 6:08 AM, Uri Shachar <us...@hotmail.com> wrote:
> On Thu, 17 Oct 2013 15:49:19 -0700 James Peach wrote:
>> On Oct 17, 2013, at 3:23 PM, Uri Shachar <us...@hotmail.com> wrote:
>>> On Thu, 17 Oct 2013 11:45:41 -0700 James Peach wrote:
>>>> On Oct 13, 2013, at 2:15 PM, Uri Shachar <us...@hotmail.com> wrote:
>>>>> On Wed, 9 Oct 2013 10:41:42 -0700 James Peach wrote:
>>>>>> On Oct 8, 2013, at 1:19 PM, ushachar@apache.org wrote:
>>>>>>
>>>>>>> Updated Branches:
>>>>>>> refs/heads/master 7ba121c9a -> 3c0c835c1
>>>>>>>
>>>>>>>
>>>>>>> TS-2268 Add support for opening protocol traffic sockets through the
>>>>>>> traffic_manager.
>>>>>>
>> There's some interesting features that you are talking about here. I'd like to bring the discussion back to the specific API. Does this API help us get there? What can we do to address the concerns that I raised about it?
>
> I think we've reached an agreement on most of your concerns - the only item you raised which is still somewhat open is allowing protocol plugins to sit after the SSL engine. I feel that implementing this properly (with full support for plugin chaining, maybe even converting the SSL engine into a protocol plugin) is beyond the scope of this change, and it is not required to make the new API useful.
> (I'm all for it - just not in this context).
>
> As discussed - I'm going to change TSPluginDescriptorAccept(cont) into TSNamedAccept(STRING, cont) and add a "name" tag to the port descriptors so you'd configure 8085:plugin:name=myplugin:tr-full in proxy.config.http.server_ports (I want to separate the name from the "plugin accept" functionality to allow for future extensions).
API-wise that sounds fine. Perhaps TSNamedAccept is a uncomfortably close to TSNetAcceptNamedProtocol. How about TSNetAcceptNamedDescriptor()?
Implementation-wise, is this still going to be implemented as a TransportType? I think that it should be a separate property on HttpProxyPort so that you can accept on SSL and other transport types.
J
RE: git commit: TS-2268 Add support for opening protocol traffic
sockets through the traffic_manager.
Posted by Uri Shachar <us...@hotmail.com>.
On Thu, 17 Oct 2013 15:49:19 -0700 James Peach wrote:
> On Oct 17, 2013, at 3:23 PM, Uri Shachar <us...@hotmail.com> wrote:
>> On Thu, 17 Oct 2013 11:45:41 -0700 James Peach wrote:
>>> On Oct 13, 2013, at 2:15 PM, Uri Shachar <us...@hotmail.com> wrote:
>>>> On Wed, 9 Oct 2013 10:41:42 -0700 James Peach wrote:
>>>>> On Oct 8, 2013, at 1:19 PM, ushachar@apache.org wrote:
>>>>>
>>>>>> Updated Branches:
>>>>>> refs/heads/master 7ba121c9a -> 3c0c835c1
>>>>>>
>>>>>>
>>>>>> TS-2268 Add support for opening protocol traffic sockets through the
>>>>>> traffic_manager.
>>>>>
> There's some interesting features that you are talking about here. I'd like to bring the discussion back to the specific API. Does this API help us get there? What can we do to address the concerns that I raised about it?
I think we've reached an agreement on most of your concerns - the only item you raised which is still somewhat open is allowing protocol plugins to sit after the SSL engine. I feel that implementing this properly (with full support for plugin chaining, maybe even converting the SSL engine into a protocol plugin) is beyond the scope of this change, and it is not required to make the new API useful.
(I'm all for it - just not in this context).
As discussed - I'm going to change TSPluginDescriptorAccept(cont) into TSNamedAccept(STRING, cont) and add a "name" tag to the port descriptors so you'd configure 8085:plugin:name=myplugin:tr-full in proxy.config.http.server_ports (I want to separate the name from the "plugin accept" functionality to allow for future extensions).
Cheers,
Uri
Re: git commit: TS-2268 Add support for opening protocol traffic
sockets through the traffic_manager.
Posted by James Peach <jp...@apache.org>.
On Oct 17, 2013, at 3:23 PM, Uri Shachar <us...@hotmail.com> wrote:
> On Thu, 17 Oct 2013 11:45:41 -0700 James Peach wrote:
>> On Oct 13, 2013, at 2:15 PM, Uri Shachar <us...@hotmail.com> wrote:
>>> On Wed, 9 Oct 2013 10:41:42 -0700 James Peach wrote:
>>>> On Oct 8, 2013, at 1:19 PM, ushachar@apache.org wrote:
>>>>
>>>>> Updated Branches:
>>>>> refs/heads/master 7ba121c9a -> 3c0c835c1
>>>>>
>>>>>
>>>>> TS-2268 Add support for opening protocol traffic sockets through the
>>>>> traffic_manager.
>>>>
>>>> - How do plugins use this to accept on SSL sockets?
>>>
>>> In the current implementation - they don't. There are a couple of issues to consider here:
>>> 1) How do we allow protocol ordering (what if I want a protocol plugin to handle the connection _before_ the SSL engine)
>>
>> What's the use case for this? A plugin that implements it's own TLS? Is that needed?
>
> I can think of a number of use-cases in my (admittedly less common) transparent deployment land - but how about this:
> ATS is the frontend to all your servers and you want ATS to handle some encrypted connections but not others - making the determination based on SNI ->
>
> Protocol plugin that parses SNI information from the request, passing connections to the SSL engine if they can be handled locally and acting as a tunnel to the OS if they can't.
OK, so in this case, the plugin is listening on a server name? Or does it need to dynamically choose to sometimes accept on a server name?
Does this API help us get closer to that feature?
>>> 2) Protocol Plugin chaining implementation
>>
>> Again, what is the use case? I'm not sure that protocol plugin chaining would look like ...
>
> Encapsulated protocols for example - where you want to have a clean state machine handling each protocol and handing it off to the next one (Sort of like what you did with SPDY).
> I'm not sure what a full implementation of this would look like either....
It sounds interesting in princple :) but maybe off topic for this API discussion?
>>>> - We already have 3 *Accept() APIs, why can't they support this requirement?
>>>
>>> Isn't it worthwhile to have a way to specify that we require the port to be pre-opened? The other option I can see (without adding API like TSPortDescriptorRendevous) would be to pass a flag....
>>
>> I'm not sure that it's worthwhile. If it's protecting against traffic_server crashes, that seems nice to have, but maybe not very important. Is that the only use case?
>
> Yes - and it's a very important one for some transparent deployments. If the proxy hangs connections for a second or two, most likely users won't notice -- if they all get a reset then you're bound to get support calls.
Ah, thanks. Thats something that I did not consider.
There's some interesting features that you are talking about here. I'd like to bring the discussion back to the specific API. Does this API help us get there? What can we do to address the concerns that I raised about it?
J
Re: git commit: TS-2268 Add support for opening protocol traffic
sockets through the traffic_manager.
Posted by James Peach <jp...@apache.org>.
On Oct 17, 2013, at 3:23 PM, Uri Shachar <us...@hotmail.com> wrote:
> On Thu, 17 Oct 2013 11:45:41 -0700 James Peach wrote:
>> On Oct 13, 2013, at 2:15 PM, Uri Shachar <us...@hotmail.com> wrote:
>>> On Wed, 9 Oct 2013 10:41:42 -0700 James Peach wrote:
>>>> On Oct 8, 2013, at 1:19 PM, ushachar@apache.org wrote:
>>>>
>>>>> Updated Branches:
>>>>> refs/heads/master 7ba121c9a -> 3c0c835c1
>>>>>
>>>>>
>>>>> TS-2268 Add support for opening protocol traffic sockets through the
>>>>> traffic_manager.
>>>>
>>>> - How do plugins use this to accept on SSL sockets?
>>>
>>> In the current implementation - they don't. There are a couple of issues to consider here:
>>> 1) How do we allow protocol ordering (what if I want a protocol plugin to handle the connection _before_ the SSL engine)
>>
>> What's the use case for this? A plugin that implements it's own TLS? Is that needed?
>
> I can think of a number of use-cases in my (admittedly less common) transparent deployment land - but how about this:
> ATS is the frontend to all your servers and you want ATS to handle some encrypted connections but not others - making the determination based on SNI ->
>
> Protocol plugin that parses SNI information from the request, passing connections to the SSL engine if they can be handled locally and acting as a tunnel to the OS if they can't.
OK, so in this case, the plugin is listening on a server name? Or does it need to dynamically choose to sometimes accept on a server name?
Does this API help us get closer to that feature?
>>> 2) Protocol Plugin chaining implementation
>>
>> Again, what is the use case? I'm not sure that protocol plugin chaining would look like ...
>
> Encapsulated protocols for example - where you want to have a clean state machine handling each protocol and handing it off to the next one (Sort of like what you did with SPDY).
> I'm not sure what a full implementation of this would look like either....
It sounds interesting in princple :) but maybe off topic for this API discussion?
>>>> - We already have 3 *Accept() APIs, why can't they support this requirement?
>>>
>>> Isn't it worthwhile to have a way to specify that we require the port to be pre-opened? The other option I can see (without adding API like TSPortDescriptorRendevous) would be to pass a flag....
>>
>> I'm not sure that it's worthwhile. If it's protecting against traffic_server crashes, that seems nice to have, but maybe not very important. Is that the only use case?
>
> Yes - and it's a very important one for some transparent deployments. If the proxy hangs connections for a second or two, most likely users won't notice -- if they all get a reset then you're bound to get support calls.
Ah, thanks. Thats something that I did not consider.
There's some interesting features that you are talking about here. I'd like to bring the discussion back to the specific API. Does this API help us get there? What can we do to address the concerns that I raised about it?
J
Re: git commit: TS-2268 Add support for opening protocol traffic
sockets through the traffic_manager.
Posted by James Peach <jp...@apache.org>.
On Oct 17, 2013, at 3:23 PM, Uri Shachar <us...@hotmail.com> wrote:
> On Thu, 17 Oct 2013 11:45:41 -0700 James Peach wrote:
>> On Oct 13, 2013, at 2:15 PM, Uri Shachar <us...@hotmail.com> wrote:
>>> On Wed, 9 Oct 2013 10:41:42 -0700 James Peach wrote:
>>>> On Oct 8, 2013, at 1:19 PM, ushachar@apache.org wrote:
>>>>
>>>>> Updated Branches:
>>>>> refs/heads/master 7ba121c9a -> 3c0c835c1
>>>>>
>>>>>
>>>>> TS-2268 Add support for opening protocol traffic sockets through the
>>>>> traffic_manager.
>>>>
>>>> - How do plugins use this to accept on SSL sockets?
>>>
>>> In the current implementation - they don't. There are a couple of issues to consider here:
>>> 1) How do we allow protocol ordering (what if I want a protocol plugin to handle the connection _before_ the SSL engine)
>>
>> What's the use case for this? A plugin that implements it's own TLS? Is that needed?
>
> I can think of a number of use-cases in my (admittedly less common) transparent deployment land - but how about this:
> ATS is the frontend to all your servers and you want ATS to handle some encrypted connections but not others - making the determination based on SNI ->
>
> Protocol plugin that parses SNI information from the request, passing connections to the SSL engine if they can be handled locally and acting as a tunnel to the OS if they can't.
OK, so in this case, the plugin is listening on a server name? Or does it need to dynamically choose to sometimes accept on a server name?
Does this API help us get closer to that feature?
>>> 2) Protocol Plugin chaining implementation
>>
>> Again, what is the use case? I'm not sure that protocol plugin chaining would look like ...
>
> Encapsulated protocols for example - where you want to have a clean state machine handling each protocol and handing it off to the next one (Sort of like what you did with SPDY).
> I'm not sure what a full implementation of this would look like either....
It sounds interesting in princple :) but maybe off topic for this API discussion?
>>>> - We already have 3 *Accept() APIs, why can't they support this requirement?
>>>
>>> Isn't it worthwhile to have a way to specify that we require the port to be pre-opened? The other option I can see (without adding API like TSPortDescriptorRendevous) would be to pass a flag....
>>
>> I'm not sure that it's worthwhile. If it's protecting against traffic_server crashes, that seems nice to have, but maybe not very important. Is that the only use case?
>
> Yes - and it's a very important one for some transparent deployments. If the proxy hangs connections for a second or two, most likely users won't notice -- if they all get a reset then you're bound to get support calls.
Ah, thanks. Thats something that I did not consider.
There's some interesting features that you are talking about here. I'd like to bring the discussion back to the specific API. Does this API help us get there? What can we do to address the concerns that I raised about it?
J
Re: git commit: TS-2268 Add support for opening protocol traffic
sockets through the traffic_manager.
Posted by James Peach <jp...@apache.org>.
On Oct 17, 2013, at 3:23 PM, Uri Shachar <us...@hotmail.com> wrote:
> On Thu, 17 Oct 2013 11:45:41 -0700 James Peach wrote:
>> On Oct 13, 2013, at 2:15 PM, Uri Shachar <us...@hotmail.com> wrote:
>>> On Wed, 9 Oct 2013 10:41:42 -0700 James Peach wrote:
>>>> On Oct 8, 2013, at 1:19 PM, ushachar@apache.org wrote:
>>>>
>>>>> Updated Branches:
>>>>> refs/heads/master 7ba121c9a -> 3c0c835c1
>>>>>
>>>>>
>>>>> TS-2268 Add support for opening protocol traffic sockets through the
>>>>> traffic_manager.
>>>>
>>>> - How do plugins use this to accept on SSL sockets?
>>>
>>> In the current implementation - they don't. There are a couple of issues to consider here:
>>> 1) How do we allow protocol ordering (what if I want a protocol plugin to handle the connection _before_ the SSL engine)
>>
>> What's the use case for this? A plugin that implements it's own TLS? Is that needed?
>
> I can think of a number of use-cases in my (admittedly less common) transparent deployment land - but how about this:
> ATS is the frontend to all your servers and you want ATS to handle some encrypted connections but not others - making the determination based on SNI ->
>
> Protocol plugin that parses SNI information from the request, passing connections to the SSL engine if they can be handled locally and acting as a tunnel to the OS if they can't.
OK, so in this case, the plugin is listening on a server name? Or does it need to dynamically choose to sometimes accept on a server name?
Does this API help us get closer to that feature?
>>> 2) Protocol Plugin chaining implementation
>>
>> Again, what is the use case? I'm not sure that protocol plugin chaining would look like ...
>
> Encapsulated protocols for example - where you want to have a clean state machine handling each protocol and handing it off to the next one (Sort of like what you did with SPDY).
> I'm not sure what a full implementation of this would look like either....
It sounds interesting in princple :) but maybe off topic for this API discussion?
>>>> - We already have 3 *Accept() APIs, why can't they support this requirement?
>>>
>>> Isn't it worthwhile to have a way to specify that we require the port to be pre-opened? The other option I can see (without adding API like TSPortDescriptorRendevous) would be to pass a flag....
>>
>> I'm not sure that it's worthwhile. If it's protecting against traffic_server crashes, that seems nice to have, but maybe not very important. Is that the only use case?
>
> Yes - and it's a very important one for some transparent deployments. If the proxy hangs connections for a second or two, most likely users won't notice -- if they all get a reset then you're bound to get support calls.
Ah, thanks. Thats something that I did not consider.
There's some interesting features that you are talking about here. I'd like to bring the discussion back to the specific API. Does this API help us get there? What can we do to address the concerns that I raised about it?
J
RE: git commit: TS-2268 Add support for opening protocol traffic
sockets through the traffic_manager.
Posted by Uri Shachar <us...@hotmail.com>.
On Thu, 17 Oct 2013 11:45:41 -0700 James Peach wrote:
> On Oct 13, 2013, at 2:15 PM, Uri Shachar <us...@hotmail.com> wrote:
>> On Wed, 9 Oct 2013 10:41:42 -0700 James Peach wrote:
>>> On Oct 8, 2013, at 1:19 PM, ushachar@apache.org wrote:
>>>
>>>> Updated Branches:
>>>> refs/heads/master 7ba121c9a -> 3c0c835c1
>>>>
>>>>
>>>> TS-2268 Add support for opening protocol traffic sockets through the
>>>> traffic_manager.
>>>
>>> - How do plugins use this to accept on SSL sockets?
>>
>> In the current implementation - they don't. There are a couple of issues to consider here:
>> 1) How do we allow protocol ordering (what if I want a protocol plugin to handle the connection _before_ the SSL engine)
>
> What's the use case for this? A plugin that implements it's own TLS? Is that needed?
I can think of a number of use-cases in my (admittedly less common) transparent deployment land - but how about this:
ATS is the frontend to all your servers and you want ATS to handle some encrypted connections but not others - making the determination based on SNI ->
Protocol plugin that parses SNI information from the request, passing connections to the SSL engine if they can be handled locally and acting as a tunnel to the OS if they can't.
>
>> 2) Protocol Plugin chaining implementation
>
> Again, what is the use case? I'm not sure that protocol plugin chaining would look like ...
Encapsulated protocols for example - where you want to have a clean state machine handling each protocol and handing it off to the next one (Sort of like what you did with SPDY).
I'm not sure what a full implementation of this would look like either....
>
>>
>>
>>> - We already have 3 *Accept() APIs, why can't they support this requirement?
>>
>> Isn't it worthwhile to have a way to specify that we require the port to be pre-opened? The other option I can see (without adding API like TSPortDescriptorRendevous) would be to pass a flag....
>
> I'm not sure that it's worthwhile. If it's protecting against traffic_server crashes, that seems nice to have, but maybe not very important. Is that the only use case?
Yes - and it's a very important one for some transparent deployments. If the proxy hangs connections for a second or two, most likely users won't notice -- if they all get a reset then you're bound to get support calls.
>
>> I don't see a problem with creating multiple variants (in the API layer only) to emphasise the difference in behaviour.
>
> No, I think it's fine if it's necessary. But it's preferable to have a smaller API surface so that developers can reuse their knowledge and have fewer APIs to learn. We also have an easier time documenting and teaching a smaller API set.
>
>>
>>> - The name breaks API conventions (ie. there's no such object as a TSPluginDescriptor)
>>
>> Agreed - TSNamedAccept(STRING) maybe....
>>
>>> I think that a more generally useful facility would be to consider extending the TSPortDescriptor to handle requirement (2). The trick for this will be that traffic_manager has to open the sockets and listen(), but not ever accept(). This means that we need to communicate the descriptor string to traffic_manager. Here are some suggestions on how we could do that:
>>>
>>> a) Add a owner=STRING tag to the socket descriptor. This could cause traffic_manager to listen but not accept. A subsequent call to a new API TSPortDescriptorRendevous(STRING) would return a corresponding TSPortDescriptor that can be used with TSPortDescriptorAccept().
>>
>> In progress on the first half - I dislike TSPortDescriptorRendevous for two reasons:
>> 1) 99.9% of uses would call TSPortDescriptorRendevous and then TSPortDescriptorAccept
>
> Yep, but that's trivial to do.
>
>> 2) Multiple ports with the same owner tag will make the API a bit more complicated since we'll need to return a list
>
> Good point. That's a good argument for not bouncing through a TSPortDescriptor object.
>
> J
Re: git commit: TS-2268 Add support for opening protocol traffic
sockets through the traffic_manager.
Posted by James Peach <ja...@me.com>.
On Oct 13, 2013, at 2:15 PM, Uri Shachar <us...@hotmail.com> wrote:
>
>
>
> Thanks for the review James.
> I agree with most of your points -- with a couple of exceptions:
>
> On Wed, 9 Oct 2013 10:41:42 -0700 James Peach wrote:
>> On Oct 8, 2013, at 1:19 PM, ushachar@apache.org wrote:
>>
>>> Updated Branches:
>>> refs/heads/master 7ba121c9a -> 3c0c835c1
>>>
>>>
>>> TS-2268 Add support for opening protocol traffic sockets through the
>>> traffic_manager.
>>
>> Hi Uri,
>>
>> I don't much like this patch :) From the JIRA ticket, it is intended to (1) allow traffic_manager to hold ports for plugins and (2) support the full port specification syntax.
>>
>> (2) is already supported by the TSPortDescriptor API, which actually supports the requirement better than this implementation does.
>>
>> I can see how (1) is interesting, but I don't think that this is the right API. This implementation looks a lot like it supports one specific use case (not sure what the exact requirement it), but would be difficult to use more generally.
>
> There's also a small (3) - create a single point of configuration for all ATS ports.
I can see that it's nice to have the ports in config. I don't have a strong opinion on whether it really needs to be all on the same line.
>> - How do multiple plugins use this API?
>
> I fully agree (and was planning to add support for this) - You're right that without this the API is probably too specific.
>
>> - How do plugins use this to accept on SSL sockets?
>
> In the current implementation - they don't. There are a couple of issues to consider here:
> 1) How do we allow protocol ordering (what if I want a protocol plugin to handle the connection _before_ the SSL engine)
What's the use case for this? A plugin that implements it's own TLS? Is that needed?
> 2) Protocol Plugin chaining implementation
Again, what is the use case? I'm not sure that protocol plugin chaining would look like ...
>
>
>> - We already have 3 *Accept() APIs, why can't they support this requirement?
>
> Isn't it worthwhile to have a way to specify that we require the port to be pre-opened? The other option I can see (without adding API like TSPortDescriptorRendevous) would be to pass a flag....
I'm not sure that it's worthwhile. If it's protecting against traffic_server crashes, that seems nice to have, but maybe not very important. Is that the only use case?
> I don't see a problem with creating multiple variants (in the API layer only) to emphasise the difference in behaviour.
No, I think it's fine if it's necessary. But it's preferable to have a smaller API surface so that developers can reuse their knowledge and have fewer APIs to learn. We also have an easier time documenting and teaching a smaller API set.
>
>> - The name breaks API conventions (ie. there's no such object as a TSPluginDescriptor)
>
> Agreed - TSNamedAccept(STRING) maybe....
>
>> I think that a more generally useful facility would be to consider extending the TSPortDescriptor to handle requirement (2). The trick for this will be that traffic_manager has to open the sockets and listen(), but not ever accept(). This means that we need to communicate the descriptor string to traffic_manager. Here are some suggestions on how we could do that:
>>
>> a) Add a owner=STRING tag to the socket descriptor. This could cause traffic_manager to listen but not accept. A subsequent call to a new API TSPortDescriptorRendevous(STRING) would return a corresponding TSPortDescriptor that can be used with TSPortDescriptorAccept().
>
> In progress on the first half - I dislike TSPortDescriptorRendevous for two reasons:
> 1) 99.9% of uses would call TSPortDescriptorRendevous and then TSPortDescriptorAccept
Yep, but that's trivial to do.
> 2) Multiple ports with the same owner tag will make the API a bit more complicated since we'll need to return a list
Good point. That's a good argument for not bouncing through a TSPortDescriptor object.
J
RE: git commit: TS-2268 Add support for opening protocol traffic
sockets through the traffic_manager.
Posted by Uri Shachar <us...@hotmail.com>.
Thanks for the review James.
I agree with most of your points -- with a couple of exceptions:
On Wed, 9 Oct 2013 10:41:42 -0700 James Peach wrote:
> On Oct 8, 2013, at 1:19 PM, ushachar@apache.org wrote:
>
> > Updated Branches:
> > refs/heads/master 7ba121c9a -> 3c0c835c1
> >
> >
> > TS-2268 Add support for opening protocol traffic sockets through the
> > traffic_manager.
>
> Hi Uri,
>
> I don't much like this patch :) From the JIRA ticket, it is intended to (1) allow traffic_manager to hold ports for plugins and (2) support the full port specification syntax.
>
> (2) is already supported by the TSPortDescriptor API, which actually supports the requirement better than this implementation does.
>
> I can see how (1) is interesting, but I don't think that this is the right API. This implementation looks a lot like it supports one specific use case (not sure what the exact requirement it), but would be difficult to use more generally.
There's also a small (3) - create a single point of configuration for all ATS ports.
> - How do multiple plugins use this API?
I fully agree (and was planning to add support for this) - You're right that without this the API is probably too specific.
> - How do plugins use this to accept on SSL sockets?
In the current implementation - they don't. There are a couple of issues to consider here:
1) How do we allow protocol ordering (what if I want a protocol plugin to handle the connection _before_ the SSL engine)
2) Protocol Plugin chaining implementation
> - We already have 3 *Accept() APIs, why can't they support this requirement?
Isn't it worthwhile to have a way to specify that we require the port to be pre-opened? The other option I can see (without adding API like TSPortDescriptorRendevous) would be to pass a flag....
I don't see a problem with creating multiple variants (in the API layer only) to emphasise the difference in behaviour.
> - The name breaks API conventions (ie. there's no such object as a TSPluginDescriptor)
Agreed - TSNamedAccept(STRING) maybe....
> I think that a more generally useful facility would be to consider extending the TSPortDescriptor to handle requirement (2). The trick for this will be that traffic_manager has to open the sockets and listen(), but not ever accept(). This means that we need to communicate the descriptor string to traffic_manager. Here are some suggestions on how we could do that:
>
> a) Add a owner=STRING tag to the socket descriptor. This could cause traffic_manager to listen but not accept. A subsequent call to a new API TSPortDescriptorRendevous(STRING) would return a corresponding TSPortDescriptor that can be used with TSPortDescriptorAccept().
In progress on the first half - I dislike TSPortDescriptorRendevous for two reasons:
1) 99.9% of uses would call TSPortDescriptorRendevous and then TSPortDescriptorAccept
2) Multiple ports with the same owner tag will make the API a bit more complicated since we'll need to return a list
> b) Add socket descriptor support to plugins.config. traffic_manager would parse the file and do the initial socket setup. Then you could add a TSPlugin*() API that returns the corresponding set of TSPortDescriptor objects that you can accept on.
>
> c) Add scoping to the records.config syntax. This looks a lot like (a) except it adds a general facility that might be useful in other contexts. The idea is that you allow syntax like "proxy.config.http.server_ports[SCOPE]". This would let the "sidechannel" plugin do TSMgmtStringScopedGet("sidechannel", proxy.config.http.server_ports", &result). Just like (a), you would need to teach traffic_manager about the scoped options, and implement something like TSPortDescriptorRendevous(SCOPE).
>
> J
Cheers,
Uri
Re: git commit: TS-2268 Add support for opening protocol traffic
sockets through the traffic_manager.
Posted by James Peach <jp...@apache.org>.
On Oct 8, 2013, at 1:19 PM, ushachar@apache.org wrote:
> Updated Branches:
> refs/heads/master 7ba121c9a -> 3c0c835c1
>
>
> TS-2268 Add support for opening protocol traffic sockets through the
> traffic_manager.
Hi Uri,
I don't much like this patch :) From the JIRA ticket, it is intended to (1) allow traffic_manager to hold ports for plugins and (2) support the full port specification syntax.
(2) is already supported by the TSPortDescriptor API, which actually supports the requirement better than this implementation does.
I can see how (1) is interesting, but I don't think that this is the right API. This implementation looks a lot like it supports one specific use case (not sure what the exact requirement it), but would be difficult to use more generally.
- How do multiple plugins use this API?
- How do plugins use this to accept on SSL sockets?
- We already have 3 *Accept() APIs, why can't they support this requirement?
- The name breaks API conventions (ie. there's no such object as a TSPluginDescriptor)
I think that a more generally useful facility would be to consider extending the TSPortDescriptor to handle requirement (2). The trick for this will be that traffic_manager has to open the sockets and listen(), but not ever accept(). This means that we need to communicate the descriptor string to traffic_manager. Here are some suggestions on how we could do that:
a) Add a owner=STRING tag to the socket descriptor. This could cause traffic_manager to listen but not accept. A subsequent call to a new API TSPortDescriptorRendevous(STRING) would return a corresponding TSPortDescriptor that can be used with TSPortDescriptorAccept().
b) Add socket descriptor support to plugins.config. traffic_manager would parse the file and do the initial socket setup. Then you could add a TSPlugin*() API that returns the corresponding set of TSPortDescriptor objects that you can accept on.
c) Add scoping to the records.config syntax. This looks a lot like (a) except it adds a general facility that might be useful in other contexts. The idea is that you allow syntax like "proxy.config.http.server_ports[SCOPE]". This would let the "sidechannel" plugin do TSMgmtStringScopedGet("sidechannel", proxy.config.http.server_ports", &result). Just like (a), you would need to teach traffic_manager about the scoped options, and implement something like TSPortDescriptorRendevous(SCOPE).
J
>
>
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/3c0c835c
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/3c0c835c
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/3c0c835c
>
> Branch: refs/heads/master
> Commit: 3c0c835c1b06cb5c76ae8dba5add9a8ffc07495d
> Parents: 7ba121c
> Author: Uri Shachar <us...@apache.org>
> Authored: Tue Oct 8 23:17:08 2013 +0300
> Committer: Uri Shachar <us...@apache.org>
> Committed: Tue Oct 8 23:17:08 2013 +0300
>
> ----------------------------------------------------------------------
> CHANGES | 3 +++
> lib/records/I_RecHttp.h | 8 +++++++-
> lib/records/RecHttp.cc | 5 +++++
> proxy/InkAPI.cc | 19 ++++++++++++++++++-
> proxy/api/ts/experimental.h | 16 ++++++++++++++++
> proxy/http/HttpProxyServerMain.cc | 2 +-
> 6 files changed, 50 insertions(+), 3 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3c0c835c/CHANGES
> ----------------------------------------------------------------------
> diff --git a/CHANGES b/CHANGES
> index c2c52f3..fa566c0 100644
> --- a/CHANGES
> +++ b/CHANGES
> @@ -1,6 +1,9 @@
> -*- coding: utf-8 -*-
> Changes with Apache Traffic Server 4.1.0
>
> + *) [TS-2268] Add support for opening protocol traffic sockets through the
> + traffic_manager. Added TSPluginDescriptorAccept into expiremental API.
> +
> *) [TS-2266] Add a "make rat" Makefile target, to create a RAT report. This
> is used for verifying licensing compliance on the entire source tree.
>
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3c0c835c/lib/records/I_RecHttp.h
> ----------------------------------------------------------------------
> diff --git a/lib/records/I_RecHttp.h b/lib/records/I_RecHttp.h
> index 4bd3de0..241d870 100644
> --- a/lib/records/I_RecHttp.h
> +++ b/lib/records/I_RecHttp.h
> @@ -94,7 +94,8 @@ public:
> TRANSPORT_DEFAULT = 0, ///< Default (normal HTTP).
> TRANSPORT_COMPRESSED, ///< Compressed HTTP.
> TRANSPORT_BLIND_TUNNEL, ///< Blind tunnel (no processing).
> - TRANSPORT_SSL ///< SSL connection.
> + TRANSPORT_SSL, ///< SSL connection.
> + TRANSPORT_PLUGIN /// < Protocol plugin connection
> };
>
> int m_fd; ///< Pre-opened file descriptor if present.
> @@ -134,6 +135,9 @@ public:
> /// Check for SSL port.
> bool isSSL() const;
>
> + /// Check for SSL port.
> + bool isPlugin() const;
> +
> /// Process options text.
> /// @a opts should not contain any whitespace, only the option string.
> /// This object's internal state is updated as specified by @a opts.
> @@ -258,6 +262,7 @@ public:
> static char const* const OPT_TRANSPARENT_FULL; ///< Full transparency.
> static char const* const OPT_TRANSPARENT_PASSTHROUGH; ///< Pass-through non-HTTP.
> static char const* const OPT_SSL; ///< SSL (experimental)
> + static char const* const OPT_PLUGIN; ///< Protocol Plugin handle (experimental)
> static char const* const OPT_BLIND_TUNNEL; ///< Blind tunnel.
> static char const* const OPT_COMPRESSED; ///< Compressed.
> static char const* const OPT_HOST_RES_PREFIX; ///< Set DNS family preference.
> @@ -270,6 +275,7 @@ protected:
> };
>
> inline bool HttpProxyPort::isSSL() const { return TRANSPORT_SSL == m_type; }
> +inline bool HttpProxyPort::isPlugin() const { return TRANSPORT_PLUGIN == m_type; }
>
> inline IpAddr&
> HttpProxyPort::outboundIp(uint16_t family) {
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3c0c835c/lib/records/RecHttp.cc
> ----------------------------------------------------------------------
> diff --git a/lib/records/RecHttp.cc b/lib/records/RecHttp.cc
> index 13a20a6..e9ad2b5 100644
> --- a/lib/records/RecHttp.cc
> +++ b/lib/records/RecHttp.cc
> @@ -77,6 +77,7 @@ char const* const HttpProxyPort::OPT_TRANSPARENT_OUTBOUND = "tr-out";
> char const* const HttpProxyPort::OPT_TRANSPARENT_FULL = "tr-full";
> char const* const HttpProxyPort::OPT_TRANSPARENT_PASSTHROUGH = "tr-pass";
> char const* const HttpProxyPort::OPT_SSL = "ssl";
> +char const* const HttpProxyPort::OPT_PLUGIN = "plugin";
> char const* const HttpProxyPort::OPT_BLIND_TUNNEL = "blind";
> char const* const HttpProxyPort::OPT_COMPRESSED = "compressed";
>
> @@ -264,6 +265,8 @@ HttpProxyPort::processOptions(char const* opts) {
> } else if (0 == strcasecmp(OPT_SSL, item)) {
> m_type = TRANSPORT_SSL;
> m_inbound_transparent_p = m_outbound_transparent_p = false;
> + } else if (0 == strcasecmp(OPT_PLUGIN, item)) {
> + m_type = TRANSPORT_PLUGIN;
> } else if (0 == strcasecmp(OPT_TRANSPARENT_INBOUND, item)) {
> # if TS_USE_TPROXY
> m_inbound_transparent_p = true;
> @@ -399,6 +402,8 @@ HttpProxyPort::print(char* out, size_t n) {
> zret += snprintf(out+zret, n-zret, ":%s", OPT_BLIND_TUNNEL);
> else if (TRANSPORT_SSL == m_type)
> zret += snprintf(out+zret, n-zret, ":%s", OPT_SSL);
> + else if (TRANSPORT_PLUGIN == m_type)
> + zret += snprintf(out+zret, n-zret, ":%s", OPT_PLUGIN);
> else if (TRANSPORT_COMPRESSED == m_type)
> zret += snprintf(out+zret, n-zret, ":%s", OPT_COMPRESSED);
> if (zret >= n) return n;
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3c0c835c/proxy/InkAPI.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/InkAPI.cc b/proxy/InkAPI.cc
> index 1418826..f51f4c8 100644
> --- a/proxy/InkAPI.cc
> +++ b/proxy/InkAPI.cc
> @@ -8204,7 +8204,7 @@ TSPortDescriptorParse(const char * descriptor)
> TSReturnCode
> TSPortDescriptorAccept(TSPortDescriptor descp, TSCont contp)
> {
> - Action * action;
> + Action * action = NULL;
> HttpProxyPort * port = (HttpProxyPort *)descp;
> NetProcessor::AcceptOptions net(make_net_accept_options(*port, 0 /* nthreads */));
>
> @@ -8217,6 +8217,23 @@ TSPortDescriptorAccept(TSPortDescriptor descp, TSCont contp)
> return action ? TS_SUCCESS : TS_ERROR;
> }
>
> +TSReturnCode
> +TSPluginDescriptorAccept(TSCont contp)
> +{
> + Action * action = NULL;
> +
> + HttpProxyPort::Group& proxy_ports = HttpProxyPort::global();
> + for ( int i = 0 , n = proxy_ports.length() ; i < n ; ++i ) {
> + HttpProxyPort& port = proxy_ports[i];
> + if (port.isPlugin()) {
> + NetProcessor::AcceptOptions net(make_net_accept_options(port, 0 /* nthreads */));
> + action = netProcessor.main_accept((INKContInternal *)contp, port.m_fd, net);
> + }
> + }
> + return action ? TS_SUCCESS : TS_ERROR;
> +}
> +
> +
> int
> TSHttpTxnBackgroundFillStarted(TSHttpTxn txnp)
> {
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3c0c835c/proxy/api/ts/experimental.h
> ----------------------------------------------------------------------
> diff --git a/proxy/api/ts/experimental.h b/proxy/api/ts/experimental.h
> index f13edab..75037a2 100644
> --- a/proxy/api/ts/experimental.h
> +++ b/proxy/api/ts/experimental.h
> @@ -219,6 +219,22 @@ extern "C"
> ****************************************************************************/
> tsapi int TSHttpTxnLookingUpTypeGet(TSHttpTxn txnp);
>
> + /**
> + Attempt to attach the contp continuation to sockets that have already been
> + opened by the traffic manager and defined as belonging to plugins (based on
> + records.config configuration). If a connection is successfully accepted,
> + the TS_EVENT_NET_ACCEPT is delivered to the continuation. The event
> + data will be a valid TSVConn bound to the accepted connection.
> + In order to configure such a socket, add the "plugin" keyword to a port
> + in proxy.config.http.server_ports like "8082:plugin"
> + Transparency/IP settings can also be defined, but a port cannot have
> + both the "ssl" or "plugin" keywords configured.
> +
> + Need to update records.config comments on proxy.config.http.server_ports
> + when this option is promoted from experimental.
> + */
> + tsapi TSReturnCode TSPluginDescriptorAccept(TSCont contp);
> +
>
> /**
> Opens a network connection to the host specified by the 'to' sockaddr
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3c0c835c/proxy/http/HttpProxyServerMain.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/http/HttpProxyServerMain.cc b/proxy/http/HttpProxyServerMain.cc
> index cae18bd..ad02779 100644
> --- a/proxy/http/HttpProxyServerMain.cc
> +++ b/proxy/http/HttpProxyServerMain.cc
> @@ -233,7 +233,7 @@ start_HttpProxyServer()
> if (port.isSSL()) {
> if (NULL == sslNetProcessor.main_accept(acceptor._accept, port.m_fd, acceptor._net_opt))
> return;
> - } else {
> + } else if (! port.isPlugin()) {
> if (NULL == netProcessor.main_accept(acceptor._accept, port.m_fd, acceptor._net_opt))
> return;
> }
>
Re: git commit: TS-2268 Add support for opening protocol traffic
sockets through the traffic_manager.
Posted by James Peach <jp...@apache.org>.
On Oct 8, 2013, at 1:19 PM, ushachar@apache.org wrote:
> Updated Branches:
> refs/heads/master 7ba121c9a -> 3c0c835c1
>
>
> TS-2268 Add support for opening protocol traffic sockets through the
> traffic_manager.
Hi Uri,
I don't much like this patch :) From the JIRA ticket, it is intended to (1) allow traffic_manager to hold ports for plugins and (2) support the full port specification syntax.
(2) is already supported by the TSPortDescriptor API, which actually supports the requirement better than this implementation does.
I can see how (1) is interesting, but I don't think that this is the right API. This implementation looks a lot like it supports one specific use case (not sure what the exact requirement it), but would be difficult to use more generally.
- How do multiple plugins use this API?
- How do plugins use this to accept on SSL sockets?
- We already have 3 *Accept() APIs, why can't they support this requirement?
- The name breaks API conventions (ie. there's no such object as a TSPluginDescriptor)
I think that a more generally useful facility would be to consider extending the TSPortDescriptor to handle requirement (2). The trick for this will be that traffic_manager has to open the sockets and listen(), but not ever accept(). This means that we need to communicate the descriptor string to traffic_manager. Here are some suggestions on how we could do that:
a) Add a owner=STRING tag to the socket descriptor. This could cause traffic_manager to listen but not accept. A subsequent call to a new API TSPortDescriptorRendevous(STRING) would return a corresponding TSPortDescriptor that can be used with TSPortDescriptorAccept().
b) Add socket descriptor support to plugins.config. traffic_manager would parse the file and do the initial socket setup. Then you could add a TSPlugin*() API that returns the corresponding set of TSPortDescriptor objects that you can accept on.
c) Add scoping to the records.config syntax. This looks a lot like (a) except it adds a general facility that might be useful in other contexts. The idea is that you allow syntax like "proxy.config.http.server_ports[SCOPE]". This would let the "sidechannel" plugin do TSMgmtStringScopedGet("sidechannel", proxy.config.http.server_ports", &result). Just like (a), you would need to teach traffic_manager about the scoped options, and implement something like TSPortDescriptorRendevous(SCOPE).
J
>
>
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/3c0c835c
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/3c0c835c
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/3c0c835c
>
> Branch: refs/heads/master
> Commit: 3c0c835c1b06cb5c76ae8dba5add9a8ffc07495d
> Parents: 7ba121c
> Author: Uri Shachar <us...@apache.org>
> Authored: Tue Oct 8 23:17:08 2013 +0300
> Committer: Uri Shachar <us...@apache.org>
> Committed: Tue Oct 8 23:17:08 2013 +0300
>
> ----------------------------------------------------------------------
> CHANGES | 3 +++
> lib/records/I_RecHttp.h | 8 +++++++-
> lib/records/RecHttp.cc | 5 +++++
> proxy/InkAPI.cc | 19 ++++++++++++++++++-
> proxy/api/ts/experimental.h | 16 ++++++++++++++++
> proxy/http/HttpProxyServerMain.cc | 2 +-
> 6 files changed, 50 insertions(+), 3 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3c0c835c/CHANGES
> ----------------------------------------------------------------------
> diff --git a/CHANGES b/CHANGES
> index c2c52f3..fa566c0 100644
> --- a/CHANGES
> +++ b/CHANGES
> @@ -1,6 +1,9 @@
> -*- coding: utf-8 -*-
> Changes with Apache Traffic Server 4.1.0
>
> + *) [TS-2268] Add support for opening protocol traffic sockets through the
> + traffic_manager. Added TSPluginDescriptorAccept into expiremental API.
> +
> *) [TS-2266] Add a "make rat" Makefile target, to create a RAT report. This
> is used for verifying licensing compliance on the entire source tree.
>
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3c0c835c/lib/records/I_RecHttp.h
> ----------------------------------------------------------------------
> diff --git a/lib/records/I_RecHttp.h b/lib/records/I_RecHttp.h
> index 4bd3de0..241d870 100644
> --- a/lib/records/I_RecHttp.h
> +++ b/lib/records/I_RecHttp.h
> @@ -94,7 +94,8 @@ public:
> TRANSPORT_DEFAULT = 0, ///< Default (normal HTTP).
> TRANSPORT_COMPRESSED, ///< Compressed HTTP.
> TRANSPORT_BLIND_TUNNEL, ///< Blind tunnel (no processing).
> - TRANSPORT_SSL ///< SSL connection.
> + TRANSPORT_SSL, ///< SSL connection.
> + TRANSPORT_PLUGIN /// < Protocol plugin connection
> };
>
> int m_fd; ///< Pre-opened file descriptor if present.
> @@ -134,6 +135,9 @@ public:
> /// Check for SSL port.
> bool isSSL() const;
>
> + /// Check for SSL port.
> + bool isPlugin() const;
> +
> /// Process options text.
> /// @a opts should not contain any whitespace, only the option string.
> /// This object's internal state is updated as specified by @a opts.
> @@ -258,6 +262,7 @@ public:
> static char const* const OPT_TRANSPARENT_FULL; ///< Full transparency.
> static char const* const OPT_TRANSPARENT_PASSTHROUGH; ///< Pass-through non-HTTP.
> static char const* const OPT_SSL; ///< SSL (experimental)
> + static char const* const OPT_PLUGIN; ///< Protocol Plugin handle (experimental)
> static char const* const OPT_BLIND_TUNNEL; ///< Blind tunnel.
> static char const* const OPT_COMPRESSED; ///< Compressed.
> static char const* const OPT_HOST_RES_PREFIX; ///< Set DNS family preference.
> @@ -270,6 +275,7 @@ protected:
> };
>
> inline bool HttpProxyPort::isSSL() const { return TRANSPORT_SSL == m_type; }
> +inline bool HttpProxyPort::isPlugin() const { return TRANSPORT_PLUGIN == m_type; }
>
> inline IpAddr&
> HttpProxyPort::outboundIp(uint16_t family) {
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3c0c835c/lib/records/RecHttp.cc
> ----------------------------------------------------------------------
> diff --git a/lib/records/RecHttp.cc b/lib/records/RecHttp.cc
> index 13a20a6..e9ad2b5 100644
> --- a/lib/records/RecHttp.cc
> +++ b/lib/records/RecHttp.cc
> @@ -77,6 +77,7 @@ char const* const HttpProxyPort::OPT_TRANSPARENT_OUTBOUND = "tr-out";
> char const* const HttpProxyPort::OPT_TRANSPARENT_FULL = "tr-full";
> char const* const HttpProxyPort::OPT_TRANSPARENT_PASSTHROUGH = "tr-pass";
> char const* const HttpProxyPort::OPT_SSL = "ssl";
> +char const* const HttpProxyPort::OPT_PLUGIN = "plugin";
> char const* const HttpProxyPort::OPT_BLIND_TUNNEL = "blind";
> char const* const HttpProxyPort::OPT_COMPRESSED = "compressed";
>
> @@ -264,6 +265,8 @@ HttpProxyPort::processOptions(char const* opts) {
> } else if (0 == strcasecmp(OPT_SSL, item)) {
> m_type = TRANSPORT_SSL;
> m_inbound_transparent_p = m_outbound_transparent_p = false;
> + } else if (0 == strcasecmp(OPT_PLUGIN, item)) {
> + m_type = TRANSPORT_PLUGIN;
> } else if (0 == strcasecmp(OPT_TRANSPARENT_INBOUND, item)) {
> # if TS_USE_TPROXY
> m_inbound_transparent_p = true;
> @@ -399,6 +402,8 @@ HttpProxyPort::print(char* out, size_t n) {
> zret += snprintf(out+zret, n-zret, ":%s", OPT_BLIND_TUNNEL);
> else if (TRANSPORT_SSL == m_type)
> zret += snprintf(out+zret, n-zret, ":%s", OPT_SSL);
> + else if (TRANSPORT_PLUGIN == m_type)
> + zret += snprintf(out+zret, n-zret, ":%s", OPT_PLUGIN);
> else if (TRANSPORT_COMPRESSED == m_type)
> zret += snprintf(out+zret, n-zret, ":%s", OPT_COMPRESSED);
> if (zret >= n) return n;
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3c0c835c/proxy/InkAPI.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/InkAPI.cc b/proxy/InkAPI.cc
> index 1418826..f51f4c8 100644
> --- a/proxy/InkAPI.cc
> +++ b/proxy/InkAPI.cc
> @@ -8204,7 +8204,7 @@ TSPortDescriptorParse(const char * descriptor)
> TSReturnCode
> TSPortDescriptorAccept(TSPortDescriptor descp, TSCont contp)
> {
> - Action * action;
> + Action * action = NULL;
> HttpProxyPort * port = (HttpProxyPort *)descp;
> NetProcessor::AcceptOptions net(make_net_accept_options(*port, 0 /* nthreads */));
>
> @@ -8217,6 +8217,23 @@ TSPortDescriptorAccept(TSPortDescriptor descp, TSCont contp)
> return action ? TS_SUCCESS : TS_ERROR;
> }
>
> +TSReturnCode
> +TSPluginDescriptorAccept(TSCont contp)
> +{
> + Action * action = NULL;
> +
> + HttpProxyPort::Group& proxy_ports = HttpProxyPort::global();
> + for ( int i = 0 , n = proxy_ports.length() ; i < n ; ++i ) {
> + HttpProxyPort& port = proxy_ports[i];
> + if (port.isPlugin()) {
> + NetProcessor::AcceptOptions net(make_net_accept_options(port, 0 /* nthreads */));
> + action = netProcessor.main_accept((INKContInternal *)contp, port.m_fd, net);
> + }
> + }
> + return action ? TS_SUCCESS : TS_ERROR;
> +}
> +
> +
> int
> TSHttpTxnBackgroundFillStarted(TSHttpTxn txnp)
> {
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3c0c835c/proxy/api/ts/experimental.h
> ----------------------------------------------------------------------
> diff --git a/proxy/api/ts/experimental.h b/proxy/api/ts/experimental.h
> index f13edab..75037a2 100644
> --- a/proxy/api/ts/experimental.h
> +++ b/proxy/api/ts/experimental.h
> @@ -219,6 +219,22 @@ extern "C"
> ****************************************************************************/
> tsapi int TSHttpTxnLookingUpTypeGet(TSHttpTxn txnp);
>
> + /**
> + Attempt to attach the contp continuation to sockets that have already been
> + opened by the traffic manager and defined as belonging to plugins (based on
> + records.config configuration). If a connection is successfully accepted,
> + the TS_EVENT_NET_ACCEPT is delivered to the continuation. The event
> + data will be a valid TSVConn bound to the accepted connection.
> + In order to configure such a socket, add the "plugin" keyword to a port
> + in proxy.config.http.server_ports like "8082:plugin"
> + Transparency/IP settings can also be defined, but a port cannot have
> + both the "ssl" or "plugin" keywords configured.
> +
> + Need to update records.config comments on proxy.config.http.server_ports
> + when this option is promoted from experimental.
> + */
> + tsapi TSReturnCode TSPluginDescriptorAccept(TSCont contp);
> +
>
> /**
> Opens a network connection to the host specified by the 'to' sockaddr
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3c0c835c/proxy/http/HttpProxyServerMain.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/http/HttpProxyServerMain.cc b/proxy/http/HttpProxyServerMain.cc
> index cae18bd..ad02779 100644
> --- a/proxy/http/HttpProxyServerMain.cc
> +++ b/proxy/http/HttpProxyServerMain.cc
> @@ -233,7 +233,7 @@ start_HttpProxyServer()
> if (port.isSSL()) {
> if (NULL == sslNetProcessor.main_accept(acceptor._accept, port.m_fd, acceptor._net_opt))
> return;
> - } else {
> + } else if (! port.isPlugin()) {
> if (NULL == netProcessor.main_accept(acceptor._accept, port.m_fd, acceptor._net_opt))
> return;
> }
>