All of lore.kernel.org
 help / color / mirror / Atom feed
From: DDD <Dongdong.deng@windriver.com>
To: Matt Mackall <mpm@selenic.com>, David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Subject: [PATCH 1/1] netpoll: fix race between skb_queue_len and skb_dequeue
Date: Tue, 08 Sep 2009 15:49:49 +0800	[thread overview]
Message-ID: <1252396189.16528.19.camel@dengdd-desktop> (raw)

This race will break the messages order.

Sequence of events in the problem case:

Assume there is one skb "A" in skb_queue and the next action of
netpoll_send_skb() is: sending out "B" skb.
The right order of messages should be: send A first, then send B.

But as following orders, it will send B first, then send A.

  CPU0                             CPU1
  queue_process()                  netpoll_send_skb(B)
  A = skb_dequeue()
  /****
  A was removed from skb_queue,
  so skb_queue_len = 0
  ****/

                                   Check skb_queue_len == 0 Yes
                                   sendout (B)

  sendout (A)

The solution is to introduce the txq_skbuff_len in netpoll_info struct.
The txq_skbuff_len will keep the skb queue lenght utill the skb was sent out,
so that this race condition will gone.

Signed-off-by: Dongdong Deng <dongdong.deng@windriver.com>
---
 include/linux/netpoll.h |    1 +
 net/core/netpoll.c      |    8 ++++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index 2524267..3757141 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -27,6 +27,7 @@ struct netpoll_info {
 	atomic_t refcnt;
 	int rx_flags;
 	spinlock_t rx_lock;
+	__u32 txq_skbuff_len;
 	struct netpoll *rx_np; /* netpoll that registered an rx_hook */
 	struct sk_buff_head arp_tx; /* list of arp requests to reply to */
 	struct sk_buff_head txq;
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 1b76eb1..6d22426 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -78,11 +78,14 @@ static void queue_process(struct work_struct *work)
 			__netif_tx_unlock(txq);
 			local_irq_restore(flags);
 
+			npinfo->txq_skbuff_len = skb_queue_len(&npinfo->txq);
 			schedule_delayed_work(&npinfo->tx_work, HZ/10);
 			return;
 		}
 		__netif_tx_unlock(txq);
 		local_irq_restore(flags);
+
+		npinfo->txq_skbuff_len = skb_queue_len(&npinfo->txq);
 	}
 }
 
@@ -291,7 +294,7 @@ static void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
 	}
 
 	/* don't get messages out of order, and no recursion */
-	if (skb_queue_len(&npinfo->txq) == 0 && !netpoll_owner_active(dev)) {
+	if (npinfo->txq_skbuff_len == 0 && !netpoll_owner_active(dev)) {
 		struct netdev_queue *txq;
 		unsigned long flags;
 
@@ -329,7 +332,8 @@ static void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
 
 	if (status != NETDEV_TX_OK) {
 		skb_queue_tail(&npinfo->txq, skb);
-		schedule_delayed_work(&npinfo->tx_work,0);
+		npinfo->txq_skbuff_len = skb_queue_len(&npinfo->txq);
+		schedule_delayed_work(&npinfo->tx_work, 0);
 	}
 }
 
-- 
1.6.0.4



             reply	other threads:[~2009-09-08  7:36 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-08  7:49 DDD [this message]
2009-09-08 17:27 ` [PATCH 1/1] netpoll: fix race between skb_queue_len and skb_dequeue Matt Mackall
2009-09-09 14:27   ` DDD

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=1252396189.16528.19.camel@dengdd-desktop \
    --to=dongdong.deng@windriver.com \
    --cc=davem@davemloft.net \
    --cc=mpm@selenic.com \
    --cc=netdev@vger.kernel.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.