From patchwork Tue Aug 21 17:34:40 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arvid Brodin X-Patchwork-Id: 179119 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 952FF2C0097 for ; Wed, 22 Aug 2012 03:35:14 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758223Ab2HUReu (ORCPT ); Tue, 21 Aug 2012 13:34:50 -0400 Received: from spam1.webland.se ([91.207.112.90]:56398 "EHLO spam1.webland.se" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758045Ab2HURes (ORCPT ); Tue, 21 Aug 2012 13:34:48 -0400 From: Arvid Brodin To: Nicolas Ferre CC: Ben Hutchings , "netdev@vger.kernel.org" Subject: Re: Do I need to skb_put() Ethernet frames to a minimum of 60 bytes? Thread-Topic: Do I need to skb_put() Ethernet frames to a minimum of 60 bytes? Thread-Index: AQHNek4oiC2yFw3KPESf/HhJCGRQ4ZdZoq2AgArNqwA= Date: Tue, 21 Aug 2012 17:34:40 +0000 Message-ID: <5033C6B0.4060508@xdin.com> References: <502A9EC4.4040208@xdin.com> <1344976557.2690.43.camel@bwh-desktop.uk.solarflarecom.com> In-Reply-To: <1344976557.2690.43.camel@bwh-desktop.uk.solarflarecom.com> Accept-Language: sv-SE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: user-agent: Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20120410 Thunderbird/8.0 Content-ID: <6FF128292BB75C45974C4EC33038430C@redbull.xdin.com> MIME-Version: 1.0 Received-SPF: none Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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: ... 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 --- 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);