Received-SPF: Pass (protection.outlook.com: domain of siemens.com designates
 194.138.21.71 as permitted sender) receiver=protection.outlook.com;
 client-ip=194.138.21.71; helo=hybrid.siemens.com;
Date:   Tue, 4 Jan 2022 12:38:14 +0100
From:   Henning Schild <henning.schild@siemens.com>
To:     Aaron Ma <aaron.ma@canonical.com>
CC:     <davem@davemloft.net>, <kuba@kernel.org>, <hayeswang@realtek.com>,
        <tiwai@suse.de>, <linux-usb@vger.kernel.org>,
        <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] net: usb: r8152: Add MAC passthrough support for more
 Lenovo Docks
Message-ID: <20220104123814.32bf179e@md1za8fc.ad001.siemens.net>
In-Reply-To: <20211116141917.31661-1-aaron.ma@canonical.com>
References: <20211116141917.31661-1-aaron.ma@canonical.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Jan 2022 11:38:18.2157
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 80931c7e-e1fe-435c-c9b4-08d9cf76b404
X-MS-Exchange-CrossTenant-Id: 38ae3bcd-9579-4fd4-adda-b42e1495d55a
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=38ae3bcd-9579-4fd4-adda-b42e1495d55a;Ip=[194.138.21.71];Helo=[hybrid.siemens.com]
X-MS-Exchange-CrossTenant-AuthSource: DB5EUR01FT038.eop-EUR01.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR10MB2370
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

This patch is wrong and taking the MAC inheritance way too far. Now any
USB Ethernet dongle connected to a Lenovo USB Hub will go into
inheritance (which is meant for docks).

It means that such dongles plugged directly into the laptop will do
that, or travel adaptors/hubs which are not "active docks".

I have USB-Ethernet dongles on two desks and both stopped working as
expected because they took the main MAC, even with it being used at the
same time. The inheritance should (if at all) only be done for clearly
identified docks and only for one r8152 instance ... not all. Maybe
even double checking if that main PHY is "plugged" and monitoring it to
back off as soon as it is.

With this patch applied users can not use multiple ethernet devices
anymore ... if some of them are r8152 and connected to "Lenovo" ...
which is more than likely!

Reverting that patch solved my problem, but i later went to disabling
that very questionable BIOS feature to disable things for good without
having to patch my kernel.

I strongly suggest to revert that. And if not please drop the defines of

> -		case DEVICE_ID_THINKPAD_THUNDERBOLT3_DOCK_GEN2:
> -		case DEVICE_ID_THINKPAD_USB_C_DOCK_GEN2:

And instead of crapping out with "(unnamed net_device) (uninitialized):
Invalid header when reading pass-thru MAC addr" when the BIOS feature
is turned off, one might want to check
DSDT/WMT1/ITEM/"MACAddressPassThrough" which is my best for asking the
BIOS if the feature is wanted.

regards,
Henning

Am Tue, 16 Nov 2021 22:19:17 +0800
schrieb Aaron Ma <aaron.ma@canonical.com>:

> Like ThinkaPad Thunderbolt 4 Dock, more Lenovo docks start to use the
> original Realtek USB ethernet chip ID 0bda:8153.
> 
> Lenovo Docks always use their own IDs for usb hub, even for older
> Docks. If parent hub is from Lenovo, then r8152 should try MAC
> passthrough. Verified on Lenovo TBT3 dock too.
> 
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
>  drivers/net/usb/r8152.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 4a02f33f0643..f9877a3e83ac 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -9603,12 +9603,9 @@ static int rtl8152_probe(struct usb_interface
> *intf, netdev->hw_features &= ~NETIF_F_RXCSUM;
>  	}
>  
> -	if (le16_to_cpu(udev->descriptor.idVendor) ==
> VENDOR_ID_LENOVO) {
> -		switch (le16_to_cpu(udev->descriptor.idProduct)) {
> -		case DEVICE_ID_THINKPAD_THUNDERBOLT3_DOCK_GEN2:
> -		case DEVICE_ID_THINKPAD_USB_C_DOCK_GEN2:
> -			tp->lenovo_macpassthru = 1;
> -		}
> +	if (udev->parent &&
> +
> le16_to_cpu(udev->parent->descriptor.idVendor) == VENDOR_ID_LENOVO) {
> +		tp->lenovo_macpassthru = 1;
>  	}
>  
>  	if (le16_to_cpu(udev->descriptor.bcdDevice) == 0x3011 &&
> udev->serial &&