[PATCH] BUILD/MEDIUM defer-accept flag support for FreeBSD proposal
Hi hope this little patch will find its use.
Thanks.
Regards.
From 02dc058b4f0f41ad1deeb581653e1c3cfb2b2432 Mon Sep 17 00:00:00 2001
From: David Carlier
Date: Sat, 6 Feb 2021 12:11:11 +
Subject: [PATCH] BUILD/MEDIUM: proto_tcp defer-accept flag support for
FreeBSD.
FreeBSD has a kernel feature (accf) and a sockopt flag similar to the Linux's TCP_DEFER_ACCEPT to filter incoming data upon ACK. The main difference is the filter needs to be placed when the socket actually listens.
---
src/cfgparse-tcp.c | 4 ++--
src/proto_tcp.c| 12
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/src/cfgparse-tcp.c b/src/cfgparse-tcp.c
index 4dc39d547..e7868e6bf 100644
--- a/src/cfgparse-tcp.c
+++ b/src/cfgparse-tcp.c
@@ -61,7 +61,7 @@ static int bind_parse_transparent(char **args, int cur_arg, struct proxy *px, st
}
#endif
-#ifdef TCP_DEFER_ACCEPT
+#if defined(TCP_DEFER_ACCEPT) || defined(SO_ACCEPTFILTER)
/* parse the "defer-accept" bind keyword */
static int bind_parse_defer_accept(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err)
{
@@ -243,7 +243,7 @@ static int srv_parse_tcp_ut(char **args, int *cur_arg, struct proxy *px, struct
* not enabled.
*/
static struct bind_kw_list bind_kws = { "TCP", { }, {
-#ifdef TCP_DEFER_ACCEPT
+#if defined(TCP_DEFER_ACCEPT) || defined(SO_ACCEPTFILTER)
{ "defer-accept", bind_parse_defer_accept, 0 }, /* wait for some data for 1 second max before doing accept */
#endif
#ifdef SO_BINDTODEVICE
diff --git a/src/proto_tcp.c b/src/proto_tcp.c
index 485603d57..85cd56360 100644
--- a/src/proto_tcp.c
+++ b/src/proto_tcp.c
@@ -711,6 +711,18 @@ int tcp_bind_listener(struct listener *listener, char *errmsg, int errlen)
goto tcp_close_return;
}
+#if defined(SO_ACCEPTFILTER)
+ /* the socket needs to listen first */
+ if (listener->options & LI_O_DEF_ACCEPT) {
+ struct accept_filter_arg accept;
+ memset(, 0, sizeof(accept));
+ strcpy(accept.af_name, "dataready");
+ if (setsockopt(fd, SOL_SOCKET, SO_ACCEPTFILTER, , sizeof(accept)) == -1) {
+ msg = "cannot enable ACCEPT_FILTER";
+ err |= ERR_WARN;
+ }
+ }
+#endif
#if defined(TCP_QUICKACK)
if (listener->options & LI_O_NOQUICKACK)
setsockopt(fd, IPPROTO_TCP, TCP_QUICKACK, , sizeof(zero));
--
2.30.0
Re: minconn, maxconn and fullconn (again, sigh!)
Lukas Tribus wrote: [dd] > > > It would be nice however to understand minconn/fullconn too. If a > > backend has several servers with identical minconn, maxconn and weight, > > what's the point of having minconn? The load will be always distributed > > evenly between all the servers notwithstanding minconn/fullconn, > > correct? > > If the load is REALLY the same, sure. That's just never the case in > real life for a number of reasons: > > - different load-balancing algorithms > - different client behavior > - session persistence > - long-running TCP connections (websocket, et all) Oh, that was the missing link to understand the thing. > > But yes, like I already mentioned, minconn/fullconn is addressing a > very specific requirement that I don't think comes up very often. Thank you Lukas, you have been very helpful. -- Victor Sudakov, VAS4-RIPE, VAS47-RIPN 2:5005/49@fidonet http://vas.tomsk.ru/
Re: minconn, maxconn and fullconn (again, sigh!)
On Thu, 11 Feb 2021 at 05:31, Victor Sudakov wrote: > > Lukas Tribus wrote: > > > > On Wed, 10 Feb 2021 at 16:55, Victor Sudakov wrote: > > > > > > I can even phrase my question in simpler terms. What happens if the sum > > > total of all servers' maxconns in a backend is less than the maxconn > > > value in the frontend pointing to the said backend? > > > > Queueing for "timeout queue" amount of time, and then return 503 error > > And what happens in TCP mode, after the "timeout queue" amount of time? > I suppose the TCP connection with the client is reset? Yes, that's the only choice. > > > > > See: > > > > timeout queue > > https://cbonte.github.io/haproxy-dconv/2.2/configuration.html#4.2-timeout%20queue > > > > maxconn > > https://cbonte.github.io/haproxy-dconv/2.2/configuration.html#5.2-maxconn > > > > > > I really suggest you ignore minconn and fullconn, and focus on maxconn > > instead. The latter is a must-have (and must-understand). Really > > maxconn (global, frontend and per server ) is the single most > > important performance knob in haproxy. > > Maxconn is rather clear, especially when one is sure about two things: > > 1. A server's maxconn value is always a hard limit (notwithstanding if > there is a minconn configured for the server). Yes. > 2. Connections outnumbering the sum total of a backend's servers > maxconns are queued for the "timeout queue" amount of time and then > dropped. If, for any reason, we can't use another server with free slots, yes. > It would be nice however to understand minconn/fullconn too. If a > backend has several servers with identical minconn, maxconn and weight, > what's the point of having minconn? The load will be always distributed > evenly between all the servers notwithstanding minconn/fullconn, > correct? If the load is REALLY the same, sure. That's just never the case in real life for a number of reasons: - different load-balancing algorithms - different client behavior - session persistence - long-running TCP connections (websocket, et all) But yes, like I already mentioned, minconn/fullconn is addressing a very specific requirement that I don't think comes up very often. Lukas
[PATCH v3 4/5] MEDIUM: server: support {check,agent}_addr, agent_port in server state
logical followup from cli commands addition, so that the state server
file stays compatible with the changes made at runtime; use previously
added helper to load server attributes.
also alloc a specific chunk to avoid mixing with other called functions
using it
Signed-off-by: William Dauchy
---
doc/management.txt| 5 +-
include/haproxy/server-t.h| 9 ++-
.../checks/1be_40srv_odd_health_checks.vtc| 2 +-
.../checks/40be_2srv_odd_health_checks.vtc| 2 +-
reg-tests/checks/4be_1srv_health_checks.vtc | 6 +-
src/proxy.c | 41 ++-
src/server.c | 68 +--
7 files changed, 87 insertions(+), 46 deletions(-)
diff --git a/doc/management.txt b/doc/management.txt
index 423c614b2..60e25c7e1 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -2455,7 +2455,10 @@ show servers state []
srv_port:Server port.
srvrecord: DNS SRV record associated to this SRV.
srv_use_ssl: use ssl for server connections.
- srv_check_port: Server check port.
+ srv_check_port: Server health check port.
+ srv_check_addr: Server health check address.
+ srv_agent_addr: Server health agent address.
+ srv_agent_port: Server health agent port.
show sess
Dump all known sessions. Avoid doing this on slow connections as this can
diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index 2ec70dde4..11cb71489 100644
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -126,11 +126,14 @@ enum srv_initaddr {
"srv_port " \
"srvrecord " \
"srv_use_ssl "\
-"srv_check_port"
+"srv_check_port " \
+"srv_check_addr " \
+"srv_agent_addr " \
+"srv_agent_port"
-#define SRV_STATE_FILE_MAX_FIELDS 22
+#define SRV_STATE_FILE_MAX_FIELDS 25
#define SRV_STATE_FILE_NB_FIELDS_VERSION_1 20
-#define SRV_STATE_FILE_NB_FIELDS_VERSION_2 22
+#define SRV_STATE_FILE_NB_FIELDS_VERSION_2 25
#define SRV_STATE_LINE_MAXLEN 512
/* server flags -- 32 bits */
diff --git a/reg-tests/checks/1be_40srv_odd_health_checks.vtc
b/reg-tests/checks/1be_40srv_odd_health_checks.vtc
index f01205295..c279972aa 100644
--- a/reg-tests/checks/1be_40srv_odd_health_checks.vtc
+++ b/reg-tests/checks/1be_40srv_odd_health_checks.vtc
@@ -112,6 +112,6 @@ syslog S -wait
haproxy h1 -cli {
send "show servers state"
-expect ~ "# be_id be_name srv_id srv_name srv_addr srv_op_state
srv_admin_state srv_uweight srv_iweight srv_time_since_last_change
srv_check_status srv_check_result srv_check_health srv_check_state
srv_agent_state bk_f_forced_id srv_f_forced_id srv_fqdn srv_port srvrecord
srv_use_ssl srv_check_port\n2 be1 1 srv0 ${s0_addr} 2 0 1 1 [[:digit:]]+ 1 0 1
0 0 0 0 - ${s0_port} - 0 0\n2 be1 2 srv1 ${s1_addr} 2 0 1 1 [[:digit:]]+ 6
([[:digit:]]+ ){3}0 0 0 - ${s1_port} - 0 0\n2 be1 3 srv2 ${s2_addr} 2 0 1 1
[[:digit:]]+ 1 0 1 0 0 0 0 - ${s2_port} - 0 0\n2 be1 4 srv3 ${s3_addr} 2 0 1 1
[[:digit:]]+ 6 ([[:digit:]]+ ){3}0 0 0 - ${s3_port} - 0 0\n2 be1 5 srv4
${s4_addr} 2 0 1 1 [[:digit:]]+ 1 0 1 0 0 0 0 - ${s4_port} - 0 0\n2 be1 6 srv5
${s5_addr} 2 0 1 1 [[:digit:]]+ 6 ([[:digit:]]+ ){3}0 0 0 - ${s5_port} - 0 0\n2
be1 7 srv6 ${s6_addr} 2 0 1 1 [[:digit:]]+ 1 0 1 0 0 0 0 - ${s6_port} - 0 0\n2
be1 8 srv7 ${s7_addr} 2 0 1 1 [[:digit:]]+ 6 ([[:digit:]]+ ){3}0 0 0 -
${s7_port} - 0 0\n2 be1 9 srv8 ${s8_addr} 2 0 1 1 [[:digit:]]+ 1 0 1 0 0 0 0 -
${s8_port} - 0 0\n2 be1 10 srv9 ${s9_addr} 2 0 1 1 [[:digit:]]+ 6 ([[:digit:]]+
){3}0 0 0 - ${s9_port} - 0 0\n2 be1 11 srv10 ${s10_addr} 2 0 1 1 [[:digit:]]+ 1
0 1 0 0 0 0 - ${s10_port} - 0 0\n2 be1 12 srv11 ${s11_addr} 2 0 1 1
[[:digit:]]+ 6 ([[:digit:]]+ ){3}0 0 0 - ${s11_port} - 0 0\n2 be1 13 srv12
${s12_addr} 2 0 1 1 [[:digit:]]+ 1 0 1 0 0 0 0 - ${s12_port} - 0 0\n2 be1 14
srv13 ${s13_addr} 2 0 1 1 [[:digit:]]+ 6 ([[:digit:]]+ ){3}0 0 0 - ${s13_port}
- 0 0\n2 be1 15 srv14 ${s14_addr} 2 0 1 1 [[:digit:]]+ 1 0 1 0 0 0 0 -
${s14_port} - 0 0\n2 be1 16 srv15 ${s15_addr} 2 0 1 1 [[:digit:]]+ 6
([[:digit:]]+ ){3}0 0 0 - ${s15_port} - 0 0\n2 be1 17 srv16 ${s16_addr} 2 0 1 1
[[:digit:]]+ 1 0 1 0 0 0 0 - ${s16_port} - 0 0\n2 be1 18 srv17 ${s17_addr} 2 0
1 1 [[:digit:]]+ 6 ([[:digit:]]+ ){3}0 0 0 - ${s17_port} - 0 0\n2 be1 19 srv18
${s18_addr} 2 0 1 1 [[:digit:]]+ 1 0 1 0 0 0 0 - ${s18_port} - 0 0\n2 be1 20
srv19 ${s19_addr} 2 0 1 1 [[:digit:]]+ 6 ([[:digit:]]+ ){3}0 0 0 - ${s19_port}
- 0 0\n2 be1 21 srv20 ${s20_addr} 2 0 1 1 [[:digit:]]+ 1 0 1 0 0 0 0 -
${s20_port} - 0 0\n2 be1 22 srv21 ${s21_addr} 2 0 1 1 [[:digit:]]+ 6
([[:digit:]]+ ){3}0 0 0 - ${s21_port} - 0 0\n2 be1 23 srv22 ${s22_addr} 2 0 1 1
[[:digit:]]+ 1 0 1 0 0 0 0 - ${s22_port} - 0 0\n2 be1 24 srv23
[PATCH v3 1/5] MEDIUM: cli: add check-addr command
this patch allows to set server health check address at runtime. In
order to align with `addr` command, also allow to set port optionnaly.
This led to a small refactor in order to use the same function for both
`check-addr` and `check-port` commands.
for `check-port`, we however don't permit the change anymore if checks
are not enabled on the server.
This command becomes more and more useful for people having a consul
like architecture:
- the backend server is located on a container with its own IP
- the health checks are done the consul instance located on the host
with the host IP
Signed-off-by: William Dauchy
---
doc/management.txt | 4 ++
src/server.c | 95 ++
2 files changed, 84 insertions(+), 15 deletions(-)
diff --git a/doc/management.txt b/doc/management.txt
index b74aba769..bff770e4e 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -1842,6 +1842,10 @@ set server / health [ up | stopping |
down ]
switch a server's state regardless of some slow health checks for example.
Note that the change is propagated to tracking servers if any.
+set server / check-addr [port ]
+ Change the IP address used for server health checks.
+ Optionally, change the port used for server health checks.
+
set server / check-port
Change the port used for health checking to
diff --git a/src/server.c b/src/server.c
index 453c59866..74bd972e2 100644
--- a/src/server.c
+++ b/src/server.c
@@ -54,6 +54,8 @@ static int srv_set_fqdn(struct server *srv, const char *fqdn,
int dns_locked);
static void srv_state_parse_line(char *buf, const int version, char **params,
char **srv_params);
static int srv_state_get_version(FILE *f);
static void srv_cleanup_connections(struct server *srv);
+static const char *update_server_check_addr_port(struct server *s, const char
*addr,
+const char *port);
/* List head of all known server keywords */
static struct srv_kw_list srv_keywords = {
@@ -3574,6 +3576,59 @@ int update_server_addr(struct server *s, void *ip, int
ip_sin_family, const char
return 0;
}
+/* update server health check address and port
+ * addr must be ip4 or ip6, it won't be resolved
+ * if one error occurs, don't apply anything
+ * must be called with the server lock held.
+ */
+static const char *update_server_check_addr_port(struct server *s, const char
*addr,
+const char *port)
+{
+ struct sockaddr_storage sk;
+ struct buffer *msg;
+ int new_port;
+
+ msg = get_trash_chunk();
+ chunk_reset(msg);
+
+ if (!(s->check.state & CHK_ST_ENABLED)) {
+ chunk_strcat(msg, "health checks are not enabled on this
server");
+ goto out;
+ }
+ if (addr) {
+ memset(, 0, sizeof(struct sockaddr_storage));
+ if (str2ip2(addr, , 0) == NULL) {
+ chunk_appendf(msg, "invalid addr '%s'", addr);
+ goto out;
+ }
+ }
+ if (port) {
+ if (strl2irc(port, strlen(port), _port) != 0) {
+ chunk_appendf(msg, "provided port is not an integer");
+ goto out;
+ }
+ if (new_port < 0 || new_port > 65535) {
+ chunk_appendf(msg, "provided port is invalid");
+ goto out;
+ }
+ /* prevent the update of port to 0 if MAPPORTS are in use */
+ if ((s->flags & SRV_F_MAPPORTS) && new_port == 0) {
+ chunk_appendf(msg, "can't unset 'port' since MAPPORTS
is in use");
+ goto out;
+ }
+ }
+out:
+ if (msg->data)
+ return msg->area;
+ else {
+ if (addr)
+ s->check.addr = sk;
+ if (port)
+ s->check.port = new_port;
+ }
+ return NULL;
+}
+
/*
* This function update a server's addr and port only for AF_INET and AF_INET6
families.
*
@@ -4406,23 +4461,32 @@ static int cli_parse_set_server(char **args, char
*payload, struct appctx *appct
cli_err(appctx, "cannot allocate memory for new
string.\n");
}
}
- else if (strcmp(args[3], "check-port") == 0) {
- int i = 0;
- if (strl2irc(args[4], strlen(args[4]), ) != 0) {
- cli_err(appctx, "'set server check-port' expects
an integer as argument.\n");
- goto out_unlock;
- }
- if ((i < 0) || (i > 65535)) {
- cli_err(appctx, "provided port is not valid.\n");
+ else if (strcmp(args[3], "check-addr") == 0) {
+ char *addr = NULL;
+ char *port = NULL;
+ if (strlen(args[4]) == 0) {
+
[PATCH v3 5/5] MINOR: server: enhance error precision when applying server state
server health checks and agent parameters are written the same way as
others to be able to enahcne code reuse: basically we make use of
parsing and assignment at the same place. It makes it difficult for
error handling to know whether srv object was modified partially or not.
The problem was already present with SRV resolution though.
I was a bit puzzled about the approach to take to be honest, and I did
not wanted to go into a full refactor, so I assumed it was ok to simply
notify whether the line was failed or partially applied.
Signed-off-by: William Dauchy
---
src/server.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/server.c b/src/server.c
index 739bcfba6..b0a8f90d4 100644
--- a/src/server.c
+++ b/src/server.c
@@ -2625,6 +2625,7 @@ static void srv_update_state(struct server *srv, int
version, char **params)
unsigned int port_svc;
char *srvrecord;
char *addr;
+ int partial_apply = 0;
#ifdef USE_OPENSSL
int use_ssl;
#endif
@@ -2795,6 +2796,7 @@ static void srv_update_state(struct server *srv, int
version, char **params)
/* don't apply anything if one error has been detected */
if (msg->data)
goto out;
+ partial_apply = 1;
/* recover operational state and apply it to this server
* and all servers tracking this one */
@@ -3032,8 +3034,12 @@ static void srv_update_state(struct server *srv, int
version, char **params)
HA_SPIN_UNLOCK(SERVER_LOCK, >lock);
if (msg->data) {
chunk_appendf(msg, "\n");
- ha_warning("server-state application failed for server
'%s/%s'%s",
- srv->proxy->id, srv->id, msg->area);
+ if (partial_apply == 1)
+ ha_warning("server-state partially applied for server
'%s/%s'%s",
+ srv->proxy->id, srv->id, msg->area);
+ else
+ ha_warning("server-state application failed for server
'%s/%s'%s",
+ srv->proxy->id, srv->id, msg->area);
}
end:
free_trash_chunk(msg);
--
2.30.0
[PATCH v3 3/5] MEDIUM: server: add server-states version 2
Even if it is possibly too much work for the current usage, it makes
sure we don't break states file from v2.3 to v2.4; indeed, since v2.3,
we introduced two new fields, so we put them aside to guarantee we can
easily reload from a version 1.
The diff seems huge but there is no specific change apart from:
- introduce v2 where it is needed (parsing, update)
- move away from switch/case in update to be able to reuse code
- move srv lock to the whole function to make it easier
this patch confirm how painful it is to maintain this functionality.
Signed-off-by: William Dauchy
---
include/haproxy/server-t.h | 7 +-
src/server.c | 742 ++---
2 files changed, 367 insertions(+), 382 deletions(-)
diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index b326305e8..2ec70dde4 100644
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -101,9 +101,9 @@ enum srv_initaddr {
} __attribute__((packed));
/* server-state-file version */
-#define SRV_STATE_FILE_VERSION 1
+#define SRV_STATE_FILE_VERSION 2
#define SRV_STATE_FILE_VERSION_MIN 1
-#define SRV_STATE_FILE_VERSION_MAX 1
+#define SRV_STATE_FILE_VERSION_MAX 2
#define SRV_STATE_FILE_FIELD_NAMES \
"be_id " \
"be_name "\
@@ -129,7 +129,8 @@ enum srv_initaddr {
"srv_check_port"
#define SRV_STATE_FILE_MAX_FIELDS 22
-#define SRV_STATE_FILE_NB_FIELDS_VERSION_1 22
+#define SRV_STATE_FILE_NB_FIELDS_VERSION_1 20
+#define SRV_STATE_FILE_NB_FIELDS_VERSION_2 22
#define SRV_STATE_LINE_MAXLEN 512
/* server flags -- 32 bits */
diff --git a/src/server.c b/src/server.c
index 9a2a8c520..c2b272068 100644
--- a/src/server.c
+++ b/src/server.c
@@ -2632,391 +2632,383 @@ static void srv_update_state(struct server *srv, int
version, char **params)
fqdn = NULL;
port_svc = port_check = 0;
msg = get_trash_chunk();
- switch (version) {
- case 1:
- /*
-* now we can proceed with server's state update:
-* srv_addr: params[0]
-* srv_op_state: params[1]
-* srv_admin_state: params[2]
-* srv_uweight: params[3]
-* srv_iweight: params[4]
-* srv_last_time_change: params[5]
-* srv_check_status: params[6]
-* srv_check_result: params[7]
-* srv_check_health: params[8]
-* srv_check_state: params[9]
-* srv_agent_state: params[10]
-* bk_f_forced_id: params[11]
-* srv_f_forced_id: params[12]
-* srv_fqdn: params[13]
-* srv_port: params[14]
-* srvrecord:params[15]
-* srv_use_ssl: params[16]
-* srv_check_port: params[17]
-*/
+ HA_SPIN_LOCK(SERVER_LOCK, >lock);
+
+ if (version >= 1) {
+ /* srv_addr: params[0]
+* srv_op_state: params[1]
+* srv_admin_state: params[2]
+* srv_uweight: params[3]
+* srv_iweight: params[4]
+* srv_last_time_change: params[5]
+* srv_check_status: params[6]
+* srv_check_result: params[7]
+* srv_check_health: params[8]
+* srv_check_state: params[9]
+* srv_agent_state: params[10]
+* bk_f_forced_id: params[11]
+* srv_f_forced_id: params[12]
+* srv_fqdn: params[13]
+* srv_port: params[14]
+* srvrecord:params[15]
+*/
- /* validating srv_op_state */
- p = NULL;
- errno = 0;
- srv_op_state = strtol(params[1], , 10);
- if ((p == params[1]) || errno == EINVAL || errno ==
ERANGE ||
- (srv_op_state != SRV_ST_STOPPED &&
-srv_op_state != SRV_ST_STARTING &&
-srv_op_state != SRV_ST_RUNNING &&
-srv_op_state != SRV_ST_STOPPING)) {
- chunk_appendf(msg, ", invalid srv_op_state
value '%s'", params[1]);
- }
+ /* validating srv_op_state */
+ p = NULL;
+ errno = 0;
+ srv_op_state = strtol(params[1], , 10);
+ if ((p == params[1]) || errno == EINVAL || errno == ERANGE ||
+
[PATCH v3 2/5] MEDIUM: cli: add agent-port command
this patch allows to set agent port at runtime. In order to align with
both `addr` and `check-addr` commands, also add the possibility to
optionnaly set port on `agent-addr` command. This led to a small
refactor in order to use the same function for both `agent-addr` and
`agent-port` commands.
Signed-off-by: William Dauchy
---
doc/management.txt | 6 +++-
src/server.c | 84 +-
2 files changed, 80 insertions(+), 10 deletions(-)
diff --git a/doc/management.txt b/doc/management.txt
index bff770e4e..423c614b2 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -1828,10 +1828,14 @@ set server / agent [ up | down ]
switch a server's state regardless of some slow agent checks for example.
Note that the change is propagated to tracking servers if any.
-set server / agent-addr
+set server / agent-addr [port ]
Change addr for servers agent checks. Allows to migrate agent-checks to
another address at runtime. You can specify both IP and hostname, it will be
resolved.
+ Optionally, change the port agent.
+
+set server / agent-port
+ Change the port used for agent checks.
set server / agent-send
Change agent string sent to agent check target. Allows to update string while
diff --git a/src/server.c b/src/server.c
index 74bd972e2..9a2a8c520 100644
--- a/src/server.c
+++ b/src/server.c
@@ -56,6 +56,8 @@ static int srv_state_get_version(FILE *f);
static void srv_cleanup_connections(struct server *srv);
static const char *update_server_check_addr_port(struct server *s, const char
*addr,
const char *port);
+static const char *update_server_agent_addr_port(struct server *s, const char
*addr,
+const char *port);
/* List head of all known server keywords */
static struct srv_kw_list srv_keywords = {
@@ -3576,6 +3578,54 @@ int update_server_addr(struct server *s, void *ip, int
ip_sin_family, const char
return 0;
}
+/* update agent health check address and port
+ * addr can be ip4/ip6 or a hostname
+ * if one error occurs, don't apply anything
+ * must be called with the server lock held.
+ */
+static const char *update_server_agent_addr_port(struct server *s, const char
*addr,
+const char *port)
+{
+ struct sockaddr_storage sk;
+ struct buffer *msg;
+ int new_port;
+
+ msg = get_trash_chunk();
+ chunk_reset(msg);
+
+ if (!(s->agent.state & CHK_ST_ENABLED)) {
+ chunk_strcat(msg, "agent checks are not enabled on this
server");
+ goto out;
+ }
+ if (addr) {
+ memset(, 0, sizeof(struct sockaddr_storage));
+ if (str2ip(addr, ) == NULL) {
+ chunk_appendf(msg, "invalid addr '%s'", addr);
+ goto out;
+ }
+ }
+ if (port) {
+ if (strl2irc(port, strlen(port), _port) != 0) {
+ chunk_appendf(msg, "provided port is not an integer");
+ goto out;
+ }
+ if (new_port < 0 || new_port > 65535) {
+ chunk_appendf(msg, "provided port is invalid");
+ goto out;
+ }
+ }
+out:
+ if (msg->data)
+ return msg->area;
+ else {
+ if (addr)
+ set_srv_agent_addr(s, );
+ if (port)
+ set_srv_agent_port(s, new_port);
+ }
+ return NULL;
+}
+
/* update server health check address and port
* addr must be ip4 or ip6, it won't be resolved
* if one error occurs, don't apply anything
@@ -4443,15 +4493,31 @@ static int cli_parse_set_server(char **args, char
*payload, struct appctx *appct
cli_err(appctx, "'set server agent' expects 'up'
or 'down'.\n");
}
else if (strcmp(args[3], "agent-addr") == 0) {
- struct sockaddr_storage sk;
-
- memset(, 0, sizeof(sk));
- if (!(sv->agent.state & CHK_ST_ENABLED))
- cli_err(appctx, "agent checks are not enabled on this
server.\n");
- else if (str2ip(args[4], ))
- set_srv_agent_addr(sv, );
- else
- cli_err(appctx, "incorrect addr address given for
agent.\n");
+ char *addr = NULL;
+ char *port = NULL;
+ if (strlen(args[4]) == 0) {
+ cli_err(appctx, "set server / agent-addr requires"
+ " an address and optionally a port.\n");
+ goto out_unlock;
+ }
+ addr = args[4];
+ if (strcmp(args[5], "port") == 0)
+ port = args[6];
+ warning = update_server_agent_addr_port(sv, addr, port);
+
[PATCH v3 0/5] cli commands for checks and agent
Hello Christopher,
Here is some work to finish you week.
I believe I addressed all the points raised:
- warning are no longer emitted when we have "0" or "-" values
- I enhanced the warning output as well, and understood my mistake for
my previous CLEANUP patch removing a space... so I removed this patch
as well.
- Fixed all the chunk_appendf issues.
- I was a bit lazy to address the partial vs complete failure in parsing
as I was a bit puzzled about the approach to take. I think it would be
sad to duplicate the code for pre validation, but on the other hand I
agree it was clear to assume the whole line was not applied at all. I
however concluded it was acceptable to simply know the line was not
fully applied. I believe in that case the user should worry. We can
probably add a way to show where it stopped, but I felt discouraged
with the srv resolution, in the middle of srv port, where it is harder
to have a kinda generic way to know where we stopped.
William Dauchy (5):
MEDIUM: cli: add check-addr command
MEDIUM: cli: add agent-port command
MEDIUM: server: add server-states version 2
MEDIUM: server: support {check,agent}_addr, agent_port in server state
MINOR: server: enhance error precision when applying server state
doc/management.txt| 15 +-
include/haproxy/server-t.h| 16 +-
.../checks/1be_40srv_odd_health_checks.vtc| 2 +-
.../checks/40be_2srv_odd_health_checks.vtc| 2 +-
reg-tests/checks/4be_1srv_health_checks.vtc | 6 +-
src/proxy.c | 41 +-
src/server.c | 971 ++
7 files changed, 612 insertions(+), 441 deletions(-)
--
2.30.0
[PATCH] fix freebsd ci, update freebsd image
Hello, attached patch fix freebsd builds. Ilya From 6b2c848cfa563f5b0c8cd34f3fa5ce99847d91a3 Mon Sep 17 00:00:00 2001 From: Ilya Shipitsin Date: Thu, 11 Feb 2021 23:32:30 +0500 Subject: [PATCH] CI: cirrus: update FreeBSD image to 12.2 we already tried to run FreeBSD-stable. it is pain, so we use FreeBSD releases, we need to keep packages and release in sync. let us update to released FreeBSD-12.2 --- .cirrus.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index e8b70de6d..0ac21bf0d 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -1,7 +1,7 @@ FreeBSD_task: freebsd_instance: matrix: - image_family: freebsd-12-1 + image_family: freebsd-12-2 only_if: $CIRRUS_BRANCH =~ 'master|next' install_script: - pkg update -f && pkg upgrade -y && pkg install -y openssl git gmake lua53 socat -- 2.29.2
Re: [PATCH] cleanup: remove unused variable assignment
Le 10/02/2021 à 09:09, Илья Шипицин a écrit : Hello, this is pure cleanup. should fix #1048 Thanks Ilya, now merged ! -- Christopher Faulet
Re: TCP mode and ultra short lived connection
Thank you very much, Willy! Turning off abortonclose (it was enabled globally) for this particular session really helped :) -- Best regards, Maksim вт, 9 февр. 2021 г. в 17:46, Willy Tarreau : > Hi guys, > > > > I faced a problem dealing with l4 (tcp mode) haproxy-based proxy over > > > Graphite's component receiving metrics from clients and clients who are > > > connecting just to send one or two Graphite-metrics and disconnecting > right > > > after. > > > > > > It looks like this > > > 1. Client connects to haproxy (SYN/SYN-ACK/ACK) > > > 2. Client sends one line of metric > > > 3. Haproxy acknowledges receiving this line (ACK to client) > > > 4. Client disconnects (FIN, FIN-ACK) > > > 5. Haproxy writes 1/-1/0/0 CC-termination state to log without even > trying to connect to a backend and send client's data to it. > > > 6. Metric is lost :( > > > > > > If the client is slow enough between steps 1 and 2 or it sends a bunch > of metrics so haproxy has time to connect to a backend - everything works > like a charm. > > > > The issue though is the client disconnect. If we delay the client > > disconnect, it could work. Try playing with tc by delaying the > > incoming FIN packets for a few hundred milliseconds (make sure you > > only apply this to this particular traffic, for example this > > particular destination port). > > > > In fact it's not that black-or-white. A client disconnecting first > in TCP is *always* a protocol design issue, because it leaves the > source port in TIME_WAIT on the client side for 1 minute (even 4 on > certain legacy stacks), and once all source ports are blocked like > this, the client cannot establish new connections anymore. > > However, this is a situation we *normally* deal with in haproxy: > > - in TCP, we're *supposed* to respect exactly this sequence, and > do the same on the other side since it might be the only way to > pass the protocol from end-to-end ; there's even an series of > test for this one in the old test-fsm.cfg ; > > - in HTTP, we normally pass the request as-is, and prepare for > closing after delivering the response (since some clients are > just netcat scripts). > > But it's well known that in HTTP, a FIN from a client after the request > and before the respones usually corresponds to a browser closing by the > user clicking "stop" or closing a tab. For this reason there's an > option "abortonclose" which is used to abort the request before passing > it to the other side, or while it's still waiting for a connection to > establish. > > It turns out that this "abortonclose" option also works for TCP and > totally makes sense there for a number of protocols. Thus, one > possible explanation is that this option is present in the original > config (maybe even inherited from the defaults section), in which case > this is the desired behavior. It would also correspond to the CC log > output (client closed during connect). > > But it's also possible that we broke something again. This half-closed > client situation was broken a few times in the past because it doesn't > get enough love. It essentially corresponds to a denial-of-service > attempt and rarely to a normal behavior, and is rarely tested from this > last perspective. In addition, the idea of leaving blocked source ports > behind doesn't sound appealing to anyone for a reg-test :-/ > > > In TCP mode, we need to propagate the close from one side to the > > other, as we are not aware of the protocol. Not sure if it is possible > > (or a good idea) to keep sending buffer contents to the backend server > > when the client is already gone. > > It's expected to work and is indeed not a good idea at the same time, > because this forces haproxy to consume all of its source ports very > quickly and makes it trivial for a client to block all of its outgoing > communications by maintaining a load of only ~500 connections per second. > Once this is assumed however, it must be possible (barring any bug, again). > > > "[no] option abortonclose" only affects HTTP, according to the docs. > > I'm pretty sure it's not limited to HTTP because I've met PR_O_ABRT_CLOSE > or something like this quite a few times in the connection setup code. > However it's very possible that the doc isn't clear about this or only > focuses on HTTP since it's where this usually matters. > > Hoping this helps, > Willy >
