Re: [PATCH 1/3 v3] net: usb: r8152: Check used MAC passthrough address - Kai-Heng Feng

From: Kai-Heng Feng <kai.heng.feng@canonical.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Oliver Neukum <oneukum@suse.com>,
	Aaron Ma <aaron.ma@canonical.com>,
	kuba@kernel.org, henning.schild@siemens.com,
	linux-usb@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, davem@davemloft.net,
	hayeswang@realtek.com, tiwai@suse.de
Subject: Re: [PATCH 1/3 v3] net: usb: r8152: Check used MAC passthrough address
Date: Fri, 7 Jan 2022 10:01:33 +0800	[thread overview]
Message-ID: <CAAd53p5YnQZ0fDiwwo-q3bNMVFTJSMLcdkUuH-7=OSaRrW954Q@mail.gmail.com> (raw)
In-Reply-To: <YdbuXbtc64+Knbhm@lunn.ch>

On Thu, Jan 6, 2022 at 9:28 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > Can udev in current form really handle the MAC race? Unless there's a
> > new uevent right before ndo_open() so udev can decide which MAC to
> > assign? Even with that, the race can still happen...
>
> There will always be a race, since the kernel can start using the
> interface before register_netdev() has even finished, before user
> space is running. Take a look at how NFS root works.

Didn't know that, thanks for the insight.

>
> But in general, you can change the MAC address at any time. Some MAC
> drivers will return -EBUSY if the interface is up, but that is
> generally artificial. After a change of MAC address ARP will time out
> after a while and the link peers will get the new MAC address.

I think this makes the whole situation even more complex.

>
> >
> > So what if we keep the existing behavior (i.e. copy MAC from ACPI),
> > and let userspace daemon like NetworkManager to give the second NIC
> > (r8152 in this case) a random MAC if the main NIC (I219 in this case)
> > is already in use? Considering the userspace daemon has the all the
> > information required and it's the policy maker here.
>
> You should be thinking of this in more general terms. You want to
> design a system that will work for any vendors laptop and dock.
>
> You need to describe the two interfaces using some sort of bus
> address, be it PCIe, USB, or a platform device address as used by
> device tree etc.
>
> Let the kernel do whatever it wants with MAC addresses for these two
> interfaces. The only requirement you have is that the laptop internal
> interface gets the vendor allocated MAC address, and that the dock get
> some sort of MAC address, even if it is random.

Those laptops and docks are designed to have duplicated MACs. I don't
understand why but that's why Dell/HP/Lenovo did.
What if the kernel just abstract the hardware/firmware as intended, no
matter how stupid it is, and let userspace to make the right policy?

>
> On device creation, udev can check if it now has both interfaces? If
> the internal interface is up, it is probably in use. Otherwise, take
> its MAC address and assign it to the dock interface, and give the
> internal interface a random MAC address, just in case.
>
> You probably need to delay NetworkManager, systemd-networkkd,
> /etc/network/interfaces etc, so that they don't do anything until
> after udevd has settled, indicating all devices have probably been
> found.

I don't think it's a good idea. On my laptop,
systemd-udev-settle.service can add extra 5~10 seconds boot time
delay.
Furthermore, the external NIC in question is in a USB/Thunderbolt
dock, it can present pre-boot, or it can be hotplugged at any time.

>
> I suspect you will never get a 100% robust design, but you can
> probably get it working enough for the cleaning staff and the CEO, who
> have very simple setups. Power users are going to find all the corner
> cases and will want to disable the udev rule.

But power users may also need to use corporate network to work as
Aaron mentioned.
Packets from unregistered MAC can be filtered under corporate network,
and that's why MAC pass-through is a useful feature that many business
laptops have.

>
>      Andrew