From: Christopher Faulet <cfaulet@haproxy.com>
Date: Thu, 9 Nov 2023 08:54:51 +0000 (+0100)
Subject: BUG/MEDIUM: stconn: Don't update stream expiration date if already expired
X-Git-Tag: v2.9-dev10~119
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=78021ee9ef31dfb7055701b712694feecef238e3;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: stconn: Don't update stream expiration date if already expired

The commit 08d7169f4 ("MINOR: stconn: Don't queue stream task in past in
sc_notify()") tried to fix issues with epiration date set in past for the
stream in sc_notify(). However it remains some cases where the stream
expiration date may already be expired before recomputing it. This happens
when an event is reported by the mux exactly when a timeout is triggered. In
this case, depending on the scheduling, the SC may be woken up before the
stream. For these cases, we fall into the BUG_ON() preventing to queue in
the past.

So, it remains unexpected to queue a task in the past. The BUG_ON() is
correct at this place. We must just avoid to recompute the stream expiration
date if it is already expired. At worst, the stream will be woken up for
nothing. But it is not really a big deal because it will only happen on
timeouts from time to time. It is so sporadic that we can ignore it from a
performance point of view.

This patch must be backpoted to 2.8. Be careful to remove the BUG_ON() on
the 2.8.
---

diff --git a/src/stconn.c b/src/stconn.c
index 8ced6b8dde..d6e4683d43 100644
--- a/src/stconn.c
+++ b/src/stconn.c
@@ -1135,19 +1135,20 @@ static void sc_notify(struct stconn *sc)
 		task_wakeup(task, TASK_WOKEN_IO);
 	}
 	else {
-		/* Update expiration date for the task and requeue it */
-		task->expire = (tick_is_expired(task->expire, now_ms) ? 0 : task->expire);
-		task->expire = tick_first(task->expire, sc_ep_rcv_ex(sc));
-		task->expire = tick_first(task->expire, sc_ep_snd_ex(sc));
-		task->expire = tick_first(task->expire, sc_ep_rcv_ex(sco));
-		task->expire = tick_first(task->expire, sc_ep_snd_ex(sco));
-		task->expire = tick_first(task->expire, ic->analyse_exp);
-		task->expire = tick_first(task->expire, oc->analyse_exp);
-		task->expire = tick_first(task->expire, __sc_strm(sc)->conn_exp);
-
-		/* WARNING: Don't forget to remove this BUG_ON before 2.9.0 */
-		BUG_ON(tick_is_expired(task->expire, now_ms));
-		task_queue(task);
+		/* Update expiration date for the task and requeue it if not already expired */
+		if (!tick_is_expired(task->expire, now_ms)) {
+			task->expire = tick_first(task->expire, sc_ep_rcv_ex(sc));
+			task->expire = tick_first(task->expire, sc_ep_snd_ex(sc));
+			task->expire = tick_first(task->expire, sc_ep_rcv_ex(sco));
+			task->expire = tick_first(task->expire, sc_ep_snd_ex(sco));
+			task->expire = tick_first(task->expire, ic->analyse_exp);
+			task->expire = tick_first(task->expire, oc->analyse_exp);
+			task->expire = tick_first(task->expire, __sc_strm(sc)->conn_exp);
+
+			/* WARNING: Don't forget to remove this BUG_ON before 2.9.0 */
+			BUG_ON(tick_is_expired(task->expire, now_ms));
+			task_queue(task);
+		}
 	}
 
 	if (ic->flags & CF_READ_EVENT)