Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 28 Jan 2022 21:20:24 +0100
From:   Henning Schild <henning.schild@siemens.com>
To:     "Limonciello, Mario" <Mario.Limonciello@amd.com>
Cc:     Andrew Lunn <andrew@lunn.ch>, Aaron Ma <aaron.ma@canonical.com>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "hayeswang@realtek.com" <hayeswang@realtek.com>,
        "tiwai@suse.de" <tiwai@suse.de>
Subject: Re: [PATCH v3] net: usb: r8152: Add MAC passthrough support for
 RTL8153BL
Message-ID: <20220128212024.57ef9c59@md1za8fc.ad001.siemens.net>
In-Reply-To: <BL1PR12MB515773B15441F5BC375E452DE2229@BL1PR12MB5157.namprd12.prod.outlook.com>
References: <20220127100109.12979-1-aaron.ma@canonical.com>
        <20220128043207.14599-1-aaron.ma@canonical.com>
        <20220128092103.1fa2a661@md1za8fc.ad001.siemens.net>
        <YfQwpy1Kkz3wheTi@lunn.ch>
        <BL1PR12MB515773B15441F5BC375E452DE2229@BL1PR12MB5157.namprd12.prod.outlook.com>
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?UQHqjebhayTmF/27n4o3E7lOL5gXDOG7nHagaSWS2hxOJjFpYjJj0bFVbP4t?=
 =?us-ascii?Q?HH2uDWM/EtQxZRMnOjVk7vR6ZwjPcsNIRoJYji107stK74QWfnIxyU+gT3Nq?=
 =?us-ascii?Q?KK91rkHpdfGMf7W40Kt+sVuRSGfryUftMzkWGTsWUNEWhvZaSGH4Z4cWCr0+?=
 =?us-ascii?Q?dqOusiLtguYakJIQTa1PEwbJHvtfv8CHr7NcezJ6+5LJaOmmx04/igG0plo8?=
 =?us-ascii?Q?tKiEAzQMfbNJPDz4xqjVVpvWJu3D/2cNbfin7BDctVlW6K2A19RfPDGxw343?=
 =?us-ascii?Q?cwLEhxmiOop8gnxZM82j4Y7Vw2hP2iu78b/uIC2Id4xvcUyl21ioHYGeXB3I?=
 =?us-ascii?Q?RGBQ/EZ7pse3JG1wWQLuC2btk6lGBddl1HhjjZEZsbECL0A1b0lFyFdsBXwM?=
 =?us-ascii?Q?fxmWfwaxS1erXTRxCu0YUZEiRwXXSco2E1q5LBylkmHgAAvuwSet/+x3zZrf?=
 =?us-ascii?Q?Jx0UlJwPC1GbaLlP6HFtSZeuxbCFTiFEhkzbpInE+fE+gcJZEQruf8sX3+ij?=
 =?us-ascii?Q?Lh968vt5cD0cNfg0mjt83o5Kxsxl8tHkyCv2bGVfvBY4hY/Cqr2Tr36CSLr5?=
 =?us-ascii?Q?Q0OTh1dzL4A2wnWbw8+zwbI5dPiM8I1ljD+jYptyiXz0wipAK7LhWuj7tk22?=
 =?us-ascii?Q?Y/+7SBdfa3qBjkbZ1yRic67A0t2ZTOtB1uehBWtNkznZxmHbHFkbBbfv5lCH?=
 =?us-ascii?Q?4M2TDVAPmmxWLEWEw4j8Hr09wlehrcftFKoSC0SmyWlARwrsqRNrxmAUPe6E?=
 =?us-ascii?Q?yA5iWBFCNYcZ+bCNxdPWhAzi0TyLLO6af8Td2qupbjvjCHn6oYmlj3NF4CDZ?=
 =?us-ascii?Q?9Y1rGiPlR3uOfbD40G7l7x/Vv2mDqtzLJe4woHeaeT2dsOYErw20wJ9QaEio?=
 =?us-ascii?Q?uDhVewDvzACvlxnNug3j7vfGIfUNTGhlz5DUhHZTKrLsic8Mv/iXQf44Qyuq?=
 =?us-ascii?Q?euU8A3dMjJhZtFJjgBuYCxQbN6eLmyMAbchpBaM8oK0jlQioPrItQcFAnYZb?=
 =?us-ascii?Q?hGBNF+tjkT5ObVJy6aSWDynFTfChrpYuxUAsMbZejCFmHvokA/35zZWJWUJO?=
 =?us-ascii?Q?2X+Oa37DlGQfKhgo7BtoA2VVBkHhGJzcsyAL/4PYLYkUYpSe84QUv9cX1xjQ?=
 =?us-ascii?Q?SfKvVDS/liE9p4zlFnIT2p06RzUaRem8A8omN6SR3M1MzfOCpTUIqo6WpB3Q?=
 =?us-ascii?Q?9bNcAIMtq/kn+MPyiHVgokTCWw23ndpb5dnp836TiauHNIgA5b7SkQO8iRAH?=
 =?us-ascii?Q?++J1qo76jmewvfx3b+ISunT4LmSQw+lntDr/Z2hjlK4P2cQ694CbOaWw9nAQ?=
 =?us-ascii?Q?ZnG4pEQx22qAbEvqshWpX4+vh6740NYwmjKLF8yr9MU6e6VjHayhb2nc6c9I?=
 =?us-ascii?Q?/BrINY6xRmG77yngzzt0YtWSBLTT8udyACHMt3WOrWwTfW5ijhjbrCl8HoI+?=
 =?us-ascii?Q?Yuffc2vO9tIMtWKl8TVLXMdTtz2QK1E0jyKVWc5nKdO0pFbqs/alK1jk5pB1?=
 =?us-ascii?Q?nTI6ZHUW7hjWtWf0Vm2UbTG0bteuGWzHgr00Y81JgUfFXz82ghRr5wQIBgey?=
 =?us-ascii?Q?8dFbOQSGOUO2cqalGWm/zjSvDnI0KtgwPrcJPLttEZW3rJNbpZigyg/TNnwE?=
 =?us-ascii?Q?8oY7pK8NnjPwhYCkaYilEnVseWUS8V9uZdh9rBn09csdFAGV52BeXNcX/I84?=
 =?us-ascii?Q?nAla3l75kmgzzzKJIV4NvMVs5Za9Wfd3EN57CiSy1hnoD5jiL6Mjt9v8Vdvr?=
 =?us-ascii?Q?uqHXEY+4mg=3D=3D?=
