From: nicolas.ferre@atmel.com (Nicolas Ferre)
To: linux-arm-kernel@lists.infradead.org
Subject: Do I need to skb_put() Ethernet frames to a minimum of 60 bytes?
Date: Mon, 17 Dec 2012 14:43:11 +0100 [thread overview]
Message-ID: <50CF216F.2010107@atmel.com> (raw)
In-Reply-To: <5033C6B0.4060508@xdin.com>
On 08/21/2012 07:34 PM, Arvid Brodin :
> On 2012-08-14 22:35, Ben Hutchings wrote:
>> On Tue, 2012-08-14 at 18:53 +0000, Arvid Brodin wrote:
>>> Hi,
>>>
>>> If I create an sk_buff with a payload of less than 28 bytes (ethheader + data),
>>> and send it using the cadence/macb (Ethernet) driver, I get
>>>
>>> eth0: TX underrun, resetting buffers
>>>
>>> Now I know the minimum Ethernet frame size is 64 bytes (including the 4-byte
>>> FCS), but whose responsibility is it to pad the frame to this size if necessary?
>>> Mine or the driver's - i.e. should I just skb_put() to the minimum size or
>>> should I report the underrun as a driver bug?
>>
>> If the hardware doesn't pad frames automatically then it's the driver's
>> reponsibility to do so.
>>
>
> Nicolas, can you take a look at this? At the moment I'm using the following change
> in macb.c to avoid TX underruns on short packages:
>
> --- a/drivers/net/ethernet/cadence/macb.c 2012-05-04 19:14:41.927719667 +0200
> +++ b/drivers/net/ethernet/cadence/macb.c 2012-08-21 19:22:40.063739049 +0200
> @@ -618,6 +618,7 @@ static void macb_poll_controller(struct
> }
> #endif
>
> +#define MIN_ETHFRAME_LEN 60
> static int macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
> {
> struct macb *bp = netdev_priv(dev);
> @@ -635,6 +636,12 @@ static int macb_start_xmit(struct sk_buf
> printk("\n");
> #endif
>
> + if (skb->len < MIN_ETHFRAME_LEN) {
> + /* Pad skb to minium Ethernet frame size */
> + if (skb_tailroom(skb) >= MIN_ETHFRAME_LEN - skb->len)
> + memset(skb_put(skb, MIN_ETHFRAME_LEN - skb->len), 0,
> + MIN_ETHFRAME_LEN - skb->len);
> + }
> len = skb->len;
> spin_lock_irqsave(&bp->lock, flags);
>
>
> ... but as you can see this is limited to linear skbs which has been allocated with
> enough tailroom. Perhaps there are better ways to fix the problem? (Maybe the hardware
> is actually doing the padding already and the problem has to do with the way the DMA
> transfer is set up?)
I come back to this issue. It seems to me that the macb Cadence IP is
padding automatically a too little packet. It is the usual behavior
unless you specify otherwise in the CTRL register embedded in the tx
descriptor. I also verified this with wireshark on both ICMP and UDP
packets.
The error that you are experiencing is on at91sam9260 or at91sam9263
SoCs, am I right?
Best regards,
--
Nicolas Ferre
WARNING: multiple messages have this Message-ID (diff)
From: Nicolas Ferre <nicolas.ferre@atmel.com>
To: Arvid Brodin <Arvid.Brodin@xdin.com>
Cc: Ben Hutchings <bhutchings@solarflare.com>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
Eric Dumazet <eric.dumazet@gmail.com>,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: Do I need to skb_put() Ethernet frames to a minimum of 60 bytes?
Date: Mon, 17 Dec 2012 14:43:11 +0100 [thread overview]
Message-ID: <50CF216F.2010107@atmel.com> (raw)
In-Reply-To: <5033C6B0.4060508@xdin.com>
On 08/21/2012 07:34 PM, Arvid Brodin :
> On 2012-08-14 22:35, Ben Hutchings wrote:
>> On Tue, 2012-08-14 at 18:53 +0000, Arvid Brodin wrote:
>>> Hi,
>>>
>>> If I create an sk_buff with a payload of less than 28 bytes (ethheader + data),
>>> and send it using the cadence/macb (Ethernet) driver, I get
>>>
>>> eth0: TX underrun, resetting buffers
>>>
>>> Now I know the minimum Ethernet frame size is 64 bytes (including the 4-byte
>>> FCS), but whose responsibility is it to pad the frame to this size if necessary?
>>> Mine or the driver's - i.e. should I just skb_put() to the minimum size or
>>> should I report the underrun as a driver bug?
>>
>> If the hardware doesn't pad frames automatically then it's the driver's
>> reponsibility to do so.
>>
>
> Nicolas, can you take a look at this? At the moment I'm using the following change
> in macb.c to avoid TX underruns on short packages:
>
> --- a/drivers/net/ethernet/cadence/macb.c 2012-05-04 19:14:41.927719667 +0200
> +++ b/drivers/net/ethernet/cadence/macb.c 2012-08-21 19:22:40.063739049 +0200
> @@ -618,6 +618,7 @@ static void macb_poll_controller(struct
> }
> #endif
>
> +#define MIN_ETHFRAME_LEN 60
> static int macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
> {
> struct macb *bp = netdev_priv(dev);
> @@ -635,6 +636,12 @@ static int macb_start_xmit(struct sk_buf
> printk("\n");
> #endif
>
> + if (skb->len < MIN_ETHFRAME_LEN) {
> + /* Pad skb to minium Ethernet frame size */
> + if (skb_tailroom(skb) >= MIN_ETHFRAME_LEN - skb->len)
> + memset(skb_put(skb, MIN_ETHFRAME_LEN - skb->len), 0,
> + MIN_ETHFRAME_LEN - skb->len);
> + }
> len = skb->len;
> spin_lock_irqsave(&bp->lock, flags);
>
>
> ... but as you can see this is limited to linear skbs which has been allocated with
> enough tailroom. Perhaps there are better ways to fix the problem? (Maybe the hardware
> is actually doing the padding already and the problem has to do with the way the DMA
> transfer is set up?)
I come back to this issue. It seems to me that the macb Cadence IP is
padding automatically a too little packet. It is the usual behavior
unless you specify otherwise in the CTRL register embedded in the tx
descriptor. I also verified this with wireshark on both ICMP and UDP
packets.
The error that you are experiencing is on at91sam9260 or at91sam9263
SoCs, am I right?
Best regards,
--
Nicolas Ferre
next prev parent reply other threads:[~2012-12-17 13:43 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-14 18:53 Do I need to skb_put() Ethernet frames to a minimum of 60 bytes? Arvid Brodin
2012-08-14 20:35 ` Ben Hutchings
2012-08-21 17:34 ` Arvid Brodin
2012-08-21 17:42 ` Eric Dumazet
2012-08-21 18:07 ` Ben Hutchings
2012-12-17 13:43 ` Nicolas Ferre [this message]
2012-12-17 13:43 ` Nicolas Ferre
2012-12-17 15:15 ` Arvid Brodin
2012-12-17 15:15 ` Arvid Brodin
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=50CF216F.2010107@atmel.com \
--to=nicolas.ferre@atmel.com \
--cc=linux-arm-kernel@lists.infradead.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.