| Message ID | 1327343540-30348-1-git-send-email-torne@google.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, 2012-01-23 at 18:32 +0000, Torne (Richard Coles) wrote: > From: "Torne (Richard Coles)" <torne@google.com> > > If the default MAC address stored in the card is invalid, replace it > with a random address and complain about it. [] > diff --git 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); 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); } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 23, 2012 at 1:32 PM, Torne (Richard Coles) <torne@google.com> wrote: > From: "Torne (Richard Coles)" <torne@google.com> > > 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. > > Signed-off-by: Torne (Richard Coles) <torne@google.com> > --- > 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 > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 23 Jan 2012 18:32:20 +0000 "Torne (Richard Coles)" <torne@google.com> wrote: > From: "Torne (Richard Coles)" <torne@google.com> > > 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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 23 January 2012 20:53, Paul Gortmaker <paul.gortmaker@gmail.com> wrote: > On Mon, Jan 23, 2012 at 1:32 PM, Torne (Richard Coles) <torne@google.com> wrote: >> From: "Torne (Richard Coles)" <torne@google.com> >> >> 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).
> 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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 23 January 2012 21:43, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote: >> 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.
Hi! > > 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
Pavel Machek <pavel@ucw.cz> : [...] > 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/
On Tue, 2012-01-24 at 19:28 +0100, Francois Romieu wrote: > Pavel Machek <pavel@ucw.cz> : > [...] > > 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.
From: Ben Hutchings <bhutchings@solarflare.com> 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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);