X-OriginatorOrg: siemens.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c23e4e28-1e22-44a0-5eda-08d9e29b9fa8
X-MS-Exchange-CrossTenant-AuthSource: AM0PR10MB3459.EURPRD10.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Jan 2022 20:20:27.8087
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 38ae3bcd-9579-4fd4-adda-b42e1495d55a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Mlf5t7DD/Da/l3sWmZOq968doCiDgO02HeTfOJ1rzXBigbjXSMlUrFOsohW0FYpaBJiw+ldPwDuwrZ5OyKlipifYj1Yh3gQI4gX2NBU8KYU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB7PR10MB2153
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Am Fri, 28 Jan 2022 18:41:16 +0000
schrieb "Limonciello, Mario" <Mario.Limonciello@amd.com>:

> [Public]
> 
> 
> 
> > -----Original Message-----
> > From: Andrew Lunn <andrew@lunn.ch>
> > Sent: Friday, January 28, 2022 12:07
> > To: Henning Schild <henning.schild@siemens.com>
> > Cc: Aaron Ma <aaron.ma@canonical.com>; Limonciello, Mario
> > <Mario.Limonciello@amd.com>; kuba@kernel.org;
> > linux-usb@vger.kernel.org; netdev@vger.kernel.org;
> > linux-kernel@vger.kernel.org; gregkh@linuxfoundation.org;
> > davem@davemloft.net; hayeswang@realtek.com; tiwai@suse.de
> > Subject: Re: [PATCH v3] net: usb: r8152: Add MAC passthrough
> > support for RTL8153BL
> > 
> > On Fri, Jan 28, 2022 at 09:21:03AM +0100, Henning Schild wrote:  
> > > I am still very much against any patches in that direction. The
> > > feature as the vendors envision it does not seem to be really
> > > understood or even explained.
> > > Just narrowing down the device matching caters for vendor lock-in
> > > and confusion when that pass through is happening and when not.
> > > And seems to lead to unmaintainable spaghetti-code.
> > > People that use this very dock today will see an unexpected
> > > mac-change once they update to a kernel with this patch applied.  
> > 
> > I've not yet been convinced by replies that the proposed code really
> > does only match the given dock, and not random USB dongles.   
> 
> Didn't Realtek confirm this bit is used to identify the Lenovo
> devices?

