BUG/MEDIUM: Don't apply a max value on room_needed in sc_need_room()

In sc_need_room(), we compute the maximum room that can be requested to
restarted reading to be sure to be able to unblock the SC. At worst when the
buffer is emptied. Here, the buffer reserve is considered but it is an issue.

Counting the reserve can lead to a wicked bug with the H1 multiplexer, when
small amount of data are found at the end of the HTX buffer. In this case,
to not wrap, the H1 mux requests more room. It is an optim to be able to
resync the buffer with the consumer side and to be able to perform zero-copy
transfers. However, if this amount of data is smaller than the reserve and
if the consumer is congested, we fall in a loop because the wrong value is
used to request more room. The H1 mux continues to pretend there is not
enough space in the buffer, while the effective requested value is lower
than the free space in the buffer. While the consumer is congested and does
not consume these data, the is no way to stop the loop.

We can fix the function by removing the buffer reserve from the
computation. But it remains a dangerous decision to apply a max value on
room_needed. It is safer to require the caller must set a correct value. For
now, it is true. But at the end, it is totally unexepected to wait for more
room than an empty buffer can contain.

This patch must be backported to 2.8.

diff --git a/include/haproxy/stconn.h b/include/haproxy/stconn.h
index 3b24b217e5379106e6f94f2eeec04e6760ce3df3..4e8722e4a71fcdc0d97c5b63eb38b1f889f463eb 100644 (file)
--- a/include/haproxy/stconn.h
+++ b/include/haproxy/stconn.h
@@ -457,13 +457,13 @@ static inline void sc_have_room(struct stconn *sc)
  * SE_FL_HAVE_NO_DATA to be called again as soon as SC_FL_NEED_ROOM is cleared.
  *
  * The caller is responsible to specified the amount of free space required to
- * progress. However, to be sure the SC can be unblocked a max value cannot be
- * eceeded : (BUFSIZE - RESERVE - HTX OVERHEAD)
+ * progress. It must take care to not exceed the buffer size.
  */
 static inline void sc_need_room(struct stconn *sc, ssize_t room_needed)
 {
        sc->flags |= SC_FL_NEED_ROOM;
-       sc->room_needed = MIN((ssize_t)(global.tune.bufsize - global.tune.maxrewrite - sizeof(struct htx)), room_needed);
+       BUG_ON_HOT(room_needed > (ssize_t)global.tune.bufsize);
+       sc->room_needed = room_needed;
 }
 
 /* The stream endpoint indicates that it's ready to consume data from the