Re: [PATCH mptcp-next v7 2/4] mptcp: implement .read_sock

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

Thank you for the new version!

(Thank you, Paolo, for the previous reviews!)

On 07/07/2025 11:34, 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 is not implemented in MPTCP.
> 
> This patch implements it with reference to __mptcp_recvmsg_mskq().

I still think more explanations are needed to help reviewers and
developers later, e.g. explaining what this interface is supposed to do,
what are the particularities linked to MPTCP and the differences with
TCP, why you cannot re-use something already there or the differences
with __mptcp_recvmsg_mskq(), etc.

> v2:
>  - first check the sk_state (Matt), but not look for the end of the
> end of a connection like TCP in __tcp_read_sock():
> 
> 	if (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN)
> 		break;
> 
> This will cause a use-after-free error:
> 
> 	BUG: KASAN: slab-use-after-free in mptcp_read_sock.
> 
> v3:
>  - Use sk->sk_rcvbuf instead of INT_MAX as the max len.
> 
> v4:
>  - invoke __mptcp_move_skbs.

Thank you for the changelog, it is very useful, but please move it in
the comment section, under the last Signed-off-by, after a line
containing "---":

  (...)
  Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
  ---
  Notes:
    v2:
      - (...)

Otherwise, the changelog will be part of the commit message when sending
this patch to netdev.

Same in the other patches.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.