Re: [PATCH] MINOR: promex: backend aggregated server check status
On Mon, Nov 08, 2021 at 02:31:32PM +0100, William Dauchy wrote: > On Mon, Nov 8, 2021 at 1:52 PM Willy Tarreau wrote: > > Just to be sure, is this something you want to merge into 2.5 or is it > > to be queued next ? I'm fine with both, but I prefer to ask as it's not > > just a one-liner and I don't know the impact on promex. > > I know Christopher was possibly thinking about a more advanced > implementation but I thought it could be ok for a first version. > Probably a good idea to wait for Christopher feedbacks anyway. > I was indeed targeting a late v2.5, despite being a new thing. Sorry > for that and I forgot to add a message about it. > If it works well enough, I will also probably ask for a backport in > 2.4 before the end of the year as I know large clusters are waiting > for this patch to lower their memory consumption in prometheus. OK, thanks for the details. Then we'll just add a hint about the possibilty of a backport to the commit message, as keeping track of the original author's intent is often useful. Thanks, Willy
Re: [PATCH] MINOR: promex: backend aggregated server check status
On Mon, Nov 8, 2021 at 1:52 PM Willy Tarreau wrote: > Just to be sure, is this something you want to merge into 2.5 or is it > to be queued next ? I'm fine with both, but I prefer to ask as it's not > just a one-liner and I don't know the impact on promex. I know Christopher was possibly thinking about a more advanced implementation but I thought it could be ok for a first version. Probably a good idea to wait for Christopher feedbacks anyway. I was indeed targeting a late v2.5, despite being a new thing. Sorry for that and I forgot to add a message about it. If it works well enough, I will also probably ask for a backport in 2.4 before the end of the year as I know large clusters are waiting for this patch to lower their memory consumption in prometheus. Thanks, -- William
Re: [ANNOUNCE] haproxy-2.2.18
Great to hear - thanks ! On Sat, Nov 6, 2021 at 12:58 AM Vincent Bernat wrote: > ❦ 5 November 2021 17:05 -06, Jim Freeman: > > > Might this (or something 2.4-ish) be heading towards bullseye-backports ? > > https://packages.debian.org/search?keywords=haproxy > > https://packages.debian.org/bullseye-backports/ > > 2.4 will be in bullseye-backports. > -- > Don't patch bad code - rewrite it. > - The Elements of Programming Style (Kernighan & Plauger) >
Re: [PATCH] MINOR: promex: backend aggregated server check status
Hi William, On Sun, Nov 07, 2021 at 10:18:47AM +0100, William Dauchy wrote: > - add new metric: `haproxy_backend_agg_server_check_status` > it counts the number of servers matching a specific check status > this permits to exclude per server check status as the usage is often > to rely on the total. Indeed in large setup having thousands of > servers per backend the memory impact is not neglible to store the per > server metric. > - realign promex_str_metrics array > > quite simple implementation - we could improve it later by adding an > internal state to the prometheus exporter, thus to avoid counting at > every dump. > > this patch is an attempt to close github issue #1312 Just to be sure, is this something you want to merge into 2.5 or is it to be queued next ? I'm fine with both, but I prefer to ask as it's not just a one-liner and I don't know the impact on promex. Willy
Re: [PATCH] DOC: stats: fix location of the text representation
On Sat, Nov 06, 2021 at 12:30:43PM +0100, William Dauchy wrote: > `info_field_names` and `stat_field_names` no longer exist and have been > moved in stats.c > To avoid changing this comment, just mention the name of the new table > `info_fields` and `stat_fields` Merged, thanks William. Willy
Re: [PATCH 0/6] Probably final Coccinelle Cleanup
On Mon, Nov 08, 2021 at 12:53:00PM +0100, Tim Düsterhus wrote: > Willy, > > On 11/8/21 11:43 AM, Willy Tarreau wrote: > > > You're totally right. Not only it is redundant, but it is wrong (which > > > is why it is redundant). By being called strncat() one would hope that > > > it never copies more than the source, but here it ignores the source's > > > size and always takes the len argument. I suggest that we replace it > > > with chunk_memcat() everywhere and remove it to avoid any issue. At > > > first glance for now this is harmless, fortunately, but for how long? > > > > I'm going to do it, and have already applied your 6 first patches here. > > Thanks for handling this. > > Can you please revert b9656e48377a9e5359494bce6a413a9915c8f74b then? This > Coccinelle patch is no longer needed / correct now that the function is > gone. OK done. Willy
Re: [PATCH 0/6] Probably final Coccinelle Cleanup
Willy, On 11/8/21 11:43 AM, Willy Tarreau wrote: You're totally right. Not only it is redundant, but it is wrong (which is why it is redundant). By being called strncat() one would hope that it never copies more than the source, but here it ignores the source's size and always takes the len argument. I suggest that we replace it with chunk_memcat() everywhere and remove it to avoid any issue. At first glance for now this is harmless, fortunately, but for how long? I'm going to do it, and have already applied your 6 first patches here. Thanks for handling this. Can you please revert b9656e48377a9e5359494bce6a413a9915c8f74b then? This Coccinelle patch is no longer needed / correct now that the function is gone. Best regards Tim Düsterhus
Limit requests with peers on 2 independent HAProxies to one backend
Hi.
I have 2 LB's which should limit the connection to one backend.
I would try to use "conn_cur" in a stick table and share it via peers.
Have anyone such a solution already in place?
That's my assuption for the config.
```
peers be_pixel_peers
bind 9123
log global
localpeer {{ ansible_nodename }}
server lb1 lb1.domain.com:1024
server lb2 lb2.domain.com:1024
backend be_pixel_persons
log global
acl port_pixel dst_port {{ dst_ports["pixel"] }}
tcp-request content silent-drop if port_pixel !{ src -f
/etc/haproxy/whitelist.acl }
option httpchk GET /alive
http-check connect ssl
timeout check 20s
timeout server 300s
# limit connection to backend
stick-table type ip size 1m expire 10m store conn_cur peers be_pixel_peers
http-request deny if { src,table_table_conn_cur(sc_conn_cur) gt 100 }
http-request capture req.fhdr(Referer) id 0
http-request capture req.fhdr(User-Agent) id 1
http-request capture req.hdr(host) id 2
http-request capture var(txn.cap_alg_keysize) id 3
http-request capture var(txn.cap_cipher) id 4
http-request capture var(txn.cap_protocol) id 5
http-response set-header X-Server %s
balance roundrobin
server pixel_persons1 {{ hosts["pixel_persons1"] }}:8184 resolvers mydns ssl
check check-ssl ca-file /etc/haproxy/letsencryptauthorityx3.pem maxconn 2 weight 20
server pixel_persons2 {{ hosts["pixel_persons2"] }}:8184 resolvers mydns ssl
check check-ssl ca-file /etc/haproxy/letsencryptauthorityx3.pem maxconn 2 weight 20
server pixel_persons3 {{ hosts["pixel_persons3"] }}:8184 resolvers mydns ssl
check check-ssl ca-file /etc/haproxy/letsencryptauthorityx3.pem maxconn 8 weight 80
```
Regards
Alex
Re: [PATCH 0/6] Probably final Coccinelle Cleanup
On Mon, Nov 08, 2021 at 11:41:52AM +0100, Willy Tarreau wrote: > Hi Tim, > > On Mon, Nov 08, 2021 at 09:04:59AM +0100, Tim Duesterhus wrote: > > Hi Willy, > > > > find my (probably :-) ) final CLEANUP series for 2.5. > > > > Regarding the final patch: > > > > 'chunk_strncat()' appears to be completely redundant, it simply passes > > through > > the arguments and even takes an int instead of a size_t. Should it be > > removed? > > You're totally right. Not only it is redundant, but it is wrong (which > is why it is redundant). By being called strncat() one would hope that > it never copies more than the source, but here it ignores the source's > size and always takes the len argument. I suggest that we replace it > with chunk_memcat() everywhere and remove it to avoid any issue. At > first glance for now this is harmless, fortunately, but for how long? I'm going to do it, and have already applied your 6 first patches here. Willy
Re: [PATCH 0/6] Probably final Coccinelle Cleanup
Hi Tim, On Mon, Nov 08, 2021 at 09:04:59AM +0100, Tim Duesterhus wrote: > Hi Willy, > > find my (probably :-) ) final CLEANUP series for 2.5. > > Regarding the final patch: > > 'chunk_strncat()' appears to be completely redundant, it simply passes through > the arguments and even takes an int instead of a size_t. Should it be removed? You're totally right. Not only it is redundant, but it is wrong (which is why it is redundant). By being called strncat() one would hope that it never copies more than the source, but here it ignores the source's size and always takes the len argument. I suggest that we replace it with chunk_memcat() everywhere and remove it to avoid any issue. At first glance for now this is harmless, fortunately, but for how long? Willy
[PATCH 5/6] CLEANUP: Apply ist.cocci
This is to make use of `chunk_istcat()`.
---
src/cache.c | 2 +-
src/http_fetch.c | 2 +-
src/http_htx.c | 4 ++--
src/mux_fcgi.c | 10 +-
src/tcpcheck.c | 4 ++--
5 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/cache.c b/src/cache.c
index e871a7b30..ee42947c1 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -1644,7 +1644,7 @@ int sha1_hosturi(struct stream *s)
chunk_istcat(trash, ctx.value);
}
- chunk_memcat(trash, uri.ptr, uri.len);
+ chunk_istcat(trash, uri);
/* hash everything */
blk_SHA1_Init(_ctx);
diff --git a/src/http_fetch.c b/src/http_fetch.c
index aa4965d0f..99dc89a51 100644
--- a/src/http_fetch.c
+++ b/src/http_fetch.c
@@ -874,7 +874,7 @@ static int smp_fetch_hdr_names(const struct arg *args,
struct sample *smp, const
if (temp->data)
temp->area[temp->data++] = del;
- chunk_memcat(temp, n.ptr, n.len);
+ chunk_istcat(temp, n);
}
smp->data.type = SMP_T_STR;
diff --git a/src/http_htx.c b/src/http_htx.c
index 6b06336e6..3535fa713 100644
--- a/src/http_htx.c
+++ b/src/http_htx.c
@@ -432,7 +432,7 @@ int http_replace_req_path(struct htx *htx, const struct ist
path, int with_qs)
vsn = ist2(temp->area + meth.len, HTX_SL_REQ_VLEN(sl));
chunk_memcat(temp, uri.ptr, p.ptr - uri.ptr); /* uri: host part
*/
- chunk_memcat(temp, path.ptr, path.len); /* uri: new path
*/
+ chunk_istcat(temp, path); /* uri: new path
*/
chunk_memcat(temp, p.ptr + plen, p.len - plen); /* uri: QS part */
uri = ist2(temp->area + meth.len + vsn.len, uri.len - plen + path.len);
@@ -711,7 +711,7 @@ int http_update_authority(struct htx *htx, struct htx_sl
*sl, const struct ist h
vsn = ist2(temp->area + meth.len, HTX_SL_REQ_VLEN(sl));
chunk_memcat(temp, uri.ptr, authority.ptr - uri.ptr);
- chunk_memcat(temp, host.ptr, host.len);
+ chunk_istcat(temp, host);
chunk_memcat(temp, istend(authority), istend(uri) - istend(authority));
uri = ist2(temp->area + meth.len + vsn.len, host.len + uri.len -
authority.len); /* uri */
diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
index f20b46b71..ba3a54617 100644
--- a/src/mux_fcgi.c
+++ b/src/mux_fcgi.c
@@ -1242,17 +1242,17 @@ static int fcgi_set_default_param(struct fcgi_conn
*fconn, struct fcgi_strm *fst
if (!(params->mask & FCGI_SP_REQ_METH)) {
p = htx_sl_req_meth(sl);
params->meth = ist2(b_tail(params->p), p.len);
- chunk_memcat(params->p, p.ptr, p.len);
+ chunk_istcat(params->p, p);
}
if (!(params->mask & FCGI_SP_REQ_URI)) {
p = h1_get_uri(sl);
params->uri = ist2(b_tail(params->p), p.len);
- chunk_memcat(params->p, p.ptr, p.len);
+ chunk_istcat(params->p, p);
}
if (!(params->mask & FCGI_SP_SRV_PROTO)) {
p = htx_sl_req_vsn(sl);
params->vsn = ist2(b_tail(params->p), p.len);
- chunk_memcat(params->p, p.ptr, p.len);
+ chunk_istcat(params->p, p);
}
if (!(params->mask & FCGI_SP_SRV_PORT)) {
char *end;
@@ -1361,7 +1361,7 @@ static int fcgi_set_default_param(struct fcgi_conn
*fconn, struct fcgi_strm *fst
/* Decode the path. it must first be copied to keep the URI
* untouched.
*/
- chunk_memcat(params->p, path.ptr, path.len);
+ chunk_istcat(params->p, path);
path.ptr = b_tail(params->p) - path.len;
len = url_decode(ist0(path), 0);
if (len < 0)
@@ -1415,7 +1415,7 @@ static int fcgi_set_default_param(struct fcgi_conn
*fconn, struct fcgi_strm *fst
struct ist sn = params->scriptname;
params->scriptname = ist2(b_tail(params->p),
len+fconn->app->index.len);
- chunk_memcat(params->p, sn.ptr, sn.len);
+ chunk_istcat(params->p, sn);
chunk_memcat(params->p, fconn->app->index.ptr,
fconn->app->index.len);
}
}
diff --git a/src/tcpcheck.c b/src/tcpcheck.c
index bf497fec5..294e49bcc 100644
--- a/src/tcpcheck.c
+++ b/src/tcpcheck.c
@@ -429,7 +429,7 @@ static void tcpcheck_expect_onerror_message(struct buffer
*msg, struct check *ch
* 4. Otherwise produce the generic tcp-check info message
*/
if (istlen(info)) {
- chunk_strncat(msg, istptr(info), istlen(info));
+ chunk_istcat(msg, info);
goto comment;
}
else if (!LIST_ISEMPTY(>expect.onerror_fmt)) {
@@ -517,7 +517,7 @@ static void tcpcheck_expect_onsuccess_message(struct buffer
*msg, struct check *
* 4. Otherwise produce
[PATCH 6/6] CLEANUP: chunk: Remove duplicated chunk_Xcat implementation
Delegate chunk_istcat, chunk_cat and chunk_strncat to the most generic
chunk_memcat.
---
include/haproxy/chunk.h | 41 +
1 file changed, 13 insertions(+), 28 deletions(-)
diff --git a/include/haproxy/chunk.h b/include/haproxy/chunk.h
index af9ef816b..05fd16121 100644
--- a/include/haproxy/chunk.h
+++ b/include/haproxy/chunk.h
@@ -107,28 +107,6 @@ static inline int chunk_cpy(struct buffer *chk, const
struct buffer *src)
return 1;
}
-/* appends chunk after . Returns 0 in case of failure. */
-static inline int chunk_cat(struct buffer *chk, const struct buffer *src)
-{
- if (unlikely(chk->data + src->data > chk->size))
- return 0;
-
- memcpy(chk->area + chk->data, src->area, src->data);
- chk->data += src->data;
- return 1;
-}
-
-/* appends ist after . Returns 0 in case of failure. */
-static inline int chunk_istcat(struct buffer *chk, const struct ist src)
-{
- if (unlikely(chk->data + src.len > chk->size))
- return 0;
-
- memcpy(chk->area + chk->data, src.ptr, src.len);
- chk->data += src.len;
- return 1;
-}
-
/* copies memory area into for bytes. Returns 0 in
* case of failure. No trailing zero is added.
*/
@@ -158,6 +136,18 @@ static inline int chunk_memcat(struct buffer *chk, const
char *src,
return 1;
}
+/* appends ist after . Returns 0 in case of failure. */
+static inline int chunk_istcat(struct buffer *chk, const struct ist src)
+{
+ return chunk_memcat(chk, istptr(src), istlen(src));
+}
+
+/* appends chunk after . Returns 0 in case of failure. */
+static inline int chunk_cat(struct buffer *chk, const struct buffer *src)
+{
+ return chunk_memcat(chk, src->area, src->data);
+}
+
/* copies str into followed by a trailing zero. Returns 0 in
* case of failure.
*/
@@ -218,12 +208,7 @@ static inline int chunk_strcat(struct buffer *chk, const
char *str)
*/
static inline int chunk_strncat(struct buffer *chk, const char *str, int nb)
{
- if (unlikely(chk->data + nb >= chk->size))
- return 0;
-
- memcpy(chk->area + chk->data, str, nb);
- chk->data += nb;
- return 1;
+ return chunk_memcat(chk, str, nb);
}
/* Adds a trailing zero to the current chunk and returns the pointer to the
--
2.33.1
[PATCH 0/6] Probably final Coccinelle Cleanup
Hi Willy, find my (probably :-) ) final CLEANUP series for 2.5. Regarding the final patch: 'chunk_strncat()' appears to be completely redundant, it simply passes through the arguments and even takes an int instead of a size_t. Should it be removed? Best regards Tim Düsterhus Tim Duesterhus (6): DEV: coccinelle: Add rule to use `isttrim()` where possible CLEANUP: Apply ist.cocci DEV: coccinelle: Add rule to use `chunk_istcat()` instead of `chunk_memcat()` DEV: coccinelle: Add rule to use `chunk_istcat()` instead of `chunk_strncat()` CLEANUP: Apply ist.cocci CLEANUP: chunk: Remove duplicated chunk_Xcat implementation dev/coccinelle/ist.cocci | 24 +++ include/haproxy/chunk.h | 41 +--- src/cache.c | 5 ++--- src/flt_trace.c | 3 +-- src/hlua.c | 6 ++ src/http_ana.c | 3 +-- src/http_fetch.c | 2 +- src/http_htx.c | 4 ++-- src/log.c| 6 ++ src/mux_fcgi.c | 10 +- src/tcpcheck.c | 4 ++-- 11 files changed, 55 insertions(+), 53 deletions(-) -- 2.33.1
[PATCH 1/6] DEV: coccinelle: Add rule to use `isttrim()` where possible
This replaces `if (i.len > e) i.len = e;` by `isttrim(i, e)`.
---
dev/coccinelle/ist.cocci | 8
1 file changed, 8 insertions(+)
diff --git a/dev/coccinelle/ist.cocci b/dev/coccinelle/ist.cocci
index 5b6aa6b2c..7e9a6ac05 100644
--- a/dev/coccinelle/ist.cocci
+++ b/dev/coccinelle/ist.cocci
@@ -44,6 +44,14 @@ struct ist i;
- (\(i.ptr\|istptr(i)\) + \(i.len\|istlen(i)\))
+ istend(i)
+@@
+struct ist i;
+expression e;
+@@
+
+- if (\(i.len\|istlen(i)\) > e) { i.len = e; }
++ i = isttrim(i, e);
+
@@
struct ist i;
@@
--
2.33.1
[PATCH 3/6] DEV: coccinelle: Add rule to use `chunk_istcat()` instead of `chunk_memcat()`
This replaces `chunk_memcat()` with `chunk_istcat()` if the parameters are the
ist's `.ptr` and `.len`.
---
dev/coccinelle/ist.cocci | 8
1 file changed, 8 insertions(+)
diff --git a/dev/coccinelle/ist.cocci b/dev/coccinelle/ist.cocci
index 7e9a6ac05..4945141b2 100644
--- a/dev/coccinelle/ist.cocci
+++ b/dev/coccinelle/ist.cocci
@@ -52,6 +52,14 @@ expression e;
- if (\(i.len\|istlen(i)\) > e) { i.len = e; }
+ i = isttrim(i, e);
+@@
+struct ist i;
+struct buffer *b;
+@@
+
+- chunk_memcat(b, \(i.ptr\|istptr(i)\) , \(i.len\|istlen(i)\));
++ chunk_istcat(b, i);
+
@@
struct ist i;
@@
--
2.33.1
[PATCH 2/6] CLEANUP: Apply ist.cocci
Make use of the new rules to use `isttrim()`. --- src/cache.c | 3 +-- src/flt_trace.c | 3 +-- src/hlua.c | 6 ++ src/http_ana.c | 3 +-- src/log.c | 6 ++ 5 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/cache.c b/src/cache.c index ba2b63c49..e871a7b30 100644 --- a/src/cache.c +++ b/src/cache.c @@ -622,8 +622,7 @@ cache_store_http_payload(struct stream *s, struct filter *filter, struct http_ms case HTX_BLK_DATA: v = htx_get_blk_value(htx, blk); v = istadv(v, offset); - if (v.len > len) - v.len = len; + v = isttrim(v, len); info = (type << 28) + v.len; chunk_memcat(, (char *), sizeof(info)); diff --git a/src/flt_trace.c b/src/flt_trace.c index b3efea6f9..5aabcb2b0 100644 --- a/src/flt_trace.c +++ b/src/flt_trace.c @@ -146,8 +146,7 @@ trace_htx_hexdump(struct htx *htx, unsigned int offset, unsigned int len) v = istadv(v, offset); offset = 0; - if (v.len > len) - v.len = len; + v = isttrim(v, len); len -= v.len; if (type == HTX_BLK_DATA) trace_hexdump(v); diff --git a/src/hlua.c b/src/hlua.c index c2e56b3b9..94f656234 100644 --- a/src/hlua.c +++ b/src/hlua.c @@ -6329,8 +6329,7 @@ static int _hlua_http_msg_dup(struct http_msg *msg, lua_State *L, size_t offset, case HTX_BLK_DATA: v = htx_get_blk_value(htx, blk); v = istadv(v, offset); - if (v.len > len) - v.len = len; + v = isttrim(v, len); luaL_addlstring(, v.ptr, v.len); ret += v.len; @@ -6431,8 +6430,7 @@ static void _hlua_http_msg_delete(struct http_msg *msg, struct filter *filter, s goto end; v = htx_get_blk_value(htx, blk); v.ptr += htxret.ret; - if (v.len > len) - v.len = len; + v = isttrim(v, len); blk = htx_replace_blk_value(htx, blk, v, IST_NULL); len -= v.len; ret += v.len; diff --git a/src/http_ana.c b/src/http_ana.c index 9d11284a5..c037261cf 100644 --- a/src/http_ana.c +++ b/src/http_ana.c @@ -4912,8 +4912,7 @@ static void http_capture_headers(struct htx *htx, char **cap, struct cap_hdr *ca } v = htx_get_blk_value(htx, blk); - if (v.len > h->len) - v.len = h->len; + v = isttrim(v, h->len); memcpy(cap[h->index], v.ptr, v.len); cap[h->index][v.len]=0; diff --git a/src/log.c b/src/log.c index 81bf97b34..e7607c2c4 100644 --- a/src/log.c +++ b/src/log.c @@ -1665,8 +1665,7 @@ static inline void __do_send_log(struct logsrv *logsrv, int nblogger, int level, struct ist msg; msg = ist2(message, size); - if (msg.len > logsrv->maxlen) - msg.len = logsrv->maxlen; + msg = isttrim(msg, logsrv->maxlen); sent = sink_write(logsrv->sink, , 1, level, logsrv->facility, metadata); } @@ -1674,8 +1673,7 @@ static inline void __do_send_log(struct logsrv *logsrv, int nblogger, int level, struct ist msg; msg = ist2(message, size); - if (msg.len > logsrv->maxlen) - msg.len = logsrv->maxlen; + msg = isttrim(msg, logsrv->maxlen); sent = fd_write_frag_line(*plogfd, logsrv->maxlen, msg_header, nbelem, , 1, 1); } -- 2.33.1
[PATCH 4/6] DEV: coccinelle: Add rule to use `chunk_istcat()` instead of `chunk_strncat()`
This replaces `chunk_strncat()` with `chunk_istcat()` if the parameters are the ist's `.ptr` and `.len`. --- dev/coccinelle/ist.cocci | 8 1 file changed, 8 insertions(+) diff --git a/dev/coccinelle/ist.cocci b/dev/coccinelle/ist.cocci index 4945141b2..680afbade 100644 --- a/dev/coccinelle/ist.cocci +++ b/dev/coccinelle/ist.cocci @@ -60,6 +60,14 @@ struct buffer *b; - chunk_memcat(b, \(i.ptr\|istptr(i)\) , \(i.len\|istlen(i)\)); + chunk_istcat(b, i); +@@ +struct ist i; +struct buffer *b; +@@ + +- chunk_strncat(b, \(i.ptr\|istptr(i)\) , \(i.len\|istlen(i)\)); ++ chunk_istcat(b, i); + @@ struct ist i; @@ -- 2.33.1
