Message ID | A6A1774AFD79E346AE6D49A33CB294530DC19FC4@EX-BE-017-SFO.shared.themessagecenter.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
From: "Ben Menchaca (ben@bigfootnetworks.com)" <ben@bigfootnetworks.com> Date: Wed, 24 Mar 2010 08:05:02 -0700 > From: Ben Menchaca <ben@bigfootnetworks.com> > > Fix undo of reserve() before RX recycle > > gfar_new_skb reserve()s space in the SKB to align it. If an error occurs, > and the skb needs to be returned to the RX recycle queue, the current code > attempts to reset head, but did not reset tail. This patch remembers the > alignment amount, and reverses the reserve() when needed. > > Signed-off-by: Ben Menchaca <ben@bigfootnetworks.com> So, is this tested at least a little bit? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> So, is this tested at least a little bit?
We run a 2.6.31 variant on our board, and this patch fixes the skb->len issue, and we are not observing any strangeness under the stress conditions after this patch, and our asserts are no longer firing. 2.6.31 was pre-multiq for gianfar, though, and that was a large change...I would definitely prefer getting some modern-ish kernel time on it. If needed, I can probably move our board forward to -next for a bit.
For some history, and to see the original issue addressed, this appears to have been introduced shortly after recycling in 2.6.30, here:
commit d4a76f8a619b5d7dfd5a0f122666fee24bb3dcb9
gianfar: fix BUG under load after introduction of skb recycling
- Ben
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: "Ben Menchaca (ben@bigfootnetworks.com)" <ben@bigfootnetworks.com> Date: Wed, 24 Mar 2010 13:13:22 -0700 > I would definitely prefer getting some modern-ish kernel time on it. > If needed, I can probably move our board forward to -next for a bit. If anyone can test this on a current kernel that would be enough for me. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: "Ben Menchaca (ben@bigfootnetworks.com)" <ben@bigfootnetworks.com> Date: Wed, 24 Mar 2010 08:05:02 -0700 > From: Ben Menchaca <ben@bigfootnetworks.com> > > Fix undo of reserve() before RX recycle > > gfar_new_skb reserve()s space in the SKB to align it. If an error occurs, > and the skb needs to be returned to the RX recycle queue, the current code > attempts to reset head, but did not reset tail. This patch remembers the > alignment amount, and reverses the reserve() when needed. > > Signed-off-by: Ben Menchaca <ben@bigfootnetworks.com> Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c index b671555..669de02 100644 --- a/drivers/net/gianfar.c +++ b/drivers/net/gianfar.c @@ -2393,6 +2393,7 @@ struct sk_buff * gfar_new_skb(struct net_device *dev) * as many bytes as needed to align the data properly */ skb_reserve(skb, alignamount); + GFAR_CB(skb)->alignamount = alignamount; return skb; } @@ -2533,13 +2534,13 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit) newskb = skb; else if (skb) { /* - * We need to reset ->data to what it + * We need to un-reserve() the skb to what it * was before gfar_new_skb() re-aligned * it to an RXBUF_ALIGNMENT boundary * before we put the skb back on the * recycle list. */ - skb->data = skb->head + NET_SKB_PAD; + skb_reserve(skb, -GFAR_CB(skb)->alignamount); __skb_queue_head(&priv->rx_recycle, skb); } } else { diff --git a/drivers/net/gianfar.h b/drivers/net/gianfar.h index 3d72dc4..06044dd 100644 --- a/drivers/net/gianfar.h +++ b/drivers/net/gianfar.h @@ -566,6 +566,12 @@ struct rxfcb { u16 vlctl; /* VLAN control word */ }; +struct gianfar_skb_cb { + int alignamount; +}; + +#define GFAR_CB(skb) ((struct gianfar_skb_cb *)((skb)->cb)) + struct rmon_mib { u32 tr64; /* 0x.680 - Transmit and Receive 64-byte Frame Counter */