MINOR: server: define pool-conn-name keyword

Define a new server keyword pool-conn-name. The purpose of this keyword
will be to identify connections inside the idle connections pool,
replacing SNI in case SSL is not wanted.

This keyword uses a sample expression argument. It thus can reuse
existing function parse_srv_expr() for parsing. In the future, it may be
necessary to define a keyword variant which uses a logformat for
extensability.

This patch only implement parsing. Argument is stored inside new server
field <pool_conn_name> and expression is generated in
_srv_parse_finalize() into <pool_conn_name_expr>.

If pool-conn-name is not set but SNI is, the latter is reused
automatically as pool-conn-name via _srv_parse_finalize(). This ensures
current reuse behavior remains compatible and idle connection reuse will
not mix connections with different SNIs by mistake.

Main usage will be for rhttp when SSL is not wanted between the two
haproxy instances. Previously, it was possible to use "sni" keyword even
without SSL on a server line which have a similar effect. However,
having a dedicated "pool-conn-name" keyword is deemed clearer. Besides,
it would allow for more complex configuration where pool-conn-name and
SNI are use in parallel with different values.

doc/configuration.txt

@@ -7937,10 +7937,11 @@ http-reuse { never | safe | aggressive | always }
 
   When http connection sharing is enabled, a great care is taken to respect the
   connection properties and compatibility. Indeed, some properties are specific
-  and it is not possible to reuse it blindly. Those are the SSL SNI, source
-  and destination address, proxy protocol block as well as tos and mark
-  sockopts. A connection is reused only if it shares the same set of properties
-  with the request.
+  and it is not possible to reuse it blindly. Those are the source and
+  destination address, proxy protocol block as well as tos and mark sockopts.
+  It is also possible to manually define an extra expression identifier using
+  "pool-conn-name" server keyword, or "sni" as a fallback. A connection is
+  reused only if it shares the same set of properties with the request.
 
   Also note that connections with certain bogus authentication schemes (relying
   on the connection) like NTLM are marked private if possible and never shared.
@@ -14329,8 +14330,9 @@ attach-srv <srv> [name <expr>] [ EXPERIMENTAL ]
   An extra parameter <expr> can be specified. Its value is interpreted as a
   sample expression to name the connection inside the server idle pool. When
   routing an outgoing request through this server, this name will be matched
-  against the 'sni' parameter of the server line. Otherwise, the connection
-  will have no name and will only match requests without SNI.
+  against the 'pool-conn-name' parameter of the server line, or 'sni' as a
+  fallback. Otherwise, the connection will have no extra identifier and will
+  only match requests without name.
 
   This rule is only valid for frontend in HTTP mode. Also all listeners must
   not require a protocol different from HTTP/2.
@@ -17575,6 +17577,14 @@ on-marked-up <action>
 
   Actions are disabled by default
 
+pool-conn-name <expr>
+  May be used in the following contexts: http
+
+  Set an expression which will be interpreted to differentiate connections
+  inside the server idle pool. On reuse, only connection with identical name
+  will be eligible. If SNI expression is defined but not this parameter, it
+  will act as idle pool identifier to guarantee that SNI match on reuse.
+
 pool-low-conn <max>
   May be used in the following contexts: http
 

include/haproxy/server-t.h

@@ -301,6 +301,8 @@ struct server {
 	struct srv_per_tgroup *per_tgrp;        /* array of per-tgroup stuff such as idle conns */
 	unsigned int *curr_idle_thr;            /* Current number of orphan idling connections per thread */
 
+	char *pool_conn_name;
+	struct sample_expr *pool_conn_name_expr;
 	unsigned int pool_purge_delay;          /* Delay before starting to purge the idle conns pool */
 	unsigned int low_idle_conns;            /* min idle connection count to start picking from other threads */
 	unsigned int max_idle_conns;            /* Max number of connection allowed in the orphan connections list */

src/http_client.c

@@ -1397,6 +1397,16 @@ static int httpclient_postcheck_proxy(struct proxy *curproxy)
 			err_code |= ERR_ALERT | ERR_FATAL;
 			goto err;
 		}
+
+		srv_ssl->pool_conn_name = strdup(srv_ssl->sni_expr);
+		srv_ssl->pool_conn_name_expr = _parse_srv_expr(srv_ssl->pool_conn_name,
+		                                               &curproxy->conf.args,
+		                                               NULL, 0, NULL);
+		if (!srv_ssl->pool_conn_name_expr) {
+			memprintf(&errmsg, "failed to configure pool-conn-name.");
+			err_code |= ERR_ALERT | ERR_FATAL;
+			goto err;
+		}
 	}
 #endif
 

src/server.c

@@ -1153,6 +1153,26 @@ static int srv_parse_pool_purge_delay(char **args, int *cur_arg, struct proxy *c
 	return 0;
 }
 
+static int srv_parse_pool_conn_name(char **args, int *cur_arg, struct proxy *curproxy, struct server *newsrv, char **err)
+{
+	char *arg;
+
+	arg = args[*cur_arg + 1];
+	if (!*arg) {
+		memprintf(err, "'%s' expects <value> as argument", args[*cur_arg]);
+		return ERR_ALERT | ERR_FATAL;
+	}
+
+	ha_free(&newsrv->pool_conn_name);
+	newsrv->pool_conn_name = strdup(arg);
+	if (!newsrv->pool_conn_name) {
+		memprintf(err, "'%s' : out of memory", args[*cur_arg]);
+		return ERR_ALERT | ERR_FATAL;
+	}
+
+	return 0;
+}
+
 static int srv_parse_pool_low_conn(char **args, int *cur_arg, struct proxy *curproxy, struct server *newsrv, char **err)
 {
 	char *arg;
@@ -2289,6 +2309,7 @@ static struct srv_kw_list srv_kws = { "ALL", { }, {
 	{ "on-error",             srv_parse_on_error,             1,  1,  1 }, /* Configure the action on check failure */
 	{ "on-marked-down",       srv_parse_on_marked_down,       1,  1,  1 }, /* Configure the action when a server is marked down */
 	{ "on-marked-up",         srv_parse_on_marked_up,         1,  1,  1 }, /* Configure the action when a server is marked up */
+	{ "pool-conn-name",       srv_parse_pool_conn_name,       1,  1,  1 }, /* Define expression to identify connections in idle pool */
 	{ "pool-low-conn",        srv_parse_pool_low_conn,        1,  1,  1 }, /* Set the min number of orphan idle connecbefore being allowed to pick from other threads */
 	{ "pool-max-conn",        srv_parse_pool_max_conn,        1,  1,  1 }, /* Set the max number of orphan idle connections, -1 means unlimited */
 	{ "pool-purge-delay",     srv_parse_pool_purge_delay,     1,  1,  1 }, /* Set the time before we destroy orphan idle connections, defaults to 1s */
@@ -2819,6 +2840,8 @@ void srv_settings_cpy(struct server *srv, const struct server *src, int srv_tmpl
 	srv->tcp_ut = src->tcp_ut;
 #endif
 	srv->mux_proto = src->mux_proto;
+	if (srv->pool_conn_name)
+		srv->pool_conn_name = strdup(srv->pool_conn_name);
 	srv->pool_purge_delay = src->pool_purge_delay;
 	srv->low_idle_conns = src->low_idle_conns;
 	srv->max_idle_conns = src->max_idle_conns;
@@ -2922,6 +2945,8 @@ void srv_free_params(struct server *srv)
 	free(srv->per_thr);
 	free(srv->per_tgrp);
 	free(srv->curr_idle_thr);
+	free(srv->pool_conn_name);
+	release_sample_expr(srv->pool_conn_name_expr);
 	free(srv->resolvers_id);
 	free(srv->addr_node.key);
 	free(srv->lb_nodes);
@@ -3119,6 +3144,19 @@ static int _srv_parse_tmpl_init(struct server *srv, struct proxy *px)
 		srv_settings_cpy(newsrv, srv, 1);
 		srv_prepare_for_resolution(newsrv, srv->hostname);
 
+		/* Use sni as fallback if pool_conn_name isn't set */
+		if (!newsrv->pool_conn_name && newsrv->sni_expr) {
+			newsrv->pool_conn_name = strdup(newsrv->sni_expr);
+			if (!newsrv->pool_conn_name)
+				goto err;
+		}
+
+		if (newsrv->pool_conn_name) {
+			newsrv->pool_conn_name_expr = _parse_srv_expr(srv->pool_conn_name, &px->conf.args, NULL, 0, NULL);
+			if (!newsrv->pool_conn_name_expr)
+				goto err;
+		}
+
 		if (newsrv->sni_expr) {
 			newsrv->ssl_ctx.sni = _parse_srv_expr(srv->sni_expr, &px->conf.args, NULL, 0, NULL);
 			if (!newsrv->ssl_ctx.sni)
@@ -3543,6 +3581,24 @@ static int _srv_parse_finalize(char **args, int cur_arg,
 		return ret;
 	}
 
+	/* Use sni as fallback if pool_conn_name isn't set */
+	if (!srv->pool_conn_name && srv->sni_expr) {
+		srv->pool_conn_name = strdup(srv->sni_expr);
+		if (!srv->pool_conn_name) {
+			ha_alert("out of memory\n");
+			return ERR_ALERT | ERR_FATAL;
+		}
+	}
+
+	if ((ret = parse_srv_expr(srv->pool_conn_name, &srv->pool_conn_name_expr,
+	                          px, &errmsg))) {
+		if (errmsg) {
+			ha_alert("error detected while parsing pool-conn-name expression : %s.\n", errmsg);
+			free(errmsg);
+		}
+		return ret;
+	}
+
 	/* A dynamic server is disabled on startup. It must not be counted as
 	 * an active backend entry.
 	 */

src/tcp_act.c

@@ -521,13 +521,13 @@ static int tcp_check_attach_srv(struct act_rule *rule, struct proxy *px, char **
 	}
 
 	if (rule->arg.attach_srv.name) {
-		if (!srv->sni_expr) {
-			memprintf(err, "attach-srv rule has a name argument while server '%s/%s' does not have an sni argument; either add a sni argument to the server or remove the name argument from this attach-srv rule", ist0(be_name), ist0(sv_name));
+		if (!srv->pool_conn_name) {
+			memprintf(err, "attach-srv rule has a name argument while server '%s/%s' does not use pool-conn-name; either reconfigure the server or remove the name argument from this attach-srv rule", ist0(be_name), ist0(sv_name));
 			return 0;
 		}
 	} else {
-		if (srv->sni_expr) {
-			memprintf(err, "attach-srv rule has no name argument while server '%s/%s' has an sni argument; either add a name argument to the attach-srv rule or remove the sni argument from the server", ist0(be_name), ist0(sv_name));
+		if (srv->pool_conn_name) {
+			memprintf(err, "attach-srv rule has no name argument while server '%s/%s' uses pool-conn-name; either add a name argument to the attach-srv rule or reconfigure the server", ist0(be_name), ist0(sv_name));
 			return 0;
 		}
 	}