search for: last_xmit_skb

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...

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...

...: 7bit Content-Disposition: inline Message-Id: <200807151340.00446.rusty at rustcorp.com.au> From: Mark McLoughlin <markmc at redhat.com> On Mon, 2008-05-26 at 17:42 +1000, Rusty Russell wrote: > 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. ... > diff -r 564237b31993 drivers/net/virtio_net.c > --- a/driv...

...: 7bit Content-Disposition: inline Message-Id: <200807151340.00446.rusty at rustcorp.com.au> From: Mark McLoughlin <markmc at redhat.com> On Mon, 2008-05-26 at 17:42 +1000, Rusty Russell wrote: > 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. ... > diff -r 564237b31993 drivers/net/virtio_net.c > --- a/driv...

...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 @@ -47,9 +47,6 @@ struct virtnet_info struct napi_struct napi; unsigned int status; - /* The skb we couldn't send because buffers were full. */ - struct sk_buff *last_xmit_skb; - /* If we need to free in a timer, this is it. */ struct timer_list xmit_free_timer; @@ -116,9 +113,8 @@ static void skb_xmit_done(struct virtque /* We were probably waiting for more output buffers. */ netif_wake_queue(vi->dev); - /* Make sure we re-xmit last_xmit_skb: if there are...

...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 @@ -47,9 +47,6 @@ struct virtnet_info struct napi_struct napi; unsigned int status; - /* The skb we couldn't send because buffers were full. */ - struct sk_buff *last_xmit_skb; - /* If we need to free in a timer, this is it. */ struct timer_list xmit_free_timer; @@ -116,9 +113,8 @@ static void skb_xmit_done(struct virtque /* We were probably waiting for more output buffers. */ netif_wake_queue(vi->dev); - /* Make sure we re-xmit last_xmit_skb: if there are...

...;free_in_tasklet) + if (err >= 0 && !vi->free_in_tasklet) mod_timer(&vi->xmit_free_timer, jiffies + (HZ/10)); return err; @@ -538,7 +538,7 @@ static void xmit_tasklet(unsigned long d struct virtnet_info *vi = (void *)data; netif_tx_lock_bh(vi->dev); - if (vi->last_xmit_skb && xmit_skb(vi, vi->last_xmit_skb) == 0) { + if (vi->last_xmit_skb && xmit_skb(vi, vi->last_xmit_skb) >= 0) { vi->svq->vq_ops->kick(vi->svq); vi->last_xmit_skb = NULL; } @@ -557,7 +557,7 @@ again: /* If we has a buffer left over from last time,...

...;free_in_tasklet) + if (err >= 0 && !vi->free_in_tasklet) mod_timer(&vi->xmit_free_timer, jiffies + (HZ/10)); return err; @@ -538,7 +538,7 @@ static void xmit_tasklet(unsigned long d struct virtnet_info *vi = (void *)data; netif_tx_lock_bh(vi->dev); - if (vi->last_xmit_skb && xmit_skb(vi, vi->last_xmit_skb) == 0) { + if (vi->last_xmit_skb && xmit_skb(vi, vi->last_xmit_skb) >= 0) { vi->svq->vq_ops->kick(vi->svq); vi->last_xmit_skb = NULL; } @@ -557,7 +557,7 @@ again: /* If we has a buffer left over from last time,...

...Thu Apr 17 05:03:18 2008 +1000 +++ b/drivers/net/virtio_net.c Thu Apr 17 06:15:40 2008 +1000 @@ -40,6 +40,9 @@ struct virtnet_info struct virtqueue *rvq, *svq; struct net_device *dev; struct napi_struct napi; + + /* The skb we couldn't send because buffers were full. */ + struct sk_buff *last_xmit_skb; /* Number of input buffers, and max we've ever had. */ unsigned int num, max; @@ -227,10 +230,9 @@ static void free_old_xmit_skbs(struct vi } } -static int start_xmit(struct sk_buff *skb, struct net_device *dev) +static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb) { -...

...Thu Apr 17 05:03:18 2008 +1000 +++ b/drivers/net/virtio_net.c Thu Apr 17 06:15:40 2008 +1000 @@ -40,6 +40,9 @@ struct virtnet_info struct virtqueue *rvq, *svq; struct net_device *dev; struct napi_struct napi; + + /* The skb we couldn't send because buffers were full. */ + struct sk_buff *last_xmit_skb; /* Number of input buffers, and max we've ever had. */ unsigned int num, max; @@ -227,10 +230,9 @@ static void free_old_xmit_skbs(struct vi } } -static int start_xmit(struct sk_buff *skb, struct net_device *dev) +static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb) { -...

...Tue Apr 22 13:03:07 2008 +1000 +++ b/drivers/net/virtio_net.c Thu Apr 24 12:05:24 2008 +1000 @@ -40,6 +40,9 @@ struct virtnet_info struct virtqueue *rvq, *svq; struct net_device *dev; struct napi_struct napi; + + /* The skb we couldn't send because buffers were full. */ + struct sk_buff *last_xmit_skb; /* Number of input buffers, and max we've ever had. */ unsigned int num, max; @@ -227,17 +230,16 @@ static void free_old_xmit_skbs(struct vi } } -static int start_xmit(struct sk_buff *skb, struct net_device *dev) +static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb) {...

...Tue Apr 22 13:03:07 2008 +1000 +++ b/drivers/net/virtio_net.c Thu Apr 24 12:05:24 2008 +1000 @@ -40,6 +40,9 @@ struct virtnet_info struct virtqueue *rvq, *svq; struct net_device *dev; struct napi_struct napi; + + /* The skb we couldn't send because buffers were full. */ + struct sk_buff *last_xmit_skb; /* Number of input buffers, and max we've ever had. */ unsigned int num, max; @@ -227,17 +230,16 @@ static void free_old_xmit_skbs(struct vi } } -static int start_xmit(struct sk_buff *skb, struct net_device *dev) +static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb) {...

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

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