RE: [PATCH v2] r8152: disable RX aggregation on Dell TB16 dock

[Mario.Limonciello]

> -Original Message-
> From: Kai Heng Feng [mailto:kai.heng.f...@canonical.com]
> Sent: Thursday, January 18, 2018 10:57 AM
> To: David Miller <da...@davemloft.net>
> Cc: Hayes Wang <hayesw...@realtek.com>; gre...@linuxfoundation.org; linux-
> u...@vger.kernel.org; netdev@vger.kernel.org; linux-ker...@vger.kernel.org;
> Limonciello, Mario <mario_limoncie...@dell.com>; nic_s...@realtek.com
> Subject: Re: [PATCH v2] r8152: disable RX aggregation on Dell TB16 dock
> 
> 
> 
> > On 18 Jan 2018, at 10:50 PM, David Miller <da...@davemloft.net> wrote:
> >
> > From: Hayes Wang <hayesw...@realtek.com>
> > Date: Thu, 18 Jan 2018 03:04:08 +
> >
> >> [...]
> >>>> r8153 on Dell TB15/16 dock corrupts rx packets.
> >>>>
> >>>> This change is suggested by Realtek. They guess that the XHCI
> >>>> controller doesn't have enough buffer, and their guesswork is correct,
> >>>> once the RX aggregation gets disabled, the issue is gone.
> >>>>
> >>>> ASMedia is currently working on a real sulotion for this issue.
> >>>>
> >>>> Dell and ODM confirm the bcdDevice and iSerialNumber is unique for TB16.
> >>>>
> >>>> Note that TB15 has different bcdDevice and iSerialNumber, which are
> >>>> not unique values. If you still have TB15, please contact Dell to
> >>>> replace it with TB16.
> >>
> >> Excuse me. I don't understand why this patch is for specific USB nic 
> >> rather than
> xHCI.
> >> It seems to make the specific USB nic working and the other ones keeping 
> >> error.
> >
> > Well, are we sure that the device being in the TB16 dock doesn't
> > contribute to the issue as well?

Previous version of this patch checked the parent device to ensure it was in 
TB16.
I believe there was negative feedback to that approach, which prompted the 
discussion
to check bcdDevice and iSerialNumber with all vendors involved.

If it's still desirable to analyze parentage tree, I suppose bcdDevice, 
iSerialNumber
and parent's USB device VID/PID can be analyzed  all at the same time.

> 
> This is what vendors concluded for now. The very same NIC on WD15 doesn’t
> have the issue.

And just so it's extra clear to everyone on this list - WD15 has different 
bcdDevice
iSerialNumber, and doesn't connect to ASMedia host controller.

Re: [PATCH v2] r8152: disable RX aggregation on Dell TB16 dock

[Kai Heng Feng]

> On 18 Jan 2018, at 10:50 PM, David Miller  wrote:
> 
> From: Hayes Wang 
> Date: Thu, 18 Jan 2018 03:04:08 +
> 
>> [...]
 r8153 on Dell TB15/16 dock corrupts rx packets.
 
 This change is suggested by Realtek. They guess that the XHCI
 controller doesn't have enough buffer, and their guesswork is correct,
 once the RX aggregation gets disabled, the issue is gone.
 
 ASMedia is currently working on a real sulotion for this issue.
 
 Dell and ODM confirm the bcdDevice and iSerialNumber is unique for TB16.
 
 Note that TB15 has different bcdDevice and iSerialNumber, which are
 not unique values. If you still have TB15, please contact Dell to
 replace it with TB16.
>> 
>> Excuse me. I don't understand why this patch is for specific USB nic rather 
>> than xHCI.
>> It seems to make the specific USB nic working and the other ones keeping 
>> error.
> 
> Well, are we sure that the device being in the TB16 dock doesn't
> contribute to the issue as well?

This is what vendors concluded for now. The very same NIC on WD15 doesn’t
have the issue.

> 
> If the problem only shows up with XHCI and this dock, then this patch
> was the appropriate way to deal with the problem for now.

Re: [PATCH v2] r8152: disable RX aggregation on Dell TB16 dock

[David Miller]

From: Hayes Wang 
Date: Thu, 18 Jan 2018 03:04:08 +

> [...]
>> > r8153 on Dell TB15/16 dock corrupts rx packets.
>> >
>> > This change is suggested by Realtek. They guess that the XHCI
>> > controller doesn't have enough buffer, and their guesswork is correct,
>> > once the RX aggregation gets disabled, the issue is gone.
>> >
>> > ASMedia is currently working on a real sulotion for this issue.
>> >
>> > Dell and ODM confirm the bcdDevice and iSerialNumber is unique for TB16.
>> >
>> > Note that TB15 has different bcdDevice and iSerialNumber, which are
>> > not unique values. If you still have TB15, please contact Dell to
>> > replace it with TB16.
> 
> Excuse me. I don't understand why this patch is for specific USB nic rather 
> than xHCI.
> It seems to make the specific USB nic working and the other ones keeping 
> error.

Well, are we sure that the device being in the TB16 dock doesn't
contribute to the issue as well?

If the problem only shows up with XHCI and this dock, then this patch
was the appropriate way to deal with the problem for now.

Re: [PATCH v2] r8152: disable RX aggregation on Dell TB16 dock

[Kai Heng Feng]

