[PATCH] net: usb: r8152: Add MAC passthrough support for more Lenovo Docks

[PATCH] net: usb: r8152: Add MAC passthrough support for more Lenovo Docks

[Aaron Ma]

Like ThinkaPad Thunderbolt 4 Dock, more Lenovo docks start to use the original
Realtek USB ethernet chip ID 0bda:8153.

Lenovo Docks always use their own IDs for usb hub, even for older Docks.
If parent hub is from Lenovo, then r8152 should try MAC passthrough.
Verified on Lenovo TBT3 dock too.

Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
---
 drivers/net/usb/r8152.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 4a02f33f0643..f9877a3e83ac 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -9603,12 +9603,9 @@ static int rtl8152_probe(struct usb_interface *intf,
 		netdev->hw_features &= ~NETIF_F_RXCSUM;
 	}
 
-	if (le16_to_cpu(udev->descriptor.idVendor) == VENDOR_ID_LENOVO) {
-		switch (le16_to_cpu(udev->descriptor.idProduct)) {
-		case DEVICE_ID_THINKPAD_THUNDERBOLT3_DOCK_GEN2:
-		case DEVICE_ID_THINKPAD_USB_C_DOCK_GEN2:
-			tp->lenovo_macpassthru = 1;
-		}
+	if (udev->parent &&
+			le16_to_cpu(udev->parent->descriptor.idVendor) == VENDOR_ID_LENOVO) {
+		tp->lenovo_macpassthru = 1;
 	}
 
 	if (le16_to_cpu(udev->descriptor.bcdDevice) == 0x3011 && udev->serial &&
-- 
2.25.1

Re: [PATCH] net: usb: r8152: Add MAC passthrough support for more Lenovo Docks

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Tue, 16 Nov 2021 22:19:17 +0800 you wrote:
> Like ThinkaPad Thunderbolt 4 Dock, more Lenovo docks start to use the original
> Realtek USB ethernet chip ID 0bda:8153.
> 
> Lenovo Docks always use their own IDs for usb hub, even for older Docks.
> If parent hub is from Lenovo, then r8152 should try MAC passthrough.
> Verified on Lenovo TBT3 dock too.
> 
> [...]

Here is the summary with links:
  - net: usb: r8152: Add MAC passthrough support for more Lenovo Docks
    https://git.kernel.org/netdev/net/c/f77b83b5bbab

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH] net: usb: r8152: Add MAC passthrough support for more Lenovo Docks

[Henning Schild]

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.

regards,
Henning

Am Tue, 16 Nov 2021 22:19:17 +0800
schrieb Aaron Ma <aaron.ma@canonical.com>:

> Like ThinkaPad Thunderbolt 4 Dock, more Lenovo docks start to use the
> original Realtek USB ethernet chip ID 0bda:8153.
> 
> Lenovo Docks always use their own IDs for usb hub, even for older
> Docks. If parent hub is from Lenovo, then r8152 should try MAC
> passthrough. Verified on Lenovo TBT3 dock too.
> 
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
>  drivers/net/usb/r8152.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 4a02f33f0643..f9877a3e83ac 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -9603,12 +9603,9 @@ static int rtl8152_probe(struct usb_interface
> *intf, netdev->hw_features &= ~NETIF_F_RXCSUM;
>  	}
>  
> -	if (le16_to_cpu(udev->descriptor.idVendor) ==
> VENDOR_ID_LENOVO) {
> -		switch (le16_to_cpu(udev->descriptor.idProduct)) {
> -		case DEVICE_ID_THINKPAD_THUNDERBOLT3_DOCK_GEN2:
> -		case DEVICE_ID_THINKPAD_USB_C_DOCK_GEN2:
> -			tp->lenovo_macpassthru = 1;
> -		}
> +	if (udev->parent &&
> +
> le16_to_cpu(udev->parent->descriptor.idVendor) == VENDOR_ID_LENOVO) {
> +		tp->lenovo_macpassthru = 1;
>  	}
>  
>  	if (le16_to_cpu(udev->descriptor.bcdDevice) == 0x3011 &&
> udev->serial &&

Re: [PATCH] net: usb: r8152: Add MAC passthrough support for more Lenovo Docks

[Jakub Kicinski]

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.

Re: [PATCH] net: usb: r8152: Add MAC passthrough support for more Lenovo Docks

[Henning Schild]

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

Re: [PATCH] net: usb: r8152: Add MAC passthrough support for more Lenovo Docks

[Aaron Ma]

On 1/5/22 01:07, Henning Schild wrote:
> 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.
> 

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.

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.

Aaron


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

Re: [PATCH] net: usb: r8152: Add MAC passthrough support for more Lenovo Docks

[Henning Schild]

Am Wed, 5 Jan 2022 01:40:42 +0800
schrieb Aaron Ma <aaron.ma@canonical.com>:

> On 1/5/22 01:07, Henning Schild wrote:
> > 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. 
> 
> 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".

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

Henning

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

Re: [PATCH] net: usb: r8152: Add MAC passthrough support for more Lenovo Docks

[Jakub Kicinski]

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?

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

Re: [PATCH] net: usb: r8152: Add MAC passthrough support for more Lenovo Docks

[Henning Schild]

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

Re: [PATCH] net: usb: r8152: Add MAC passthrough support for more Lenovo Docks

[Henning Schild]

Am Wed, 5 Jan 2022 01:40:42 +0800
schrieb Aaron Ma <aaron.ma@canonical.com>:

> On 1/5/22 01:07, Henning Schild wrote:
> > 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. 
> 
> 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.
> 
> For now I suggest to disable it in BIOS if you got multiple r8152.

I can do that. But as i expect that to be coming from "managed devices"
where IT departments dream that they can identify a machine by its MAC
... those uses likely can not.

So there should maybe be an additional module param, even if it
defaults to "on" as well.

Henning

> Let me try to make some changes to limit this feature in one r8152.
> 
> Aaron
> 
> 
> > MAC spoofing is something NetworkManager and others can take care
> > of, or udev ... doing that in the driver is ... spooky.
> > 
> > regards,
> > Henning