linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
@ 2016-06-07 18:22 Mario Limonciello
  2016-06-11  5:51 ` David Miller
  0 siblings, 1 reply; 16+ messages in thread
From: Mario Limonciello @ 2016-06-07 18:22 UTC (permalink / raw)
  To: hayeswang
  Cc: LKML, Netdev, Linux USB, pali.rohar, anthony.wong, Greg KH,
	Mario Limonciello

The RTL8153-AD supports a persistent system specific MAC address.
This means a device plugged into two different systems with host side
support will show different (but persistent) MAC addresses.

This information for the system's persistent MAC address is burned in when
the system HW is built and available under \_SB.AMAC in the DSDT at runtime.

This technology is currently implemented in the Dell TB15 and WD15 Type-C
docks.  More information is available here:
http://www.dell.com/support/article/us/en/04/SLN301147

Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
---
Changes from v5:
 * Correct return value if hex2bin succesful but invalid ether addr

 drivers/net/usb/r8152.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 74 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 3f9f6ed..fc58669 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -26,6 +26,7 @@
 #include <linux/mdio.h>
 #include <linux/usb/cdc.h>
 #include <linux/suspend.h>
+#include <linux/acpi.h>
 
 /* Information for net-next */
 #define NETNEXT_VERSION		"08"
@@ -455,6 +456,11 @@
 /* SRAM_IMPEDANCE */
 #define RX_DRIVING_MASK		0x6000
 
+/* MAC PASSTHRU */
+#define AD_MASK			0xfee0
+#define EFUSE			0xcfdb
+#define PASS_THRU_MASK		0x1
+
 enum rtl_register_content {
 	_1000bps	= 0x10,
 	_100bps		= 0x08,
@@ -1030,6 +1036,65 @@ out1:
 	return ret;
 }
 
+/* Devices containing RTL8153-AD can support a persistent
+ * host system provided MAC address.
+ * Examples of this are Dell TB15 and Dell WD15 docks
+ */
+static int vendor_mac_passthru_addr_read(struct r8152 *tp, struct sockaddr *sa)
+{
+	acpi_status status;
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *obj;
+	int ret = -EINVAL;
+	u32 ocp_data;
+	unsigned char buf[6];
+
+	/* test for -AD variant of RTL8153 */
+	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0);
+	if ((ocp_data & AD_MASK) != 0x1000)
+		return -ENODEV;
+
+	/* test for MAC address pass-through bit */
+	ocp_data = ocp_read_byte(tp, MCU_TYPE_USB, EFUSE);
+	if ((ocp_data & PASS_THRU_MASK) != 1)
+		return -ENODEV;
+
+	/* returns _AUXMAC_#AABBCCDDEEFF# */
+	status = acpi_evaluate_object(NULL, "\\_SB.AMAC", NULL, &buffer);
+	obj = (union acpi_object *)buffer.pointer;
+	if (!ACPI_SUCCESS(status))
+		return -ENODEV;
+	if (obj->type != ACPI_TYPE_BUFFER || obj->string.length != 0x17) {
+		netif_warn(tp, probe, tp->netdev,
+			   "Invalid buffer when reading pass-thru MAC addr: "
+			   "(%d, %d)\n",
+			   obj->type, obj->string.length);
+		goto amacout;
+	}
+	if (strncmp(obj->string.pointer, "_AUXMAC_#", 9) != 0 ||
+	    strncmp(obj->string.pointer + 0x15, "#", 1) != 0) {
+		netif_warn(tp, probe, tp->netdev,
+			   "Invalid header when reading pass-thru MAC addr\n");
+		goto amacout;
+	}
+	ret = hex2bin(buf, obj->string.pointer + 9, 6);
+	if (!(ret == 0 && is_valid_ether_addr(buf))) {
+		netif_warn(tp, probe, tp->netdev,
+			   "Invalid MAC when reading pass-thru MAC addr: "
+			   "%d, %pM\n", ret, buf);
+		ret = -EINVAL;
+		goto amacout;
+	}
+	memcpy(sa->sa_data, buf, 6);
+	ether_addr_copy(tp->netdev->dev_addr, sa->sa_data);
+	netif_info(tp, probe, tp->netdev,
+		   "Using pass-thru MAC addr %pM\n", sa->sa_data);
+
+amacout:
+	kfree(obj);
+	return ret;
+}
+
 static int set_ethernet_addr(struct r8152 *tp)
 {
 	struct net_device *dev = tp->netdev;
@@ -1038,8 +1103,15 @@ static int set_ethernet_addr(struct r8152 *tp)
 
 	if (tp->version == RTL_VER_01)
 		ret = pla_ocp_read(tp, PLA_IDR, 8, sa.sa_data);
-	else
-		ret = pla_ocp_read(tp, PLA_BACKUP, 8, sa.sa_data);
+	else {
+		/* if this is not an RTL8153-AD, no eFuse mac pass thru set,
+		 * 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 (ret < 0)
+			ret = pla_ocp_read(tp, PLA_BACKUP, 8, sa.sa_data);
+	}
 
 	if (ret < 0) {
 		netif_err(tp, probe, dev, "Get ether addr fail\n");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
  2016-06-07 18:22 [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD Mario Limonciello
@ 2016-06-11  5:51 ` David Miller
  2016-06-11 15:39   ` Andrew Lunn
  0 siblings, 1 reply; 16+ messages in thread
From: David Miller @ 2016-06-11  5:51 UTC (permalink / raw)
  To: mario_limonciello
  Cc: hayeswang, linux-kernel, netdev, linux-usb, pali.rohar,
	anthony.wong, gregkh

From: Mario Limonciello <mario_limonciello@dell.com>
Date: Tue,  7 Jun 2016 13:22:37 -0500

> The RTL8153-AD supports a persistent system specific MAC address.
> This means a device plugged into two different systems with host side
> support will show different (but persistent) MAC addresses.
> 
> This information for the system's persistent MAC address is burned in when
> the system HW is built and available under \_SB.AMAC in the DSDT at runtime.
> 
> This technology is currently implemented in the Dell TB15 and WD15 Type-C
> docks.  More information is available here:
> http://www.dell.com/support/article/us/en/04/SLN301147
> 
> Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
> ---
> Changes from v5:
>  * Correct return value if hex2bin succesful but invalid ether addr

Have things calmed down enough now that I can apply this?

Let's see some ACKs.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
  2016-06-11  5:51 ` David Miller