> On 18 Jan 2018, at 11:04 AM, Hayes Wang  wrote:
> 
> [...]
>>> r8153 on Dell TB15/16 dock corrupts rx packets.
>>> 
>>> This change is suggested by Realtek. They guess that the XHCI
>>> controller doesn't have enough buffer, and their guesswork is correct,
>>> once the RX aggregation gets disabled, the issue is gone.
>>> 
>>> ASMedia is currently working on a real sulotion for this issue.
>>> 
>>> Dell and ODM confirm the bcdDevice and iSerialNumber is unique for TB16.
>>> 
>>> Note that TB15 has different bcdDevice and iSerialNumber, which are
>>> not unique values. If you still have TB15, please contact Dell to
>>> replace it with TB16.
> 
> Excuse me. I don't understand why this patch is for specific USB nic rather 
> than xHCI.
> It seems to make the specific USB nic working and the other ones keeping 
> error.

This patch is transient, once ASMedia find the correct solution, the patch
can be reverted.

This patch only targets the builtin 8153 because externally plugged r8152 or
asix do not suffered from this issue.

Kai-Heng

> 
> 
> Best Regards,
> Hayes
> 
> 

RE: [PATCH v2] r8152: disable RX aggregation on Dell TB16 dock

[Hayes Wang]

[...]
> > r8153 on Dell TB15/16 dock corrupts rx packets.
> >
> > This change is suggested by Realtek. They guess that the XHCI
> > controller doesn't have enough buffer, and their guesswork is correct,
> > once the RX aggregation gets disabled, the issue is gone.
> >
> > ASMedia is currently working on a real sulotion for this issue.
> >
> > Dell and ODM confirm the bcdDevice and iSerialNumber is unique for TB16.
> >
> > Note that TB15 has different bcdDevice and iSerialNumber, which are
> > not unique values. If you still have TB15, please contact Dell to
> > replace it with TB16.

Excuse me. I don't understand why this patch is for specific USB nic rather 
than xHCI.
It seems to make the specific USB nic working and the other ones keeping error.


Best Regards,
Hayes

Re: [PATCH v2] r8152: disable RX aggregation on Dell TB16 dock

[David Miller]

From: Kai-Heng Feng 
Date: Tue, 16 Jan 2018 16:46:27 +0800

> r8153 on Dell TB15/16 dock corrupts rx packets.
> 
> This change is suggested by Realtek. They guess that the XHCI controller
> doesn't have enough buffer, and their guesswork is correct, once the RX
> aggregation gets disabled, the issue is gone.
> 
> ASMedia is currently working on a real sulotion for this issue.
> 
> Dell and ODM confirm the bcdDevice and iSerialNumber is unique for TB16.
> 
> Note that TB15 has different bcdDevice and iSerialNumber, which are not
> unique values. If you still have TB15, please contact Dell to replace it
> with TB16.
> 
> BugLink: https://bugs.launchpad.net/bugs/1729674
> Cc: Mario Limonciello 
> Signed-off-by: Kai-Heng Feng 
> ---
> v2:
> - Disable RX aggregation instead of disable RX checksum
> - Use bcdDevice and iSerialNumber to uniquely identify Dell TB16

Ok, since this is very restricted it's an acceptable way to deal with
this problem.

Applied, thanks.

[PATCH v2] r8152: disable RX aggregation on Dell TB16 dock

[Kai-Heng Feng]

r8153 on Dell TB15/16 dock corrupts rx packets.

This change is suggested by Realtek. They guess that the XHCI controller
doesn't have enough buffer, and their guesswork is correct, once the RX
aggregation gets disabled, the issue is gone.

ASMedia is currently working on a real sulotion for this issue.

Dell and ODM confirm the bcdDevice and iSerialNumber is unique for TB16.

Note that TB15 has different bcdDevice and iSerialNumber, which are not
unique values. If you still have TB15, please contact Dell to replace it
with TB16.

BugLink: https://bugs.launchpad.net/bugs/1729674
Cc: Mario Limonciello 
Signed-off-by: Kai-Heng Feng 
---
v2:
- Disable RX aggregation instead of disable RX checksum
- Use bcdDevice and iSerialNumber to uniquely identify Dell TB16

 drivers/net/usb/r8152.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index d51d9abf7986..0657203ffb91 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -606,6 +606,7 @@ enum rtl8152_flags {
PHY_RESET,
SCHEDULE_NAPI,
GREEN_ETHERNET,
+   DELL_TB_RX_AGG_BUG,
 };
 
 /* Define these values to match your device */
@@ -1798,6 +1799,9 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct 
tx_agg *agg)
dev_kfree_skb_any(skb);
 
remain = agg_buf_sz - (int)(tx_agg_align(tx_data) - agg->head);
+
+   if (test_bit(DELL_TB_RX_AGG_BUG, >flags))
+   break;
}
 
if (!skb_queue_empty(_head)) {
@@ -4133,6 +4137,9 @@ static void r8153_init(struct r8152 *tp)
/* rx aggregation */
ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_USB_CTRL);
ocp_data &= ~(RX_AGG_DISABLE | RX_ZERO_EN);
+   if (test_bit(DELL_TB_RX_AGG_BUG, >flags))
+   ocp_data |= RX_AGG_DISABLE;
+
ocp_write_word(tp, MCU_TYPE_USB, USB_USB_CTRL, ocp_data);
 
rtl_tally_reset(tp);
@@ -5207,6 +5214,12 @@ static int rtl8152_probe(struct usb_interface *intf,
netdev->hw_features &= ~NETIF_F_RXCSUM;
}
 
+   if (le16_to_cpu(udev->descriptor.bcdDevice) == 0x3011 &&
+   udev->serial && !strcmp(udev->serial, "0100")) {
+   dev_info(>dev, "Dell TB16 Dock, disable RX aggregation");
+   set_bit(DELL_TB_RX_AGG_BUG, >flags);
+   }
+
netdev->ethtool_ops = 
netif_set_gso_max_size(netdev, RTL_LIMITED_TSO_SIZE);
 
-- 
2.15.1