similar to: [PATCH 3/4] virtio_net: don't free buffers in xmit ring

This effectively reverts 99ffc696d10b28580fe93441d627cf290ac4484c "virtio: wean net driver off NETDEV_TX_BUSY". The complexity of queuing an skb (setting a tasklet to re-xmit) is questionable, especially once we get rid of the other reason for the tasklet in the next patch. If the skb won't fit in the tx queue, just return NETDEV_TX_BUSY. It might be frowned upon, but it's

This effectively reverts 99ffc696d10b28580fe93441d627cf290ac4484c "virtio: wean net driver off NETDEV_TX_BUSY". The complexity of queuing an skb (setting a tasklet to re-xmit) is questionable, especially once we get rid of the other reason for the tasklet in the next patch. If the skb won't fit in the tx queue, just return NETDEV_TX_BUSY. It might be frowned upon, but it's

From: Mark McLoughlin <markmc at redhat.com> hdr->csum_start is the offset from the start of the ethernet header to the transport layer checksum field. skb->csum_start is the offset from skb->head. skb_partial_csum_set() assumes that skb->data points to the ethernet header - i.e. it computes skb->csum_start by adding the headroom to hdr->csum_start. Since

This API change means that virtio_net can tell how much capacity remains for buffers. It's necessarily fuzzy, since VIRTIO_RING_F_INDIRECT_DESC means we can fit any number of descriptors in one, *if* we can kmalloc. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> Cc: Dinesh Subhraveti <dineshs at us.ibm.com> --- drivers/block/virtio_blk.c | 2 +-

This API change means that virtio_net can tell how much capacity remains for buffers. It's necessarily fuzzy, since VIRTIO_RING_F_INDIRECT_DESC means we can fit any number of descriptors in one, *if* we can kmalloc. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> Cc: Dinesh Subhraveti <dineshs at us.ibm.com> --- drivers/block/virtio_blk.c | 2 +-

If we fail to transmit a packet, we assume the queue is full and put the skb into last_xmit_skb. However, if more space frees up before we xmit it, we loop, and the result can be transmitting the same skb twice. Fix is simple: set skb to NULL if we've used it in some way, and check before sending. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> --- drivers/net/virtio_net.c |

If we fail to transmit a packet, we assume the queue is full and put the skb into last_xmit_skb. However, if more space frees up before we xmit it, we loop, and the result can be transmitting the same skb twice. Fix is simple: set skb to NULL if we've used it in some way, and check before sending. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> --- drivers/net/virtio_net.c |

The patch virtio_net: limit xmit polling got the logic reversed: it polled while we had capacity not while ring was empty. Fix it up and clean up a bit by using a for loop. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- OK, turns out that patch was borken. Here's a fix that survived stress test on my box. Pushed on my branch, I'll send a rebased series with Rusty's

The patch virtio_net: limit xmit polling got the logic reversed: it polled while we had capacity not while ring was empty. Fix it up and clean up a bit by using a for loop. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- OK, turns out that patch was borken. Here's a fix that survived stress test on my box. Pushed on my branch, I'll send a rebased series with Rusty's

On Thursday 01 May 2008 00:31:46 Mark McLoughlin wrote: > virtio_net currently only frees old transmit skbs just > before queueing new ones. If the queue is full, it then > enables interrupts and waits for notification that more > work has been performed. Hi Mark, This patch is fine, but it's better to do it from skb_xmit_done(). Of course, this is usually called from an

On Thursday 01 May 2008 00:31:46 Mark McLoughlin wrote: > virtio_net currently only frees old transmit skbs just > before queueing new ones. If the queue is full, it then > enables interrupts and waits for notification that more > work has been performed. Hi Mark, This patch is fine, but it's better to do it from skb_xmit_done(). Of course, this is usually called from an

skb is only NULL the first time around: it's more correct to test for being under-budget. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> diff -r 2a94425ac7d5 drivers/net/virtio_net.c --- a/drivers/net/virtio_net.c Thu Nov 15 13:47:28 2007 +1100 +++ b/drivers/net/virtio_net.c Thu Nov 15 23:10:44 2007 +1100 @@ -198,8 +198,8 @@ again: if (vi->num < vi->max / 2)

skb is only NULL the first time around: it's more correct to test for being under-budget. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> diff -r 2a94425ac7d5 drivers/net/virtio_net.c --- a/drivers/net/virtio_net.c Thu Nov 15 13:47:28 2007 +1100 +++ b/drivers/net/virtio_net.c Thu Nov 15 23:10:44 2007 +1100 @@ -198,8 +198,8 @@ again: if (vi->num < vi->max / 2)

commit 763769621d271d92204ed27552d75448587c1ac0 Author: Dor Laor <dor.laor@qumranet.com> Date: Wed Dec 12 14:52:00 2007 +0200 [virtio-net][PATCH] Don't arm tx hrtimer with a constant 50us each transmit The current start_xmit sets 500us hrtimer to kick the host. The problem is that if another xmit happens before the timer was fired then the first xmit will have to

commit 763769621d271d92204ed27552d75448587c1ac0 Author: Dor Laor <dor.laor@qumranet.com> Date: Wed Dec 12 14:52:00 2007 +0200 [virtio-net][PATCH] Don't arm tx hrtimer with a constant 50us each transmit The current start_xmit sets 500us hrtimer to kick the host. The problem is that if another xmit happens before the timer was fired then the first xmit will have to

We check for finished xmit skbs on every xmit, or on a timer (unless the host promises to force an interrupt when the xmit ring is empty). This can penalize userspace tasks which fill their sockbuf. Not much difference with TSO, but measurable with large numbers of packets. There are a finite number of packets which can be in the transmission queue. We could fire the timer more than every

We check for finished xmit skbs on every xmit, or on a timer (unless the host promises to force an interrupt when the xmit ring is empty). This can penalize userspace tasks which fill their sockbuf. Not much difference with TSO, but measurable with large numbers of packets. There are a finite number of packets which can be in the transmission queue. We could fire the timer more than every

Herbert tells me that returning NETDEV_TX_BUSY from hard_start_xmit is seen as a poor thing to do; we should cache the packet and stop the queue. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> --- drivers/net/virtio_net.c | 57 +++++++++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 19 deletions(-) diff -r a936d320f6d5 drivers/net/virtio_net.c ---

Herbert tells me that returning NETDEV_TX_BUSY from hard_start_xmit is seen as a poor thing to do; we should cache the packet and stop the queue. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> --- drivers/net/virtio_net.c | 57 +++++++++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 19 deletions(-) diff -r a936d320f6d5 drivers/net/virtio_net.c ---

Herbert tells me that returning NETDEV_TX_BUSY from hard_start_xmit is seen as a poor thing to do; we should cache the packet and stop the queue. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> --- drivers/net/virtio_net.c | 58 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 18 deletions(-) diff -r 26ba2dd67f46 drivers/net/virtio_net.c ---