[PATCH] r8169: Randomise invalid MAC addresses

From: "Torne (Richard Coles)" <redacted>

If the default MAC address stored in the card is invalid, replace it
with a random address and complain about it.

Signed-off-by: Torne (Richard Coles) <redacted>
---
 drivers/net/ethernet/realtek/r8169.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 7a0c800..ec5ebbb 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c

@@ -4103,6 +4103,14 @@ rtl8169_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	/* Get MAC address */
 	for (i = 0; i < ETH_ALEN; i++)
 		dev->dev_addr[i] = RTL_R8(MAC0 + i);
+
+	if (!is_valid_ether_addr(dev->dev_addr)) {
+		/* Report it and use a random ethernet address instead */
+		netdev_err(dev, "Invalid MAC address: %pM\n", dev->dev_addr);
+		random_ether_addr(dev->dev_addr);
+		netdev_info(dev, "Using random MAC address: %pM\n",
+			    dev->dev_addr);
+	}
 	memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
 
 	SET_ETHTOOL_OPS(dev, &rtl8169_ethtool_ops);

-- 
1.7.7.3

On Mon, 2012-01-23 at 18:32 +0000, Torne (Richard Coles) wrote:

From: "Torne (Richard Coles)" <redacted>

If the default MAC address stored in the card is invalid, replace it
with a random address and complain about it.

[]

[]

Perhaps this should be on 1 line.
If KERN_level filtering is higher then KERN_INFO,
the new MAC will not be shown.

	if (!is_valid_ether_addr(dev->dev_addr)) {
		u8 mac[ETH_ALEN];
		random_ether_addr(mac);
		netdev_err(dev, "Hardware has invalid mac address %pM, using %pM\n",
			   dev->dev_addr, mac);
		memcpy(dev->dev_addr, mac, ETH_ALEN);
	}

On Mon, Jan 23, 2012 at 1:32 PM, Torne (Richard Coles) [off-list ref] wrote:

From: "Torne (Richard Coles)" <redacted>

If the default MAC address stored in the card is invalid, replace it
with a random address and complain about it.

You might want to have a look at this thread and its outcome.

https://lkml.org/lkml/2012/1/14/75

Paul.

On Mon, 23 Jan 2012 18:32:20 +0000
"Torne (Richard Coles)" [off-list ref] wrote:

From: "Torne (Richard Coles)" <redacted>

If the default MAC address stored in the card is invalid, replace it
with a random address and complain about it.

See the discussion about the pch ethernet card. Detect the error and warn
about it, block opening it and let the user set it with ifconfig xx hw
aa:bb:cc:dd:ee:ff

Alan

On 23 January 2012 20:53, Paul Gortmaker [off-list ref] wrote:

On Mon, Jan 23, 2012 at 1:32 PM, Torne (Richard Coles) [off-list ref] wrote:

quoted

From: "Torne (Richard Coles)" <redacted>

If the default MAC address stored in the card is invalid, replace it
with a random address and complain about it.

You might want to have a look at this thread and its outcome.

https://lkml.org/lkml/2012/1/14/75

While I can appreciate the argument that people shouldn't build
hardware that relies on this kind of behaviour, I am in the same
situation as Darren there: I have a retail device I have bought that
has a garbage MAC address (an OpenPeak Joggler), and so do all the
others. Requiring that this be worked around in userspace makes it
tricky to just install a standard Linux distribution onto this piece
of hardware that's otherwise pretty much x86-pc-compatible (albeit
with a weird bootloader)

The vendor's distribution clones the USB wifi adapter's MAC onto the
ethernet device in userspace at boot, which is rather unpleasant (it
never uses both interfaces at once, admittedly).

So, is this just not going to be acceptable in any form? What about
refactoring the existing drivers that do this so that this code
doesn't need to be repeated in every driver, if that would help? I'd
really quite like to get standard linux distros to be compatible with
the Joggler, and this is one of the few changes that's actually needed
(one way or another).

-- 
Torne (Richard Coles)
torne@google.com

So, is this just not going to be acceptable in any form? What about
refactoring the existing drivers that do this so that this code
doesn't need to be repeated in every driver, if that would help? I'd
really quite like to get standard linux distros to be compatible with
the Joggler, and this is one of the few changes that's actually needed
(one way or another).

It ought to be about a ten line change in Red Hat/Fedora, you could also
do it generically for most distributions as a udev rule.

Alan

On 23 January 2012 21:43, Alan Cox [off-list ref] wrote:

quoted

So, is this just not going to be acceptable in any form? What about
refactoring the existing drivers that do this so that this code
doesn't need to be repeated in every driver, if that would help? I'd
really quite like to get standard linux distros to be compatible with
the Joggler, and this is one of the few changes that's actually needed
(one way or another).

It ought to be about a ten line change in Red Hat/Fedora, you could also
do it generically for most distributions as a udev rule.

OK; I'll work on that instead. Thanks for the feedback all.

-- 
Torne (Richard Coles)
torne@google.com

Hi!

quoted

If the default MAC address stored in the card is invalid, replace it
with a random address and complain about it.

See the discussion about the pch ethernet card. Detect the error and warn
about it, block opening it and let the user set it with ifconfig xx hw
aa:bb:cc:dd:ee:ff

Kernel should provide hw abstraction, and that should mean doing
something about commonly-bad ethernet cards.

Userspace does not quite cut it, think about nfsroot for example.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Pavel Machek [off-list ref] :
[...]

Kernel should provide hw abstraction, and that should mean doing
something about commonly-bad ethernet cards.

Ask for a refund ?

Userspace does not quite cut it, think about nfsroot for example.

nfsroot without pxe or such ? A bit contrieved imho.

The eeprom may be writable though. Or not :o/

-- 
Ueimor

On Tue, 2012-01-24 at 19:28 +0100, Francois Romieu wrote:

Pavel Machek [off-list ref] :
[...]

quoted

Kernel should provide hw abstraction, and that should mean doing
something about commonly-bad ethernet cards.

Ask for a refund ?

[...]

This is mostly being done in embedded systems where the system
developers control both the kernel and all of userspace and can easily
bodge things in the userland init process.

Then some adventurous users want to run custom kernels on those systems
and have a reasonable way to deal with that.  The original system worked
and they cannot replace just the network interface, so it is no good
asking for a refund.

None of the push-back from netdev is going to have any effect on the
embedded system developers who are failing to program MAC addresses
properly; it's only going to hurt those adventurous users.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

From: Ben Hutchings <redacted>
Date: Tue, 24 Jan 2012 18:47:33 +0000

None of the push-back from netdev is going to have any effect on the
embedded system developers who are failing to program MAC addresses
properly; it's only going to hurt those adventurous users.

The implementation of denying interface bringup until a valid MAC is
configured is actually helping these adventurous users if the embedded
guys (which in the Intel situation, was the case) put the hack into
userspace to set the MAC address before bringing the interface up.

And this is really the only solution I think is warranted.