Commit 14c998f0 authored by Mark McLoughlin's avatar Mark McLoughlin Committed by Jeff Garzik

virtio: virtio_net free transmit skbs in a timer

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.

However, a side-effect of this scheme is that there are
always xmit skbs left dangling when no new packets are
sent, against the Documentation/networking/driver.txt
guideline:

  "... it is not allowed for your TX mitigation scheme
   to let TX packets "hang out" in the TX ring unreclaimed
   forever if no new TX packets are sent."

Add a timer to ensure that any time we queue new TX
skbs, we will shortly free them again.

This fixes an easily reproduced hang at shutdown where
iptables attempts to unload nf_conntrack and nf_conntrack
waits for an skb it is tracking to be freed, but virtio_net
never frees it.
Signed-off-by: default avatarMark McLoughlin <markmc@redhat.com>
Signed-off-by: default avatarRusty Russell <rusty@rustcorp.com.au>
Signed-off-by: default avatarJeff Garzik <jgarzik@redhat.com>
parent 2506ece0
...@@ -44,6 +44,8 @@ struct virtnet_info ...@@ -44,6 +44,8 @@ struct virtnet_info
/* The skb we couldn't send because buffers were full. */ /* The skb we couldn't send because buffers were full. */
struct sk_buff *last_xmit_skb; struct sk_buff *last_xmit_skb;
struct timer_list xmit_free_timer;
/* Number of input buffers, and max we've ever had. */ /* Number of input buffers, and max we've ever had. */
unsigned int num, max; unsigned int num, max;
...@@ -240,9 +242,23 @@ static void free_old_xmit_skbs(struct virtnet_info *vi) ...@@ -240,9 +242,23 @@ static void free_old_xmit_skbs(struct virtnet_info *vi)
} }
} }
static void xmit_free(unsigned long data)
{
struct virtnet_info *vi = (void *)data;
netif_tx_lock(vi->dev);
free_old_xmit_skbs(vi);
if (!skb_queue_empty(&vi->send))
mod_timer(&vi->xmit_free_timer, jiffies + (HZ/10));
netif_tx_unlock(vi->dev);
}
static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb) static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
{ {
int num; int num, err;
struct scatterlist sg[2+MAX_SKB_FRAGS]; struct scatterlist sg[2+MAX_SKB_FRAGS];
struct virtio_net_hdr *hdr; struct virtio_net_hdr *hdr;
const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest; const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
...@@ -285,7 +301,11 @@ static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb) ...@@ -285,7 +301,11 @@ static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
vnet_hdr_to_sg(sg, skb); vnet_hdr_to_sg(sg, skb);
num = skb_to_sgvec(skb, sg+1, 0, skb->len) + 1; num = skb_to_sgvec(skb, sg+1, 0, skb->len) + 1;
return vi->svq->vq_ops->add_buf(vi->svq, sg, num, 0, skb); err = vi->svq->vq_ops->add_buf(vi->svq, sg, num, 0, skb);
if (!err)
mod_timer(&vi->xmit_free_timer, jiffies + (HZ/10));
return err;
} }
static void xmit_tasklet(unsigned long data) static void xmit_tasklet(unsigned long data)
...@@ -456,6 +476,8 @@ static int virtnet_probe(struct virtio_device *vdev) ...@@ -456,6 +476,8 @@ static int virtnet_probe(struct virtio_device *vdev)
tasklet_init(&vi->tasklet, xmit_tasklet, (unsigned long)vi); tasklet_init(&vi->tasklet, xmit_tasklet, (unsigned long)vi);
setup_timer(&vi->xmit_free_timer, xmit_free, (unsigned long)vi);
err = register_netdev(dev); err = register_netdev(dev);
if (err) { if (err) {
pr_debug("virtio_net: registering device failed\n"); pr_debug("virtio_net: registering device failed\n");
...@@ -493,6 +515,8 @@ static void virtnet_remove(struct virtio_device *vdev) ...@@ -493,6 +515,8 @@ static void virtnet_remove(struct virtio_device *vdev)
/* Stop all the virtqueues. */ /* Stop all the virtqueues. */
vdev->config->reset(vdev); vdev->config->reset(vdev);
del_timer_sync(&vi->xmit_free_timer);
/* Free our skbs in send and recv queues, if any. */ /* Free our skbs in send and recv queues, if any. */
while ((skb = __skb_dequeue(&vi->recv)) != NULL) { while ((skb = __skb_dequeue(&vi->recv)) != NULL) {
kfree_skb(skb); kfree_skb(skb);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment