MEDIUM: check: Revamp the way the protocol and xprt are determined

Storing the protocol directly into the check was not a good idea,
because the protocol may not be determined until after a DNS resolution
on the server, and may even change at runtime, if the DNS changes.
What we can, however, figure out at start up, is the net_addr_type,
which will contain all that we need to find out which protocol to use
later.
Also revert the changes made by commit 07edaed1918a6433126b4d4d61b7f7b0e9324b30
that would not reuse the server xprt if a different alpn is set for
checks. The alpn is just a string, and should not influence the choice
of the xprt.
We'll now make sure to use the server xprt, unless an address is
provided, in which case we'll use whatever xprt matches that address, or
a port, in which case we'll assume we want TCP, and use check_ssl to
know whetver we want the SSL xprt or not.
Now that the check contains all that is needed to know which protocol to
look up, always just use that when creating a new check connection if it
is the default check connection, and for now, always use TCP when a
tcp-check or http-check connect rule is used (which means those can't be
used for QUIC so far).

This should hopefully fix github issue #3324.

diff --git a/include/haproxy/check-t.h b/include/haproxy/check-t.h
index 720d2ff02951b131c8c5fd1d3e3c1412d875d861..f9f378e8a8c7927f9decd449a3ae4e740dd7239b 100644 (file)
--- a/include/haproxy/check-t.h
+++ b/include/haproxy/check-t.h
@@ -24,6 +24,7 @@
 #include <haproxy/connection-t.h>
 #include <haproxy/dynbuf-t.h>
 #include <haproxy/obj_type-t.h>
+#include <haproxy/tools-t.h>
 #include <haproxy/vars-t.h>
 
 /* Please note: this file tends to commonly be part of circular dependencies,
@@ -189,7 +190,8 @@ struct check {
        char **envp;                            /* the environment to use if running a process-based check */
        struct pid_list *curpid;                /* entry in pid_list used for current process-based test, or -1 if not in test */
        struct sockaddr_storage addr;           /* the address to check */
-       struct protocol *proto;                 /* protocol used for check, may be different from the server's one */
+       struct net_addr_type addr_type;         /* Address type (dgram/stream for both protocol and XPRT) */
+       int alt_proto;                          /* Needed to know exactly which protocol we are after */
        char *pool_conn_name;                   /* conn name used on reuse */
        char *sni;                              /* Server name */
        char *alpn_str;                         /* ALPN to use for checks */

diff --git a/src/check.c b/src/check.c
index 6a29ebe5728e3c05569438179e476f9d8aecd721..45baca97928c5c8e606b73d9d69e12d9e4ef28cb 100644 (file)
--- a/src/check.c
+++ b/src/check.c
@@ -1802,29 +1802,42 @@ int init_srv_check(struct server *srv)
                 * specified.
                 */
                if (!srv->check.port && !is_addr(&srv->check.addr)) {
-                       /*
-                        * If any setting is set for the check, then we can't
-                        * assume we'll use the same XPRT as the server, the
-                        * server may be QUIC, but we want a TCP check.
-                        */
-                       if (!srv->check.use_ssl && srv->use_ssl != -1 &&
-                           !srv->check.via_socks4 && !srv->check.send_proxy &&
-                           (!srv->check.alpn_len || (srv->check.alpn_len == srv->ssl_ctx.alpn_len && !strncmp(srv->check.alpn_str, srv->ssl_ctx.alpn_str, srv->check.alpn_len))) &&
-                           (!srv->check.mux_proto || srv->check.mux_proto != srv->mux_proto))
+                       if ((!srv->check.use_ssl && srv->use_ssl != -1) ||
+                           (srv->check.use_ssl == srv->use_ssl))
                                srv->check.xprt = srv->xprt;
                        else if (srv->check.use_ssl == 1)
                                srv->check.xprt = xprt_get(XPRT_SSL);
                        srv->check.send_proxy |= (srv->pp_opts);
+                       srv->check.addr_type = srv->addr_type;
+                       srv->check.alt_proto = srv->alt_proto;
+               } else {
+                       /* Only port was specified, so let's go with TCP */
+                       if (!is_addr(&srv->check.addr)) {
+                               srv->check.addr_type.proto_type = PROTO_TYPE_STREAM;
+                               srv->check.addr_type.xprt_type = PROTO_TYPE_STREAM;
+                       }
+                       if (net_addr_type_is_quic(&srv->check.addr_type))
+                               srv->check.xprt = xprt_get(XPRT_QUIC);
+                       else
+                               if (srv->check.use_ssl == 1)
+                                       srv->check.xprt = xprt_get(XPRT_SSL);
                }
