Re: [PATCH v4 1/2] mptcp: add MSG_PEEK support

[Paolo Abeni <pabeni@redhat.com>]

On Mon, 2021-04-19 at 10:53 +0800, Yonglong Li wrote:
> This patch adds support for MSG_PEEK flag. Packets are not removed
> from the receive_queue if MSG_PEEK set in recv() system call.
> 
> Signed-off-by: Yonglong Li <liyonglong@chinatelecom.cn>
> ---
>  net/mptcp/protocol.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 16d73ec..75f4420 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1739,12 +1739,12 @@ static void mptcp_wait_data(struct sock *sk, long *timeo)
>  
>  static int __mptcp_recvmsg_mskq(struct mptcp_sock *msk,
>  				struct msghdr *msg,
> -				size_t len)
> +				size_t len, int flags)
>  {
> -	struct sk_buff *skb;
> +	struct sk_buff *skb, *tmp;
>  	int copied = 0;
>  
> -	while ((skb = skb_peek(&msk->receive_queue)) != NULL) {
> +	skb_queue_walk_safe(&msk->receive_queue, skb, tmp) {
>  		u32 offset = MPTCP_SKB_CB(skb)->offset;
>  		u32 data_len = skb->len - offset;
>  		u32 count = min_t(size_t, len - copied, data_len);
> @@ -1760,15 +1760,18 @@ static int __mptcp_recvmsg_mskq(struct mptcp_sock *msk,
>  		copied += count;
>  
>  		if (count < data_len) {
> -			MPTCP_SKB_CB(skb)->offset += count;
> +			if (!(flags & MSG_PEEK))
> +				MPTCP_SKB_CB(skb)->offset += count;
>  			break;
>  		}
>  
> -		/* we will bulk release the skb memory later */
> -		skb->destructor = NULL;
> -		msk->rmem_released += skb->truesize;
> -		__skb_unlink(skb, &msk->receive_queue);
> -		__kfree_skb(skb);
> +		if (!(flags & MSG_PEEK)) {
> +			/* we will bulk release the skb memory later */
> +			skb->destructor = NULL;
> +			msk->rmem_released += skb->truesize;
> +			__skb_unlink(skb, &msk->receive_queue);
> +			__kfree_skb(skb);
> +		}
>  
>  		if (copied >= len)
>  			break;
> @@ -1945,7 +1948,7 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
>  	int target;
>  	long timeo;
>  
> -	if (msg->msg_flags & ~(MSG_WAITALL | MSG_DONTWAIT))
> +	if (flags & ~(MSG_PEEK | MSG_WAITALL | MSG_DONTWAIT))
>  		return -EOPNOTSUPP;
>  
>  	mptcp_lock_sock(sk, __mptcp_splice_receive_queue(sk));
> @@ -1962,7 +1965,7 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
>  	while (copied < len) {
>  		int bytes_read;
>  
> -		bytes_read = __mptcp_recvmsg_mskq(msk, msg, len - copied);
> +		bytes_read = __mptcp_recvmsg_mskq(msk, msg, len - copied, flags);
>  		if (unlikely(bytes_read < 0)) {
>  			if (!copied)
>  				copied = bytes_read;
> @@ -2046,7 +2049,8 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
>  	pr_debug("msk=%p data_ready=%d rx queue empty=%d copied=%d",
>  		 msk, test_bit(MPTCP_DATA_READY, &msk->flags),
>  		 skb_queue_empty_lockless(&sk->sk_receive_queue), copied);
> -	mptcp_rcv_space_adjust(msk, copied);
> +	if (!(flags & MSG_PEEK))
> +		mptcp_rcv_space_adjust(msk, copied);
>  
>  	release_sock(sk);
>  	return copied;

LGTM,

Acked-by: Paolo Abeni <pabeni@redhat.com>

Note that the first chunk mptcp_recvmsg() will conflict with other
pending changes ("mptcp: ignore unsupported msg flags"), but I guess
Mattbe can handle that ;) - likely no need to resubmit.

Thanks!

Paolo