@ 2016-06-11 15:39   ` Andrew Lunn
  2016-06-11 17:42     ` David Miller
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Lunn @ 2016-06-11 15:39 UTC (permalink / raw)
  To: David Miller
  Cc: mario_limonciello, hayeswang, linux-kernel, netdev, linux-usb,
	pali.rohar, anthony.wong, gregkh

On Fri, Jun 10, 2016 at 10:51:56PM -0700, David Miller wrote:
> From: Mario Limonciello <mario_limonciello@dell.com>
> Date: Tue,  7 Jun 2016 13:22:37 -0500
> 
> > The RTL8153-AD supports a persistent system specific MAC address.
> > This means a device plugged into two different systems with host side
> > support will show different (but persistent) MAC addresses.
> > 
> > This information for the system's persistent MAC address is burned in when
> > the system HW is built and available under \_SB.AMAC in the DSDT at runtime.
> > 
> > This technology is currently implemented in the Dell TB15 and WD15 Type-C
> > docks.  More information is available here:
> > http://www.dell.com/support/article/us/en/04/SLN301147
> > 
> > Signed-off-by: Mario Limonciello <mario_limonciello@dell.com>
> > ---
> > Changes from v5:
> >  * Correct return value if hex2bin succesful but invalid ether addr
> 
> Have things calmed down enough now that I can apply this?

Hi David

I think the code has reaching the level of maturity needed for
acceptance. So for the code quality:

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

What is still open is do we want to accept it at all? Do we accept the
concept of putting the same MAC address on multiple interfaces at
hotplug time? Do we trust BIOS vendors to not keep changing DSDT
property name, since it is not standardised?

Do we want this at all should be decided by somebody more senior then
those passing comments on the code.

	Andrew

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
  2016-06-11 15:39   ` Andrew Lunn
@ 2016-06-11 17:42     ` David Miller
  2016-06-14 15:08       ` Mario_Limonciello
  2016-06-14 16:28       ` Pali Rohár
  0 siblings, 2 replies; 16+ messages in thread
