netdev - Re: Do I need to skb_put() Ethernet frames to a minimum of 60 bytes?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 17 Dec 2012 14:43:11 +0100
From:	Nicolas Ferre <nicolas.ferre@...el.com>
To:	Arvid Brodin <Arvid.Brodin@...n.com>
CC:	Ben Hutchings <bhutchings@...arflare.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Eric Dumazet <eric.dumazet@...il.com>,
	linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: Do I need to skb_put() Ethernet frames to a minimum of 60 bytes?

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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html