]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: ring: adjust maxlen consistency check
authorAurelien DARRAGON <adarragon@haproxy.com>
Fri, 23 Jun 2023 16:44:53 +0000 (18:44 +0200)
committerChristopher Faulet <cfaulet@haproxy.com>
Wed, 6 Sep 2023 14:06:39 +0000 (16:06 +0200)
When user specifies a maxlen parameter that is greater than the size of
a given ring section, a warning is emitted to inform that the max length
exceeds size, and then the maxlen is forced to size.

The logic is good, but imprecise, because it doesn't take into account
the slight overhead from storing payloads into the ring.

In practise, we cannot store a single message which is exactly the same
length than size. Doing so will result in the message being dropped at
runtime.

Thanks to the ring_max_payload() function introduced in "MINOR: ring: add
a function to compute max ring payload", we can now deduce the maximum
value for the maxlen parameter before it could result in messages being
dropped.

When maxlen value is set to an improper value, the warning will be emitted
and maxlen will be forced to the maximum "single" payload len that could
fit in the ring buffer, preventing messages from being dropped
unexpectedly.

This commit depends on:
 - "MINOR: ring: add a function to compute max ring payload"

This may be backported as far as 2.2

src/sink.c

index 8e8e7e6d4b6c5e18cba368f63a5e2300f89df439..3c3554fdaec44efedad0f370c33995133b43d541 100644 (file)
@@ -810,6 +810,10 @@ int cfg_parse_ring(const char *file, int linenum, char **args, int kwm)
                        err_code |= ERR_ALERT | ERR_FATAL;
                        goto err;
                }
+               /* set maxlen value to 0 for now, we rely on this in postparsing
+                * to know if it was explicitly set using the "maxlen" parameter
+                */
+               cfg_sink->maxlen = 0;
 
                /* allocate new proxy to handle forwards */
                p = calloc(1, sizeof *p);
@@ -1185,10 +1189,13 @@ int cfg_post_parse_ring()
        struct server *srv;
 
        if (cfg_sink && (cfg_sink->type == SINK_TYPE_BUFFER)) {
-               if (cfg_sink->maxlen > b_size(&cfg_sink->ctx.ring->buf)) {
-                       ha_warning("ring '%s' event max length '%u' exceeds size, forced to size '%lu'.\n",
-                                  cfg_sink->name, cfg_sink->maxlen, (unsigned long)b_size(&cfg_sink->ctx.ring->buf));
-                       cfg_sink->maxlen = b_size(&cfg_sink->ctx.ring->buf);
+               if (!cfg_sink->maxlen)
+                       cfg_sink->maxlen = BUFSIZE; // maxlen not set: use default value
+               else if (cfg_sink->maxlen > ring_max_payload(cfg_sink->ctx.ring)) {
+                       /* maxlen set by user however it doesn't fit: set to max value */
+                       ha_warning("ring '%s' event max length '%u' exceeds max payload size, forced to '%lu'.\n",
+                                  cfg_sink->name, cfg_sink->maxlen, (unsigned long)ring_max_payload(cfg_sink->ctx.ring));
+                       cfg_sink->maxlen = ring_max_payload(cfg_sink->ctx.ring);
                        err_code |= ERR_WARN;
                }