similar to: [PATCH net] virtio-net: fix refill races during restore

On Fri, 2013-12-27 at 12:55 +0800, Jason Wang wrote: > On 12/27/2013 05:56 AM, Eric Dumazet wrote: > > On Thu, 2013-12-26 at 13:28 -0800, Michael Dalton wrote: > >> On Mon, Dec 23, 2013 at 11:37 AM, Michael S. Tsirkin <mst at redhat.com> wrote: > >>> So there isn't a conflict with respect to locking. > >>> > >>> Is it problematic to

On Fri, 2013-12-27 at 12:55 +0800, Jason Wang wrote: > On 12/27/2013 05:56 AM, Eric Dumazet wrote: > > On Thu, 2013-12-26 at 13:28 -0800, Michael Dalton wrote: > >> On Mon, Dec 23, 2013 at 11:37 AM, Michael S. Tsirkin <mst at redhat.com> wrote: > >>> So there isn't a conflict with respect to locking. > >>> > >>> Is it problematic to

On Thu, 2013-12-26 at 13:28 -0800, Michael Dalton wrote: > On Mon, Dec 23, 2013 at 11:37 AM, Michael S. Tsirkin <mst at redhat.com> wrote: > > So there isn't a conflict with respect to locking. > > > > Is it problematic to use same page_frag with both GFP_ATOMIC and with > > GFP_KERNEL? If yes why? > > I believe it is safe to use the same page_frag and I

On Thu, 2013-12-26 at 13:28 -0800, Michael Dalton wrote: > On Mon, Dec 23, 2013 at 11:37 AM, Michael S. Tsirkin <mst at redhat.com> wrote: > > So there isn't a conflict with respect to locking. > > > > Is it problematic to use same page_frag with both GFP_ATOMIC and with > > GFP_KERNEL? If yes why? > > I believe it is safe to use the same page_frag and I

Fix theoretical races related to refill work: 1. After napi is disabled by ndo_stop, refill work can run and re-enable it. 2. Refill can reschedule itself, if this happens it can run after cancel_delayed_work_sync, and will access device after it is destroyed. As a solution, add flags to track napi state and to disable refill, and toggle them on start, stop and remove; check these flags

Fix theoretical races related to refill work: 1. After napi is disabled by ndo_stop, refill work can run and re-enable it. 2. Refill can reschedule itself, if this happens it can run after cancel_delayed_work_sync, and will access device after it is destroyed. As a solution, add flags to track napi state and to disable refill, and toggle them on start, stop and remove; check these flags

Commit 55257d72bd1c51f25106350f4983ec19f62ed1fa (virtio-net: fill only rx queues which are being used) tries to refill on demand when changing the number of channels by call try_refill_recv() directly, this may race: - the refill work who may do the refill in the same time - the try_refill_recv() called in bh since napi was not disabled Which may led guest complain during setting channels:

Commit 55257d72bd1c51f25106350f4983ec19f62ed1fa (virtio-net: fill only rx queues which are being used) tries to refill on demand when changing the number of channels by call try_refill_recv() directly, this may race: - the refill work who may do the refill in the same time - the try_refill_recv() called in bh since napi was not disabled Which may led guest complain during setting channels:

On 08/27/2014 09:45 AM, Hengjinxiao wrote: > Selftest is an important part of network driver, this patch adds selftest for > virtio-net, including loopback test, negotiate test and reset test. Loopback > test checks whether virtio-net can send and receive packets normally. Negotiate test > executes feature negotiation between virtio-net driver in Guest OS and virtio-net > device

To even the latency, refill buffer right after being used. Sign-off-by: Shirley Ma <xma at us.ibm.com> --- diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 0c7321c..c8201d4 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -429,6 +429,22 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi, gfp_t gfp) return err; } +static bool

To even the latency, refill buffer right after being used. Sign-off-by: Shirley Ma <xma at us.ibm.com> --- diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 0c7321c..c8201d4 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -429,6 +429,22 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi, gfp_t gfp) return err; } +static bool

Fix theoretical races related to refill work: 1. After napi is disabled by ndo_stop, refill work can run and re-enable it. 2. Refill can get scheduled on many cpus in parallel. if this happens it will corrupt the vq linked list as there's no locking 3. refill work is cancelled after unregister netdev For small bufs this means it can alloc an skb for a device which is

Fix theoretical races related to refill work: 1. After napi is disabled by ndo_stop, refill work can run and re-enable it. 2. Refill can get scheduled on many cpus in parallel. if this happens it will corrupt the vq linked list as there's no locking 3. refill work is cancelled after unregister netdev For small bufs this means it can alloc an skb for a device which is

Selftest is an important part of network driver, this patch adds selftest for virtio-net, including loopback test, negotiate test and reset test. Loopback test checks whether virtio-net can send and receive packets normally. Negotiate test executes feature negotiation between virtio-net driver in Guest OS and virtio-net device in Host OS. Reset test resets virtio-net. Signed-off-by: Hengjinxiao

Selftest is an important part of network driver, this patch adds selftest for virtio-net, including loopback test, negotiate test and reset test. Loopback test checks whether virtio-net can send and receive packets normally. Negotiate test executes feature negotiation between virtio-net driver in Guest OS and virtio-net device in Host OS. Reset test resets virtio-net. Signed-off-by: Hengjinxiao

Too many places poke at [rs]q->vq->vdev->priv just to get the the vi structure. Let's just pass the pointer around: seems cleaner, and might even be faster. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- drivers/net/virtio_net.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/drivers/net/virtio_net.c

Too many places poke at [rs]q->vq->vdev->priv just to get the the vi structure. Let's just pass the pointer around: seems cleaner, and might even be faster. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- drivers/net/virtio_net.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/drivers/net/virtio_net.c

Too many places poke at [rs]q->vq->vdev->priv just to get the vi structure. Let's just pass the pointer around: seems cleaner, and might even be faster. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> Reviewed-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- drivers/net/virtio_net.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+),

Too many places poke at [rs]q->vq->vdev->priv just to get the vi structure. Let's just pass the pointer around: seems cleaner, and might even be faster. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> Reviewed-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- drivers/net/virtio_net.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+),

Too many places poke at [rs]q->vq->vdev->priv just to get the vi structure. Let's just pass the pointer around: seems cleaner, and might even be faster. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> Reviewed-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- drivers/net/virtio_net.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+),