On Oct 8, 2013, at 1:19 PM, [email protected] 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 <[email protected]>
> Authored: Tue Oct 8 23:17:08 2013 +0300
> Committer: Uri Shachar <[email protected]>
> 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;
> }
>
