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

[Yonglong Li <liyonglong@chinatelecom.cn>]

On 2021/4/15 19:54, Paolo Abeni wrote:
> On Thu, 2021-04-15 at 11:19 +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 0437fe4..521e67b 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;
> 
> Ooops... the existing code (prior to your patch) is pure nonsense here.
> 
> Is probably better just drop this check entirely.
> 
ok, I will drop this check entirely in next patch.