-               else if (srv->check.use_ssl == 1)
-                       srv->check.xprt = xprt_get(XPRT_SSL);
        }
        else {
                /* For dynamic servers, check-ssl and check-send-proxy must be
                 * explicitly defined even if the check port was not
                 * overridden.
                 */
-               if (srv->check.use_ssl == 1)
+               if (!is_addr(&srv->check.addr)) {
+                       if (srv->check.port) {
+                               srv->check.addr_type.proto_type = PROTO_TYPE_STREAM;
+                               srv->check.addr_type.xprt_type = PROTO_TYPE_STREAM;
+                       } else
+                               srv->check.addr_type = srv->addr_type;
+               }
+               if (net_addr_type_is_quic(&srv->check.addr_type))
+                       srv->check.xprt = xprt_get(XPRT_QUIC);
+               else if (srv->check.use_ssl == 1)
                        srv->check.xprt = xprt_get(XPRT_SSL);
        }
 
@@ -1834,7 +1847,8 @@ int init_srv_check(struct server *srv)
        if (srv->mux_proto && !srv->check.mux_proto &&
            ((srv->mux_proto->mode == PROTO_MODE_HTTP && check_type == TCPCHK_RULES_HTTP_CHK) ||
             (srv->mux_proto->mode == PROTO_MODE_SPOP && check_type == TCPCHK_RULES_SPOP_CHK) ||
-            (srv->mux_proto->mode == PROTO_MODE_TCP && check_type != TCPCHK_RULES_HTTP_CHK))) {
+            (srv->mux_proto->mode == PROTO_MODE_TCP && check_type != TCPCHK_RULES_HTTP_CHK)) &&
+            (net_addr_type_is_quic(&srv->check.addr_type) == srv_is_quic(srv))) {
                srv->check.mux_proto = srv->mux_proto;
        }
        /* test that check proto is valid if explicitly defined */
@@ -1850,7 +1864,7 @@ int init_srv_check(struct server *srv)
 
        /* validate <srv> server health-check settings */
 
