Messages in this thread | | | Date | Wed, 5 Jan 2022 16:01:24 +0800 | Subject | Re: [PATCH] net: usb: r8152: Check used MAC passthrough address | From | Aaron Ma <> |
| |
On 1/5/22 15:55, Henning Schild wrote: > Am Wed, 5 Jan 2022 15:38:51 +0800 > schrieb Aaron Ma <aaron.ma@canonical.com>: > >> On 1/5/22 15:32, Henning Schild wrote: >>> Am Wed, 5 Jan 2022 08:23:55 +0100 >>> schrieb Henning Schild <henning.schild@siemens.com>: >>> >>>> Hi Aaron, >>>> >>>> if this or something similar goes in, please add another patch to >>>> remove the left-over defines. >>>> >> >> Sure, I will do it. >> >>>> Am Wed, 5 Jan 2022 14:17:47 +0800 >>>> schrieb Aaron Ma <aaron.ma@canonical.com>: >>>> >>>>> When plugin multiple r8152 ethernet dongles to Lenovo Docks >>>>> or USB hub, MAC passthrough address from BIOS should be >>>>> checked if it had been used to avoid using on other dongles. >>>>> >>>>> Currently builtin r8152 on Dock still can't be identified. >>>>> First detected r8152 will use the MAC passthrough address. >>>>> >>>>> Signed-off-by: Aaron Ma <aaron.ma@canonical.com> >>>>> --- >>>>> drivers/net/usb/r8152.c | 10 ++++++++++ >>>>> 1 file changed, 10 insertions(+) >>>>> >>>>> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c >>>>> index f9877a3e83ac..77f11b3f847b 100644 >>>>> --- a/drivers/net/usb/r8152.c >>>>> +++ b/drivers/net/usb/r8152.c >>>>> @@ -1605,6 +1605,7 @@ static int >>>>> vendor_mac_passthru_addr_read(struct r8152 *tp, struct sockaddr >>>>> *sa) char *mac_obj_name; acpi_object_type mac_obj_type; >>>>> int mac_strlen; >>>>> + struct net_device *ndev; >>>>> >>>>> if (tp->lenovo_macpassthru) { >>>>> mac_obj_name = "\\MACA"; >>>>> @@ -1662,6 +1663,15 @@ static int >>>>> vendor_mac_passthru_addr_read(struct r8152 *tp, struct sockaddr >>>>> *sa) ret = -EINVAL; goto amacout; >>>>> } >>>>> + rcu_read_lock(); >>>>> + for_each_netdev_rcu(&init_net, ndev) { >>>>> + if (strncmp(buf, ndev->dev_addr, 6) == 0) { >>>>> + rcu_read_unlock(); >>>>> + goto amacout; >>>> >>>> Since the original PCI netdev will always be there, that would >>>> disable inheritance would it not? >>>> I guess a strncmp(MODULE_NAME, info->driver, strlen(MODULE_NAME)) >>>> is needed as well. >>>> >> >> PCI ethernet could be a builtin one on dock since there will be TBT4 >> dock. > > In my X280 there is a PCI device in the laptop, always there. And its > MAC is the one found in ACPI. Did not try but i think for such devices > there would never be inheritance even if one wanted and used a Lenovo > dock that is supposed to do it. >
There will more TBT4 docks in market, the new ethernet is just the same as PCI device, connected by thunderbolt.
For exmaple, connect a TBT4 dock which uses i225 pcie base ethernet, then connect another TBT3 dock which uses r8152. If skip PCI check, then i225 and r8152 will use the same MAC.
Aaron
> Maybe i should try the patch but it seems like it defeats inheritance > completely. Well depending on probe order ... > > regards, > Henning > > >>>> Maybe leave here with >>>> netif_info() >>>> >> >> Not good to print in rcu lock. >> >>>> And move the whole block up, we can skip the whole ACPI story if we >>>> find the MAC busy. >>> >>> That is wrong, need to know that MAC so can not move up too much. >>> But maybe above the is_valid_ether_addr >> >> The MAC passthough address is read from ACPI. >> ACPI read only happens once during r8152 driver probe. >> To keep the lock less time, do it after is_valid_ether_addr. >> >>> >>> Henning >>> >>>>> + } >>>>> + } >>>>> + rcu_read_unlock(); >>>> >>>> Not sure if this function is guaranteed to only run once at a time, >>>> otherwise i think that is a race. Multiple instances could make it >>>> to this very point at the same time. >>>> >> >> Run once for one device. >> So add a safe lock. >> >> Aaron >> >>>> Henning >>>> >>>>> memcpy(sa->sa_data, buf, 6); >>>>> netif_info(tp, probe, tp->netdev, >>>>> "Using pass-thru MAC addr %pM\n", >>>>> sa->sa_data); >>>> >>> >
| |