From a70a3548bcb0d9100f6418ee859accdb4a7967e1 Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Wed, 30 Mar 2022 17:13:02 +0200 Subject: [PATCH] MINOR: stream: Only save previous connection state for the server side The previous connection state on the client side was only used for debugging purpose to report client close. But this may be handled when the client stream-interface is switched from SI_ST_DIS to SI_ST_CLO. So, there only remains the previous connection state on the server side that is used by the stream, in process_stream(), to be able to set the correct termination flags. Thus, instead of keeping this info in the stream-interface for only one side, the info is now stored in the stream itself. --- include/haproxy/stream-t.h | 1 + include/haproxy/stream_interface-t.h | 1 - include/haproxy/stream_interface.h | 4 +- src/cli.c | 2 +- src/stream.c | 61 +++++++++++++++------------- src/stream_interface.c | 3 +- 6 files changed, 37 insertions(+), 35 deletions(-) diff --git a/include/haproxy/stream-t.h b/include/haproxy/stream-t.h index e179e5417..02b8789f1 100644 --- a/include/haproxy/stream-t.h +++ b/include/haproxy/stream-t.h @@ -143,6 +143,7 @@ struct stream { int conn_retries; /* number of connect retries performed */ unsigned int conn_exp; /* wake up time for connect, queue, turn-around, ... */ + enum si_state prev_conn_state; /* SI_ST*, copy of previous state of the server conn-stream */ struct list list; /* position in the thread's streams list */ struct mt_list by_srv; /* position in server stream list */ diff --git a/include/haproxy/stream_interface-t.h b/include/haproxy/stream_interface-t.h index bb2fdbdc4..08c30ad22 100644 --- a/include/haproxy/stream_interface-t.h +++ b/include/haproxy/stream_interface-t.h @@ -112,7 +112,6 @@ enum { struct stream_interface { /* struct members used by the "buffer" side */ enum si_state state; /* SI_ST* */ - enum si_state prev_state;/* SI_ST*, copy of previous state */ /* 16-bit hole here */ unsigned int flags; /* SI_FL_* */ struct conn_stream *cs; /* points to the conn-streams that owns the endpoint (connection or applet) */ diff --git a/include/haproxy/stream_interface.h b/include/haproxy/stream_interface.h index 0b02c9e33..1b7fd13fa 100644 --- a/include/haproxy/stream_interface.h +++ b/include/haproxy/stream_interface.h @@ -109,7 +109,7 @@ static inline int si_init(struct stream_interface *si) si->err_type = SI_ET_NONE; si->flags &= SI_FL_ISBACK; si->cs = NULL; - si->state = si->prev_state = SI_ST_INI; + si->state = SI_ST_INI; si->ops = &si_embedded_ops; si->wait_event.tasklet = tasklet_new(); if (!si->wait_event.tasklet) @@ -126,7 +126,7 @@ static inline int si_init(struct stream_interface *si) */ static inline void si_set_state(struct stream_interface *si, int state) { - si->state = si->prev_state = state; + si->state = si_strm(si)->prev_conn_state = state; } /* returns a bit for a stream-int state, to match against SI_SB_* */ diff --git a/src/cli.c b/src/cli.c index f55999e03..a428b691d 100644 --- a/src/cli.c +++ b/src/cli.c @@ -2766,7 +2766,7 @@ int pcli_wait_for_response(struct stream *s, struct channel *rep, int an_bit) sockaddr_free(&s->csb->dst); - cs_si(s->csb)->state = cs_si(s->csb)->prev_state = SI_ST_INI; + si_set_state(cs_si(s->csb), SI_ST_INI); cs_si(s->csb)->err_type = SI_ET_NONE; cs_si(s->csb)->flags &= SI_FL_ISBACK; /* we're in the context of process_stream */ s->csb->flags &= CS_FL_ISBACK | CS_FL_DONT_WAKE; /* we're in the context of process_stream */ diff --git a/src/stream.c b/src/stream.c index 61e66601c..7ab4f5c14 100644 --- a/src/stream.c +++ b/src/stream.c @@ -422,6 +422,7 @@ struct stream *stream_new(struct session *sess, struct conn_stream *cs, struct b s->pending_events = 0; s->conn_retries = 0; s->conn_exp = TICK_ETERNITY; + s->prev_conn_state = SI_ST_INI; t->process = process_stream; t->context = s; t->expire = TICK_ETERNITY; @@ -958,7 +959,7 @@ static void sess_set_term_flags(struct stream *s) s->flags |= SF_FINST_Q; else if (si_state_in(cs_si(s->csb)->state, SI_SB_REQ|SI_SB_TAR|SI_SB_ASS|SI_SB_CON|SI_SB_CER|SI_SB_RDY)) s->flags |= SF_FINST_C; - else if (cs_si(s->csb)->state == SI_ST_EST || cs_si(s->csb)->prev_state == SI_ST_EST) + else if (cs_si(s->csb)->state == SI_ST_EST || s->prev_conn_state == SI_ST_EST) s->flags |= SF_FINST_D; else s->flags |= SF_FINST_L; @@ -1780,9 +1781,23 @@ struct task *process_stream(struct task *t, void *context, unsigned int state) DBG_TRACE_POINT(STRM_EV_STRM_PROC, s); /* nothing special to be done on client side */ - if (unlikely(si_f->state == SI_ST_DIS)) + if (unlikely(si_f->state == SI_ST_DIS)) { si_f->state = SI_ST_CLO; + /* This is needed only when debugging is enabled, to indicate + * client-side close. + */ + if (unlikely((global.mode & MODE_DEBUG) && + (!(global.mode & MODE_QUIET) || + (global.mode & MODE_VERBOSE)))) { + chunk_printf(&trash, "%08x:%s.clicls[%04x:%04x]\n", + s->uniq_id, s->be->id, + (unsigned short)conn_fd(cs_conn(si_f->cs)), + (unsigned short)conn_fd(cs_conn(si_b->cs))); + DISGUISE(write(1, trash.area, trash.data)); + } + } + /* When a server-side connection is released, we have to count it and * check for pending connections on this server. */ @@ -1798,6 +1813,21 @@ struct task *process_stream(struct task *t, void *context, unsigned int state) if (may_dequeue_tasks(srv, s->be)) process_srv_queue(srv); } + + /* This is needed only when debugging is enabled, to indicate + * server-side close. + */ + if (unlikely((global.mode & MODE_DEBUG) && + (!(global.mode & MODE_QUIET) || + (global.mode & MODE_VERBOSE)))) { + if (s->prev_conn_state == SI_ST_EST) { + chunk_printf(&trash, "%08x:%s.srvcls[%04x:%04x]\n", + s->uniq_id, s->be->id, + (unsigned short)conn_fd(__cs_conn(si_f->cs)), + (unsigned short)conn_fd(cs_conn(si_b->cs))); + DISGUISE(write(1, trash.area, trash.data)); + } + } } /* @@ -2392,33 +2422,6 @@ struct task *process_stream(struct task *t, void *context, unsigned int state) s->csf->flags &= ~CS_FL_DONT_WAKE; s->csb->flags &= ~CS_FL_DONT_WAKE; - /* This is needed only when debugging is enabled, to indicate - * client-side or server-side close. Please note that in the unlikely - * event where both sides would close at once, the sequence is reported - * on the server side first. - */ - if (unlikely((global.mode & MODE_DEBUG) && - (!(global.mode & MODE_QUIET) || - (global.mode & MODE_VERBOSE)))) { - if (si_b->state == SI_ST_CLO && - si_b->prev_state == SI_ST_EST) { - chunk_printf(&trash, "%08x:%s.srvcls[%04x:%04x]\n", - s->uniq_id, s->be->id, - (unsigned short)conn_fd(cs_conn(si_f->cs)), - (unsigned short)conn_fd(cs_conn(si_b->cs))); - DISGUISE(write(1, trash.area, trash.data)); - } - - if (si_f->state == SI_ST_CLO && - si_f->prev_state == SI_ST_EST) { - chunk_printf(&trash, "%08x:%s.clicls[%04x:%04x]\n", - s->uniq_id, s->be->id, - (unsigned short)conn_fd(cs_conn(si_f->cs)), - (unsigned short)conn_fd(cs_conn(si_b->cs))); - DISGUISE(write(1, trash.area, trash.data)); - } - } - if (likely((si_f->state != SI_ST_CLO) || !si_state_in(si_b->state, SI_SB_INI|SI_SB_CLO) || (req->analysers & AN_REQ_FLT_END) || (res->analysers & AN_RES_FLT_END))) { if ((sess->fe->options & PR_O_CONTSTATS) && (s->flags & SF_BE_ASSIGNED) && !(s->flags & SF_IGNORE)) diff --git a/src/stream_interface.c b/src/stream_interface.c index 1198215e0..3f38fb785 100644 --- a/src/stream_interface.c +++ b/src/stream_interface.c @@ -988,8 +988,7 @@ void si_update_both(struct stream_interface *si_f, struct stream_interface *si_b req->flags &= ~(CF_READ_NULL|CF_READ_PARTIAL|CF_READ_ATTACHED|CF_WRITE_NULL|CF_WRITE_PARTIAL); res->flags &= ~(CF_READ_NULL|CF_READ_PARTIAL|CF_READ_ATTACHED|CF_WRITE_NULL|CF_WRITE_PARTIAL); - si_f->prev_state = si_f->state; - si_b->prev_state = si_b->state; + si_strm(si_b)->prev_conn_state = si_b->state; /* let's recompute both sides states */ if (si_state_in(si_f->state, SI_SB_RDY|SI_SB_EST))