From: Amaury Denoyelle Date: Wed, 28 Sep 2022 13:15:51 +0000 (+0200) Subject: BUG/MINOR: quic: fix subscribe operation X-Git-Tag: v2.7-dev9~129 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bbb1c68508ceebb98ac4234c906a65a42596e6ea;p=thirdparty%2Fhaproxy.git BUG/MINOR: quic: fix subscribe operation Subscribing was not properly designed between quic-conn and quic MUX layers. Align this as with in other haproxy components : field is moved from the MUX to the quic-conn structure. All mention of qcc MUX is cleaned up in quic_conn_subscribe()/quic_conn_unsubscribe(). Thanks to this change, ACK reception notification has been simplified. It's now unnecessary to check for the MUX existence before waking it. Instead, if quic-conn field is set, just wake-up the upper layer tasklet without mentionning MUX. This should probably be extended to other part in quic-conn code. This should be backported up to 2.6. --- diff --git a/include/haproxy/mux_quic-t.h b/include/haproxy/mux_quic-t.h index a55f3db26d..d716e08d52 100644 --- a/include/haproxy/mux_quic-t.h +++ b/include/haproxy/mux_quic-t.h @@ -97,7 +97,6 @@ struct qcc { struct list send_retry_list; /* list of qcs eligible to send retry */ struct wait_event wait_event; /* To be used if we're waiting for I/Os */ - struct wait_event *subs; struct proxy *proxy; diff --git a/include/haproxy/quic_conn-t.h b/include/haproxy/quic_conn-t.h index b493f85f9c..65a4ab0225 100644 --- a/include/haproxy/quic_conn-t.h +++ b/include/haproxy/quic_conn-t.h @@ -706,6 +706,7 @@ struct quic_conn { int stream_buf_count; /* total count of allocated stream buffers for this connection */ struct wait_event wait_event; + struct wait_event *subs; /* MUX */ struct qcc *qcc; diff --git a/src/mux_quic.c b/src/mux_quic.c index 142b4e4624..e97621caa6 100644 --- a/src/mux_quic.c +++ b/src/mux_quic.c @@ -1990,7 +1990,6 @@ static int qc_init(struct connection *conn, struct proxy *prx, LIST_INIT(&qcc->send_retry_list); - qcc->subs = NULL; qcc->wait_event.tasklet->process = qc_io_cb; qcc->wait_event.tasklet->context = qcc; qcc->wait_event.events = 0; diff --git a/src/quic_conn.c b/src/quic_conn.c index 8b04695249..c10a8ca98c 100644 --- a/src/quic_conn.c +++ b/src/quic_conn.c @@ -1716,14 +1716,12 @@ static inline void qc_treat_acked_tx_frm(struct quic_conn *qc, qc_release_frm(qc, frm); } - if (stream_acked && qc->mux_state == QC_MUX_READY) { - struct qcc *qcc = qc->qcc; - - if (qcc->subs && qcc->subs->events & SUB_RETRY_SEND) { - tasklet_wakeup(qcc->subs->tasklet); - qcc->subs->events &= ~SUB_RETRY_SEND; - if (!qcc->subs->events) - qcc->subs = NULL; + if (stream_acked) { + if (qc->subs && qc->subs->events & SUB_RETRY_SEND) { + tasklet_wakeup(qc->subs->tasklet); + qc->subs->events &= ~SUB_RETRY_SEND; + if (!qc->subs->events) + qc->subs = NULL; } } leave: @@ -4614,15 +4612,12 @@ struct task *qc_process_timer(struct task *task, void *ctx, unsigned int state) if (qc->path->in_flight) { pktns = quic_pto_pktns(qc, qc->state >= QUIC_HS_ST_COMPLETE, NULL); - if (qc->mux_state == QC_MUX_READY && qc->qcc->subs && - qc->qcc->subs->events & SUB_RETRY_SEND) { - struct qcc *qcc = qc->qcc; - + if (qc->subs && qc->subs->events & SUB_RETRY_SEND) { pktns->tx.pto_probe = QUIC_MAX_NB_PTO_DGRAMS; - tasklet_wakeup(qcc->subs->tasklet); - qcc->subs->events &= ~SUB_RETRY_SEND; - if (!qcc->subs->events) - qcc->subs = NULL; + tasklet_wakeup(qc->subs->tasklet); + qc->subs->events &= ~SUB_RETRY_SEND; + if (!qc->subs->events) + qc->subs = NULL; } else { qc->flags |= QUIC_FL_CONN_RETRANS_NEEDED; @@ -4865,6 +4860,7 @@ static struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4, */ qc->tid = quic_get_cid_tid(qc->scid.data, l->bind_conf); qc->wait_event.tasklet->tid = qc->tid; + qc->subs = NULL; if (qc_conn_alloc_ssl_ctx(qc) || !quic_conn_init_timer(qc) || diff --git a/src/xprt_quic.c b/src/xprt_quic.c index e908925351..9e9258f057 100644 --- a/src/xprt_quic.c +++ b/src/xprt_quic.c @@ -47,21 +47,24 @@ static void quic_close(struct connection *conn, void *xprt_ctx) static int quic_conn_subscribe(struct connection *conn, void *xprt_ctx, int event_type, struct wait_event *es) { struct quic_conn *qc = conn->handle.qc; - struct qcc *qcc = qc->qcc; TRACE_ENTER(QUIC_EV_CONN_SUB, qc); BUG_ON(event_type & ~(SUB_RETRY_SEND|SUB_RETRY_RECV)); - BUG_ON(qcc->subs && qcc->subs != es); + BUG_ON(qc->subs && qc->subs != es); es->events |= event_type; - qcc->subs = es; + qc->subs = es; + + /* TODO implement a check_events to detect if subscriber should be + * woken up immediately ? + */ if (event_type & SUB_RETRY_RECV) - TRACE_DEVEL("subscribe(recv)", QUIC_EV_CONN_XPRTRECV, qc, qcc); + TRACE_DEVEL("subscribe(recv)", QUIC_EV_CONN_XPRTRECV, qc); if (event_type & SUB_RETRY_SEND) - TRACE_DEVEL("subscribe(send)", QUIC_EV_CONN_XPRTSEND, qc, qcc); + TRACE_DEVEL("subscribe(send)", QUIC_EV_CONN_XPRTSEND, qc); TRACE_LEAVE(QUIC_EV_CONN_SUB, qc); @@ -74,22 +77,24 @@ static int quic_conn_subscribe(struct connection *conn, void *xprt_ctx, int even */ static int quic_conn_unsubscribe(struct connection *conn, void *xprt_ctx, int event_type, struct wait_event *es) { - int ret; struct quic_conn *qc = conn->handle.qc; - struct qcc *qcc = qc->qcc; TRACE_ENTER(QUIC_EV_CONN_SUB, qc); if (event_type & SUB_RETRY_RECV) - TRACE_DEVEL("unsubscribe(recv)", QUIC_EV_CONN_XPRTRECV, qc, qcc); + TRACE_DEVEL("unsubscribe(recv)", QUIC_EV_CONN_XPRTRECV, qc); if (event_type & SUB_RETRY_SEND) - TRACE_DEVEL("unsubscribe(send)", QUIC_EV_CONN_XPRTSEND, qc, qcc); + TRACE_DEVEL("unsubscribe(send)", QUIC_EV_CONN_XPRTSEND, qc); + + es->events &= ~event_type; + if (!es->events) + qc->subs = NULL; - ret = conn_unsubscribe(conn, xprt_ctx, event_type, es); + /* TODO implement ignore_events similar to conn_unsubscribe() ? */ TRACE_LEAVE(QUIC_EV_CONN_SUB, qc); - return ret; + return 0; } /* Store in the context attached to .