Message-ID: <BL1PR12MB5157796F50C523D32F0F3C5DE2229@BL1PR12MB5157.namprd12.prod.outlook.com>
Date:   Fri, 28 Jan 2022 20:29:58 +0000
From:   "Limonciello, Mario" <Mario.Limonciello@....com>
To:     Henning Schild <henning.schild@...mens.com>
CC:     Andrew Lunn <andrew@...n.ch>, Aaron Ma <aaron.ma@...onical.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "hayeswang@...ltek.com" <hayeswang@...ltek.com>,
        "tiwai@...e.de" <tiwai@...e.de>
Subject: RE: [PATCH v3] net: usb: r8152: Add MAC passthrough support for
 RTL8153BL

[Public]

> > > 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?).

Yeah I think if this is to be reverted, step 1 is going to be to export that
data into sysfs from some Dell and Lenovo drivers so userspace can get it.
No funny sed/grep/awk to parse binary.

DMI doesn't contain it (at least for Dell).

> 
> > 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.

Reverting it has the possibility to really mess up machines people have in the
field that have behavior built around it.  I think a clear set of rules for what is allow
to use it is the only safe way forward.  You need to "clearly identify the device" or
something.

Just FYI I'm not intentionally acting as a proxy to anyone at Dell on behalf of AMD,
my only involvement here is because I did the original implementation for Dell
when I was there and so I can speak the how things were and thought processes at
that time.

If the consensus is to revert this then someone from Dell probably does need to
speak up.

> 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.wikip
> edia.org%2Fwiki%2FMAC_address&amp;data=04%7C01%7CMario.Limonciello%
> 40amd.com%7C385b6dd02672490749d208d9e29ba192%7C3dd8961fe4884e60
> 8e11a82d994e183d%7C0%7C0%7C637789980357612909%7CUnknown%7CTWF
> pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
> Mn0%3D%7C3000&amp;sdata=9vmPi1%2FF%2BxrkqpjoYoLmmupAJ1vBMckLLo
> 5hDMqTuZA%3D&amp;reserved=0
> 
> > 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

Powered by blists - more mailing lists