[PATCH] r8152: limit MAC pass-through to one device

[PATCH] r8152: limit MAC pass-through to one device

[Oliver Neukum]

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(-)

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

RE: [PATCH] r8152: limit MAC pass-through to one device

[Mario.Limonciello]

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

Re: [PATCH] r8152: limit MAC pass-through to one device

[Oliver Neukum]

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

Re: [PATCH] r8152: limit MAC pass-through to one device

[David Miller]

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.