From patchwork Tue May 17 03:36:15 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Stultz X-Patchwork-Id: 622840 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 3r830c0Gbsz9sdb for ; Tue, 17 May 2016 13:36:48 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b=kGEawwQr; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752948AbcEQDgW (ORCPT ); Mon, 16 May 2016 23:36:22 -0400 Received: from mail-pf0-f174.google.com ([209.85.192.174]:36719 "EHLO mail-pf0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752945AbcEQDgU (ORCPT ); Mon, 16 May 2016 23:36:20 -0400 Received: by mail-pf0-f174.google.com with SMTP id c189so1896058pfb.3 for ; Mon, 16 May 2016 20:36:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=r7Sw5+iwGnI4xjLYuUCitdWIOMyzfbnW4qls3oq7LNo=; b=kGEawwQraSuvKDkhhols2BQESV6nyk2/j6hEsvdW/gJAteWOuOOnrQjcIuD3bj4f8f 5co05zxDHgLhEvcqaitFhrHglBnI8cTRoihLHSYQHCtks+KjruHbNPiaN1nNEv+oQXDL +TikAyHU7YZu4gQlAgT9ZuInqnjFmhmMUrB18= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=r7Sw5+iwGnI4xjLYuUCitdWIOMyzfbnW4qls3oq7LNo=; b=YpFrb7dPVwC/GthhxkdQwKKMVPqJ+WrNOkW6j+hBuFCdNqv7m6j4cNIW2QXbKncVj3 8Mny/8gj+TRl8mRkw7G7OmL3E1Nk4B00gmZi3BQVnZuyfwznr9/jkO36heX1PydJC946 qMAWwmlc/mO6cXk7F3emtS7pQsWgA30fJ6ZrK8MowQij9Sz34AXfLzBGnlcwR/TtyjAu 7zvz8HW7muZkUbAX4dk7L4I9qj3G/iHl8F7vteXSzhJ240ZpVrcTzA6EchEXaZZhqyPF 5h6sSDK/WiLJ2JYJOvrmGxmJgJSkOdeUcg9jwEH1Ad7oGgIA3C0vjOWPNaRqQErS7EUY TUKA== X-Gm-Message-State: AOPr4FX6Nrnyxk9I92JcQewD1TArDWuwsnE0EFBMQKBYFmFNlBy73y9zMAlJ5WHymMw7tUDT X-Received: by 10.98.52.195 with SMTP id b186mr50743272pfa.9.1463456179211; Mon, 16 May 2016 20:36:19 -0700 (PDT) Received: from localhost.localdomain (c-73-67-244-238.hsd1.or.comcast.net. [73.67.244.238]) by smtp.gmail.com with ESMTPSA id cr16sm599863pac.5.2016.05.16.20.36.18 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 16 May 2016 20:36:18 -0700 (PDT) From: John Stultz To: lkml Cc: John Stultz , Dean Jenkins , "David B. Robins" , Mark Craske , Emil Goode , "David S. Miller" , YongQin Liu , Guodong Xu , Ivan Vecera , linux-usb@vger.kernel.org, netdev@vger.kernel.org, stable Subject: [PATCH] asix: Fix offset calculation in asix_rx_fixup() causing slow transmissions Date: Mon, 16 May 2016 20:36:15 -0700 Message-Id: <1463456175-6674-1-git-send-email-john.stultz@linaro.org> X-Mailer: git-send-email 1.9.1 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org In testing with HiKey, we found that since commit 3f30b158eba5 ("asix: On RX avoid creating bad Ethernet frames"), we're seeing lots of noise during network transfers: [ 239.027993] asix 1-1.1:1.0 eth0: asix_rx_fixup() Data Header synchronisation was lost, remaining 988 [ 239.037310] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length 0x54ebb5ec, offset 4 [ 239.045519] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length 0xcdffe7a2, offset 4 [ 239.275044] asix 1-1.1:1.0 eth0: asix_rx_fixup() Data Header synchronisation was lost, remaining 988 [ 239.284355] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length 0x1d36f59d, offset 4 [ 239.292541] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length 0xaef3c1e9, offset 4 [ 239.518996] asix 1-1.1:1.0 eth0: asix_rx_fixup() Data Header synchronisation was lost, remaining 988 [ 239.528300] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length 0x2881912, offset 4 [ 239.536413] asix 1-1.1:1.0 eth0: asix_rx_fixup() Bad Header Length 0x5638f7e2, offset 4 And network throughput ends up being pretty bursty and slow with a overall throughput of at best ~30kB/s (where as previously we got 1.1MB/s with the slower USB1.1 "full speed" host). We found the issue also was reproducible on a x86_64 system, using a "high-speed" USB2.0 port but the throughput did not measurably drop (possibly due to the scp transfer being cpu bound on my slow test hardware). After lots of debugging, I found the check added in the problematic commit seems to be calculating the offset incorrectly. In the normal case, in the main loop of the function, we do: (where offset is zero, or set to "offset += (copy_length + 1) & 0xfffe" in the previous loop) rx->header = get_unaligned_le32(skb->data + offset); offset += sizeof(u32); But the problematic patch calculates: offset = ((rx->remaining + 1) & 0xfffe) + sizeof(u32); rx->header = get_unaligned_le32(skb->data + offset); Adding some debug logic to check those offset calculation used to find rx->header, the one in problematic code is always too large by sizeof(u32). Thus, this patch removes the incorrect " + sizeof(u32)" addition in the problematic calculation, and resolves the issue. Cc: Dean Jenkins Cc: "David B. Robins" Cc: Mark Craske Cc: Emil Goode Cc: "David S. Miller" Cc: YongQin Liu Cc: Guodong Xu Cc: Ivan Vecera Cc: linux-usb@vger.kernel.org Cc: netdev@vger.kernel.org Cc: stable #4.4+ Reported-by: Yongqin Liu Signed-off-by: John Stultz --- drivers/net/usb/asix_common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c index 0c5c22b..7de5ab5 100644 --- a/drivers/net/usb/asix_common.c +++ b/drivers/net/usb/asix_common.c @@ -66,7 +66,7 @@ int asix_rx_fixup_internal(struct usbnet *dev, struct sk_buff *skb, * buffer. */ if (rx->remaining && (rx->remaining + sizeof(u32) <= skb->len)) { - offset = ((rx->remaining + 1) & 0xfffe) + sizeof(u32); + offset = ((rx->remaining + 1) & 0xfffe); rx->header = get_unaligned_le32(skb->data + offset); offset = 0;