Re: [PATCH mptcp-next v15 2/6] mptcp: implement .read_sock

[Geliang Tang <geliang@kernel.org>]

Hi Matt,

On Thu, 2025-12-11 at 19:22 +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> Thank you for working on that!
> 
> On 06/12/2025 13:33, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > nvme_tcp_try_recv() needs to call .read_sock interface of struct
> > proto_ops, but it's not implemented in MPTCP.
> > 
> > This patch implements it with reference to __tcp_read_sock() and
> > __mptcp_recvmsg_mskq().
> > 
> > Corresponding to tcp_recv_skb(), a new helper for MPTCP named
> > mptcp_recv_skb() is added to peek a skb from sk->sk_receive_queue.
> > 
> > Compared with __mptcp_recvmsg_mskq(), mptcp_read_sock() uses
> > sk->sk_rcvbuf as the max read length. The LISTEN status is checked
> > before the while loop, and mptcp_recv_skb() and
> > mptcp_cleanup_rbuf()
> > are invoked after the loop. In the loop, all flags checks for
> > __mptcp_recvmsg_mskq() are removed.
> 
> (...)
> 
> > +static int mptcp_read_sock(struct sock *sk, read_descriptor_t
> > *desc,
> > +			   sk_read_actor_t recv_actor)
> > +{
> > +	return __mptcp_read_sock(sk, desc, recv_actor, false);
> 
> In this series, it looks like __mptcp_read_sock() is only called
> here,
> always with noack == false then. Will you call this helper with noack
> set to true in another series?

Yes, indeed. This read_sock will be called in all three series that I
am currently developing.

1. In "NVME over MPTCP" series, mptcp_read_sock() will be used, see:

    consumed = sock->ops->read_sock(sk, &rd_desc, nvme_tcp_recv_skb);

in nvme_tcp_try_recv().

2. In "MPTCP sockmap support" series (issue #521),
mptcp_read_sock_noack() will be used, see:

    copied = tcp_read_sock_noack(sk, desc, recv_actor, true,
                                 &psock->copied_seq);

in tcp_bpf_strp_read_sock().

3. In "MPTCP KTLS support" series (issue #480), tcp_read_sock() is used
in tls_strp_read_copyin():

    tcp_read_sock(strp->sk, &desc, tls_strp_copyin);

I originally intended to use mptcp_read_sock() here, but it would cause
a deadlock, so I had to switch to using tcp_read_sock_noack() instead.

> 
> (Maybe the extension could be introduced later, but if the next
> series
> follows, that's fine to keep it like that.)

So I think it's better to keep this patch as is.

Thanks,
-Geliang

> 
> Cheers,
> Matt