BUG/MEDIUM: listener: fix pause_listener() suspend return value handling
Legacy suspend() return value handling in pause_listener() has been altered
over the time.
First with fb76bd5ca ("BUG/MEDIUM: listeners: correctly report pause() errors")
Then with e03204c8e ("MEDIUM: listeners: implement protocol level
->suspend/resume() calls")
We aim to restore original function behavior and comply with resume_listener()
function description.
This is required for resume_listener() and pause_listener() to work as a whole
Now, it is made explicit that pause_listener() may stop a listener if the
listener doesn't support the LI_PAUSED state (depending on the protocol
family, ie: ABNS sockets), in this case l->state will be set to LI_ASSIGNED
and this won't be considered as an error.
This could be backported up to 2.4 after a reasonable observation period
to make sure that this change doesn't cause unwanted side-effects.
--
Backport notes:
This commit depends on:
- "MINOR: listener: make sure we don't pause/resume bypassed listeners"
-> 2.4: manual change required because "MINOR: proxy/listener: support
for additional PAUSED state" was not backported: the contextual patch
lines don't match.
Replace this:
| if (px && !px->li_ready) {
| /* PROXY_LOCK is required */
By this:
| if (px && !px->li_ready) {
| ha_warning("Paused %s %s.\n", proxy_cap_str(px->cap), px->id);
(cherry picked from commit 7a15fa58b1087db4af60d4beaca0a11bbc8c0778)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 3c1e3cbe22397120f62bf4ad7db76e8b3adbfae2)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit 82eddf6f2ab7e2b3b75d9181318c0ccf77271734)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit e822967ceb14d2ba14a6ae84c417c9253a813bfd)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/src/listener.c b/src/listener.c
index e151e10..e262e85 100644
--- a/src/listener.c
+++ b/src/listener.c
@@ -363,19 +363,17 @@
/* default function called to suspend a listener: it simply passes the call to
* the underlying receiver. This is find for most socket-based protocols. This
- * must be called under the listener's lock. It will return non-zero on success,
- * 0 on failure. If no receiver-level suspend is provided, the operation is
- * assumed to succeed.
+ * must be called under the listener's lock. It will return < 0 in case of
+ * failure, 0 if the listener was totally stopped, or > 0 if correctly paused..
+ * If no receiver-level suspend is provided, the operation is assumed
+ * to succeed.
*/
int default_suspend_listener(struct listener *l)
{
- int ret = 1;
-
if (!l->rx.proto->rx_suspend)
return 1;
- ret = l->rx.proto->rx_suspend(&l->rx);
- return ret > 0 ? ret : 0;
+ return l->rx.proto->rx_suspend(&l->rx);
}
@@ -426,6 +424,8 @@
* closes upon SHUT_WR and refuses to rebind. So a common validation path
* involves SHUT_WR && listen && SHUT_RD. In case of success, the FD's polling
* is disabled. It normally returns non-zero, unless an error is reported.
+ * pause() may totally stop a listener if it doesn't support the PAUSED state,
+ * in which case state will be set to ASSIGNED.
* It will need to operate under the proxy's lock and the listener's lock.
* The caller is responsible for indicating in lpx, lli whether the respective
* locks are already held (non-zero) or not (zero) so that the function pick
@@ -449,12 +449,27 @@
if (!(l->flags & LI_F_FINALIZED) || l->state <= LI_PAUSED)
goto end;
- if (l->rx.proto->suspend)
+ if (l->rx.proto->suspend) {
ret = l->rx.proto->suspend(l);
+ /* if the suspend() fails, we don't want to change the
+ * current listener state
+ */
+ if (ret < 0)
+ goto end;
+ }
MT_LIST_DELETE(&l->wait_queue);
- listener_set_state(l, LI_PAUSED);
+ /* ret == 0 means that the suspend() has been turned into
+ * an unbind(), meaning the listener is now stopped (ie: ABNS), we need
+ * to report this state change properly
+ */
+ listener_set_state(l, ((ret) ? LI_PAUSED : LI_ASSIGNED));
+
+ /* at this point, everything is under control, no error should be
+ * returned to calling function
+ */
+ ret = 1;
if (px && !px->li_ready) {
ha_warning("Paused %s %s.\n", proxy_cap_str(px->cap), px->id);