From: Christopher Faulet <cfaulet@haproxy.com>
Date: Mon, 27 Feb 2023 15:38:12 +0000 (+0100)
Subject: BUG/MEDIUM: stconn: Report a blocked send if some output data are not consumed
X-Git-Tag: v2.8-dev5~90
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=59b240c30ca03fb46e0a0c63af0cff54274702b3;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: stconn: Report a blocked send if some output data are not consumed

Instead of reporting a blocked send if nothing is send, we do it if some
output data remain blocked after a write attempts or after a call the the
applet's I/O handler. It is mandatory to properly handle write timeouts.

Indeed, if an endpoint is blocked for a while but it partially consumed
output data, no timeout is triggered. It is especially true for
connections. But the same may happen for applet, there is no reason.

Of course, if the endpoint decides to partially consume output data because
it must wait to move on for any reason, it should use the se/applet API
(se/applet_will_consume(), se/applet_wont_consume() and
se/applet_need_more_data()).

This bug was introduced during the channels timeouts refactoring. No
backport is needed.
---

diff --git a/src/applet.c b/src/applet.c
index bfdab6f91d..2900013932 100644
--- a/src/applet.c
+++ b/src/applet.c
@@ -250,18 +250,16 @@ struct task *task_run_applet(struct task *t, void *context, unsigned int state)
 	if (count != co_data(sc_oc(sc))) {
 		sc_oc(sc)->flags |= CF_WRITE_EVENT | CF_WROTE_DATA;
 		sc_have_room(sc_opposite(sc));
-		sc_ep_report_send_activity(sc);
-	}
-	else {
-		if (sc_ep_test(sc, SE_FL_WONT_CONSUME))
-			sc_ep_report_send_activity(sc);
-		else
-			sc_ep_report_blocked_send(sc);
 	}
 
 	if (sc_ic(sc)->flags & CF_READ_EVENT)
 		sc_ep_report_read_activity(sc);
 
+	if (channel_is_empty(sc_oc(sc)))
+		sc_ep_report_send_activity(sc);
+	else
+		sc_ep_report_blocked_send(sc);
+
 	/* measure the call rate and check for anomalies when too high */
 	if (((b_size(sc_ib(sc)) && sc->flags & SC_FL_NEED_BUFF) || // asks for a buffer which is present
 	     (b_size(sc_ib(sc)) && !b_data(sc_ib(sc)) && sc->flags & SC_FL_NEED_ROOM) || // asks for room in an empty buffer
diff --git a/src/stconn.c b/src/stconn.c
index 3c19cfc38c..39299b19a6 100644
--- a/src/stconn.c
+++ b/src/stconn.c
@@ -1667,12 +1667,8 @@ static int sc_conn_send(struct stconn *sc)
 		oc->flags |= CF_WRITE_EVENT | CF_WROTE_DATA;
 		if (sc->state == SC_ST_CON)
 			sc->state = SC_ST_RDY;
-
 		sc_have_room(sc_opposite(sc));
-		sc_ep_report_send_activity(sc);
 	}
-	else
-		sc_ep_report_blocked_send(sc);
 
 	if (sc_ep_test(sc, SE_FL_ERROR | SE_FL_ERR_PENDING)) {
 		oc->flags |= CF_WRITE_EVENT;
@@ -1681,9 +1677,14 @@ static int sc_conn_send(struct stconn *sc)
 		return 1;
 	}
 
-	/* We couldn't send all of our data, let the mux know we'd like to send more */
-	if (!channel_is_empty(oc))
+	if (channel_is_empty(oc))
+		sc_ep_report_send_activity(sc);
+	else {
+		/* We couldn't send all of our data, let the mux know we'd like to send more */
 		conn->mux->subscribe(sc, SUB_RETRY_SEND, &sc->wait_event);
+		sc_ep_report_blocked_send(sc);
+	}
+
 	return did_send;
 }