RepoMirrors
/
haproxy
mirror of http://git.haproxy.org/git/haproxy.git/
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

BUG/MINOR: stream: do not use client-fin/server-fin with HTX 

Historically the client-fin and server-fin timeouts were made to allow
a connection closure to be effective quickly if the last data were sent
down a socket and the client didn't close, something that can happen
when the peer's FIN is lost and retransmits are blocked by a firewall
for example. This made complete sense in 1.5 for TCP and HTTP in close
mode. But nowadays with muxes, it's not done at the right layer anymore
and even the description doesn't match what is being done, because what
happens is that the stream will abort the whole transfer after it's done
sending to the mux and this timeout expires.

We've seen in GH issue 2095 that this can happen with very short timeout
values, and while this didn't trigger often before, now that the muxes
(h2 & quic) properly report an end of stream before even the first
sc_conn_sync_recv(), it seems that it can happen more often, and have
two undesirable effects:
  - logging a timeout when that's not the case
  - aborting the request channel, hence the server-side conn, possibly
    before it had a chance to be put back to the idle list, causing
    this connection to be closed and not reusable.

Unfortunately for TCP (mux_pt) this remains necessary because the mux
doesn't have a timeout task. So here we're adding tests to only do
this through an HTX mux. But to be really clean we should in fact
completely drop all of this and implement these timeouts in the mux
itself.

This needs to be backported to 2.8 where the issue was discovered,
and maybe carefully to older versions, though that is not sure at
all. In any case, using a higher timeout or removing client-fin in
HTTP proxies is sufficient to make the issue disappear.
This commit is contained in:
Willy Tarreau 2023-06-02 16:19:51 +02:00
parent ae0f8be011
commit 4ad1c9635a
2 changed files with 9 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
include/haproxy/sc_strm.h
View File

@ -389,6 +389,9 @@ static inline void sc_set_hcto(struct stconn *sc)
{ {
struct stream *strm = __sc_strm(sc); struct stream *strm = __sc_strm(sc);
if (IS_HTX_STRM(strm))
return;
if (sc->flags & SC_FL_ISBACK) { if (sc->flags & SC_FL_ISBACK) {
if ((strm->flags & SF_BE_ASSIGNED) && tick_isset(strm->be->timeout.serverfin)) if ((strm->flags & SF_BE_ASSIGNED) && tick_isset(strm->be->timeout.serverfin))
sc->ioto = strm->be->timeout.serverfin; sc->ioto = strm->be->timeout.serverfin;

10
src/stream.c
View File

@ -2453,10 +2453,12 @@ struct task *process_stream(struct task *t, void *context, unsigned int state)
if (!req->analysers && s->tunnel_timeout) { if (!req->analysers && s->tunnel_timeout) {
scf->ioto = scb->ioto = s->tunnel_timeout; scf->ioto = scb->ioto = s->tunnel_timeout;
if ((scf->flags & (SC_FL_EOS|SC_FL_ABRT_DONE|SC_FL_SHUT_DONE)) && tick_isset(sess->fe->timeout.clientfin)) if (!IS_HTX_STRM(s)) {
scf->ioto = sess->fe->timeout.clientfin; if ((scf->flags & (SC_FL_EOS|SC_FL_ABRT_DONE|SC_FL_SHUT_DONE)) && tick_isset(sess->fe->timeout.clientfin))
if ((scb->flags & (SC_FL_EOS|SC_FL_ABRT_DONE|SC_FL_SHUT_DONE)) && tick_isset(s->be->timeout.serverfin)) scf->ioto = sess->fe->timeout.clientfin;
scb->ioto = s->be->timeout.serverfin; if ((scb->flags & (SC_FL_EOS|SC_FL_ABRT_DONE|SC_FL_SHUT_DONE)) && tick_isset(s->be->timeout.serverfin))
scb->ioto = s->be->timeout.serverfin;
}
} }
} }