]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
[MAJOR] buffers: automatically compute the maximum buffer length
authorWilly Tarreau <w@1wt.eu>
Sun, 13 Dec 2009 14:53:05 +0000 (15:53 +0100)
committerWilly Tarreau <w@1wt.eu>
Tue, 22 Dec 2009 09:06:34 +0000 (10:06 +0100)
We used to apply a limit to each buffer's size in order to leave
some room to rewrite headers, then we used to remove this limit
once the session switched to a data state.

Proceeding that way becomes a problem with keepalive because we
have to know when to stop reading too much data into the buffer
so that we can leave some room again to process next requests.

The principle we adopt here consists in only relying on to_forward+send_max.
Indeed, both of those data define how many bytes will leave the buffer.
So as long as their sum is larger than maxrewrite, we can safely
fill the buffers. If they are smaller, then we refrain from filling
the buffer. This means that we won't risk to fill buffers when
reading last data chunk followed by a POST request and its contents.

The only impact identified so far is that we must ensure that the
BF_FULL flag is correctly dropped when starting to forward. Right
now this is OK because nobody inflates to_forward without using
buffer_forward().

include/proto/buffers.h
include/types/buffers.h
src/buffers.c
src/client.c
src/proto_http.c
src/proto_tcp.c
src/session.c
src/stream_sock.c

index 6f10553b0e612447302274b88e285dc782466a7b..c19a284fc94b93ffeae149bf7af38ef6dbc06f2f 100644 (file)
@@ -1,23 +1,23 @@
 /*
-  include/proto/buffers.h
-  Buffer management definitions, macros and inline functions.
-
-  Copyright (C) 2000-2009 Willy Tarreau - w@1wt.eu
-
-  This library is free software; you can redistribute it and/or
-  modify it under the terms of the GNU Lesser General Public
-  License as published by the Free Software Foundation, version 2.1
-  exclusively.
-
-  This library is distributed in the hope that it will be useful,
-  but WITHOUT ANY WARRANTY; without even the implied warranty of
-  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-  Lesser General Public License for more details.
-
-  You should have received a copy of the GNU Lesser General Public
-  License along with this library; if not, write to the Free Software
-  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
-*/
* include/proto/buffers.h
* Buffer management definitions, macros and inline functions.
+ *
* Copyright (C) 2000-2009 Willy Tarreau - w@1wt.eu
+ *
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation, version 2.1
* exclusively.
+ *
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
* Lesser General Public License for more details.
+ *
* You should have received a copy of the GNU Lesser General Public
* License along with this library; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ */
 
 #ifndef _PROTO_BUFFERS_H
 #define _PROTO_BUFFERS_H
 #include <common/time.h>
 
 #include <types/buffers.h>
+#include <types/global.h>
 
 extern struct pool_head *pool2_buffer;
 
 /* perform minimal intializations, report 0 in case of error, 1 if OK. */
 int init_buffer();
 
