RE: [PATCH net] vhost_net: don't continue to call the recvmsg when meet errors

-----Original Message-----
From: Jason Wang [mailto:jasowang@redhat.com] 
Sent: Thursday, December 01, 2016 11:37 AM
To: Michael S. Tsirkin
Cc: wangyunjian; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; caihe
Subject: Re: [PATCH net] vhost_net: don't continue to call the recvmsg when meet errors



On 2016年12月01日 11:27, Michael S. Tsirkin wrote:

quoted

On Thu, Dec 01, 2016 at 11:26:21AM +0800, Jason Wang wrote:

quoted

quoted

On 2016年12月01日 11:21, Michael S. Tsirkin wrote:

quoted

quoted

On Thu, Dec 01, 2016 at 02:48:59AM +0000, wangyunjian wrote:

quoted

quoted

quoted

quoted

-----Original Message-----
From: Michael S. Tsirkin [mailto:mst@redhat.com]
Sent: Wednesday, November 30, 2016 9:41 PM
To: wangyunjian
Cc:jasowang@redhat.com;netdev@vger.kernel.org;linux-kernel@
vger.kernel.org; caihe
Subject: Re: [PATCH net] vhost_net: don't continue to call 
the recvmsg when meet errors

On Wed, Nov 30, 2016 at 08:10:57PM +0800, Yunjian Wang wrote:

quoted

quoted

When we meet an error(err=-EBADFD) recvmsg,

How do you get EBADFD? Won't vhost_net_rx_peek_head_len 
return 0 in this case, breaking the loop?

We started many guest VMs while attaching/detaching some virtio-net nics for loop.
The soft lockup might happened. The err is -EBADFD.

OK, I'd like to figure out what happened here. why don't we get 0 
when we peek at the head?

EBADFD is from here:
         struct tun_struct *tun = __tun_get(tfile); ...
         if (!tun)
                 return -EBADFD;

but then:
static int tun_peek_len(struct socket *sock) {

...

         struct tun_struct *tun; ...
         tun = __tun_get(tfile);
         if (!tun)
                 return 0;


so peek len should return 0.

then while will exit:
         while ((sock_len = vhost_net_rx_peek_head_len(net, 
sock->sk))) ...

Consider this case: user do ip link del link tap0 before recvmsg() 
but after
tun_peek_len() ?

Sure, this can happen, but I think we'll just exit on the next loop, 
won't we?

Right, this is the only case I can image for -EBADFD, let's wait for the author to the steps.

Thanks, I understand it don't happen in the latest kernel version.
My problem happened using kernel version 3.10.0-xx

The peek len willn't return 0.

static int peek_head_len(struct sock *sk)
{
	struct sk_buff *head;
	int len = 0;
	unsigned long flags;

	spin_lock_irqsave(&sk->sk_receive_queue.lock, flags);
	head = skb_peek(&sk->sk_receive_queue);
	if (likely(head)) {
		len = head->len;
		if (skb_vlan_tag_present(head))
			len += VLAN_HLEN;
	}

	spin_unlock_irqrestore(&sk->sk_receive_queue.lock, flags);
	return len;
}