From: David Miller @ 2016-06-11 17:42 UTC (permalink / raw)
  To: andrew
  Cc: mario_limonciello, hayeswang, linux-kernel, netdev, linux-usb,
	pali.rohar, anthony.wong, gregkh

From: Andrew Lunn <andrew@lunn.ch>
Date: Sat, 11 Jun 2016 17:39:21 +0200

> What is still open is do we want to accept it at all? Do we accept the
> concept of putting the same MAC address on multiple interfaces at
> hotplug time? Do we trust BIOS vendors to not keep changing DSDT
> property name, since it is not standardised?
> 
> Do we want this at all should be decided by somebody more senior then
> those passing comments on the code.

Indeed, I think the behavior of using the same MAC address on multiple
interfaces if we plug several of these in at once is not good.

We shouldn't behave this way just because the Microsoft driver does.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
  2016-06-11 17:42     ` David Miller
@ 2016-06-14 15:08       ` Mario_Limonciello
  2016-06-14 16:28       ` Pali Rohár
  1 sibling, 0 replies; 16+ messages in thread
From: Mario_Limonciello @ 2016-06-14 15:08 UTC (permalink / raw)
  To: davem, andrew
  Cc: hayeswang, linux-kernel, netdev, linux-usb, pali.rohar,
	anthony.wong, gregkh

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Saturday, June 11, 2016 12:42 PM
> To: andrew@lunn.ch
> Cc: Limonciello, Mario <Mario_Limonciello@Dell.com>;
> hayeswang@realtek.com; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; linux-usb@vger.kernel.org; pali.rohar@gmail.com;
> anthony.wong@canonical.com; gregkh@linuxfoundation.org
> Subject: Re: [PATCH v6] r8152: Add support for setting pass through MAC
> address on RTL8153-AD
> 
> From: Andrew Lunn <andrew@lunn.ch>
> Date: Sat, 11 Jun 2016 17:39:21 +0200
> 
> > What is still open is do we want to accept it at all? Do we accept the
> > concept of putting the same MAC address on multiple interfaces at
> > hotplug time? Do we trust BIOS vendors to not keep changing DSDT
> > property name, since it is not standardised?
> >

It's worth saying - standardized a property name doesn't indemnify it.
Properties change all the time from one version of a spec to another.

I can only speak for Dell, but it's in our best interest to keep the BIOS
side of the codebase around this simpler too.

> > Do we want this at all should be decided by somebody more senior then
> > those passing comments on the code.
> 
> Indeed, I think the behavior of using the same MAC address on multiple
> interfaces if we plug several of these in at once is not good.
> 
> We shouldn't behave this way just because the Microsoft driver does.

This is really grasping at an extreme corner case scenario.

Dell TB15 and WD15 docks are currently only ones on the market with
RTL8135-AD and MAC address pass through efuse bit set.  These docks
are not inexpensive.  If someone really wants multiple USB NIC's plugged
in, they can pick up a second USB NIC from the web for far cheaper than
buying a second dock.

Also for what it's worth, the docks don't allow daisy chaining.  The dock EC's 
will reject the second dock from functioning through the downstream 
connection.  The only way that two docks could be hooked up and
functional is on a machine with multiple type C ports.

If you still think it's worth solving, what would you like done as an 
alternative?  I would really like to have some implementation of this
that you guys are comfortable with upstream.

There was already discussion and an implementation in this 
thread about tracking if the aux MAC was assigned to something and only 
allowing one device to get that assignment.  There was a limitation that 
you won't be able to know which device gets the auxiliary MAC address.  
It will be based solely upon hotplug order.

There was also in one version of the patch a way to turn off this behavior
In a module parameter.  Greg KH wasn't fond of that, so it's not present
in the current version.

I can add either of those back in if they would help the case.

Another option I wanted to offer was turning this behavior on via kernel
configuration option and let the distros decide if they want to turn it on
for users.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
  2016-06-11 17:42     ` David Miller
  2016-06-14 15:08       ` Mario_Limonciello
@ 2016-06-14 16:28       ` Pali Rohár
  2016-06-14 16:40         ` Greg KH
  1 sibling, 1 reply; 16+ messages in thread
From: Pali Rohár @ 2016-06-14 16:28 UTC (permalink / raw)
  To: David Miller
  Cc: andrew, mario_limonciello, hayeswang, linux-kernel, netdev,
	linux-usb, anthony.wong, gregkh

[-- Attachment #1: Type: Text/Plain, Size: 1530 bytes --]

On Saturday 11 June 2016 19:42:26 David Miller wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> Date: Sat, 11 Jun 2016 17:39:21 +0200
> 
> > What is still open is do we want to accept it at all? Do we accept
> > the concept of putting the same MAC address on multiple interfaces
> > at hotplug time? Do we trust BIOS vendors to not keep changing
> > DSDT property name, since it is not standardised?
> > 
> > Do we want this at all should be decided by somebody more senior
> > then those passing comments on the code.
> 
> Indeed, I think the behavior of using the same MAC address on
> multiple interfaces if we plug several of these in at once is not
> good.
> 
> We shouldn't behave this way just because the Microsoft driver does.

I agree, but in some cases it is night mare for local admins when 
booting different OS cause changing MAC address on local network.

Another similar situation: Imagine that you have two USB network cards 
and both have "burned" into their registers same MAC address. If you 
connect both those USB network cards, linux kernel bind appropriate 
driver which read MAC address for both those cards. But those addresses 
are same. What will linux kernel do in this case?

This is very similar situation as those Dell usb network cards told us 
"hey, use address which is in ACPI DSDT table".

Either we should trust what network card what told us, or not and then 
generate MAC addresses in better way.

Just my opinion...

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
  2016-06-14 16:28       ` Pali Rohár