-/* Initializes all fields in the buffer. The ->max_len field is initialized last
- * so that the compiler can optimize it away if changed immediately after the
- * call to this function. By default, it is set to the full size of the buffer.
- * This implies that buffer_init() must only be called once ->size is set !
- * The BF_OUT_EMPTY flags is set.
- */
+/* Initialize all fields in the buffer. The BF_OUT_EMPTY flags is set. */
 static inline void buffer_init(struct buffer *buf)
 {
        buf->send_max = 0;
@@ -54,7 +50,19 @@ static inline void buffer_init(struct buffer *buf)
        buf->cons = NULL;
        buf->flags = BF_OUT_EMPTY;
        buf->r = buf->lr = buf->w = buf->data;
-       buf->max_len = buf->size;
+}
+
+/* Return the max number of bytes the buffer can contain so that once all the
+ * pending bytes are forwarded, the buffer still has global.tune.maxrewrite
+ * bytes free. The result sits between buf->size - maxrewrite and buf->size.
+ */
+static inline int buffer_max_len(struct buffer *buf)
+{
+       if (buf->to_forward == BUF_INFINITE_FORWARD ||
+           buf->to_forward + buf->send_max >= global.tune.maxrewrite)
+               return buf->size;
+       else
+               return buf->size - global.tune.maxrewrite + buf->to_forward + buf->send_max;
 }
 
 /* Check buffer timeouts, and set the corresponding flags. The
@@ -102,12 +110,16 @@ static inline void buffer_forward(struct buffer *buf, unsigned long bytes)
        if (buf->send_max)
                buf->flags &= ~BF_OUT_EMPTY;
 
-       if (buf->to_forward == BUF_INFINITE_FORWARD)
-               return;
+       if (buf->to_forward != BUF_INFINITE_FORWARD) {
+               buf->to_forward += bytes - data_left;
+               if (bytes == BUF_INFINITE_FORWARD)
+                       buf->to_forward = bytes;
+       }
 
-       buf->to_forward += bytes - data_left;
-       if (bytes == BUF_INFINITE_FORWARD)
-               buf->to_forward = bytes;
+       if (buf->l < buffer_max_len(buf))
+               buf->flags &= ~BF_FULL;
+       else
+               buf->flags |= BF_FULL;
 }
 
 /* Schedule all remaining buffer data to be sent. send_max is not touched if it
@@ -135,8 +147,6 @@ static inline void buffer_erase(struct buffer *buf)
        buf->flags &= ~(BF_FULL | BF_OUT_EMPTY);
        if (!buf->pipe)
                buf->flags |= BF_OUT_EMPTY;
-       if (!buf->max_len)
-               buf->flags |= BF_FULL;
 }
 
 /* Cut the "tail" of the buffer, which means strip it to the length of unsent
@@ -159,7 +169,7 @@ static inline void buffer_cut_tail(struct buffer *buf)
                buf->r -= buf->size;
        buf->lr = buf->r;
        buf->flags &= ~BF_FULL;
-       if (buf->l >= buf->max_len)
+       if (buf->l >= buffer_max_len(buf))
                buf->flags |= BF_FULL;
 }
 
@@ -238,18 +248,6 @@ static inline int buffer_max(const struct buffer *buf)
                return buf->w - buf->r;
 }
 
-/* sets the buffer read limit to <size> bytes, and adjusts the FULL
- * flag accordingly.
- */
-static inline void buffer_set_rlim(struct buffer *buf, int size)
-{
-       buf->max_len = size;
-       if (buf->l < size)
-               buf->flags &= ~BF_FULL;
-       else
-               buf->flags |= BF_FULL;
-}
-
 /*
  * Tries to realign the given buffer, and returns how many bytes can be written
  * there at once without overwriting anything.
@@ -275,15 +273,15 @@ static inline int buffer_contig_space(struct buffer *buf)
 
        if (buf->l == 0) {
                buf->r = buf->w = buf->lr = buf->data;
-               ret = buf->max_len;
+               ret = buffer_max_len(buf);
        }
        else if (buf->r > buf->w) {
-               ret = buf->data + buf->max_len - buf->r;
+               ret = buf->data + buffer_max_len(buf) - buf->r;
        }
        else {
                ret = buf->w - buf->r;
-               if (ret > buf->max_len)
-                       ret = buf->max_len;
+               if (ret > buffer_max_len(buf))
+                       ret = buffer_max_len(buf);
        }
        return ret;
 }
@@ -337,7 +335,7 @@ static inline void buffer_skip(struct buffer *buf, int len)
        if (!buf->l)
                buf->r = buf->w = buf->lr = buf->data;
 
-       if (buf->l < buf->max_len)
+       if (buf->l < buffer_max_len(buf))
                buf->flags &= ~BF_FULL;
 
        buf->send_max -= len;
@@ -378,7 +376,7 @@ static inline int buffer_si_putchar(struct buffer *buf, char c)
        *buf->r = c;
 
        buf->l++;
-       if (buf->l >= buf->max_len)
+       if (buf->l >= buffer_max_len(buf))
                buf->flags |= BF_FULL;
 
        buf->r++;
index e8ac6419ac047478bb563db939146c125af502e1..322f21ef8a5859759fe709a4a8769cebc57601e7 100644 (file)
@@ -1,23 +1,23 @@
 /*
-  include/types/buffers.h
-  Buffer management definitions, macros and inline functions.
-
-  Copyright (C) 2000-2009 Willy Tarreau - w@1wt.eu
-
-  This library is free software; you can redistribute it and/or
-  modify it under the terms of the GNU Lesser General Public
-  License as published by the Free Software Foundation, version 2.1
-  exclusively.
-
-  This library is distributed in the hope that it will be useful,
-  but WITHOUT ANY WARRANTY; without even the implied warranty of
-  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-  Lesser General Public License for more details.
-
-  You should have received a copy of the GNU Lesser General Public
-  License along with this library; if not, write to the Free Software
-  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
-*/
* include/types/buffers.h
* Buffer management definitions, macros and inline functions.
+ *
* Copyright (C) 2000-2009 Willy Tarreau - w@1wt.eu
+ *
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation, version 2.1
* exclusively.
+ *
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
* Lesser General Public License for more details.
+ *
* You should have received a copy of the GNU Lesser General Public
* License along with this library; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ */
 
 #ifndef _TYPES_BUFFERS_H
 #define _TYPES_BUFFERS_H
@@ -57,7 +57,7 @@
 #define BF_READ_ERROR     0x000008  /* unrecoverable error on producer side */
 #define BF_READ_ACTIVITY  (BF_READ_NULL|BF_READ_PARTIAL|BF_READ_ERROR)
 
-#define BF_FULL           0x000010  /* buffer cannot accept any more data (l >= max_len) */
+#define BF_FULL           0x000010  /* buffer cannot accept any more data (l >= max len) */
 #define BF_SHUTR          0x000020  /* producer has already shut down */
 #define BF_SHUTR_NOW      0x000040  /* the producer must shut down for reads ASAP */
 #define BF_READ_NOEXP     0x000080  /* producer should not expire */
@@ -173,7 +173,6 @@ struct buffer {
        unsigned int l;                 /* data length */
        char *r, *w, *lr;               /* read ptr, write ptr, last read */
        unsigned int size;              /* buffer size in bytes */
-       unsigned int max_len;           /* read limit, used to keep room for header rewriting */
        unsigned int send_max;          /* number of bytes the sender can consume om this buffer, <= l */
        unsigned long to_forward;       /* number of bytes to forward after send_max without a wake-up */
        unsigned int analysers;         /* bit field indicating what to do on the buffer */
@@ -238,6 +237,24 @@ struct buffer {
    whole buffer, and reduce ->to_forward to 8000. After that, the producer may
    try to feed the additional data through the invisible buffer using a
    platform-specific method such as splice().
+
+   The ->to_forward entry is also used to detect whether we can fill the buffer
+   or not. The idea is that we need to save some space for data manipulation
+   (mainly header rewriting in HTTP) so we don't want to have a full buffer on
+   input before processing a request or response. Thus, we ensure that there is
+   always global.maxrewrite bytes of free space. Since we don't want to forward
+   chunks without filling the buffer, we rely on ->to_forward. When ->to_forward
+   is null, we may have some processing to do so we don't want to fill the
+   buffer. When ->to_forward is non-null, we know we don't care for at least as
+   many bytes. In the end, we know that each of the ->to_forward bytes will
+   eventually leave the buffer. So as long as ->to_forward is larger than
+   global.maxrewrite, we can fill the buffer. If ->to_forward is smaller than
+   global.maxrewrite, then we don't want to fill the buffer with more than
+   ->size - global.maxrewrite + ->to_forward.
+
+   Note that this also means that anyone touching ->to_forward must also take
+   care of updating the BF_FULL flag. For this reason, it's really advised to
+   use buffer_forward() only.
  */
 
 #endif /* _TYPES_BUFFERS_H */
index 68ede265b96adcd53058d704aae92acc40b6e3bb..939b03d047ad161ae0fc442c8ed0047b3cf46f2a 100644 (file)
@@ -66,7 +66,7 @@ int buffer_write(struct buffer *buf, const char *msg, int len)
                buf->r = buf->data;
 
        buf->flags &= ~(BF_OUT_EMPTY|BF_FULL);
-       if (buf->l >= buf->max_len)
+       if (buf->l >= buffer_max_len(buf))
                buf->flags |= BF_FULL;
 
        return -1;
@@ -86,7 +86,7 @@ int buffer_feed2(struct buffer *buf, const char *str, int len)
        if (len == 0)
                return -1;
 
-       if (len > buf->max_len) {
+       if (len > buffer_max_len(buf)) {
                /* we can't write this chunk and will never be able to, because
                 * it is larger than the buffer's current max size.
                 */
@@ -117,7 +117,7 @@ int buffer_feed2(struct buffer *buf, const char *str, int len)
                buf->r = buf->data;
 
        buf->flags &= ~BF_FULL;
-       if (buf->l >= buf->max_len)
+       if (buf->l >= buffer_max_len(buf))
                buf->flags |= BF_FULL;
 
        /* notify that some data was read from the SI into the buffer */
@@ -211,7 +211,7 @@ int buffer_replace(struct buffer *b, char *pos, char *end, const char *str)
        b->flags &= ~BF_FULL;
        if (b->l == 0)
                b->r = b->w = b->lr = b->data;
-       if (b->l >= b->max_len)
+       if (b->l >= buffer_max_len(b))
                b->flags |= BF_FULL;
 
        return delta;
@@ -252,7 +252,7 @@ int buffer_replace2(struct buffer *b, char *pos, char *end, const char *str, int
        b->flags &= ~BF_FULL;
        if (b->l == 0)
                b->r = b->w = b->lr = b->data;
-       if (b->l >= b->max_len)
+       if (b->l >= buffer_max_len(b))
                b->flags |= BF_FULL;
 
        return delta;
@@ -294,7 +294,7 @@ int buffer_insert_line2(struct buffer *b, char *pos, const char *str, int len)
        b->l += delta;
 
        b->flags &= ~BF_FULL;
-       if (b->l >= b->max_len)
+       if (b->l >= buffer_max_len(b))
                b->flags |= BF_FULL;
 
        return delta;
index 79bb9d9417abf42bb2af93f9a71e619b02df9912..492cae706954523a2c8867071b3fe798cef41221 100644 (file)
@@ -400,10 +400,8 @@ int event_accept(int fd) {
 
                s->req->flags |= BF_READ_ATTACHED; /* the producer is already connected */
 
-               if (p->mode == PR_MODE_HTTP) { /* reserve some space for header rewriting */
-                       s->req->max_len -= global.tune.maxrewrite;
+               if (p->mode == PR_MODE_HTTP)
                        s->req->flags |= BF_READ_DONTWAIT; /* one read is usually enough */
-               }
 
                /* activate default analysers enabled for this listener */
                s->req->analysers = l->analysers;
index 02be35ce1b3bee2d982b5b8c241bd13f710cc481..f61bfe6b2ad4be67ca8492841e509dcc96260ce8 100644 (file)
@@ -2694,7 +2694,6 @@ int http_process_request(struct session *s, struct buffer *req, int an_bit)
        req->analyse_exp = TICK_ETERNITY;
        req->analysers &= ~an_bit;
 
-       buffer_set_rlim(req, req->size); /* no more rewrite needed */
        s->logs.tv_request = now;
        /* OK let's go on with the BODY now */
        return 1;
@@ -3569,7 +3568,6 @@ int http_process_res_common(struct session *t, struct buffer *rep, int an_bit, s
                 * could. Let's switch to the DATA state.                    *
                 ************************************************************/
 
-               buffer_set_rlim(rep, rep->size); /* no more rewrite needed */
                t->logs.t_data = tv_ms_elapsed(&t->logs.tv_accept, &now);
 
                /* if the user wants to log as soon as possible, without counting
index f13c321c59fd3cdc22a879d38c93c14cbff7cda4..b2efa0b78298d489ff80c89ab2880f1cea66c439 100644 (file)
@@ -1021,8 +1021,8 @@ acl_fetch_req_ssl_ver(struct proxy *px, struct session *l4, void *l7, int dir,
         * all the part of the request which fits in a buffer is already
         * there.
         */
-       if (msg_len > l4->req->max_len + l4->req->data - l4->req->w)
-               msg_len = l4->req->max_len + l4->req->data - l4->req->w;
+       if (msg_len > buffer_max_len(l4->req) + l4->req->data - l4->req->w)
+               msg_len = buffer_max_len(l4->req) + l4->req->data - l4->req->w;
 
        if (bleft < msg_len)
                goto too_short;
index 81e4a3206a76be7356cc18adcaf8b6a32d2bc220..b86526ac84b552f57f86f335f29b8a913cf9e6fe 100644 (file)
@@ -334,8 +334,6 @@ void sess_establish(struct session *s, struct stream_interface *si)
                health_adjust(s->srv, HANA_STATUS_L4_OK);
 
        if (s->be->mode == PR_MODE_TCP) { /* let's allow immediate data connection in this case */
-               buffer_set_rlim(rep, rep->size); /* no rewrite needed */
-
                /* if the user wants to log as soon as possible, without counting
                 * bytes from the server, then this is the right moment. */
                if (s->fe->to_log && !(s->logs.logwait & LW_BYTES)) {
@@ -344,7 +342,6 @@ void sess_establish(struct session *s, struct stream_interface *si)
                }
        }
        else {
-               buffer_set_rlim(rep, req->size - global.tune.maxrewrite); /* rewrite needed */
                s->txn.rsp.msg_state = HTTP_MSG_RPBEFORE;
                /* reset hdr_idx which was already initialized by the request.
                 * right now, the http parser does it.
index c1f0dfae8c5b26a641391066e493a3ff29992b5b..e75bdafe836ce6401049817afce7bc656907e0e6 100644 (file)
@@ -302,15 +302,15 @@ int stream_sock_read(int fd) {
                if (b->l == 0) {
                        /* let's realign the buffer to optimize I/O */
                        b->r = b->w = b->lr = b->data;
-                       max = b->max_len;
+                       max = buffer_max_len(b);
                }
                else if (b->r > b->w) {
-                       max = b->data + b->max_len - b->r;
+                       max = b->data + buffer_max_len(b) - b->r;
                }
                else {
                        max = b->w - b->r;
-                       if (max > b->max_len)
-                               max = b->max_len;
+                       if (max > buffer_max_len(b))
+                               max = buffer_max_len(b);
                }
 
                if (max == 0) {
@@ -363,7 +363,7 @@ int stream_sock_read(int fd) {
 
                        b->total += ret;
 
-                       if (b->l >= b->max_len) {
+                       if (b->l >= buffer_max_len(b)) {
                                /* The buffer is now full, there's no point in going through
                                 * the loop again.
                                 */
@@ -642,7 +642,7 @@ static int stream_sock_write_loop(struct stream_interface *si, struct buffer *b)
                                b->w = b->data; /* wrap around the buffer */
 
                        b->l -= ret;
-                       if (likely(b->l < b->max_len))
+                       if (likely(b->l < buffer_max_len(b)))
                                b->flags &= ~BF_FULL;
 
                        if (likely(!b->l))