• John Stultz's avatar
    asix: Fix offset calculation in asix_rx_fixup() causing slow transmissions · cd9e2e5d
    John Stultz authored
    In testing with HiKey, we found that since
    commit 3f30b158 ("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 <Dean_Jenkins@mentor.com>
    Cc: "David B. Robins" <linux@davidrobins.net>
    Cc: Mark Craske <Mark_Craske@mentor.com>
    Cc: Emil Goode <emilgoode@gmail.com>
    Cc: "David S. Miller" <davem@davemloft.net>
    Cc: YongQin Liu <yongqin.liu@linaro.org>
    Cc: Guodong Xu <guodong.xu@linaro.org>
    Cc: Ivan Vecera <ivecera@redhat.com>
    Cc: linux-usb@vger.kernel.org
    Cc: netdev@vger.kernel.org
    Cc: stable <stable@vger.kernel.org> #4.4+
    Reported-by: default avatarYongqin Liu <yongqin.liu@linaro.org>
    Signed-off-by: default avatarJohn Stultz <john.stultz@linaro.org>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    cd9e2e5d
asix_common.c 16 KB