Re: mod_proxy's aside connections proposal

[Eric Covener]

On Tue, Mar 3, 2015 at 9:20 AM, Jim Jagielski  wrote:
> I guess the main question is what are the actual use-cases?


NTLM at the very least. That's the context it comes up in periodically.

Re: mod_proxy's aside connections proposal

[Jim Jagielski]

I guess the main question is what are the actual use-cases?

 On Mar 3, 2015, at 6:29 AM, Jan Kaluža jkal...@redhat.com wrote:
 
 On 09/30/2014 04:47 PM, Yann Ylavic wrote:
 Hello,
 
 I have proposed a patch for PR39673 but I'm not sure it would be
 accepted for mainline httpd, so here I am.
 
 Hi,
 
 I would like to get more opinions on the patch Yann proposed in this email. I 
 fully understand that NTLM is not HTTP/1.1 compatible, because it's not 
 stateless. The question here is if we want to use the approach Yann showed in 
 this patch or we would say that we won't support protocols like that.
 
 Regards,
 Jan Kaluza
 
 The patch adds the possibility (for a module) to acquire a connection
 aside from the worker's reslist, so that it won't be acquired from the
 reslist nor put back to it once released (nor reused for another
 client connection, unless the module does it on its own).
 
 The connection is still bound to a worker, hence it will be
 established/closed according to the worker's configuration (hostname,
 port, SSL, is_address_reusable, TTL...), and the module can still (and
 should) call ap_proxy_determine_connection(),
 ap_proxy_connect_backend() and ap_proxy_release_connection() to do
 that work.
 
 The new function to acquire an aside connection is :
 
 PROXY_DECLARE(int) ap_proxy_acquire_connection_ex(const char *proxy_function,
   proxy_conn_rec **conn,
   proxy_worker *worker,
   server_rec *s, void *baton,
   ap_proxy_acquire_fn 
 acquire,
   ap_proxy_release_fn 
 release);
 
 When called with both baton and acquire set to NULL,
 ap_proxy_acquire_connection_ex() is the same as
 ap_proxy_acquire_connection(), and will acquire connections from the
 reslist.
 
 When acquire is not NULL, it is used as the constructor for *conn
 (given the baton), so it's up to the caller to create a new (aside)
 connection (with the new function ap_proxy_aside_connection() also
 provided) or reuse an existing one according to its criteria (and the
 baton). If release is not NULL, it will be called upon
 ap_proxy_release_connection() with the same baton.
 
 When acquire is NULL but baton isn't, a built-in acquire function is
 used to select an existing or create a new connection associated to a
 conn_rec (and still the worker). The baton is assumed to be a conn_rec
 (eg. the one of the client's connection), and this mode is hence a
 return back to the days of httpd = 2.0.
 This is the trick to respond to PR39673 (NTLM forwarding), or any
 issue due to a session associated to a TCP connection (some
 load-balancers configured to do so expect that), which mod_proxy can't
 forward currently.
 
 The patch then uses the new ap_proxy_acquire_connection_ex() function
 (latter mode) in mod_proxy_http and mod_proxy_ajp (which both have the
 ability to reuse connections), but only when the environment var
 proxy-aside-c is defined. This allows for the administrator to use
 SetEnv[If] or a RewriteRule to enable the functionality, according to
 specific conditions, like (PR39673) :
 
 RewriteCond %{HTTP:Authorization} ^NTLM
 RewriteRule ^ - [E=proxy-aside-c]
 
 The patch is attached, and I tried to make it as generic as I could so
 that it is not PR39673 (NTLM) specific, aside connections looks like a
 nice to have for modules.
 
 This version is slightly different from the one proposed in the PR, in
 that it will always close aside connections associated with a
 non-reusable worker upon release (the PR's one failed to do that), and
 the default release callback (when none is given) for aside
 connections is now to apply the worker's configured TTL (if any).
 
 Do you think this can/should (not) be applied to httpd?
 
 Regards,
 Yann.
 
 

Re: mod_proxy's aside connections proposal

[Jan Kaluža]

On 09/30/2014 04:47 PM, Yann Ylavic wrote:

Hello,

I have proposed a patch for PR39673 but I'm not sure it would be
accepted for mainline httpd, so here I am.


Hi,

I would like to get more opinions on the patch Yann proposed in this 
email. I fully understand that NTLM is not HTTP/1.1 compatible, because 
it's not stateless. The question here is if we want to use the approach 
Yann showed in this patch or we would say that we won't support 
protocols like that.


Regards,
Jan Kaluza


The patch adds the possibility (for a module) to acquire a connection
aside from the worker's reslist, so that it won't be acquired from the
reslist nor put back to it once released (nor reused for another
client connection, unless the module does it on its own).

The connection is still bound to a worker, hence it will be
established/closed according to the worker's configuration (hostname,
port, SSL, is_address_reusable, TTL...), and the module can still (and
should) call ap_proxy_determine_connection(),
ap_proxy_connect_backend() and ap_proxy_release_connection() to do
that work.

The new function to acquire an aside connection is :

PROXY_DECLARE(int) ap_proxy_acquire_connection_ex(const char *proxy_function,
   proxy_conn_rec **conn,
   proxy_worker *worker,
   server_rec *s, void *baton,
   ap_proxy_acquire_fn acquire,
   ap_proxy_release_fn release);

When called with both baton and acquire set to NULL,
ap_proxy_acquire_connection_ex() is the same as
ap_proxy_acquire_connection(), and will acquire connections from the
reslist.

When acquire is not NULL, it is used as the constructor for *conn
(given the baton), so it's up to the caller to create a new (aside)
connection (with the new function ap_proxy_aside_connection() also
provided) or reuse an existing one according to its criteria (and the
baton). If release is not NULL, it will be called upon
ap_proxy_release_connection() with the same baton.

When acquire is NULL but baton isn't, a built-in acquire function is
used to select an existing or create a new connection associated to a
conn_rec (and still the worker). The baton is assumed to be a conn_rec
(eg. the one of the client's connection), and this mode is hence a
return back to the days of httpd = 2.0.
This is the trick to respond to PR39673 (NTLM forwarding), or any
issue due to a session associated to a TCP connection (some
load-balancers configured to do so expect that), which mod_proxy can't
forward currently.

The patch then uses the new ap_proxy_acquire_connection_ex() function
(latter mode) in mod_proxy_http and mod_proxy_ajp (which both have the
ability to reuse connections), but only when the environment var
proxy-aside-c is defined. This allows for the administrator to use
SetEnv[If] or a RewriteRule to enable the functionality, according to
specific conditions, like (PR39673) :

 RewriteCond %{HTTP:Authorization} ^NTLM
 RewriteRule ^ - [E=proxy-aside-c]

The patch is attached, and I tried to make it as generic as I could so
that it is not PR39673 (NTLM) specific, aside connections looks like a
nice to have for modules.

This version is slightly different from the one proposed in the PR, in
that it will always close aside connections associated with a
non-reusable worker upon release (the PR's one failed to do that), and
the default release callback (when none is given) for aside
connections is now to apply the worker's configured TTL (if any).

Do you think this can/should (not) be applied to httpd?

Regards,
Yann.

Re: mod_proxy's aside connections proposal

[Yann Ylavic]

On Tue, Sep 30, 2014 at 4:47 PM, Yann Ylavic ylavic@gmail.com wrote:

 When acquire is NULL but baton isn't, a built-in acquire function is
 used to select an existing or create a new connection associated to a
 conn_rec (and still the worker). The baton is assumed to be a conn_rec
 (eg. the one of the client's connection), and this mode is hence a
 return back to the days of httpd = 2.0.

This mode (the one used when env proxy-aside-c is defined) currently
issues no locking to use the conn_rec, ie. for each request, mod_proxy
will call :

+/* For the given conn_rec (baton), handle a connection per worker */
+static apr_status_t proxy_acquire_from_c(void *baton, proxy_conn_rec **conn,
+ proxy_worker *worker, server_rec *s)
+{
+apr_hash_t *conns;
+conn_rec *c = baton;
+
+conns = ap_get_module_config(c-conn_config, proxy_module);
+if (!conns) {
+conns = apr_hash_make(c-pool);
+ap_set_module_config(c-conn_config, proxy_module, conns);
+*conn = NULL;
+}
+else {
+*conn = apr_hash_get(conns, worker-s, sizeof worker-s);
+}
+if (!*conn) {
+ap_proxy_aside_connection(conn, worker, c-pool);
+apr_hash_set(conns, worker-s, sizeof worker-s, *conn);
+}
+else if ((*conn)-aside-expiry) {
+if (apr_time_now() = (*conn)-aside-expiry) {
+socket_cleanup(*conn);
+}
+(*conn)-aside-expiry = 0;
+}
+
+return APR_SUCCESS;
+}

This assumes there will be only one request handled at a time for a
single connection, and I wonder if, for some MPM (event/windows?),
there may be multiple threads using the same client's conn_rec (eg.
pipelined requests on the same connection), is that ever possible?

In this case, I'd have to add locking there...

mod_proxy's aside connections proposal

[Yann Ylavic]

Hello,

I have proposed a patch for PR39673 but I'm not sure it would be
accepted for mainline httpd, so here I am.

The patch adds the possibility (for a module) to acquire a connection
aside from the worker's reslist, so that it won't be acquired from the
reslist nor put back to it once released (nor reused for another
client connection, unless the module does it on its own).

The connection is still bound to a worker, hence it will be
established/closed according to the worker's configuration (hostname,
port, SSL, is_address_reusable, TTL...), and the module can still (and
should) call ap_proxy_determine_connection(),
ap_proxy_connect_backend() and ap_proxy_release_connection() to do
that work.

The new function to acquire an aside connection is :

PROXY_DECLARE(int) ap_proxy_acquire_connection_ex(const char *proxy_function,
  proxy_conn_rec **conn,
  proxy_worker *worker,
  server_rec *s, void *baton,
  ap_proxy_acquire_fn acquire,
  ap_proxy_release_fn release);

When called with both baton and acquire set to NULL,
ap_proxy_acquire_connection_ex() is the same as
ap_proxy_acquire_connection(), and will acquire connections from the
reslist.

When acquire is not NULL, it is used as the constructor for *conn
(given the baton), so it's up to the caller to create a new (aside)
connection (with the new function ap_proxy_aside_connection() also
provided) or reuse an existing one according to its criteria (and the
baton). If release is not NULL, it will be called upon
ap_proxy_release_connection() with the same baton.

When acquire is NULL but baton isn't, a built-in acquire function is
used to select an existing or create a new connection associated to a
conn_rec (and still the worker). The baton is assumed to be a conn_rec
(eg. the one of the client's connection), and this mode is hence a
return back to the days of httpd = 2.0.
This is the trick to respond to PR39673 (NTLM forwarding), or any
issue due to a session associated to a TCP connection (some
load-balancers configured to do so expect that), which mod_proxy can't
forward currently.

The patch then uses the new ap_proxy_acquire_connection_ex() function
(latter mode) in mod_proxy_http and mod_proxy_ajp (which both have the
ability to reuse connections), but only when the environment var
proxy-aside-c is defined. This allows for the administrator to use
SetEnv[If] or a RewriteRule to enable the functionality, according to
specific conditions, like (PR39673) :

RewriteCond %{HTTP:Authorization} ^NTLM
RewriteRule ^ - [E=proxy-aside-c]

The patch is attached, and I tried to make it as generic as I could so
that it is not PR39673 (NTLM) specific, aside connections looks like a
nice to have for modules.

This version is slightly different from the one proposed in the PR, in
that it will always close aside connections associated with a
non-reusable worker upon release (the PR's one failed to do that), and
the default release callback (when none is given) for aside
connections is now to apply the worker's configured TTL (if any).

Do you think this can/should (not) be applied to httpd?

Regards,
Yann.
Index: modules/proxy/mod_proxy_ajp.c
===
--- modules/proxy/mod_proxy_ajp.c	(revision 1628385)
+++ modules/proxy/mod_proxy_ajp.c	(working copy)
@@ -730,6 +730,12 @@ static int proxy_ajp_handler(request_rec *r, proxy
 ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00895) serving URL %s, url);
 
 /* create space for state information */
+if (apr_table_get(r-subprocess_env, proxy-aside-c)) {
+status = ap_proxy_acquire_connection_ex(scheme, backend, worker,
+r-server, r-connection,
+NULL, NULL);
+}
+else
 status = ap_proxy_acquire_connection(scheme, backend, worker,
  r-server);
 if (status != OK) {
Index: modules/proxy/proxy_util.c
===
--- modules/proxy/proxy_util.c	(revision 1628385)
+++ modules/proxy/proxy_util.c	(working copy)
@@ -72,6 +72,13 @@ static struct wstat {
 {0x0, '\0', NULL}
 };
 
+/* Opaque aside connection record */
+struct ap_proxy_aside_rec {
+void *baton;
+ap_proxy_release_fn release;
+apr_time_t expiry; /* idle connection expiry */
+};
+
 /* Global balancer counter */
 int PROXY_DECLARE_DATA proxy_lb_workers = 0;
 static int lb_workers_limit = 0;
@@ -1351,6 +1358,14 @@ PROXY_DECLARE(int) ap_proxy_connection_reusable(pr
 return ! (conn-close || !worker-s-is_address_reusable || worker-s-disablereuse);
 }
 
+static void socket_cleanup(proxy_conn_rec *conn)
+{
+conn-sock = NULL;
+

Re: mod_proxy's aside connections proposal

[Micha Lenk]

Hi,

On 30.09.2014 16:47, Yann Ylavic wrote:

Do you think this can/should (not) be applied to httpd?


I would love to see this applied to httpd.

Regards,
Micha