Received-SPF: Pass (protection.outlook.com: domain of siemens.com designates
 194.138.21.73 as permitted sender) receiver=protection.outlook.com;
 client-ip=194.138.21.73; helo=hybrid.siemens.com;
Date:   Tue, 4 Jan 2022 18:07:15 +0100
From:   Henning Schild <henning.schild@siemens.com>
To:     Jakub Kicinski <kuba@kernel.org>
CC:     Aaron Ma <aaron.ma@canonical.com>, <davem@davemloft.net>,
        <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: <20220104180715.7ecb0980@md1za8fc.ad001.siemens.net>
In-Reply-To: <20220104065326.2a73f674@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
References: <20211116141917.31661-1-aaron.ma@canonical.com>
        <20220104123814.32bf179e@md1za8fc.ad001.siemens.net>
        <20220104065326.2a73f674@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
MIME-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Jan 2022 17:07:18.5745
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 38753eba-a235-41f5-7dc0-08d9cfa4aa30
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.73];Helo=[hybrid.siemens.com]
X-MS-Exchange-CrossTenant-AuthSource: DB5EUR01FT026.eop-EUR01.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB8PR10MB2809
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Am Tue, 4 Jan 2022 06:53:26 -0800
schrieb Jakub Kicinski <kuba@kernel.org>:

> On Tue, 4 Jan 2022 12:38:14 +0100 Henning Schild wrote:
> > 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.  
> 
> Thank you for the report!
> 
> Aaron, will you be able to fix this quickly? 5.16 is about to be
> released.

If you guys agree with a revert and potentially other actions, i would
be willing to help. In any case it is not super-urgent since we can
maybe agree an regression and push it back into stable kernels.

I first wanted to place the report and see how people would react ...
if you guys agree that this is a bug and the inheritance is going "way
too far".

But i would only do some repairs on the surface, the feature itself is
horrific to say the least and i am very happy with that BIOS switch to
ditch it for good. Giving the MAC out is something a dock physically
blocking the original PHY could do ... but year ... only once and it
might be pretty hard to say which r8152 is built-in from the hub and
which is plugged in additionally in that very hub.
Not to mention multiple hubs of the same type ... in a nice USB-C chain.

MAC spoofing is something NetworkManager and others can take care of,
or udev ... doing that in the driver is ... spooky.

regards,
Henning