From patchwork Thu Apr 25 10:56:30 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Bader X-Patchwork-Id: 239491 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 6709C2C00D2 for ; Thu, 25 Apr 2013 20:56:52 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756375Ab3DYK4p (ORCPT ); Thu, 25 Apr 2013 06:56:45 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:38560 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754630Ab3DYK4n (ORCPT ); Thu, 25 Apr 2013 06:56:43 -0400 Received: from p5b2e49c1.dip0.t-ipconnect.de ([91.46.73.193] helo=[192.168.2.5]) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1UVJqy-0001b8-Ma; Thu, 25 Apr 2013 10:56:40 +0000 Message-ID: <51790BDE.7090500@canonical.com> Date: Thu, 25 Apr 2013 12:56:30 +0200 From: Stefan Bader User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130329 Thunderbird/17.0.5 MIME-Version: 1.0 To: hayeswang CC: 'Francois Romieu' , netdev@vger.kernel.org, 'nic_swsd' Subject: Re: rtl8168e-vl dropping tftp ack References: <516EB23B.3080504@canonical.com> <20130417213251.GA13790@electric-eye.fr.zoreil.com> <516FC218.3060002@canonical.com> <20130418215501.GB8944@electric-eye.fr.zoreil.com> <20130419061425.GA14843@electric-eye.fr.zoreil.com> <55FF74168B2A4152AD875C000535466F@realtek.com.tw> In-Reply-To: <55FF74168B2A4152AD875C000535466F@realtek.com.tw> X-Enigmail-Version: 1.4.6 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 19.04.2013 09:49, hayeswang wrote: >> I don't get it: arp aside, the normal trace in the capture >> file exhibits no >> sub-60 bytes packet. Could you reformulate ? >> > > In brief, when the packet < 60, that is skb->len < 60, the hw should pad the > packet to 60 bytes automatically. However, in my memory, the rtl8168e-vl > wouldn't do this, and the packet wouldn't be sent. Therefore, the patch would be > similar with the followings. > > --- r8169.c.org 2013-04-19 22:35:40.785759473 +0800 > +++ r8169.c 2013-04-19 22:38:24.227189535 +0800 > @@ -5760,12 +5760,29 @@ static inline void rtl8169_tso_csum(stru > } else if (skb->ip_summed == CHECKSUM_PARTIAL) { > const struct iphdr *ip = ip_hdr(skb); > > + if (unlikely(skb->len < ETH_ZLEN && > + (tp->mac_version == RTL_GIGA_MAC_VER_34))) { > + if (skb_padto(skb, ETH_ZLEN)) > + return false; > + skb_checksum_help(skb); > + skb_put(skb, ETH_ZLEN - skb->len); > + return true; > + } > + > if (ip->protocol == IPPROTO_TCP) > opts[offset] |= info->checksum.tcp; > else if (ip->protocol == IPPROTO_UDP) > opts[offset] |= info->checksum.udp; > else > WARN_ON_ONCE(1); > + } else { > + if (unlikely(skb->len < ETH_ZLEN && > + (tp->mac_version == RTL_GIGA_MAC_VER_34))) { > + if (skb_padto(skb, ETH_ZLEN)) > + return false; > + skb_put(skb, ETH_ZLEN - skb->len); > + return true; Hardware is now back and I ran a couple of tests. First, the crash I ran into previously was because I accidentally slipped in a skp_checksum_help into the else case that Hayes proposed. Sorry. So with the right version and both hunks in rtl8169_tso_csum, I see the problem go away. I do observe the else case being hit on other occasions (not only while doing pxe boots) from time to time. So that may happen more often than it causes visible problems. I never saw the checksum case being hit. Naively assuming that the checksum path never gets executed (which may be wrong) I condensed the changes to: This variant also seems to work (and does not drop the tftp packet). Whichever path looks more suitable to you, you could add my tested-by to both. We should also flag whichever change results from this as stable material as I could see this happen even in v3.2 (just did not try further back). Thanks, Stefan > + } > } > } > diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek index 4ecbe64..1519a2f 100644 --- a/drivers/net/ethernet/realtek/r8169.c +++ b/drivers/net/ethernet/realtek/r8169.c @@ -5790,12 +5790,23 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *sk if (unlikely(le32_to_cpu(txd->opts1) & DescOwn)) goto err_stop_0; + /* + * 8168E-VL hardware does not automatically pad to minimum + * length. + */ + if (unlikely(skb->len < ETH_ZLEN && + (tp->mac_version == RTL_GIGA_MAC_VER_34))) { + if (skb_padto(skb, ETH_ZLEN)) + goto err_update_stats_0; + skb_put(skb, ETH_ZLEN - skb->len); + } + len = skb_headlen(skb); mapping = dma_map_single(d, skb->data, len, DMA_TO_DEVICE); if (unlikely(dma_mapping_error(d, mapping))) { if (net_ratelimit()) netif_err(tp, drv, dev, "Failed to map TX DMA!\n"); - goto err_dma_0; + goto err_free_skb_1; } tp->tx_skb[entry].len = len; @@ -5808,7 +5819,7 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb, frags = rtl8169_xmit_frags(tp, skb, opts); if (frags < 0) - goto err_dma_1; + goto err_unmap_dma_1; else if (frags) opts[0] |= FirstFrag; else { @@ -5854,10 +5865,11 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *sk return NETDEV_TX_OK; -err_dma_1: +err_unmap_dma_1: rtl8169_unmap_tx_skb(d, tp->tx_skb + entry, txd); -err_dma_0: +err_free_skb_1: dev_kfree_skb(skb); +err_update_stats_0: dev->stats.tx_dropped++; return NETDEV_TX_OK;