From: Frédéric Lécaille <flecaille@haproxy.com>
Date: Wed, 8 Feb 2023 15:08:28 +0000 (+0100)
Subject: MINOR: quic: Move code to wakeup the timer task to avoid anti-amplication deadlock
X-Git-Tag: v2.8-dev5~182
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=75c8ad549002b01de0b0d139a26b356973af57a9;p=thirdparty%2Fhaproxy.git

MINOR: quic: Move code to wakeup the timer task to avoid anti-amplication deadlock

This code was there because the timer task was not running on the same thread
as the one which parse the QUIC packets. Now that this is no more the case,
we can wake up this task directly.

Must be backported to 2.7.
---

diff --git a/include/haproxy/quic_conn-t.h b/include/haproxy/quic_conn-t.h
index 6560bd5f3d..cbf3f686aa 100644
--- a/include/haproxy/quic_conn-t.h
+++ b/include/haproxy/quic_conn-t.h
@@ -606,7 +606,6 @@ enum qc_mux_state {
 
 /* Flags at connection level */
 #define QUIC_FL_CONN_ANTI_AMPLIFICATION_REACHED  (1U << 0)
-#define QUIC_FL_CONN_IO_CB_WAKEUP                (1U << 1)
 #define QUIC_FL_CONN_POST_HANDSHAKE_FRAMES_BUILT (1U << 2)
 #define QUIC_FL_CONN_LISTENER                    (1U << 3)
 #define QUIC_FL_CONN_ACCEPT_REGISTERED           (1U << 4)
diff --git a/src/quic_conn.c b/src/quic_conn.c
index 458f13de52..503e7bb84e 100644
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -4466,20 +4466,6 @@ struct task *quic_conn_io_cb(struct task *t, void *context, unsigned int state)
 		qc_dgrams_retransmit(qc);
 	}
 
-	if (qc->flags & QUIC_FL_CONN_IO_CB_WAKEUP) {
-		qc->flags &= ~QUIC_FL_CONN_IO_CB_WAKEUP;
-		TRACE_DEVEL("needs to wakeup the timer task after the anti-amplicaiton limit was reached",
-		            QUIC_EV_CONN_IO_CB, qc);
-		/* The I/O handler has been woken up by the dgram parser (qc_lstnr_pkt_rcv())
-		 * after the anti-amplification was reached.
-		 *
-		 * TODO: this part should be removed. This was there because the
-		 * datagram parser was not executed by only one thread.
-		 */
-		qc_set_timer(qc);
-		if (qc->timer_task && tick_isset(qc->timer) && tick_is_lt(qc->timer, now_ms))
-			task_wakeup(qc->timer_task, TASK_WOKEN_MSG);
-	}
 	ssl_err = SSL_ERROR_NONE;
 	zero_rtt = st < QUIC_HS_ST_COMPLETE &&
 		quic_tls_has_rx_sec(eqel) &&
@@ -6544,7 +6530,6 @@ static void qc_rx_pkt_handle(struct quic_conn *qc, struct quic_rx_packet *pkt,
 		 * when sending the next packet if reached again.
 		 */
 		qc->flags &= ~QUIC_FL_CONN_ANTI_AMPLIFICATION_REACHED;
-		qc->flags |= QUIC_FL_CONN_IO_CB_WAKEUP;
 		io_cb_wakeup = 1;
 	}
 
@@ -6601,11 +6586,13 @@ static void qc_rx_pkt_handle(struct quic_conn *qc, struct quic_rx_packet *pkt,
  drop:
 	HA_ATOMIC_INC(&qc->prx_counters->dropped_pkt);
  err:
-	/* Wakeup the I/O handler callback if the PTO timer must be armed.
-	 * This cannot be done by this thread.
-	 */
-	if (io_cb_wakeup)
-		tasklet_wakeup(qc->wait_event.tasklet);
+	if (io_cb_wakeup) {
+		TRACE_DEVEL("needs to wakeup the timer task after the amplification limit was reached",
+		            QUIC_EV_CONN_LPKT, qc);
+		qc_set_timer(qc);
+		if (qc->timer_task && tick_isset(qc->timer) && tick_is_lt(qc->timer, now_ms))
+			task_wakeup(qc->timer_task, TASK_WOKEN_MSG);
+	}
 
 	TRACE_LEAVE(QUIC_EV_CONN_LPKT, qc ? qc : NULL, pkt, NULL, qv);
 }