linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mark Lord <mlord@pobox.com>
To: Hayes Wang <hayeswang@realtek.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Cc: nic_swsd <nic_swsd@realtek.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>
Subject: Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
Date: Fri, 18 Nov 2016 07:03:28 -0500	[thread overview]
Message-ID: <b9809516-d036-bfc3-b7a3-6563033ec957@pobox.com> (raw)
In-Reply-To: <0835B3720019904CB8F7AA43166CEEB201050B7E@RTITMBSV03.realtek.com.tw>

On 16-11-18 02:57 AM, Hayes Wang wrote:
..
> Besides, the maximum data length which the RTL8152 would send to
> the host is 16KB. That is, if the agg_buf_sz is 16KB, the host
> wouldn't split it. However, you still see problems for it.

How does the RTL8152 know that the limit is 16KB,
rather than some other number?  Is this a hardwired number
in the hardware, or is it a parameter that the software
sends to the chip during initialization?

I have a USB analyzer, but it is difficult to figure out how
to program an appropriate trigger point for the capture,
since the problem (with 16KB URBs) takes minutes to hours
or even days to trigger.

And the output from the analyzer is in some proprietary format.
The in-kernel software analzer could be useful, but I have never
figured out how to use it.  :)

Since my earlier email, I have figured out another piece of the
puzzle with this dongle.

The first issue is that a packet sometimes begins in one URB,
and completes in the next URB, without an rx_desc at the start
of the second URB.  This I have already reported earlier.

But the driver, as written, sometimes accesses bytes outside
of the 16KB URB buffer, because it trusts the non-existent
rx_desc in these cases, and also because it accesses bytes
from the rx_desc without first checking whether there is
sufficient remaining space in the URB to hold an rx_desc.

These incorrect accesses sometimes touch memory outside
of the URB buffer.  Since the driver allocates all of its
rx URB buffers at once, they are highly likely to be
physically (and therefore virtually) adjacent in memory.

So mistakenly accessing beyond the end of one buffer will
often result in a read from memory of the next URB buffer.
Which causes a portion of it to be loaded in the the D-cache.

When that URB is subsequently filled by DMA, there then exists
a data-consistency issue:  the D-cache contains stale information
from before the latest DMA cycle.

So this explains the strange memory behaviour observed earlier on.
When I add a call to invalidate_dcache_range() to the driver
just before it begins examining a new rx URB, the problems go away.
So this confirms the observations.

Using non-cacheable RAM also makes the problem go away.
But neither is a fix for the real buffer overrun accesses in the driver.

Fix the "packet spans URBs" bug, and fix the driver to ALWAYS
test lengths/ranges before accessing the actual buffer,
and everything should begin working reliably.

Cheers
-- 
Mark Lord
Real-Time Remedies Inc.
mlord@pobox.com

  reply	other threads:[~2016-11-18 12:03 UTC|newest]

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-11  7:15 [PATCH net 0/2] r8152: rx patches Hayes Wang
2016-11-11  7:15 ` [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable Hayes Wang
2016-11-17  3:36   ` Hayes Wang
2016-11-17 14:14     ` Mark Lord
2016-11-17 14:25       ` Mark Lord
     [not found]     ` <d683c019-4e0f-6fe6-368c-c4fc86c72fe6@pobox.com>
2016-11-18  7:57       ` Hayes Wang
2016-11-18 12:03         ` Mark Lord [this message]
2016-11-22 13:12           ` Mark Lord
2016-11-23  3:52           ` Hayes Wang
2016-11-23 13:41             ` Mark Lord
2016-11-23 15:12               ` Hayes Wang
2016-11-23 19:29                 ` Mark Lord
2016-11-24  3:24                   ` Hayes Wang
2016-11-24 12:31                   ` Mark Lord
2016-11-24 13:26                     ` Hayes Wang
2016-11-24 15:24                       ` Mark Lord
2016-11-25  6:11                         ` Hayes Wang
2016-11-25 12:36                           ` Mark Lord
2016-11-24 16:21                       ` David Miller
2016-11-24 16:43                         ` Mark Lord
2016-11-24 17:00                           ` Mark Lord
2016-11-24 17:13                             ` David Miller
2016-11-24 17:11                           ` David Miller
2016-11-24 18:34                             ` Mark Lord
2016-11-24 18:49                               ` Mark Lord
2016-11-24 19:00                               ` Greg KH
2016-11-24 19:10                                 ` Mark Lord
2016-11-24 19:17                                   ` Greg KH
2016-11-25  9:52                                   ` Hayes Wang
2016-11-25 13:32                                     ` Mark Lord
2016-11-25  0:27                               ` Francois Romieu
2016-11-25  3:49                                 ` Mark Lord
2016-11-25  9:53                                   ` Greg KH
2016-11-25 12:34                                     ` Mark Lord
2016-11-25 12:41                                       ` Mark Lord
2016-11-25 14:22                                         ` Greg KH
2016-11-25 14:35                                           ` Mark Lord
2016-11-25 12:49                                     ` Mark Lord
2016-11-25 14:24                                       ` Greg KH
2016-11-25 16:58                                       ` David Miller
2016-11-30 11:58                                         ` Hayes Wang
2016-12-09  3:23                                           ` Hayes Wang
2016-12-09 13:05                                             ` Mark Lord
2017-01-01  0:07                                           ` Ansis Atteka
2017-01-03  0:40                                             ` Ansis Atteka
2017-01-03 13:19                                               ` Mark Lord
2017-01-09  7:58                                               ` Hayes Wang
2019-01-05 14:14                                             ` r8152: data corruption in various scenarios Mark Lord
2019-01-05 14:22                                               ` Mark Lord
2019-01-06 19:14                                               ` Kai Heng Feng
2019-01-06 21:13                                                 ` Mark Lord
2019-01-06 21:16                                                   ` Mark Lord
2019-01-07  3:53                                                     ` Hayes Wang
2019-01-07 16:01                                                       ` Mario.Limonciello
2019-01-07 18:06                                                         ` Mark Lord
2019-01-07 18:27                                                           ` Mario.Limonciello
2019-01-07 19:24                                                             ` Mark Lord
2019-01-07  4:09                                                     ` Kai Heng Feng
2019-01-07  4:13                                                       ` Mark Lord
2019-01-07  6:46                                                         ` Kai Heng Feng
2019-01-07  7:01                                                           ` Mark Lord
2016-11-24 18:42                           ` [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable Greg KH
2016-11-24 18:58                             ` Mark Lord
2016-11-25  6:31                           ` Hayes Wang
2016-11-25  6:51                             ` Hayes Wang
2016-11-25 12:35                               ` Mark Lord
2016-11-24 16:19                     ` David Miller
2016-11-24 12:37               ` Hayes Wang
2016-11-11  7:15 ` [PATCH net 2/2] r8152: rx descriptor check Hayes Wang
2016-11-11 12:13   ` Francois Romieu
2016-11-12 13:21     ` Mark Lord
2016-11-14  6:43     ` Hayes Wang
2016-11-15  1:10       ` Francois Romieu
2016-11-17  3:05         ` Hayes Wang
2016-11-13 17:39   ` David Miller
2016-11-13 20:34     ` Mark Lord
2016-11-13 20:38       ` Mark Lord
2016-11-14  7:23       ` Hayes Wang
2016-11-14 17:27         ` David Miller
2016-11-14  7:03     ` Hayes Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b9809516-d036-bfc3-b7a3-6563033ec957@pobox.com \
    --to=mlord@pobox.com \
    --cc=hayeswang@realtek.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nic_swsd@realtek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).