From 871dd82117f1e9a673938c6987ddf8989485d384 Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Wed, 24 Aug 2022 11:38:03 +0200 Subject: [PATCH] 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. --- include/haproxy/tcpcheck-t.h | 1 + src/check.c | 7 +++++++ src/tcpcheck.c | 11 +++++++---- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/include/haproxy/tcpcheck-t.h b/include/haproxy/tcpcheck-t.h index dcd6e3d6eb..2b9de1f537 100644 --- 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 efca99ce62..5a6ee08a54 100644 --- 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); } + /* 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 f254c80fc8..7e7627a2c8 100644 --- 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); -- 2.39.5