Message ID | 20181010142933.31051-1-oneukum@suse.com |
---|---|
State | Rejected, archived |
Delegated to: | David Miller |
Headers | show |
Series | r8152: limit MAC pass-through to one device | expand |
On Mi, 2018-10-10 at 17:18 +0000, Mario.Limonciello@dell.com wrote: > > > > MAC address having to be unique, a MAC coming from the host > > must be used at most once at a time. Hence the users must > > be recorded and additional users must fall back to conventional > > methods. > > I checked with the internal team and actually applies pass through MAC on both > Windows Realtek driver and in UEFI network stack this applies to ALL supported > Realtek devices (R8153-AD). I may have formulated this badly. What happens if you attach two devices of this type to the same host? Regards Oliver
> > MAC address having to be unique, a MAC coming from the host > must be used at most once at a time. Hence the users must > be recorded and additional users must fall back to conventional > methods. I checked with the internal team and actually applies pass through MAC on both Windows Realtek driver and in UEFI network stack this applies to ALL supported Realtek devices (R8153-AD). > > Signed-off-by: Oliver Neukum <oneukum@suse.com> > Fixes: 34ee32c9a5696 ("r8152: Add support for setting pass through MAC address on > RTL8153-AD") > --- > drivers/net/usb/r8152.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c > index f1b5201cc320..7345a2258ee4 100644 > --- a/drivers/net/usb/r8152.c > +++ b/drivers/net/usb/r8152.c > @@ -766,6 +766,9 @@ enum tx_csum_stat { > TX_CSUM_NONE > }; > > +/* pass through MACs are per host, hence concurrent use is forbidden */ > +static struct r8152 *pass_through_user = NULL; > + > /* Maximum number of multicast addresses to filter (vs. Rx-all-multicast). > * The RTL chips use a 64 element hash table based on the Ethernet CRC. > */ > @@ -1221,7 +1224,14 @@ static int set_ethernet_addr(struct r8152 *tp) > * or system doesn't provide valid _SB.AMAC this will be > * be expected to non-zero > */ > - ret = vendor_mac_passthru_addr_read(tp, &sa); > + if (!pass_through_user) { > + ret = vendor_mac_passthru_addr_read(tp, &sa); > + if (ret >= 0) > + /* we must record the user against concurrent use > */ > + pass_through_user = tp; > + } else { > + ret = -EBUSY; > + } > if (ret < 0) > ret = pla_ocp_read(tp, PLA_BACKUP, 8, sa.sa_data); > } > @@ -5304,6 +5314,8 @@ static void rtl8152_disconnect(struct usb_interface *intf) > cancel_delayed_work_sync(&tp->hw_phy_work); > tp->rtl_ops.unload(tp); > free_netdev(tp->netdev); > + if (pass_through_user == tp) > + pass_through_user = NULL; > } > } > > -- > 2.16.4
From: Oliver Neukum <oneukum@suse.com> Date: Wed, 10 Oct 2018 19:17:05 +0200 > On Mi, 2018-10-10 at 17:18 +0000, Mario.Limonciello@dell.com wrote: >> > >> > MAC address having to be unique, a MAC coming from the host >> > must be used at most once at a time. Hence the users must >> > be recorded and additional users must fall back to conventional >> > methods. >> >> I checked with the internal team and actually applies pass through MAC on both >> Windows Realtek driver and in UEFI network stack this applies to ALL supported >> Realtek devices (R8153-AD). > > I may have formulated this badly. What happens if you attach two > devices of this type to the same host? If the devices are on different physical network segments it will work. This was common back in the day even. Old Sparc machines had a host defined MAC address, and even with multi-port cards they would all use that same MAC address unless the firmware nodes had individual device MAC addresses defined. I think it's a valid configuration. But I can see that elements of userspace such as udev will likely not be happy because their device naming schemes are built on the assumption that all MAC addresses are unique.
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index f1b5201cc320..7345a2258ee4 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -766,6 +766,9 @@ enum tx_csum_stat { TX_CSUM_NONE }; +/* pass through MACs are per host, hence concurrent use is forbidden */ +static struct r8152 *pass_through_user = NULL; + /* Maximum number of multicast addresses to filter (vs. Rx-all-multicast). * The RTL chips use a 64 element hash table based on the Ethernet CRC. */ @@ -1221,7 +1224,14 @@ static int set_ethernet_addr(struct r8152 *tp) * or system doesn't provide valid _SB.AMAC this will be * be expected to non-zero */ - ret = vendor_mac_passthru_addr_read(tp, &sa); + if (!pass_through_user) { + ret = vendor_mac_passthru_addr_read(tp, &sa); + if (ret >= 0) + /* we must record the user against concurrent use */ + pass_through_user = tp; + } else { + ret = -EBUSY; + } if (ret < 0) ret = pla_ocp_read(tp, PLA_BACKUP, 8, sa.sa_data); } @@ -5304,6 +5314,8 @@ static void rtl8152_disconnect(struct usb_interface *intf) cancel_delayed_work_sync(&tp->hw_phy_work); tp->rtl_ops.unload(tp); free_netdev(tp->netdev); + if (pass_through_user == tp) + pass_through_user = NULL; } }
MAC address having to be unique, a MAC coming from the host must be used at most once at a time. Hence the users must be recorded and additional users must fall back to conventional methods. Signed-off-by: Oliver Neukum <oneukum@suse.com> Fixes: 34ee32c9a5696 ("r8152: Add support for setting pass through MAC address on RTL8153-AD") --- drivers/net/usb/r8152.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)