lkml.org 
[lkml]   [2016]   [May]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [REGRESSION] asix: Lots of asix rx fixup() errors and slow transmissions
On 2016-05-03 17:16, Dean Jenkins wrote:
> On 03/05/16 15:42, David B. Robins wrote:
>>
>> I don't think the first one is giving you problems (except as
>> triggered by the second) but I had concerns about the second myself
>> (and emailed the author off-list, but received no reply), and we did
>> not take that commit for our own product.
>>
> Sorry, I might have missed your original E-mail.
>
>> Specifically, the second change, 3f30... (original patch:
>> https://www.mail-archive.com/netdev@vger.kernel.org/msg80720.html) (1)
>> appears to do the exact opposite of what it claims, i.e., instead of
>> "resync if this looks like a header", it does "resync if this does NOT
>> look like a (packet) header", where "looks like a header" means "bits
>> 0-10 (size) are equal to the bitwise-NOT of bits 16-26", and (2) can
>> happen by coincidence for 1/2048 32-bit values starting a continuation
>> URB (easy to hit dealing with large volumes of video data as we were).
>> It appears to expect the header for every URB whereas the rest of the
>> code at least expects it only once per network packet (look at
>> following code that only reads it for remaining == 0).
>
> David, I think that your interpretation is incorrect. Please see below.
>
> Here is the code snippet from the patch with my annotations between #
> #, I will try to explain my intentions. Feel free to point out any
> flaws:
>
> if (rx->remaining && (rx->remaining + sizeof(u32) <= skb->len)) {
> # Only runs when rx->remaining !=0 and the end of the Ethernet
> frame + next 32-bit header word is within the URB buffer. #
> # Therefore, this code does not run when the end of an
> Ethernet frame has been reached in the previous URB #
> # or when the end of the Ethernet frame + next 32-bit header
> word will be in a later URB buffer #

It may well be. I don't have the setup with me now, but I can try
tomorrow to reproduce an environment where I can add some more detailed
logging.

Since the URB length has to be >= than the remaining data plus a u32,
the devices that John Stultz and I are using (AX88772B in my case) may
be adding some additional data/padding after an Ethernet frame,
expecting it to be discarded, and running into this check and its
consequences. This may mean the device is badly behaved, if it is
specified not to send anything extra; in any case, a well-intentioned
error correction has gone badly, but I better understand the intent now.
I am curious to know how often the device you are using benefits from
this block of code.

> Regards,
> Dean

David

\
 
 \ /
  Last update: 2016-05-04 03:01    [W:0.198 / U:0.264 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site