Re: [PATCH mptcp-net] mptcp: sched: check both backup in retrans

[Mat Martineau <martineau@kernel.org>]

On Tue, 13 Aug 2024, Matthieu Baerts (NGI0) wrote:

> The 'mptcp_subflow_context' structure has two items related to the
> backup flags:
>
> - 'backup': the subflow has been marked as backup by the other peer
>
> - 'request_bkup': the backup flag has been set by the host
>
> Looking only at the 'backup' flag can make sense in some cases, but it
> is not the behaviour of the default packet scheduler when selecting
> paths.
>
> As explained in the commit b6a66e521a20 ("mptcp: sched: check both
> directions for backup"), the packet scheduler should look at both flags,
> because that was the behaviour from the beginning: the 'backup' flag was
> set by accident instead of the 'request_bkup' one. Now that the latter
> has been fixed, get_retrans() needs to be adapted as well.
>
> Fixes: 3b1d6210a957 ("mptcp: implement and use MPTCP-level retransmission")

I think this should have:

Fixes: b6a66e521a20 ("mptcp: sched: check both directions for backup")

and a Cc: stable ?


Otherwise LGTM:

Reviewed-by: Mat Martineau <martineau@kernel.org>


> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> net/mptcp/protocol.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 13777c35496c..ad60d04f4981 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2326,7 +2326,7 @@ struct sock *mptcp_subflow_get_retrans(struct mptcp_sock *msk)
> 			continue;
> 		}
>
> -		if (subflow->backup) {
> +		if (subflow->backup || subflow->request_bkup) {
> 			if (!backup)
> 				backup = ssk;
> 			continue;
>
> ---
> base-commit: 971367a56ebfd1fd70608813898e48681281da99
> change-id: 20240813-mptcp-sched-retrans-bkp-291785163b52
>
> Best regards,
> -- 
> Matthieu Baerts (NGI0) <matttbe@kernel.org>
>
>
>