diff mbox

Do not register ne2k-pci device until initialized

Message ID 1241281735.2641.7.camel@localhost.localdomain
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Lubomir Rintel May 2, 2009, 4:28 p.m. UTC
Doing it in reverse order causes uevent to be sent before
we have a MAC address, which confuses udev.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 drivers/net/ne2k-pci.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

Comments

Jeff Garzik May 2, 2009, 5:05 p.m. UTC | #1
Lubomir Rintel wrote:
> Doing it in reverse order causes uevent to be sent before
> we have a MAC address, which confuses udev.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>

Acked-by: Jeff Garzik <jgarzik@redhat.com>

Good spotting, that fixes a clear startup race


--
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
Andreas Mohr May 2, 2009, 5:58 p.m. UTC | #2
Hi,

> Good spotting, that fixes a clear startup race

I saw this and thought that this must be one of those non-singular bugs
again... ;)

(I checked for dev_addr manipulations _after_ register_netdev)

And, well...:

smsc911x.c looks like it has the same issue (in case of a non-specified
MAC address - invalid EEPROM content).

Dito ucc_geth.c

Dito arm/ixp4xx_eth.c

Dito wireless/netwave_cs.c


Given that several drivers seem to violate this rule: is there a way
to add a check against this problem?

Andreas Mohr
--
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
David Miller May 2, 2009, 8:52 p.m. UTC | #3
From: Andreas Mohr <andi@lisas.de>
Date: Sat, 2 May 2009 19:58:11 +0200

> And, well...:
> 
> smsc911x.c looks like it has the same issue (in case of a non-specified
> MAC address - invalid EEPROM content).
> 
> Dito ucc_geth.c
> 
> Dito arm/ixp4xx_eth.c
> 
> Dito wireless/netwave_cs.c
> 
> Given that several drivers seem to violate this rule: is there a way
> to add a check against this problem?

I'm pretty sure we have a valid ethernet address check
somewhere already... hmmm..
--
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
David Miller May 2, 2009, 8:52 p.m. UTC | #4
From: Jeff Garzik <jeff@garzik.org>
Date: Sat, 02 May 2009 13:05:34 -0400

> Lubomir Rintel wrote:
>> Doing it in reverse order causes uevent to be sent before
>> we have a MAC address, which confuses udev.
>> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> 
> Acked-by: Jeff Garzik <jgarzik@redhat.com>
> 
> Good spotting, that fixes a clear startup race

Applied, thanks.
--
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
Krzysztof Halasa May 3, 2009, 4:34 p.m. UTC | #5
Andreas Mohr <andi@lisas.de> writes:

> Dito arm/ixp4xx_eth.c

Right, will have a patch soon (mainline only, IMHO it's not worth it
to bother "stable" people because of no practical impact of this bug,
especially on this ARM IXP4xx arch).

Thanks.
diff mbox

Patch

diff --git a/drivers/net/ne2k-pci.c b/drivers/net/ne2k-pci.c
index eb66f65..7d83896 100644
--- a/drivers/net/ne2k-pci.c
+++ b/drivers/net/ne2k-pci.c
@@ -374,18 +374,17 @@  static int __devinit ne2k_pci_init_one (struct pci_dev *pdev,
 	dev->ethtool_ops = &ne2k_pci_ethtool_ops;
 	NS8390_init(dev, 0);
 
+	memcpy(dev->dev_addr, SA_prom, 6);
+	memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
+
 	i = register_netdev(dev);
 	if (i)
 		goto err_out_free_netdev;
 
-	for(i = 0; i < 6; i++)
-		dev->dev_addr[i] = SA_prom[i];
 	printk("%s: %s found at %#lx, IRQ %d, %pM.\n",
 	       dev->name, pci_clone_list[chip_idx].name, ioaddr, dev->irq,
 	       dev->dev_addr);
 
-	memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
-
 	return 0;
 
 err_out_free_netdev: