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)
next reply other threads:[~2009-05-29 14:16 UTC|newest]
Thread overview: 22+ 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 9:05 ` Herbert Xu
2009-06-02 13:55 ` Rusty Russell
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-14 6:45 ` Herbert Xu
2009-06-18 7:17 ` Rusty Russell
2009-06-18 7:34 ` Herbert Xu
2009-06-19 3:37 ` Rusty Russell
2009-06-19 4:36 ` Herbert Xu
2009-06-19 13:50 ` Rusty Russell
2009-06-19 14:10 ` Herbert Xu
2009-06-22 2:39 ` Rusty Russell
2009-06-22 5:46 ` Krishna Kumar2
2009-06-22 7:34 ` Herbert Xu
2009-06-22 13:41 ` Krishna Kumar2
2009-06-22 18:25 ` Matt Carlson
2009-06-23 2:54 ` Herbert Xu
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$(echo .)com.au \
--cc=herbert@gondor$(echo .)apana.org.au \
--cc=netdev@vger$(echo .)kernel.org \
--cc=virtualization@lists$(echo .)linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox