[MAJOR] buffers: split BF_WRITE_ENA into BF_AUTO_CONNECT and BF_AUTO_CLOSE
The BF_WRITE_ENA buffer flag became very complex to deal with, because
it was used to :
- enable automatic connection
- enable close forwarding
- enable data forwarding
The last point was not very true anymore since we introduced ->send_max,
but still the test remained everywhere. This was causing issues such as
impossibility to connect without forwarding data, impossibility to prevent
closing when data was forwarded, etc...
This patch clarifies the situation by getting rid of this multi-purpose
flag and replacing it with :
- data forwarding based only on ->send_max || ->pipe ;
- a new BF_AUTO_CONNECT flag to allow automatic connection and only
that ;
- ability to perform an automatic connection when ->send_max or ->pipe
indicate that data is waiting to leave the buffer ;
- a new BF_AUTO_CLOSE flag to let the producer automatically set the
BF_SHUTW_NOW flag when it gets a BF_SHUTR.
During this cleanup, it was discovered that some tests were performed
twice, or that the BF_HIJACK flag was still tested, which is not needed
anymore since ->send_max replcaed it. These places have been fixed too.
These cleanups have also revealed a few areas where the other flags
such as BF_EMPTY are not cleanly used. This will be an opportunity for
a second patch.
diff --git a/include/proto/buffers.h b/include/proto/buffers.h
index 66bee89..8df6231 100644
--- a/include/proto/buffers.h
+++ b/include/proto/buffers.h
@@ -211,54 +211,30 @@
buf->flags &= ~BF_HIJACK;
}
-/* allows the consumer to send the buffer contents */
-static inline void buffer_write_ena(struct buffer *buf)
+/* allow the consumer to try to establish a new connection. */
+static inline void buffer_auto_connect(struct buffer *buf)
{
- buf->flags |= BF_WRITE_ENA;
+ buf->flags |= BF_AUTO_CONNECT;
}
-/* prevents the consumer from sending the buffer contents */
-static inline void buffer_write_dis(struct buffer *buf)
+/* prevent the consumer from trying to establish a new connection, and also
+ * disable auto shutdown forwarding.
+ */
+static inline void buffer_dont_connect(struct buffer *buf)
{
- buf->flags &= ~BF_WRITE_ENA;
+ buf->flags &= ~(BF_AUTO_CONNECT|BF_AUTO_CLOSE);
}
-/* check if the buffer needs to be shut down for read, and perform the shutdown
- * at the stream_interface level if needed. This must not be used with a buffer
- * for which a connection is currently in queue or turn-around.
- */
-static inline void buffer_check_shutr(struct buffer *b)
+/* allow the producer to forward shutdown requests */
+static inline void buffer_auto_close(struct buffer *buf)
{
- if (b->flags & BF_SHUTR)
- return;
-
- if (!(b->flags & (BF_SHUTR_NOW|BF_SHUTW)))
- return;
-
- /* Last read, forced read-shutdown, or other end closed. We have to
- * close our read side and inform the stream_interface.
- */
- b->prod->shutr(b->prod);
+ buf->flags |= BF_AUTO_CLOSE;
}
-/* check if the buffer needs to be shut down for write, and perform the shutdown
- * at the stream_interface level if needed. This must not be used with a buffer
- * for which a connection is currently in queue or turn-around.
- */
-static inline void buffer_check_shutw(struct buffer *b)
+/* prevent the producer from forwarding shutdown requests */
+static inline void buffer_dont_close(struct buffer *buf)
{
- if (b->flags & BF_SHUTW)
- return;
-
- if ((b->flags & BF_SHUTW_NOW) ||
- (b->flags & (BF_EMPTY|BF_HIJACK|BF_WRITE_ENA|BF_SHUTR)) ==
- (BF_EMPTY|BF_WRITE_ENA|BF_SHUTR)) {
- /* Application requested write-shutdown, or other end closed
- * with empty buffer. We have to close our write side and
- * inform the stream_interface.
- */
- b->cons->shutw(b->cons);
- }
+ buf->flags &= ~BF_AUTO_CLOSE;
}
/* returns the maximum number of bytes writable at once in this buffer */
diff --git a/include/types/buffers.h b/include/types/buffers.h
index 1d15c70..b758596 100644
--- a/include/types/buffers.h
+++ b/include/types/buffers.h
@@ -71,7 +71,7 @@
#define BF_EMPTY 0x001000 /* buffer is empty */
#define BF_SHUTW 0x002000 /* consumer has already shut down */
#define BF_SHUTW_NOW 0x004000 /* the consumer must shut down for writes ASAP */
-#define BF_WRITE_ENA 0x008000 /* consumer is allowed to forward all buffer contents */
+#define BF_AUTO_CLOSE 0x008000 /* producer can forward shutdown to other side */
/* When either BF_SHUTR_NOW or BF_HIJACK is set, it is strictly forbidden for
* the producer to alter the buffer contents. When BF_SHUTW_NOW is set, the
@@ -92,7 +92,7 @@
* 1 0 closed: the consumer has closed its output channel.
* 1 1 impossible
*
- * The SHUTW_NOW flag should be set by the session processor when SHUTR and WRITE_ENA
+ * The SHUTW_NOW flag should be set by the session processor when SHUTR and AUTO_CLOSE
* are both set. It may also be set by a hijacker at the end of data. And it may also
* be set by the producer when it detects SHUTR while directly forwarding data to the
* consumer.
@@ -109,6 +109,7 @@
#define BF_READ_ATTACHED 0x100000 /* the read side is attached for the first time */
#define BF_KERN_SPLICING 0x200000 /* kernel splicing desired for this buffer */
#define BF_READ_DONTWAIT 0x400000 /* wake the task up after every read (eg: HTTP request) */
+#define BF_AUTO_CONNECT 0x800000 /* consumer may attempt to establish a new connection */
/* Use these masks to clear the flags before going back to lower layers */
#define BF_CLEAR_READ (~(BF_READ_NULL|BF_READ_PARTIAL|BF_READ_ERROR|BF_READ_ATTACHED))
@@ -119,7 +120,7 @@
#define BF_MASK_ANALYSER (BF_READ_ATTACHED|BF_READ_ACTIVITY|BF_READ_TIMEOUT|BF_ANA_TIMEOUT|BF_WRITE_ACTIVITY)
/* Mask for static flags which are not events, but might change during processing */
-#define BF_MASK_STATIC (BF_EMPTY|BF_FULL|BF_HIJACK|BF_WRITE_ENA|BF_SHUTR|BF_SHUTW|BF_SHUTR_NOW|BF_SHUTW_NOW)
+#define BF_MASK_STATIC (BF_EMPTY|BF_FULL|BF_HIJACK|BF_AUTO_CLOSE|BF_AUTO_CONNECT|BF_SHUTR|BF_SHUTW|BF_SHUTR_NOW|BF_SHUTW_NOW)
/* Analysers (buffer->analysers).
diff --git a/src/client.c b/src/client.c
index 312d896..76c91c9 100644
--- a/src/client.c
+++ b/src/client.c
@@ -403,8 +403,10 @@
s->req->analysers = l->analysers;
/* note: this should not happen anymore since there's always at least the switching rules */
- if (!s->req->analysers)
- buffer_write_ena(s->req); /* don't wait to establish connection */
+ if (!s->req->analysers) {
+ buffer_auto_connect(s->req); /* don't wait to establish connection */
+ buffer_auto_close(s->req); /* let the producer forward close requests */
+ }
s->req->rto = s->fe->timeout.client;
s->req->wto = s->be->timeout.server;
diff --git a/src/dumpstats.c b/src/dumpstats.c
index 24aab05..86e73a9 100644
--- a/src/dumpstats.c
+++ b/src/dumpstats.c
@@ -316,7 +316,7 @@
memset(&s->data_ctx.stats, 0, sizeof(s->data_ctx.stats));
s->data_source = DATA_SRC_STATS;
s->ana_state = STATS_ST_REQ;
- buffer_write_dis(s->req);
+ buffer_dont_connect(s->req);
/* fall through */
case STATS_ST_REQ:
diff --git a/src/proto_http.c b/src/proto_http.c
index 1ecdc0b..a159769 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -526,7 +526,7 @@
{
buffer_erase(si->ob);
buffer_erase(si->ib);
- buffer_write_ena(si->ib);
+ buffer_auto_close(si->ib);
if (status > 0 && msg) {
t->txn.status = status;
buffer_write(si->ib, msg->str, msg->len);
@@ -1885,7 +1885,7 @@
return 0;
}
- buffer_write_dis(req);
+ buffer_dont_connect(req);
req->flags |= BF_READ_DONTWAIT; /* try to get back here ASAP */
/* just set the request timeout once at the beginning of the request */
@@ -2036,7 +2036,7 @@
if (unlikely(msg->msg_state != HTTP_MSG_BODY)) {
/* we need more data */
- buffer_write_dis(req);
+ buffer_dont_connect(req);
return 0;
}
@@ -2094,7 +2094,7 @@
/* wipe the request out so that we can drop the connection early
* if the client closes first.
*/
- buffer_write_dis(req);
+ buffer_dont_connect(req);
req->analysers = 0; /* remove switching rules etc... */
req->analysers |= AN_REQ_HTTP_TARPIT;
req->analyse_exp = tick_add_ifset(now_ms, s->be->timeout.tarpit);
@@ -2321,7 +2321,7 @@
if (unlikely(msg->msg_state != HTTP_MSG_BODY)) {
/* we need more data */
- buffer_write_dis(req);
+ buffer_dont_connect(req);
return 0;
}
@@ -2549,7 +2549,7 @@
ctx.idx = 0;
http_find_header2("Transfer-Encoding", 17, msg->sol, &txn->hdr_idx, &ctx);
if (ctx.idx && ctx.vlen >= 7 && strncasecmp(ctx.line+ctx.val, "chunked", 7) == 0) {
- buffer_write_dis(req);
+ buffer_dont_connect(req);
req->analysers |= AN_REQ_HTTP_BODY;
}
else {
@@ -2572,7 +2572,7 @@
hint = s->be->url_param_post_limit;
/* now do we really need to buffer more data? */
if (len < hint) {
- buffer_write_dis(req);
+ buffer_dont_connect(req);
req->analysers |= AN_REQ_HTTP_BODY;
}
/* else... There are no body bytes to wait for */
@@ -2626,7 +2626,7 @@
* timeout. We just have to check that the client is still
* there and that the timeout has not expired.
*/
- buffer_write_dis(req);
+ buffer_dont_connect(req);
if ((req->flags & (BF_SHUTR|BF_READ_ERROR)) == 0 &&
!tick_is_expired(req->analyse_exp, now_ms))
return 0;
@@ -2669,7 +2669,7 @@
if (unlikely(msg->msg_state != HTTP_MSG_BODY)) {
/* we need more data */
- buffer_write_dis(req);
+ buffer_dont_connect(req);
return 0;
}
@@ -2738,7 +2738,7 @@
* request timeout once at the beginning of the
* request.
*/
- buffer_write_dis(req);
+ buffer_dont_connect(req);
if (!tick_isset(req->analyse_exp))
req->analyse_exp = tick_add_ifset(now_ms, s->be->timeout.httpreq);
return 0;
@@ -2921,14 +2921,7 @@
return 0;
}
- /* We disable sending only if we have nothing to send.
- * Note that we should not need to do this since the
- * buffer is protected by the fact that at least one
- * analyser remains. But close events could still be
- * forwarded if we don't disable the BF_WRITE_ENA flag.
- */
- if (!rep->send_max)
- buffer_write_dis(rep);
+ buffer_dont_close(rep);
return 0;
}
@@ -4635,7 +4628,7 @@
/* The request is valid, the user is authenticated. Let's start sending
* data.
*/
- buffer_write_dis(t->req);
+ buffer_dont_connect(t->req);
buffer_shutw_now(t->req);
buffer_shutr_now(t->rep);
t->logs.tv_request = now;
diff --git a/src/proto_tcp.c b/src/proto_tcp.c
index 73a2ba6..a03c809 100644
--- a/src/proto_tcp.c
+++ b/src/proto_tcp.c
@@ -668,7 +668,7 @@
if (rule->cond) {
ret = acl_exec_cond(rule->cond, s->fe, s, &s->txn, ACL_DIR_REQ | partial);
if (ret == ACL_PAT_MISS) {
- buffer_write_dis(req);
+ buffer_dont_connect(req);
/* just set the request timeout once at the beginning of the request */
if (!tick_isset(req->analyse_exp) && s->fe->tcp_req.inspect_delay)
req->analyse_exp = tick_add_ifset(now_ms, s->fe->tcp_req.inspect_delay);
diff --git a/src/session.c b/src/session.c
index 013be7f..6c9390d 100644
--- a/src/session.c
+++ b/src/session.c
@@ -769,8 +769,9 @@
if (s->req->prod->state >= SI_ST_EST) {
unsigned int last_ana = 0;
- /* it's up to the analysers to reset write_ena */
- buffer_write_ena(s->req);
+ /* it's up to the analysers to stop new connections */
+ buffer_auto_connect(s->req);
+ buffer_auto_close(s->req);
/* We will call all analysers for which a bit is set in
* s->req->analysers, following the bit order from LSB
@@ -884,8 +885,8 @@
unsigned int flags = s->rep->flags;
if (s->rep->prod->state >= SI_ST_EST) {
- /* it's up to the analysers to reset write_ena */
- buffer_write_ena(s->rep);
+ /* it's up to the analysers to reset auto_close */
+ buffer_auto_close(s->rep);
if (s->rep->analysers)
process_response(s);
}
@@ -950,6 +951,8 @@
* attached to it. If any data are left in, we'll permit them to
* move.
*/
+ buffer_auto_connect(s->req);
+ buffer_auto_close(s->req);
buffer_flush(s->req);
/* If the producer is still connected, we'll schedule large blocks
@@ -980,18 +983,15 @@
* Now forward all shutdown requests between both sides of the buffer
*/
- /* first, let's check if the request buffer needs to shutdown(write) */
- if (unlikely((s->req->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_HIJACK|BF_WRITE_ENA|BF_SHUTR)) ==
- (BF_WRITE_ENA|BF_SHUTR)))
- buffer_shutw_now(s->req);
- else if ((s->req->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_WRITE_ENA)) == (BF_WRITE_ENA) &&
- (s->req->cons->state == SI_ST_EST) &&
- s->be->options & PR_O_FORCE_CLO &&
- s->rep->flags & BF_READ_ACTIVITY) {
- /* We want to force the connection to the server to close,
- * and the server has begun to respond. That's the right
- * time.
- */
+ /* first, let's check if the request buffer needs to shutdown(write), which may
+ * happen either because the input is closed or because we want to force a close
+ * once the server has begun to respond.
+ */
+ if ((s->req->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_HIJACK|BF_AUTO_CLOSE)) == BF_AUTO_CLOSE) {
+ if (unlikely((s->req->flags & BF_SHUTR) ||
+ ((s->req->cons->state == SI_ST_EST) &&
+ (s->be->options & PR_O_FORCE_CLO) &&
+ (s->rep->flags & BF_READ_ACTIVITY))))
buffer_shutw_now(s->req);
}
@@ -1008,17 +1008,23 @@
if (unlikely((s->req->flags & (BF_SHUTR|BF_SHUTR_NOW)) == BF_SHUTR_NOW))
s->req->prod->shutr(s->req->prod);
- /* it's possible that an upper layer has requested a connection setup or abort */
- if (s->req->cons->state == SI_ST_INI &&
- (s->req->flags & (BF_WRITE_ENA|BF_SHUTW|BF_SHUTW_NOW))) {
- if ((s->req->flags & (BF_WRITE_ENA|BF_SHUTW|BF_SHUTW_NOW)) == BF_WRITE_ENA) {
- /* If we have a ->connect method, we need to perform a connection request,
- * otherwise we immediately switch to the connected state.
- */
- if (s->req->cons->connect)
- s->req->cons->state = SI_ST_REQ; /* new connection requested */
- else
- s->req->cons->state = SI_ST_EST; /* connection established */
+ /* it's possible that an upper layer has requested a connection setup or abort.
+ * There are 2 situations where we decide to establish a new connection :
+ * - there are data scheduled for emission in the buffer
+ * - the BF_AUTO_CONNECT flag is set (active connection)
+ */
+ if (s->req->cons->state == SI_ST_INI) {
+ if (!(s->req->flags & (BF_SHUTW|BF_SHUTW_NOW))) {
+ if ((s->req->flags & BF_AUTO_CONNECT) ||
+ (s->req->send_max || s->req->pipe)) {
+ /* If we have a ->connect method, we need to perform a connection request,
+ * otherwise we immediately switch to the connected state.
+ */
+ if (s->req->cons->connect)
+ s->req->cons->state = SI_ST_REQ; /* new connection requested */
+ else
+ s->req->cons->state = SI_ST_EST; /* connection established */
+ }
}
else
s->req->cons->state = SI_ST_CLO; /* shutw+ini = abort */
@@ -1065,6 +1071,7 @@
* attached to it. If any data are left in, we'll permit them to
* move.
*/
+ buffer_auto_close(s->rep);
buffer_flush(s->rep);
/* If the producer is still connected, we'll schedule large blocks
@@ -1100,8 +1107,8 @@
*/
/* first, let's check if the response buffer needs to shutdown(write) */
- if (unlikely((s->rep->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_HIJACK|BF_WRITE_ENA|BF_SHUTR)) ==
- (BF_WRITE_ENA|BF_SHUTR)))
+ if (unlikely((s->rep->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_HIJACK|BF_AUTO_CLOSE|BF_SHUTR)) ==
+ (BF_AUTO_CLOSE|BF_SHUTR)))
buffer_shutw_now(s->rep);
/* shutdown(write) pending */
@@ -1182,7 +1189,7 @@
* request timeout is set and the server has not yet sent a response.
*/
- if ((s->rep->flags & (BF_WRITE_ENA|BF_SHUTR)) == 0 &&
+ if ((s->rep->flags & (BF_AUTO_CLOSE|BF_SHUTR)) == 0 &&
(tick_isset(s->req->wex) || tick_isset(s->rep->rex))) {
s->req->flags |= BF_READ_NOEXP;
s->req->rex = TICK_ETERNITY;
diff --git a/src/stream_interface.c b/src/stream_interface.c
index d8ee9e6..2b14d2d 100644
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -89,7 +89,7 @@
buffer_write(si->ob, msg->str, msg->len);
si->ob->wex = tick_add_ifset(now_ms, si->ob->wto);
- buffer_write_ena(si->ob);
+ buffer_auto_close(si->ob);
}
/*
diff --git a/src/stream_sock.c b/src/stream_sock.c
index 715259b..0ec83cd 100644
--- a/src/stream_sock.c
+++ b/src/stream_sock.c
@@ -502,7 +502,7 @@
/* we received a shutdown */
fdtab[fd].ev &= ~FD_POLL_HUP;
b->flags |= BF_READ_NULL;
- if (b->flags & BF_WRITE_ENA)
+ if (b->flags & BF_AUTO_CLOSE)
buffer_shutw_now(b);
stream_sock_shutr(si);
goto out_wakeup;
@@ -601,7 +601,7 @@
unsigned int send_flag = MSG_DONTWAIT | MSG_NOSIGNAL;
if (MSG_MORE &&
- (((b->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_HIJACK|BF_WRITE_ENA)) == (BF_WRITE_ENA|BF_SHUTW_NOW) &&
+ (((b->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_HIJACK)) == BF_SHUTW_NOW &&
(max == b->l)) ||
(max != b->l && max != b->send_max))
&& (fdtab[si->fd].flags & FD_FL_TCP)) {
@@ -741,8 +741,7 @@
* send_max limit was reached. Maybe we just wrote the last
* chunk and need to close.
*/
- if (((b->flags & (BF_SHUTW|BF_HIJACK|BF_WRITE_ENA|BF_SHUTW_NOW)) ==
- (BF_WRITE_ENA|BF_SHUTW_NOW)) &&
+ if (((b->flags & (BF_SHUTW|BF_HIJACK|BF_SHUTW_NOW)) == BF_SHUTW_NOW) &&
(si->state == SI_ST_EST)) {
stream_sock_shutw(si);
goto out_wakeup;
@@ -926,11 +925,9 @@
/* Check if we need to close the write side */
if (!(ob->flags & BF_SHUTW)) {
/* Write not closed, update FD status and timeout for writes */
- if ((ob->send_max == 0 && !ob->pipe) ||
- (ob->flags & BF_EMPTY) ||
- (ob->flags & (BF_HIJACK|BF_WRITE_ENA)) == 0) {
+ if ((ob->send_max == 0 && !ob->pipe) || (ob->flags & BF_EMPTY)) {
/* stop writing */
- if ((ob->flags & (BF_EMPTY|BF_HIJACK|BF_WRITE_ENA)) == (BF_EMPTY|BF_WRITE_ENA))
+ if ((ob->flags & (BF_EMPTY|BF_HIJACK)) == BF_EMPTY)
si->flags |= SI_FL_WAIT_DATA;
EV_FD_COND_C(fd, DIR_WR);
ob->wex = TICK_ETERNITY;
@@ -1014,8 +1011,7 @@
if (!(si->flags & SI_FL_WAIT_DATA) || /* not waiting for data */
(fdtab[si->fd].ev & FD_POLL_OUT) || /* we'll be called anyway */
- !(ob->send_max || ob->pipe) || /* called with nothing to send ! */
- !(ob->flags & (BF_HIJACK|BF_WRITE_ENA))) /* we may not write */
+ !(ob->send_max || ob->pipe)) /* called with nothing to send ! */
return;
retval = stream_sock_write_loop(si, ob);
@@ -1045,14 +1041,14 @@
* send_max limit was reached. Maybe we just wrote the last
* chunk and need to close.
*/
- if (((ob->flags & (BF_SHUTW|BF_HIJACK|BF_WRITE_ENA|BF_SHUTW_NOW)) ==
- (BF_WRITE_ENA|BF_SHUTW_NOW)) &&
+ if (((ob->flags & (BF_SHUTW|BF_HIJACK|BF_AUTO_CLOSE|BF_SHUTW_NOW)) ==
+ (BF_AUTO_CLOSE|BF_SHUTW_NOW)) &&
(si->state == SI_ST_EST)) {
stream_sock_shutw(si);
goto out_wakeup;
}
- if ((ob->flags & (BF_SHUTW|BF_EMPTY|BF_HIJACK|BF_WRITE_ENA)) == (BF_EMPTY|BF_WRITE_ENA))
+ if ((ob->flags & (BF_SHUTW|BF_EMPTY|BF_HIJACK)) == BF_EMPTY)
si->flags |= SI_FL_WAIT_DATA;
ob->wex = TICK_ETERNITY;
}