BUG/MINOR: tcpcheck: Disable QUICKACK only if data should be sent after connect

It is only a real problem for agent-checks when there is no agent string to
send. The condition to disable TCP_QUICKACK was only based on the action
type following the connect one. But it is not always accurate. indeed, for
agent-checks, there is always a SEND action. But if there is no "agent-send"
string defined, nothing is sent. In this case, this adds 200ms of latency
with no reason.

To fix the bug, a flag is now used on the CONNECT action to instruct there
are data that should be sent after the connect. For health-checks, this flag
is set if the action following the connect is a SEND action. For
agent-checks, it is set if an "agent-send" string is defined.

This patch should fix the issue #1836. It must be backported as far as 2.2.

diff --git a/include/haproxy/tcpcheck-t.h b/include/haproxy/tcpcheck-t.h
index dcd6e3d6ebff3b69b50c3b31061d5f345af53686..2b9de1f537a8fc18cffcd7cc105bff7da23d95c1 100644 (file)
--- a/include/haproxy/tcpcheck-t.h
+++ b/include/haproxy/tcpcheck-t.h
@@ -35,6 +35,7 @@
 #define TCPCHK_OPT_DEFAULT_CONNECT 0x0008  /* Do a connect using server params */
 #define TCPCHK_OPT_IMPLICIT        0x0010  /* Implicit connect */
 #define TCPCHK_OPT_SOCKS4          0x0020  /* check the connection via socks4 proxy */
+#define TCPCHK_OPT_HAS_DATA        0x0040  /* data should be sent after conncetion */
 
 enum tcpcheck_send_type {
        TCPCHK_SEND_UNDEF = 0,  /* Send is not parsed. */

diff --git a/src/check.c b/src/check.c
index efca99ce6220447f02c14a75943d000453306e2c..5a6ee08a54992c0cf09999674c9dc215446f79e1 100644 (file)
--- a/src/check.c
+++ b/src/check.c
@@ -1688,6 +1688,13 @@ int init_srv_agent_check(struct server *srv)
                LIST_INSERT(srv->agent.tcpcheck_rules->list, &chk->list);
        }
 
+       /* <chk> is always defined here and it is a CONNECT action. If there is
+        * a preset variable, it means there is an agent string defined and data
+        * will be sent after the connect.
+        */
+       if (!LIST_ISEMPTY(&srv->agent.tcpcheck_rules->preset_vars))
+               chk->connect.options |= TCPCHK_OPT_HAS_DATA;
+
 
        err = init_check(&srv->agent, PR_O2_TCPCHK_CHK);
        if (err) {

diff --git a/src/tcpcheck.c b/src/tcpcheck.c
index f254c80fc8ee4411c6b89aaaa0d1f7d43ea28b86..7e7627a2c8fde95a7bb44f786bd0b45752991412 100644 (file)
--- a/src/tcpcheck.c
+++ b/src/tcpcheck.c
@@ -1187,10 +1187,8 @@ enum tcpcheck_eval_ret tcpcheck_eval_connect(struct check *check, struct tcpchec
        if (proto && proto->connect) {
                int flags = 0;
 
-               if (check->tcpcheck_rules->flags & TCPCHK_RULES_PROTO_CHK)
-                       flags |= CONNECT_HAS_DATA;
-               if (!next || next->action != TCPCHK_ACT_EXPECT)
-                       flags |= CONNECT_DELACK_ALWAYS;
+               if (connect->options & TCPCHK_OPT_HAS_DATA)
+                       flags = (CONNECT_HAS_DATA|CONNECT_DELACK_ALWAYS);
                status = proto->connect(conn, flags);
        }
 
@@ -3737,6 +3735,8 @@ static int check_proxy_tcpcheck(struct proxy *px)
         * comment is assigned to the following rule(s).
         */
        list_for_each_entry_safe(chk, back, px->tcpcheck_rules.list, list) {
+               struct tcpcheck_rule *next;
+
                if (chk->action != prev_action && prev_action != TCPCHK_ACT_COMMENT)
                        ha_free(&comment);
 
@@ -3751,6 +3751,9 @@ static int check_proxy_tcpcheck(struct proxy *px)
                case TCPCHK_ACT_CONNECT:
                        if (!chk->comment && comment)
                                chk->comment = strdup(comment);
+                       next = get_next_tcpcheck_rule(&px->tcpcheck_rules, chk);
+                       if (next && next->action == TCPCHK_ACT_SEND)
+                               chk->connect.options |= TCPCHK_OPT_HAS_DATA;
                        /* fall through */
                case TCPCHK_ACT_ACTION_KW:
                        ha_free(&comment);