Re: [PATCH] r8152: disable rx checksum offload on Dell TB dock

From: Greg KH
Date: Thu Nov 23 2017 - 04:24:54 EST


On Thu, Nov 23, 2017 at 04:53:41PM +0800, Kai Heng Feng wrote:
>
> > On 23 Nov 2017, at 3:58 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> >
> > On Thu, Nov 23, 2017 at 01:38:38AM -0500, Kai-Heng Feng wrote:
> >> r8153 on Dell TB dock corrupts rx packets.
> >>
> >> The root cause is not found yet, but disabling rx checksumming can
> >> workaround the issue. We can use this connection to decide if it's
> >> a Dell TB dock:
> >> Realtek r8153 <-> SMSC hub <-> ASMedia XHCI controller
> >>
> >> BugLink: https://bugs.launchpad.net/bugs/1729674
> >> Cc: Mario Limonciello <mario.limonciello@xxxxxxxx>
> >> Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
> >> ---
> >> drivers/net/usb/r8152.c | 33 ++++++++++++++++++++++++++++++++-
> >> 1 file changed, 32 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> >> index d51d9abf7986..58b80b5e7803 100644
> >> --- a/drivers/net/usb/r8152.c
> >> +++ b/drivers/net/usb/r8152.c
> >> @@ -27,6 +27,8 @@
> >> #include <linux/usb/cdc.h>
> >> #include <linux/suspend.h>
> >> #include <linux/acpi.h>
> >> +#include <linux/pci.h>
> >> +#include <linux/usb/hcd.h>
> >>
> >> /* Information for net-next */
> >> #define NETNEXT_VERSION "09"
> >> @@ -5135,6 +5137,35 @@ static u8 rtl_get_version(struct usb_interface *intf)
> >> return version;
> >> }
> >>
> >> +/* Ethernet on Dell TB 15/16 dock is connected this way:
> >> + * Realtek r8153 <-> SMSC hub <-> ASMedia XHCI controller
> >> + * We use this connection to make sure r8153 is on the Dell TB dock.
> >> + */
> >> +static bool check_dell_tb_dock(struct usb_device *udev)
> >> +{
> >> + struct usb_device *hub = udev->parent;
> >> + struct usb_device *root_hub;
> >> + struct pci_dev *controller;
> >> +
> >> + if (!hub)
> >> + return false;
> >> +
> >> + if (!(le16_to_cpu(hub->descriptor.idVendor) == 0x0424 &&
> >> + le16_to_cpu(hub->descriptor.idProduct) == 0x5537))
> >> + return false;
> >> +
> >> + root_hub = hub->parent;
> >> + if (!root_hub || root_hub->parent)
> >> + return false;
> >> +
> >> + controller = to_pci_dev(bus_to_hcd(root_hub->bus)->self.controller);
> >
> > That's a very scary, and dangerous, cast. You can not ever be sure that
> > the hub really is a "root hub" like this.
>
> What I want to do here is to finding this connection:
> Realtek r8153 <-> SMSC hub (USD ID: 0424:5537) <->
> ASMedia XHCI controller (PCI ID: 1b21:1142).
>
> Is there a safer way to do this?

Nope! You can't do that at all from within a USB driver, sorry. As you
really should not care at all :)

> >> + if (controller->vendor == 0x1b21 && controller->device == 0x1142)
> >> + return true;
> >
> > Why can't you just look at the USB device itself and go off of a quirk
> > in it? Something like a version or string or something else?
>
> I have a r8153 <-> USB 3.0 dongle which work just fine. I canât find any
> information to differentiate them. Hence I want to use the connection to
> identify if r8153 is on a Dell TB dock.

Are you sure there is nothing different in the version or release number
of the device? 'lsusb -v' shows the exact same information for both
devices?

> > This sounds like a USB host controller issue, not a USB device issue,
> > can't we fix the "real" problem here instead of this crazy work-around?
>
> Yes. From what I know, ASMedia is working on it, but not sure how long it
> will take. In the meantime, Iâd like to workaround this issue for the users.

Again, it's a host controller bug, it should be fixed there, don't try
to paper over the real issue in different individual drivers.

I think I've seen various patches on the linux-usb list for this
controller already, have you tried them?

> > Odds are any device plugged into the hub should have the same issue,
> > right?
>
> Actually no.
> I just plugged r8153 dongle into the same hub, surprisingly the issue
> doesnât happen in this scenario.

Then something seems to be wrong with the device itself, as that would
be the same exact electrical/logical path, right?

thanks,

greg k-h