From 81fdeb8ce2cb10bfe2119dbb4c63f080bb5f8be4 Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Thu, 16 Feb 2023 16:47:33 +0100 Subject: [PATCH] MEDIUM: channel: Remove CF_READ_NOEXP flag This flag was introduced in 1.3 to fix a design issue. It was untouch since then but there is no reason to still have this trick. Note it could be good to review what happens in HTTP with the server is waiting for the end of the request. It could be good to be sure a client timeout is always reported. --- include/haproxy/channel-t.h | 6 +++--- include/haproxy/channel.h | 2 +- src/cli.c | 2 +- src/stconn.c | 4 ++-- src/stream.c | 18 ++---------------- 5 files changed, 9 insertions(+), 23 deletions(-) diff --git a/include/haproxy/channel-t.h b/include/haproxy/channel-t.h index 18ebc0373..287b712ac 100644 --- a/include/haproxy/channel-t.h +++ b/include/haproxy/channel-t.h @@ -61,7 +61,7 @@ /* unused: 0x00000010 */ #define CF_SHUTR 0x00000020 /* producer has already shut down */ #define CF_SHUTR_NOW 0x00000040 /* the producer must shut down for reads ASAP */ -#define CF_READ_NOEXP 0x00000080 /* producer should not expire */ +/* 0x00000080 unused */ #define CF_WRITE_EVENT 0x00000100 /* a write event detected on consumer side */ /* unused: 0x00000200 */ @@ -136,14 +136,14 @@ static forceinline char *chn_show_flags(char *buf, size_t len, const char *delim _(0); /* flags */ _(CF_READ_EVENT, _(CF_READ_TIMEOUT, _(CF_READ_ERROR, - _(CF_SHUTR, _(CF_SHUTR_NOW, _(CF_READ_NOEXP, _(CF_WRITE_EVENT, + _(CF_SHUTR, _(CF_SHUTR_NOW, _(CF_WRITE_EVENT, _(CF_WRITE_TIMEOUT, _(CF_WRITE_ERROR, _(CF_WAKE_WRITE, _(CF_SHUTW, _(CF_SHUTW_NOW, _(CF_AUTO_CLOSE, _(CF_STREAMER, _(CF_STREAMER_FAST, _(CF_WROTE_DATA, _(CF_KERN_SPLICING, _(CF_READ_DONTWAIT, _(CF_AUTO_CONNECT, _(CF_DONT_READ, _(CF_EXPECT_MORE, _(CF_SEND_DONTWAIT, _(CF_NEVER_WAIT, _(CF_WAKE_ONCE, _(CF_FLT_ANALYZE, - _(CF_EOI, _(CF_ISRESP))))))))))))))))))))))))))); + _(CF_EOI, _(CF_ISRESP)))))))))))))))))))))))))); /* epilogue */ _(~0U); return buf; diff --git a/include/haproxy/channel.h b/include/haproxy/channel.h index 8ef5b863d..053bc24c4 100644 --- a/include/haproxy/channel.h +++ b/include/haproxy/channel.h @@ -530,7 +530,7 @@ static inline int channel_output_closed(struct channel *chn) */ static inline void channel_check_timeouts(struct channel *chn) { - if (likely(!(chn->flags & (CF_SHUTR|CF_READ_TIMEOUT|CF_READ_EVENT|CF_READ_NOEXP))) && + if (likely(!(chn->flags & (CF_SHUTR|CF_READ_TIMEOUT|CF_READ_EVENT))) && unlikely(tick_is_expired(chn->rex, now_ms))) chn->flags |= CF_READ_TIMEOUT; diff --git a/src/cli.c b/src/cli.c index a54146434..44ca4e85c 100644 --- a/src/cli.c +++ b/src/cli.c @@ -2784,7 +2784,7 @@ int pcli_wait_for_response(struct stream *s, struct channel *rep, int an_bit) sc_set_state(s->scb, SC_ST_INI); s->scb->flags &= SC_FL_ISBACK | SC_FL_DONT_WAKE; /* we're in the context of process_stream */ s->req.flags &= ~(CF_SHUTW|CF_SHUTW_NOW|CF_AUTO_CONNECT|CF_WRITE_ERROR|CF_STREAMER|CF_STREAMER_FAST|CF_NEVER_WAIT|CF_WROTE_DATA); - s->res.flags &= ~(CF_SHUTR|CF_SHUTR_NOW|CF_READ_ERROR|CF_READ_NOEXP|CF_STREAMER|CF_STREAMER_FAST|CF_WRITE_EVENT|CF_NEVER_WAIT|CF_WROTE_DATA|CF_READ_EVENT); + s->res.flags &= ~(CF_SHUTR|CF_SHUTR_NOW|CF_READ_ERROR|CF_STREAMER|CF_STREAMER_FAST|CF_WRITE_EVENT|CF_NEVER_WAIT|CF_WROTE_DATA|CF_READ_EVENT); s->flags &= ~(SF_DIRECT|SF_ASSIGNED|SF_BE_ASSIGNED|SF_FORCE_PRST|SF_IGNORE_PRST); s->flags &= ~(SF_CURR_SESS|SF_REDIRECTABLE|SF_SRV_REUSED); s->flags &= ~(SF_ERR_MASK|SF_FINST_MASK|SF_REDISP); diff --git a/src/stconn.c b/src/stconn.c index 6dfdfaff4..0b3147a00 100644 --- a/src/stconn.c +++ b/src/stconn.c @@ -1043,7 +1043,7 @@ void sc_update_rx(struct stconn *sc) if ((ic->flags & CF_EOI) || sc->flags & (SC_FL_WONT_READ|SC_FL_NEED_BUFF|SC_FL_NEED_ROOM)) ic->rex = TICK_ETERNITY; - else if (!(ic->flags & CF_READ_NOEXP) && !tick_isset(ic->rex)) + else if (!tick_isset(ic->rex)) ic->rex = tick_add_ifset(now_ms, ic->rto); sc_chk_rcv(sc); @@ -1195,7 +1195,7 @@ static void sc_notify(struct stconn *sc) } else if ((ic->flags & (CF_SHUTR|CF_READ_EVENT)) == CF_READ_EVENT) { /* we must re-enable reading if sc_chk_snd() has freed some space */ - if (!(ic->flags & CF_READ_NOEXP) && tick_isset(ic->rex)) + if (tick_isset(ic->rex)) ic->rex = tick_add_ifset(now_ms, ic->rto); } diff --git a/src/stream.c b/src/stream.c index 8f58e0d67..382aeda49 100644 --- a/src/stream.c +++ b/src/stream.c @@ -1720,8 +1720,8 @@ struct task *process_stream(struct task *t, void *context, unsigned int state) memset(&s->txn->auth, 0, sizeof(s->txn->auth)); /* This flag must explicitly be set every time */ - req->flags &= ~(CF_READ_NOEXP|CF_WAKE_WRITE); - res->flags &= ~(CF_READ_NOEXP|CF_WAKE_WRITE); + req->flags &= ~CF_WAKE_WRITE; + res->flags &= ~CF_WAKE_WRITE; /* Keep a copy of req/rep flags so that we can detect shutdowns */ rqf_last = req->flags & ~CF_MASK_ANALYSER; @@ -2544,20 +2544,6 @@ struct task *process_stream(struct task *t, void *context, unsigned int state) stream_update_both_sc(s); - /* Trick: if a request is being waiting for the server to respond, - * and if we know the server can timeout, we don't want the timeout - * to expire on the client side first, but we're still interested - * in passing data from the client to the server (eg: POST). Thus, - * we can cancel the client's request timeout if the server's - * request timeout is set and the server has not yet sent a response. - */ - - if ((res->flags & (CF_AUTO_CLOSE|CF_SHUTR)) == 0 && - (tick_isset(req->wex) || tick_isset(res->rex))) { - req->flags |= CF_READ_NOEXP; - req->rex = TICK_ETERNITY; - } - /* Reset pending events now */ s->pending_events = 0;