Re: [PATCH mptcp-next 2/3] mptcp: reset the packet scheduler on incoming MP_PRIO.

[Mat Martineau <mathew.j.martineau@linux.intel.com>]

On Fri, 18 Mar 2022, Paolo Abeni wrote:

> When an incoming MP_PRIO option changes the backup
> status of any subflow, we need to reset the packet
> scheduler status, or the next send could keep using
> the previously selected subflow, without taking in account
> the new priorities.
>
> Reported-by: Davide Caratti <dcaratti@redhat.com>
> Fixes: 40453a5c61f4 ("mptcp: add the incoming MP_PRIO support")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> net/mptcp/pm.c       | 19 +++++++++++++++----
> net/mptcp/protocol.c |  2 ++
> net/mptcp/protocol.h |  1 +
> 3 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index d0d31d5c198a..2f6ee422c6df 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -262,14 +262,25 @@ void mptcp_pm_rm_addr_received(struct mptcp_sock *msk,
> 	spin_unlock_bh(&pm->lock);
> }
>
> -void mptcp_pm_mp_prio_received(struct sock *sk, u8 bkup)
> +void mptcp_pm_mp_prio_received(struct sock *ssk, u8 bkup)
> {
> -	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
> +	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
> +	struct sock *sk = subflow->conn;
> +	struct mptcp_sock *msk;
>
> 	pr_debug("subflow->backup=%d, bkup=%d\n", subflow->backup, bkup);
> -	subflow->backup = bkup;
> +	msk = mptcp_sk(sk);
> +	if (subflow->backup == bkup) {

Above should be "if (subflow->backup != bkup)"?

> +		subflow->backup = bkup;
> +		mptcp_data_lock(sk);
> +		if (!sock_owned_by_user(sk))
> +			msk->last_snd = NULL;
> +		else
> +			__set_bit(MPTCP_RESET_SCHEDULER,  &msk->cb_flags);
> +		mptcp_data_unlock(sk);
> +	}
>
> -	mptcp_event(MPTCP_EVENT_SUB_PRIORITY, mptcp_sk(subflow->conn), sk, GFP_ATOMIC);
> +	mptcp_event(MPTCP_EVENT_SUB_PRIORITY, msk, ssk, GFP_ATOMIC);
> }
>
> void mptcp_pm_mp_fail_received(struct sock *sk, u64 fail_seq)
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index bdba1ddee2a7..cec42bc16d12 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -3128,6 +3128,8 @@ static void mptcp_release_cb(struct sock *sk)
> 			__mptcp_set_connected(sk);
> 		if (__test_and_clear_bit(MPTCP_ERROR_REPORT, &msk->cb_flags))
> 			__mptcp_error_report(sk);
> +		if (__test_and_clear_bit(MPTCP_RESET_SCHEDULER, &msk->cb_flags))
> +			msk->last_snd = NULL;
> 	}
>
> 	__mptcp_update_rmem(sk);
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index c8bada4537e2..d59c16d248a4 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -124,6 +124,7 @@
> #define MPTCP_RETRANSMIT	4
> #define MPTCP_FLUSH_JOIN_LIST	5
> #define MPTCP_CONNECTED		6
> +#define MPTCP_RESET_SCHEDULER	7
>
> static inline bool before64(__u64 seq1, __u64 seq2)
> {
> -- 
> 2.35.1



--
Mat Martineau
Intel