All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rusty Russell <rusty@rustcorp.com.au>
To: netdev@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org,
	Herbert Xu <herbert@gondor.apana.org.au>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Subject: [PATCH 2/4] virtio_net: return NETDEV_TX_BUSY instead of queueing an extra skb.
Date: Fri, 29 May 2009 23:46:04 +0930	[thread overview]
Message-ID: <200905292346.04815.rusty@rustcorp.com.au> (raw)


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 common and not going away any time
soon.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
---
 drivers/net/virtio_net.c |   49 ++++++++++-------------------------------------
 1 file changed, 11 insertions(+), 38 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
@@ -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 no more packets
-	 * queued, start_xmit won't be called. */
-	tasklet_schedule(&vi->tasklet);
+	if (vi->free_in_tasklet)
+		tasklet_schedule(&vi->tasklet);
 }
 
 static void receive_skb(struct net_device *dev, struct sk_buff *skb,
@@ -509,12 +505,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) {
-		vi->svq->vq_ops->kick(vi->svq);
-		vi->last_xmit_skb = NULL;
-	}
-	if (vi->free_in_tasklet)
-		free_old_xmit_skbs(vi);
+	free_old_xmit_skbs(vi);
 	netif_tx_unlock_bh(vi->dev);
 }
 
@@ -526,27 +517,14 @@ again:
 	/* Free up any pending old buffers before queueing new ones. */
 	free_old_xmit_skbs(vi);
 
-	/* If we has a buffer left over from last time, send it now. */
-	if (unlikely(vi->last_xmit_skb) &&
-	    xmit_skb(vi, vi->last_xmit_skb) != 0)
-		goto stop_queue;
+	/* Put new one in send queue and do transmit */
+	__skb_queue_head(&vi->send, skb);
+	if (likely(xmit_skb(vi, skb) == 0)) {
+		vi->svq->vq_ops->kick(vi->svq);
+		return NETDEV_TX_OK;
+	}
 
-	vi->last_xmit_skb = NULL;
-
-	/* Put new one in send queue and do transmit */
-	if (likely(skb)) {
-		__skb_queue_head(&vi->send, skb);
-		if (xmit_skb(vi, skb) != 0) {
-			vi->last_xmit_skb = skb;
-			skb = NULL;
-			goto stop_queue;
-		}
-	}
-done:
-	vi->svq->vq_ops->kick(vi->svq);
-	return NETDEV_TX_OK;
-
-stop_queue:
+	/* Ring too full for this packet. */
 	pr_debug("%s: virtio not prepared to send\n", dev->name);
 	netif_stop_queue(dev);
 
@@ -557,12 +535,7 @@ stop_queue:
 		netif_start_queue(dev);
 		goto again;
 	}
-	if (skb) {
-		/* Drop this skb: we only queue one. */
-		vi->dev->stats.tx_dropped++;
-		kfree_skb(skb);
-	}
-	goto done;
+	return NETDEV_TX_BUSY;
 }
 
 static int virtnet_set_mac_address(struct net_device *dev, void *p)



             reply	other threads:[~2009-05-29 14:16 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-29 14:16 Rusty Russell [this message]
2009-06-02  8:07 ` [PATCH 2/4] virtio_net: return NETDEV_TX_BUSY instead of queueing an extra skb Mark McLoughlin
2009-06-02 14:04   ` Rusty Russell
2009-06-02 14:04   ` Rusty Russell
2009-06-02  8:07 ` Mark McLoughlin
2009-06-02  9:05 ` Herbert Xu
2009-06-02  9:05 ` Herbert Xu
2009-06-02 13:55   ` Rusty Russell
2009-06-02 23:45     ` Herbert Xu
2009-06-02 23:45     ` Herbert Xu
2009-06-03  3:17       ` Rusty Russell
2009-06-08  5:22         ` Herbert Xu
2009-06-13 12:30           ` Rusty Russell
2009-06-13 12:30           ` Rusty Russell
2009-06-14  6:45             ` Herbert Xu
2009-06-18  7:17               ` Rusty Russell
2009-06-18  7:17               ` Rusty Russell
2009-06-18  7:34                 ` Herbert Xu
2009-06-18  7:34                 ` Herbert Xu
2009-06-19  3:37                   ` Rusty Russell
2009-06-19  4:36                     ` Herbert Xu
2009-06-19  4:36                     ` Herbert Xu
2009-06-19 13:50                       ` Rusty Russell
2009-06-19 14:10                         ` Herbert Xu
2009-06-19 14:10                         ` Herbert Xu
2009-06-22  2:39                           ` Rusty Russell
2009-06-22  2:39                           ` Rusty Russell
2009-06-19 13:50                       ` Rusty Russell
2009-06-22  5:46                       ` Krishna Kumar2
2009-06-22  5:46                       ` Krishna Kumar2
2009-06-22  7:34                         ` Herbert Xu
2009-06-22  7:34                         ` Herbert Xu
2009-06-22 13:41                           ` Krishna Kumar2
2009-06-22 13:41                           ` Krishna Kumar2
2009-06-22 18:25                           ` Matt Carlson
2009-06-22 18:25                           ` Matt Carlson
2009-06-23  2:54                             ` Herbert Xu
2009-06-23  2:54                             ` Herbert Xu
2009-06-19  3:37                   ` Rusty Russell
2009-06-14  6:45             ` Herbert Xu
2009-06-08  5:22         ` Herbert Xu
2009-06-03  3:17       ` Rusty Russell
2009-06-02 13:55   ` Rusty Russell
  -- strict thread matches above, loose matches on Subject: below --
2009-05-29 14:16 Rusty Russell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200905292346.04815.rusty@rustcorp.com.au \
    --to=rusty@rustcorp.com.au \
    --cc=herbert@gondor.apana.org.au \
    --cc=netdev@vger.kernel.org \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.