-       if (srv_is_quic(srv)) {
+       if (net_addr_type_is_quic(&srv->check.addr_type)) {
                if (srv->check.mux_proto && srv->check.mux_proto != get_mux_proto(ist("quic"))) {
                        ha_alert("config: %s '%s': QUIC server '%s' uses an incompatible MUX protocol for checks.\n",
                                 proxy_type_str(srv->proxy), srv->proxy->id, srv->id);
@@ -2061,8 +2075,9 @@ static int srv_parse_addr(char **args, int *cur_arg, struct proxy *curpx, struct
                          char **errmsg)
 {
        struct sockaddr_storage *sk;
-       struct protocol *proto;
+       struct net_addr_type addr_type;
        int port1, port2, err_code = 0;
+       int alt = 0;
 
 
        if (!*args[*cur_arg+1]) {
@@ -2070,7 +2085,7 @@ static int srv_parse_addr(char **args, int *cur_arg, struct proxy *curpx, struct
                goto error;
        }
 
-       sk = str2sa_range(args[*cur_arg+1], NULL, &port1, &port2, NULL, &proto, NULL, errmsg, NULL, NULL, NULL,
+       sk = str2sa_range(args[*cur_arg+1], NULL, &port1, &port2, NULL, NULL, &addr_type, errmsg, NULL, NULL, &alt,
                          PA_O_RESOLVE | PA_O_PORT_OK | PA_O_STREAM | PA_O_CONNECT);
        if (!sk) {
                memprintf(errmsg, "'%s' : %s", args[*cur_arg], *errmsg);
@@ -2078,7 +2093,8 @@ static int srv_parse_addr(char **args, int *cur_arg, struct proxy *curpx, struct
        }
 
        srv->check.addr = *sk;
-       srv->check.proto = proto;
+       srv->check.addr_type = addr_type;
+       srv->check.alt_proto = alt;
        /* if agentaddr was never set, we can use addr */
        if (!(srv->flags & SRV_F_AGENTADDR))
                srv->agent.addr = *sk;
@@ -2109,10 +2125,9 @@ static int srv_parse_agent_addr(char **args, int *cur_arg, struct proxy *curpx,
        }
        set_srv_agent_addr(srv, &sk);
        /* Agent currently only uses TCP */
-       if (sk.ss_family == AF_INET)
-               srv->agent.proto = &proto_tcpv4;
-       else
-               srv->agent.proto = &proto_tcpv6;
+       srv->agent.addr_type.proto_type = PROTO_TYPE_STREAM;
+       srv->agent.addr_type.xprt_type = PROTO_TYPE_STREAM;
+       srv->agent.alt_proto = 0;
   out:
        return err_code;
 

diff --git a/src/server.c b/src/server.c
index 9c9e09c75fb78915ddd62f0fe00aa77b9b81c730..bf8241b792328f68f4e7ef929fa2fe20167d1869 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -2948,9 +2948,11 @@ void srv_settings_cpy(struct server *srv, const struct server *src, int srv_tmpl
        }
        srv->use_ssl                  = src->use_ssl;
        srv->check.addr               = src->check.addr;
-       srv->check.proto              = src->check.proto;
+       srv->check.addr_type          = src->check.addr_type;
+       srv->check.alt_proto          = src->check.alt_proto;
        srv->agent.addr               = src->agent.addr;
-       srv->agent.proto              = src->agent.proto;
+       srv->agent.addr_type          = src->agent.addr_type;
+       srv->agent.alt_proto          = src->agent.alt_proto;
        srv->check.use_ssl            = src->check.use_ssl;
        srv->check.port               = src->check.port;
        if (src->check.sni != NULL)
@@ -4648,10 +4650,9 @@ out:
                if (port)
                        set_srv_agent_port(s, new_port);
                /* Agent currently only uses TCP */
-               if (sk.ss_family == AF_INET)
-                       s->agent.proto = &proto_tcpv4;
-               else
-                       s->agent.proto = &proto_tcpv6;
+               s->agent.addr_type.proto_type = PROTO_TYPE_STREAM;
+               s->agent.addr_type.xprt_type = PROTO_TYPE_STREAM;
+               s->agent.alt_proto = 0;
        }
        return NULL;
 }
@@ -4664,9 +4665,10 @@ out:
 const char *srv_update_check_addr_port(struct server *s, const char *addr, const char *port)
 {
        struct sockaddr_storage *sk = NULL;
-       struct protocol *proto = NULL;
+       struct net_addr_type addr_type;
        struct buffer *msg;
        int new_port;
+       int alt = 0;
 
        msg = get_trash_chunk();
        chunk_reset(msg);
@@ -4676,7 +4678,7 @@ const char *srv_update_check_addr_port(struct server *s, const char *addr, const
                goto out;
        }
        if (addr) {
-               sk = str2sa_range(addr, NULL, NULL, NULL, NULL, &proto, NULL, NULL, NULL, NULL, NULL, 0);
+               sk = str2sa_range(addr, NULL, NULL, NULL, NULL, NULL, &addr_type, NULL, NULL, NULL, &alt, 0);
                if (sk == NULL) {
                        chunk_appendf(msg, "invalid addr '%s'", addr);
                        goto out;
@@ -4703,7 +4705,8 @@ out:
        else {
                if (sk) {
                        s->check.addr = *sk;
-                       s->check.proto = proto;
+                       s->check.addr_type = addr_type;
+                       s->check.alt_proto = alt;
                }
                if (port)
                        s->check.port = new_port;

diff --git a/src/tcpcheck.c b/src/tcpcheck.c
index a53917747050fa756082332cb51b4b8ad602cf33..657fef757457db0ce3ab578f2058739ee1a8c3a4 100644 (file)
--- a/src/tcpcheck.c
+++ b/src/tcpcheck.c
@@ -1421,25 +1421,14 @@ enum tcpcheck_eval_ret tcpcheck_eval_connect(struct check *check, struct tcpchec
                      ? connect->addr
                      : (is_addr(&check->addr) ? check->addr : s->addr));
 
-       if (s && srv_is_quic(s) && tcpcheck_use_nondefault_connect(check, connect)) {
-               /* For QUIC servers, fallback to TCP checks if any specific
-                * check connection parameter is set.
+       if (connect->options & TCPCHK_OPT_DEFAULT_CONNECT)
+               proto = protocol_lookup(conn->dst->ss_family, check->addr_type.proto_type, check->alt_proto);
+       else {
+               /*
+                * For explicit tcp-check/http-check rules, always assume TCP,
+                * QUIC is not supported yet.
                 */
                proto = protocol_lookup(conn->dst->ss_family, PROTO_TYPE_STREAM, 0);
-               /* Also reset MUX protocol if set to QUIC. */
-               if (check->mux_proto == s->mux_proto)
-                       check->mux_proto = NULL;
-       }
-       else {
-               if (check->proto)
-                       proto = check->proto;
-               else {
-                       if (is_addr(&connect->addr))
-                               proto = protocol_lookup(conn->dst->ss_family, PROTO_TYPE_STREAM, 0);
-                       else
-                               proto = protocol_lookup(conn->dst->ss_family, s->addr_type.proto_type, s->alt_proto);
-
-               }
        }
 
        port = 0;