The question is really why we do all that and that answer really seems
missing. Lenovo might have a very different answer than Dell. All i saw
was Lenovo docks that suggest PXE. While that could be a good case,
bootloaders and OSs should then inherit the MAC of a NIC that is
already up. But sure not try and guess what to do and spoof on their
own. (based on weird bits and topology guesses) I am willing to bet that
grub/pxelinux/uboot/systemd-boot would all fail a PXE run even if the
OS to boot would spoof/inherit.

PXE uses spoofed MAC and bootloader will dhcp again and not spoof. dhcp
would time out or fall back to somewhere (MAC not known), Linux kernel
would not come, PXE boot failed.

In fact i have seen first hand how PXE Windows install failed on a
Lenovo dock and the real NIC worked. That was within my company and i
sure do not know the BIOS settings or what bootloader was involved and
if that Windows installer did active spoofing. But i would say that the
PXE story probably does not really "work" for Windows either.
In fact i am willing to bet the BIOS setting for spoofing was turned
on, because it seems to be the factory default.

And all stories beyond PXE-bootstrap should probably be answered with
"a MAC does not identify a machine". So people that care to ident a
machine should use something like x509, or allow network access in any
other way not relying on a MAC. If "Linux" cares to spoof for the lazy
ones, udev is a better place than the kernel.

> > To be
> > convinced i would probably like to see code which positively
> > identifies the dock, and that the USB device is on the correct port
> > of the USB hub within the dock. I doubt you can actually do that in
> > a sane way inside an Ethernet driver. As you say, it will likely
> > lead to unmaintainable spaghetti-code.
> > 
> > I also don't really think the vendor would be keen on adding code
> > which they know will get reverted as soon as it is shown to cause a
> > regression.
> > 
> > So i would prefer to NACK this, and push it to udev rules where you
> > have a complete picture of the hardware and really can identify with
> > 100% certainty it really is the docks NIC.  
> 
> I remember when I did the Dell implementation I tried userspace first.
> 
> Pushing this out to udev has a few other implications I remember
> hitting: 1) You need to also get the value you're supposed to use
> from ACPI BIOS exported some way in userland too.

Sounds like a problem with ACPI in userspace. So the kernel could
expose ACPI in a better shape. Or you will simply need to see what
systemd thinks about a funny "sed | grep | awk | bc" patch to parse
binary. DMI might contain bits, but without clear vendor instructions
we would be guessing (like on ACPI?).

> 2) You can run into race conditions with other device or MAC renaming
> rules. My first try I did it with NM and hit that continually.  So
> you would probably need to land this in systemd or so.

For sure you would end up in systemd (udev). NM is just one of many
options and would be the wrong place. You might quickly find yourself
in mdev (busybox) as well because of that PXE case or because of an
initrd.

If it was my call i would revert all mac passthough patches and request
Lenovo/Dell and others to present their case first hand. (no
canonical/amd/suse proxies)
The "feature" causes MAC collisions and is not well understood/argued by
anyone.

https://en.wikipedia.org/wiki/MAC_address

> A media access control address (MAC address) is a unique identifier
> assigned to a network interface controller (NIC) ...

unique and NIC, as opposed to colliding and machine

Henning
 
> > 
> >    Andre