@ 2016-06-14 16:40         ` Greg KH
  2016-06-14 16:47           ` Pali Rohár
  0 siblings, 1 reply; 16+ messages in thread
From: Greg KH @ 2016-06-14 16:40 UTC (permalink / raw)
  To: Pali Rohár
  Cc: David Miller, andrew, mario_limonciello, hayeswang, linux-kernel,
	netdev, linux-usb, anthony.wong

On Tue, Jun 14, 2016 at 06:28:10PM +0200, Pali Rohár wrote:
> On Saturday 11 June 2016 19:42:26 David Miller wrote:
> > From: Andrew Lunn <andrew@lunn.ch>
> > Date: Sat, 11 Jun 2016 17:39:21 +0200
> > 
> > > What is still open is do we want to accept it at all? Do we accept
> > > the concept of putting the same MAC address on multiple interfaces
> > > at hotplug time? Do we trust BIOS vendors to not keep changing
> > > DSDT property name, since it is not standardised?
> > > 
> > > Do we want this at all should be decided by somebody more senior
> > > then those passing comments on the code.
> > 
> > Indeed, I think the behavior of using the same MAC address on
> > multiple interfaces if we plug several of these in at once is not
> > good.
> > 
> > We shouldn't behave this way just because the Microsoft driver does.
> 
> I agree, but in some cases it is night mare for local admins when 
> booting different OS cause changing MAC address on local network.
> 
> Another similar situation: Imagine that you have two USB network cards 
> and both have "burned" into their registers same MAC address. If you 
> connect both those USB network cards, linux kernel bind appropriate 
> driver which read MAC address for both those cards. But those addresses 
> are same. What will linux kernel do in this case?

If you can find such a broken USB device, try it and see :)

