Re: [PATCH mptcp-next 1/2] mptcp: give MPTCP sockets a distinct lockdep class

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mptcp: passive sockets miss lockdep reclassification causing warnings
--

commit c0431dba7b7ab62b7e71ad2f4ff94fb4913cc837
Author: Geliang Tang <geliang@kernel.org>

mptcp: give MPTCP sockets a distinct lockdep class

MPTCP sockets and their subflows share the same k-sk_lock-AF_INET
lockdep class. When both are kernel sockets, lockdep flags every path
that holds the MPTCP socket lock and then locks a subflow (setsockopt,
connect, bind, worker send-ack) as "possible recursive locking".

Reclassify the MPTCP socket lock in mptcp_init_sock() to use a distinct
lock class (sk_lock-AF_INET-MPTCP for IPv4, sk_lock-AF_INET6-MPTCP for
IPv6). This approach is similar to nvme-tcp and avoids the need to
reclassify each subflow socket individually.

The implementation is based on nvme_tcp_reclassify_socket() from
drivers/nvme/host/tcp.c.

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 60cbd91be6320..e63896a605953 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
[ ... ]
> @@ -3292,6 +3327,8 @@ static int mptcp_init_sock(struct sock *sk)
>  	sk->sk_sndbuf = READ_ONCE(net->ipv4.sysctl_tcp_wmem[1]);
>  	sk->sk_write_space = sk_stream_write_space;
>  
> +	mptcp_reclassify_socket(sk->sk_socket);
> +
>  	return 0;
>  }

[Severity: Medium]
Since mptcp_init_sock() is only used for active socket creation, are cloned
(passive) connections missing this lockdep reclassification?

When an incoming MPTCP connection is accepted, the socket is cloned:

net/mptcp/protocol.c:mptcp_sk_clone_init() {
	...
	struct sock *nsk = sk_clone_lock(sk, GFP_ATOMIC);
	...
	__mptcp_init_sock(nsk);
	...
}

sk_clone_lock() initializes the lockdep class back to the generic AF_INET
default. Because mptcp_reclassify_socket() is never called for these cloned
sockets, will lockdep continue to falsely report recursive locking when an
accepted MPTCP socket and its subflow are locked simultaneously?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1792060837.git.tanggeliang@kylinos.cn?part=1