Re: [RFC PATCH mptcp-next v2 2/3] mptcp: pm: kernel: retry subflows closed after an error

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mptcp: lockdep warning due to missing nested annotation in mptcp_subflow_reconnect()
--

commit 0bd36d5db0e57b64ed74ede4cce6a078baf37f44
Author: Anil Kaushik <anilkaushikwireless@gmail.com>

mptcp: pm: kernel: retry subflows closed after an error

This commit adds an opt-in sysctl to retry MPTCP subflows closed after
an error. It reuses the same socket for the next attempt if it cannot
connect, avoiding the need for extra state or timers.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/440

> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 78e886b202585..0f1d3fae446e2 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c

[ ... ]

> @@ -1766,6 +1766,59 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_local *local,
>  	return err;
>  }
>  
> +/* Connect again an MPJ subflow that failed to connect, with the same socket.
> + * The subflow never carried data: like for the initial subflow in
> + * __mptcp_subflow_disconnect(), resetting the TCP and the subflow states is
> + * enough. Called with the msk socket lock held.
> + */
> +int mptcp_subflow_reconnect(struct sock *sk, struct sock *ssk,
> +			    const struct mptcp_pm_local *local,
> +			    const struct mptcp_addr_info *remote)
> +{
> +	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
> +	struct mptcp_sock *msk = mptcp_sk(sk);
> +	struct sockaddr_storage addr;
> +	u32 remote_token;
> +	int addrlen;
> +	int err;
> +
> +	lock_sock(ssk);

[Severity: Medium]
Will this lock_sock(ssk) trigger a lockdep splat?

The function documentation states it is called with the msk socket lock
held. Since MPTCP sockets and their TCP subflows share the same lockdep
class, acquiring the child subflow lock while the parent socket lock is
already held might be seen as a recursive locking deadlock by lockdep.

Should this use lock_sock_nested(ssk, SINGLE_DEPTH_NESTING) to avoid
system panics when CONFIG_PANIC_ON_WARN is enabled?

> +	err = tcp_disconnect(ssk, 0);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013025653.523755-1-anilkaushikwireless@gmail.com?part=2