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]

Message-ID: <5033C6B0.4060508@xdin.com>
Date:	Tue, 21 Aug 2012 17:34:40 +0000
From:	Arvid Brodin <Arvid.Brodin@...n.com>
To:	Nicolas Ferre <nicolas.ferre@...el.com>
CC:	Ben Hutchings <bhutchings@...arflare.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: Do I need to skb_put() Ethernet frames to a minimum of 60 bytes?

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?)


-- 
Arvid Brodin | Consultant (Linux)
XDIN AB | Jan Stenbecks Torg 17 | SE-164 40 Kista | Sweden | xdin.com