All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ben Menchaca (ben@bigfootnetworks.com)" <ben@bigfootnetworks.com>
To: "netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Gianfar: RX Recycle skb->len error
Date: Sat, 20 Mar 2010 12:54:59 -0700	[thread overview]
Message-ID: <A6A1774AFD79E346AE6D49A33CB294530DC19EB5@EX-BE-017-SFO.shared.themessagecenter.com> (raw)

We are seeing some random skb data length errors on RX after long-running, full-gigabit traffic.  First, my debugging and solution are based on the following invariant assumption:
(skb->tail - skb->data) == skb->len

If this is wrong, please educate.

After some tracing, here is where the error packets seem to originate:
1.  We are cleaning rx, in gfar_clean_rx_ring;
2.  A new RX skb is drawn from the rx_recycle queue, and obey the above invariant (so, in gfar_new_skb(), __skb_dequeue returns an skb);
3.  At this point skb_reserve is called, which moves data and tail by the same calculated alignamount;
4.  So, newskb is not NULL.  However, !(bdp->status & RXBD_LAST) || (bdp->status & RXBD_ERR)) is evaluates to true;
5.  Since newskb is not NULL, we arrive at the else if (skb), which is true;
6.  skb->data = skb->head + NET_SKB_PAD is applied, and then the skb is requeued for recycling.

At this point, skb->data != skb->tail, but skb->len == 0.  When this skb is used for the next RX, it is causing issues later when we skb_put trailers, and then trust skb->len.

I would propose something like:

--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -2540,6 +2540,7 @@ 
				 * recycle list.
 				 */
 				skb->data = skb->head + NET_SKB_PAD;
+				skb_reset_tail_pointer(skb);
				__skb_queue_head(&priv->rx_recycle, skb);
			}
		} else {

Ben Menchaca
Bigfoot Networks


             reply	other threads:[~2010-03-20 20:00 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-20 19:54 Ben Menchaca (ben@bigfootnetworks.com) [this message]
2010-03-22  4:46 ` Gianfar: RX Recycle skb->len error David Miller
2010-03-22 17:24   ` Anton Vorontsov
2010-03-22 21:10     ` Ben Menchaca (ben@bigfootnetworks.com)
2010-03-23  3:30       ` David Miller
2010-03-23 14:16         ` Ben Menchaca (ben@bigfootnetworks.com)
2010-03-23 20:00           ` 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=A6A1774AFD79E346AE6D49A33CB294530DC19EB5@EX-BE-017-SFO.shared.themessagecenter.com \
    --to=ben@bigfootnetworks.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.