netdev - Re: [PATCH] virtio_net: Add schedule check to napi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110210175725.GA9674@redhat.com>
Date:	Thu, 10 Feb 2011 19:57:26 +0200
From:	"Michael S. Tsirkin" <mst@...hat.com>
To:	Rusty Russell <rusty@...tcorp.com.au>
Cc:	Herbert Xu <herbert@...dor.hengli.com.au>, netdev@...r.kernel.org,
	David Miller <davem@...emloft.net>,
	virtualization@...ts.linux-foundation.org,
	Ken Stailey <kstailey@...oo.com>
Subject: Re: [PATCH] virtio_net: Add schedule check to napi_enable call

On Thu, Feb 10, 2011 at 12:32:50PM +1030, Rusty Russell wrote:
> From: "Bruce Rogers" <brogers@...ell.com>
> 
> Under harsh testing conditions, including low memory, the guest would
> stop receiving packets. With this patch applied we no longer see any
> problems in the driver while performing these tests for extended periods
> of time.
> 
> Make sure napi is scheduled subsequent to each napi_enable.
> 
> Signed-off-by: Bruce Rogers <brogers@...ell.com>
> Signed-off-by: Olaf Kirch <okir@...e.de>
> Cc: stable@...nel.org
> Signed-off-by: Rusty Russell <rusty@...tcorp.com.au>

Rusty, so this is 2.6.38 material - you'll send this to Linus? Or DaveM?

> ---
>  drivers/net/virtio_net.c |   27 ++++++++++++++++-----------
>  1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -446,6 +446,20 @@ static void skb_recv_done(struct virtque
>  	}
>  }
>  
> +static void virtnet_napi_enable(struct virtnet_info *vi)
> +{
> +	napi_enable(&vi->napi);
> +
> +	/* If all buffers were filled by other side before we napi_enabled, we
> +	 * won't get another interrupt, so process any outstanding packets
> +	 * now.  virtnet_poll wants re-enable the queue, so we disable here.
> +	 * We synchronize against interrupts via NAPI_STATE_SCHED */
> +	if (napi_schedule_prep(&vi->napi)) {
> +		virtqueue_disable_cb(vi->rvq);
> +		__napi_schedule(&vi->napi);
> +	}
> +}
> +
>  static void refill_work(struct work_struct *work)
>  {
>  	struct virtnet_info *vi;
> @@ -454,7 +468,7 @@ static void refill_work(struct work_stru
>  	vi = container_of(work, struct virtnet_info, refill.work);
>  	napi_disable(&vi->napi);
>  	still_empty = !try_fill_recv(vi, GFP_KERNEL);
> -	napi_enable(&vi->napi);
> +	virtnet_napi_enable(vi);
>  
>  	/* In theory, this can happen: if we don't get any buffers in
>  	 * we will *never* try to fill again. */
> @@ -638,16 +652,7 @@ static int virtnet_open(struct net_devic
>  {
>  	struct virtnet_info *vi = netdev_priv(dev);
>  
> -	napi_enable(&vi->napi);
> -
> -	/* If all buffers were filled by other side before we napi_enabled, we
> -	 * won't get another interrupt, so process any outstanding packets
> -	 * now.  virtnet_poll wants re-enable the queue, so we disable here.
> -	 * We synchronize against interrupts via NAPI_STATE_SCHED */
> -	if (napi_schedule_prep(&vi->napi)) {
> -		virtqueue_disable_cb(vi->rvq);
> -		__napi_schedule(&vi->napi);
> -	}
> +	virtnet_napi_enable(vi);
>  	return 0;
>  }
>  
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html