(hint, might be hard to find, I've never seen such a device before.)

I don't see how that pertains to this issue, sorry, how does broken USB
hardware compare to a working Dell device?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
  2016-06-14 16:40         ` Greg KH
@ 2016-06-14 16:47           ` Pali Rohár
  2016-06-14 16:55             ` Mario_Limonciello
  2016-06-14 18:35             ` David Miller
  0 siblings, 2 replies; 16+ messages in thread
From: Pali Rohár @ 2016-06-14 16:47 UTC (permalink / raw)
  To: Greg KH
  Cc: David Miller, andrew, mario_limonciello, hayeswang, linux-kernel,
	netdev, linux-usb, anthony.wong

[-- Attachment #1: Type: Text/Plain, Size: 2132 bytes --]

On Tuesday 14 June 2016 18:40:17 Greg KH wrote:
> On Tue, Jun 14, 2016 at 06:28:10PM +0200, Pali Rohár wrote:
> > On Saturday 11 June 2016 19:42:26 David Miller wrote:
> > > From: Andrew Lunn <andrew@lunn.ch>
> > > Date: Sat, 11 Jun 2016 17:39:21 +0200
> > > 
> > > > What is still open is do we want to accept it at all? Do we
> > > > accept the concept of putting the same MAC address on multiple
> > > > interfaces at hotplug time? Do we trust BIOS vendors to not
> > > > keep changing DSDT property name, since it is not
> > > > standardised?
> > > > 
> > > > Do we want this at all should be decided by somebody more
> > > > senior then those passing comments on the code.
> > > 
> > > Indeed, I think the behavior of using the same MAC address on
> > > multiple interfaces if we plug several of these in at once is not
> > > good.
> > > 
> > > We shouldn't behave this way just because the Microsoft driver
> > > does.
> > 
> > I agree, but in some cases it is night mare for local admins when
> > booting different OS cause changing MAC address on local network.
> > 
> > Another similar situation: Imagine that you have two USB network
> > cards and both have "burned" into their registers same MAC
> > address. If you connect both those USB network cards, linux kernel
> > bind appropriate driver which read MAC address for both those
> > cards. But those addresses are same. What will linux kernel do in
> > this case?
> 
> If you can find such a broken USB device, try it and see :)

What do you mean by broken USB device?

You have never seen two ethernet cards with same MAC addresses? Right I 
have not seen two USB, but there is non zero chance that could happen. 
Specially now when more and more people starts using USB network cards.

> (hint, might be hard to find, I've never seen such a device before.)
> 
> I don't see how that pertains to this issue, sorry, how does broken
> USB hardware compare to a working Dell device?

It is same, how to handle two network cards which tell us, that they 
have same MAC addresses.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
  2016-06-14 16:47           ` Pali Rohár
@ 2016-06-14 16:55             ` Mario_Limonciello
  2016-06-14 17:23               ` Andrew Lunn
  2016-06-14 18:35             ` David Miller
  1 sibling, 1 reply; 16+ messages in thread
From: Mario_Limonciello @ 2016-06-14 16:55 UTC (permalink / raw)
  To: pali.rohar, gregkh
  Cc: davem, andrew, hayeswang, linux-kernel, netdev, linux-usb, anthony.wong

> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Tuesday, June 14, 2016 11:48 AM
> To: Greg KH <gregkh@linuxfoundation.org>
> Cc: David Miller <davem@davemloft.net>; andrew@lunn.ch; Limonciello,
> Mario <Mario_Limonciello@Dell.com>; hayeswang@realtek.com; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org; linux-
> usb@vger.kernel.org; anthony.wong@canonical.com
> Subject: Re: [PATCH v6] r8152: Add support for setting pass through MAC
> address on RTL8153-AD
> 
> On Tuesday 14 June 2016 18:40:17 Greg KH wrote:
> > On Tue, Jun 14, 2016 at 06:28:10PM +0200, Pali Rohár wrote:
> > > On Saturday 11 June 2016 19:42:26 David Miller wrote:
> > > > From: Andrew Lunn <andrew@lunn.ch>
> > > > Date: Sat, 11 Jun 2016 17:39:21 +0200
> > > >
> > > > > What is still open is do we want to accept it at all? Do we
> > > > > accept the concept of putting the same MAC address on multiple
> > > > > interfaces at hotplug time? Do we trust BIOS vendors to not
> > > > > keep changing DSDT property name, since it is not
> > > > > standardised?
> > > > >
> > > > > Do we want this at all should be decided by somebody more
> > > > > senior then those passing comments on the code.
> > > >
> > > > Indeed, I think the behavior of using the same MAC address on
> > > > multiple interfaces if we plug several of these in at once is not
> > > > good.
> > > >
> > > > We shouldn't behave this way just because the Microsoft driver
> > > > does.
> > >
> > > I agree, but in some cases it is night mare for local admins when
> > > booting different OS cause changing MAC address on local network.
> > >
> > > Another similar situation: Imagine that you have two USB network
> > > cards and both have "burned" into their registers same MAC
> > > address. If you connect both those USB network cards, linux kernel
> > > bind appropriate driver which read MAC address for both those
> > > cards. But those addresses are same. What will linux kernel do in
> > > this case?
> >
> > If you can find such a broken USB device, try it and see :)
> 
> What do you mean by broken USB device?
> 
> You have never seen two ethernet cards with same MAC addresses? Right I
> have not seen two USB, but there is non zero chance that could happen.
> Specially now when more and more people starts using USB network cards.
> 
> > (hint, might be hard to find, I've never seen such a device before.)
> >
> > I don't see how that pertains to this issue, sorry, how does broken
> > USB hardware compare to a working Dell device?
> 
> It is same, how to handle two network cards which tell us, that they
> have same MAC addresses.
> 

The kernel handles this just fine.  In doing this patch I checked to see
what it does in that scenario.  Two devices are made.  systemd doesn't
rename the second device via the MAC name (eg enxAABBCCDDEEFF).

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
  2016-06-14 16:55             ` Mario_Limonciello
@ 2016-06-14 17:23               ` Andrew Lunn
  2016-06-14 17:58                 ` Mario_Limonciello
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Lunn @ 2016-06-14 17:23 UTC (permalink / raw)
  To: Mario_Limonciello
  Cc: pali.rohar, gregkh, davem, hayeswang, linux-kernel, netdev,
	linux-usb, anthony.wong

> > It is same, how to handle two network cards which tell us, that they
> > have same MAC addresses.
> > 
> 
> The kernel handles this just fine.  In doing this patch I checked to see
> what it does in that scenario.  Two devices are made.  systemd doesn't
> rename the second device via the MAC name (eg enxAABBCCDDEEFF).

What does you dhcp server do? Does it gives out the same IP address?
You then have two interfaces on the same network, with the same MAC
address and IP address. Then what happens?

     Andrew

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
  2016-06-14 17:23               ` Andrew Lunn
@ 2016-06-14 17:58                 ` Mario_Limonciello
  0 siblings, 0 replies; 16+ messages in thread
From: Mario_Limonciello @ 2016-06-14 17:58 UTC (permalink / raw)
  To: andrew
  Cc: pali.rohar, gregkh, davem, hayeswang, linux-kernel, netdev,
	linux-usb, anthony.wong

> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Tuesday, June 14, 2016 12:23 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: pali.rohar@gmail.com; gregkh@linuxfoundation.org;
> davem@davemloft.net; hayeswang@realtek.com; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org; linux-
> usb@vger.kernel.org; anthony.wong@canonical.com
> Subject: Re: [PATCH v6] r8152: Add support for setting pass through MAC
> address on RTL8153-AD
> 
> > > It is same, how to handle two network cards which tell us, that they
> > > have same MAC addresses.
> > >
> >
> > The kernel handles this just fine.  In doing this patch I checked to see
> > what it does in that scenario.  Two devices are made.  systemd doesn't
> > rename the second device via the MAC name (eg enxAABBCCDDEEFF).
> 
> What does you dhcp server do? Does it gives out the same IP address?
> You then have two interfaces on the same network, with the same MAC
> address and IP address. Then what happens?
> 
>      Andrew

I didn't test it on the same network, I used two separate networks.

I expect that the DHCP server would be awfully confused and you'd
run down an interesting problem path if it got the same MAC twice.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
  2016-06-14 16:47           ` Pali Rohár
  2016-06-14 16:55             ` Mario_Limonciello
@ 2016-06-14 18:35             ` David Miller
  2016-06-14 22:27               ` Mario_Limonciello
                                 ` (2 more replies)
  1 sibling, 3 replies; 16+ messages in thread
From: David Miller @ 2016-06-14 18:35 UTC (permalink / raw)
  To: pali.rohar
  Cc: gregkh, andrew, mario_limonciello, hayeswang, linux-kernel,
	netdev, linux-usb, anthony.wong

From: Pali Rohár <pali.rohar@gmail.com>
Date: Tue, 14 Jun 2016 18:47:36 +0200

> You have never seen two ethernet cards with same MAC addresses? Right I 
> have not seen two USB, but there is non zero chance that could happen. 

It would be an error scenerio, and something to be avoided.

It is a valid and correct assumption that one is able to put
several devices at the same time on the same physical network
and expect it to work.

The behavior added by the change in question invalidates that.

I'm trying to consider the long term aspects of this, which is that if
more devices adopt this scheme we're in trouble if we blindly
interpret the MAC address in this way.

This firmware MAC property facility seems to be designed with only an
extremely narrow use case being considered.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
  2016-06-14 18:35             ` David Miller
@ 2016-06-14 22:27               ` Mario_Limonciello
  2016-06-22 22:11               ` Mario_Limonciello
  2016-07-11 21:54               ` Mario_Limonciello
  2 siblings, 0 replies; 16+ messages in thread
From: Mario_Limonciello @ 2016-06-14 22:27 UTC (permalink / raw)
  To: davem, pali.rohar
  Cc: gregkh, andrew, hayeswang, linux-kernel, netdev, linux-usb, anthony.wong

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Tuesday, June 14, 2016 1:35 PM
> To: pali.rohar@gmail.com
> Cc: gregkh@linuxfoundation.org; andrew@lunn.ch; Limonciello, Mario
> <Mario_Limonciello@Dell.com>; hayeswang@realtek.com; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org; linux-
> usb@vger.kernel.org; anthony.wong@canonical.com
> Subject: Re: [PATCH v6] r8152: Add support for setting pass through MAC
> address on RTL8153-AD
> 
> From: Pali Rohár <pali.rohar@gmail.com>
> Date: Tue, 14 Jun 2016 18:47:36 +0200
> 
> > You have never seen two ethernet cards with same MAC addresses? Right
> I
> > have not seen two USB, but there is non zero chance that could happen.
> 
> It would be an error scenerio, and something to be avoided.
> 
> It is a valid and correct assumption that one is able to put
> several devices at the same time on the same physical network
> and expect it to work.
> 
> The behavior added by the change in question invalidates that.
> 
> I'm trying to consider the long term aspects of this, which is that if
> more devices adopt this scheme we're in trouble if we blindly
> interpret the MAC address in this way.
> 

Do you mean if other manufacturers start to ship devices with 
RTL8135-AD's w/ this pass through bit set and people start to try to 
mix and match?

> This firmware MAC property facility seems to be designed with only an
> extremely narrow use case being considered.

Yes, as I understand it this is the reason that it's only on such specific devices
that the mac address pass through bit is actually set on the efuse.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
  2016-06-14 18:35             ` David Miller
  2016-06-14 22:27               ` Mario_Limonciello
@ 2016-06-22 22:11               ` Mario_Limonciello
  2016-07-11 21:54               ` Mario_Limonciello
  2 siblings, 0 replies; 16+ messages in thread
From: Mario_Limonciello @ 2016-06-22 22:11 UTC (permalink / raw)
  To: davem, pali.rohar
  Cc: gregkh, andrew, hayeswang, linux-kernel, netdev, linux-usb, anthony.wong

> -----Original Message-----
> From: Limonciello, Mario
> Sent: Tuesday, June 14, 2016 5:27 PM
> To: 'David Miller' <davem@davemloft.net>; pali.rohar@gmail.com
> Cc: gregkh@linuxfoundation.org; andrew@lunn.ch;
> hayeswang@realtek.com; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; linux-usb@vger.kernel.org;
> anthony.wong@canonical.com
> Subject: RE: [PATCH v6] r8152: Add support for setting pass through MAC
> address on RTL8153-AD
> 
> > -----Original Message-----
> > From: David Miller [mailto:davem@davemloft.net]
> > Sent: Tuesday, June 14, 2016 1:35 PM
> > To: pali.rohar@gmail.com
> > Cc: gregkh@linuxfoundation.org; andrew@lunn.ch; Limonciello, Mario
> > <Mario_Limonciello@Dell.com>; hayeswang@realtek.com; linux-
> > kernel@vger.kernel.org; netdev@vger.kernel.org; linux-
> > usb@vger.kernel.org; anthony.wong@canonical.com
> > Subject: Re: [PATCH v6] r8152: Add support for setting pass through MAC
> > address on RTL8153-AD
> >
> > From: Pali Rohár <pali.rohar@gmail.com>
> > Date: Tue, 14 Jun 2016 18:47:36 +0200
> >
> > > You have never seen two ethernet cards with same MAC addresses?
> Right
> > I
> > > have not seen two USB, but there is non zero chance that could happen.
> >
> > It would be an error scenerio, and something to be avoided.
> >
> > It is a valid and correct assumption that one is able to put
> > several devices at the same time on the same physical network
> > and expect it to work.
> >
> > The behavior added by the change in question invalidates that.
> >
> > I'm trying to consider the long term aspects of this, which is that if
> > more devices adopt this scheme we're in trouble if we blindly
> > interpret the MAC address in this way.
> >
> 
> Do you mean if other manufacturers start to ship devices with
> RTL8135-AD's w/ this pass through bit set and people start to try to
> mix and match?
> 
> > This firmware MAC property facility seems to be designed with only an
> > extremely narrow use case being considered.
> 
> Yes, as I understand it this is the reason that it's only on such specific devices
> that the mac address pass through bit is actually set on the efuse.

David,

Did you have any more thoughts about this?  I'm happy to make some other
adjustments to the patch, if you have some recommendations.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
  2016-06-14 18:35             ` David Miller
  2016-06-14 22:27               ` Mario_Limonciello
  2016-06-22 22:11               ` Mario_Limonciello
@ 2016-07-11 21:54               ` Mario_Limonciello
  2016-07-11 22:05                 ` David Miller
  2 siblings, 1 reply; 16+ messages in thread
From: Mario_Limonciello @ 2016-07-11 21:54 UTC (permalink / raw)
  To: davem, pali.rohar
  Cc: gregkh, andrew, hayeswang, linux-kernel, netdev, linux-usb, anthony.wong

> David,
> 
> Did you have any more thoughts about this?  I'm happy to make some other
> adjustments to the patch, if you have some recommendations.

Hi,

I just wanted to share that the maintenance BIOSes released for the Dell 
platforms with Type-C this past week enables the MAC address pass 
through feature in UEFI, so any network booted machines will offer
the auxiliary MAC to the DHCP server.  In Windows a network booted
machine will always use auxiliary MAC now.

XPS 9350 (BIOS 1.4.4): http://goo.gl/7Sw2DZ

Please let me know what else can be done for this patch to make it
acceptable so we can have parity for Linux.

Thanks,

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
  2016-07-11 21:54               ` Mario_Limonciello
@ 2016-07-11 22:05                 ` David Miller
  0 siblings, 0 replies; 16+ messages in thread
From: David Miller @ 2016-07-11 22:05 UTC (permalink / raw)
  To: Mario_Limonciello
  Cc: pali.rohar, gregkh, andrew, hayeswang, linux-kernel, netdev,
	linux-usb, anthony.wong

From: <Mario_Limonciello@Dell.com>
Date: Mon, 11 Jul 2016 21:54:07 +0000

> Please let me know what else can be done for this patch to make it
> acceptable so we can have parity for Linux.

Just resubmit it and I'll apply it, I'm so tired of hearing about this...

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-07-11 22:05 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-07 18:22 [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD Mario Limonciello
2016-06-11  5:51 ` David Miller
2016-06-11 15:39   ` Andrew Lunn
2016-06-11 17:42     ` David Miller
2016-06-14 15:08       ` Mario_Limonciello
2016-06-14 16:28       ` Pali Rohár
2016-06-14 16:40         ` Greg KH
2016-06-14 16:47           ` Pali Rohár
2016-06-14 16:55             ` Mario_Limonciello
2016-06-14 17:23               ` Andrew Lunn
2016-06-14 17:58                 ` Mario_Limonciello
2016-06-14 18:35             ` David Miller
2016-06-14 22:27               ` Mario_Limonciello
2016-06-22 22:11               ` Mario_Limonciello
2016-07-11 21:54               ` Mario_Limonciello
2016-07-11 22:05                 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).