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);
This commit is contained in:
Aurelien DARRAGON 2023-02-07 11:23:38 +01:00 committed by Christopher Faulet
parent 2370599f96
commit 7a15fa58b1

View File

@ -399,19 +399,17 @@ void default_add_listener(struct protocol *proto, struct listener *listener)
/* 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);
}
@ -462,6 +460,8 @@ int default_resume_listener(struct listener *l)
* 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
@ -481,12 +481,27 @@ int pause_listener(struct listener *l, int lpx, int lli)
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) {
/* PROXY_LOCK is required */