Received-SPF: Pass (protection.outlook.com: domain of siemens.com designates
 194.138.21.70 as permitted sender) receiver=protection.outlook.com;
 client-ip=194.138.21.70; helo=hybrid.siemens.com;
Date:   Tue, 4 Jan 2022 21:23:08 +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: <20220104212308.6db977e6@md1za8fc.ad001.siemens.net>
In-Reply-To: <20220104120027.611f8830@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>
        <20220104180715.7ecb0980@md1za8fc.ad001.siemens.net>
        <601815fe-a10e-fe48-254c-ed2ef1accffc@canonical.com>
        <20220104193455.6b8a21fc@md1za8fc.ad001.siemens.net>
        <20220104120027.611f8830@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 20:23:11.2497
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 37fade76-a3e1-4800-ccb0-08d9cfc00756
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.70];Helo=[hybrid.siemens.com]
X-MS-Exchange-CrossTenant-AuthSource: HE1EUR01FT013.eop-EUR01.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM7PR10MB3718
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Am Tue, 4 Jan 2022 12:00:27 -0800
schrieb Jakub Kicinski <kuba@kernel.org>:

> On Tue, 4 Jan 2022 19:34:55 +0100 Henning Schild wrote:
> > Am Wed, 5 Jan 2022 01:40:42 +0800
> > schrieb Aaron Ma <aaron.ma@canonical.com>:  
> > > Yes, it's expected to be a mess if multiple r8152 are attached to
> > > Lenovo USB-C/TBT docks. The issue had been discussed for several
> > > times in LKML. Either lose this feature or add potential risk for
> > > multiple r8152.
> > > 
> > > The idea is to make the Dock work which only ship with one r8152.
> > > It's really hard to say r8152 is from dock or another plugin one.
> > > 
> > > If revert this patch, then most users with the original shipped
> > > dock may lose this feature. That's the problem this patch try to
> > > fix.    
> > 
> > I understand that. But i would say people can not expect such a crap
> > feature on Linux, or we really need very good reasoning to cause MAC
> > collisions with the real PHY and on top claim ETOOMANY of the
> > dongles.
> > 
> > The other vendors seem to check bits of the "golden" dongle. At
> > least that is how i understand BD/AD/BND_MASK
> > 
> > How about making it a module param and default to off, and dev_warn
> > if BIOS has it turned on. That sounds like a reasonable compromise
> > and whoever turns it on twice probably really wants it. (note that
> > BIOS defaults to on ... so that was never intended by users, and
> > corporate users might not be allowed/able to turn that off)
> > 
> > MACs change ... all the time, people should use radius x509. The
> > request is probably coming from corporate users, and they are all
> > on a zero trust journey and will eventually stop relying on MACs
> > anyways.
> > 
> > And if ubuntu wants to cater by default, there can always be an udev
> > rule or setting that module param to "on".  
> 
> Let's split the problem into the clear regression caused by the patch
> and support of the feature on newer docks. I think we should fix the
> regression ASAP (the patch has also been backported to 5.15, so it's
> going to get more and more widely deployed). Then we can worry about
> the MAC addr copy on newer docks and the feature in a wider context.
> Is there really nothing in the usb info of the r8152 instance to
> indicate that it's part of the dock? Does the device have EEPROM which
> could contain useful info, maybe?

As far as i can tell a simple revert will do the trick. In which case
only "well known" docks will be affected and hopefully do not have any
other r8152 dongles attached.

If a dock does not have its own USB device ID ... like travel adaptors
or plain USB ethernet thingys ... MAC inheritance might be "going too
far". And if a new "active dock" shows up ... its device ID should be
added and matched.

But it is not easy to split up really ... because people might be using
"active" docks from other vendors. Nobody said that i can not use an HP
USB-C thingy for my Lenovo machine or the other way around.
The code currently matches vendor specific ACPI with vendor specific
USB, which creates a kind of nasty vendor lock-in situation for
accessory.

Henning

> > > For now I suggest to disable it in BIOS if you got multiple r8152.
> > > 
> > > Let me try to make some changes to limit this feature in one
> > > r8152.    
> > 
> > Which one? ;) And how to deal with the real NIC once you picked one?
> > Looking forward, please Cc me.  
>