From: Eric Dumazet <eric.dumazet@gmail.com>
To: Dmitry Kravkov <dmitry@broadcom.com>, David Miller <davem@davemloft.net>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: [PATCH] net: fix a race in gro_cell_poll()
Date: Mon, 10 Dec 2012 14:32:03 -0800 [thread overview]
Message-ID: <1355178723.27891.85.camel@edumazet-glaptop> (raw)
In-Reply-To: <1355176551.27891.57.camel@edumazet-glaptop>
From: Eric Dumazet <edumazet@google.com>
Dmitry Kravkov reported packet drops for GRE packets since GRO support
was added.
There is a race in gro_cell_poll() because we call napi_complete()
without any synchronization with a concurrent gro_cells_receive()
Once bug was triggered, we queued packets but did not schedule NAPI
poll.
We can fix this issue using the spinlock protected the napi_skbs queue,
as we have to hold it to perform skb dequeue anyway.
As we open-code skb_dequeue(), we no longer need to mask IRQS, as both
producer and consumer run under BH context.
Bug added in commit c9e6bc644e (net: add gro_cells infrastructure)
Reported-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
David: I could reproduce the bug Dmitry reported, and have
verified this patch fixes the issue.
include/net/gro_cells.h | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/include/net/gro_cells.h b/include/net/gro_cells.h
index 4fd8a4b..e5062c9 100644
--- a/include/net/gro_cells.h
+++ b/include/net/gro_cells.h
@@ -17,7 +17,6 @@ struct gro_cells {
static inline void gro_cells_receive(struct gro_cells *gcells, struct sk_buff *skb)
{
- unsigned long flags;
struct gro_cell *cell = gcells->cells;
struct net_device *dev = skb->dev;
@@ -35,32 +34,37 @@ static inline void gro_cells_receive(struct gro_cells *gcells, struct sk_buff *s
return;
}
- spin_lock_irqsave(&cell->napi_skbs.lock, flags);
+ /* We run in BH context */
+ spin_lock(&cell->napi_skbs.lock);
__skb_queue_tail(&cell->napi_skbs, skb);
if (skb_queue_len(&cell->napi_skbs) == 1)
napi_schedule(&cell->napi);
- spin_unlock_irqrestore(&cell->napi_skbs.lock, flags);
+ spin_unlock(&cell->napi_skbs.lock);
}
+/* called unser BH context */
static inline int gro_cell_poll(struct napi_struct *napi, int budget)
{
struct gro_cell *cell = container_of(napi, struct gro_cell, napi);
struct sk_buff *skb;
int work_done = 0;
+ spin_lock(&cell->napi_skbs.lock);
while (work_done < budget) {
- skb = skb_dequeue(&cell->napi_skbs);
+ skb = __skb_dequeue(&cell->napi_skbs);
if (!skb)
break;
-
+ spin_unlock(&cell->napi_skbs.lock);
napi_gro_receive(napi, skb);
work_done++;
+ spin_lock(&cell->napi_skbs.lock);
}
if (work_done < budget)
napi_complete(napi);
+ spin_unlock(&cell->napi_skbs.lock);
return work_done;
}
next prev parent reply other threads:[~2012-12-10 22:32 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-08 22:35 ipgre rss is broken since gro Dmitry Kravkov
2012-12-08 23:31 ` Dmitry Kravkov
2012-12-09 2:01 ` Eric Dumazet
2012-12-09 2:04 ` Dmitry Kravkov
2012-12-09 20:49 ` Dmitry Kravkov
2012-12-09 23:27 ` Eric Dumazet
2012-12-10 11:32 ` Dmitry Kravkov
2012-12-10 15:17 ` Eric Dumazet
2012-12-10 16:54 ` Eric Dumazet
2012-12-10 19:02 ` David Miller
2012-12-10 19:20 ` Dmitry Kravkov
2012-12-10 21:55 ` Eric Dumazet
2012-12-10 22:32 ` Eric Dumazet [this message]
2012-12-10 22:46 ` [PATCH] net: fix a race in gro_cell_poll() Dmitry Kravkov
2012-12-11 17:50 ` David Miller
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=1355178723.27891.85.camel@edumazet-glaptop \
--to=eric.dumazet@gmail.com \
--cc=davem@davemloft.net \
--cc=dmitry@broadcom.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox