netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] via-rhine: do not abort due to invalid MAC address
@ 2011-03-02 16:32 Roger Luethi
  2011-03-02 18:01 ` Florian Fainelli
  0 siblings, 1 reply; 12+ messages in thread
From: Roger Luethi @ 2011-03-02 16:32 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, Alex G.

From: "Alex G." <mr.nuke.me@gmail.com>

via-rhine drops out of the init code if the hardware provides an invalid
MAC address. I have several reports of Rhine NICs doing just that. The
hardware still works, though; continuing the init process gives the user a
chance to fix the MAC address using "ifconfig ethX hw ether" which appears
to result in a perfectly useable network adapter.

Most recent report and patch provided by Alex G.

Signed-off-by: "Alex G." <mr.nuke.me@gmail.com>
Signed-off-by: Roger Luethi <rl@hellgate.ch>

diff --git a/linux-2.6.35.11/drivers/net/via-rhine.c b/via-rhine.c
index 4930f9d..a1189f4 100644
--- a/linux-2.6.35.11/drivers/net/via-rhine.c
+++ b/via-rhine.c
@@ -766,8 +766,12 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
 
 	if (!is_valid_ether_addr(dev->perm_addr)) {
 		rc = -EIO;
-		printk(KERN_ERR "Invalid MAC address\n");
-		goto err_out_unmap;
+		printk(KERN_ERR "via-rhine: invalid MAC address: %pM. "
+				"Use ifconfig to configure valid address.\n",
+				dev->dev_addr);
+		/* The device may still be used normally if a valid MAC is configured
+		 * We do not consider this a fatal error, and continue initialization
+		 */
 	}
 
 	/* For Rhine-I/II, phy_id is loaded from EEPROM */

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

* Re: [PATCH] via-rhine: do not abort due to invalid MAC address
  2011-03-02 16:32 [PATCH] via-rhine: do not abort due to invalid MAC address Roger Luethi
@ 2011-03-02 18:01 ` Florian Fainelli
  2011-03-02 18:11   ` Alex G.
  0 siblings, 1 reply; 12+ messages in thread
From: Florian Fainelli @ 2011-03-02 18:01 UTC (permalink / raw)
  To: Roger Luethi; +Cc: netdev, David S. Miller, Alex G.

Hello,

On Wednesday 02 March 2011 17:32:33 Roger Luethi wrote:
> From: "Alex G." <mr.nuke.me@gmail.com>
> 
> via-rhine drops out of the init code if the hardware provides an invalid
> MAC address. I have several reports of Rhine NICs doing just that. The
> hardware still works, though; continuing the init process gives the user a
> chance to fix the MAC address using "ifconfig ethX hw ether" which appears
> to result in a perfectly useable network adapter.
> 
> Most recent report and patch provided by Alex G.
> 
> Signed-off-by: "Alex G." <mr.nuke.me@gmail.com>
> Signed-off-by: Roger Luethi <rl@hellgate.ch>
> 
> diff --git a/linux-2.6.35.11/drivers/net/via-rhine.c b/via-rhine.c
> index 4930f9d..a1189f4 100644
> --- a/linux-2.6.35.11/drivers/net/via-rhine.c
> +++ b/via-rhine.c
> @@ -766,8 +766,12 @@ static int __devinit rhine_init_one(struct pci_dev
> *pdev,
> 
>  	if (!is_valid_ether_addr(dev->perm_addr)) {
>  		rc = -EIO;
> -		printk(KERN_ERR "Invalid MAC address\n");
> -		goto err_out_unmap;
> +		printk(KERN_ERR "via-rhine: invalid MAC address: %pM. "
> +				"Use ifconfig to configure valid address.\n",
> +				dev->dev_addr);
> +		/* The device may still be used normally if a valid MAC is 
configured
> +		 * We do not consider this a fatal error, and continue 
initialization
> +		 */
>  	}

Why not generate a valid random ethernet address using random_ether_addr() 
instead?
--
Florian

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

* Re: [PATCH] via-rhine: do not abort due to invalid MAC address
  2011-03-02 18:01 ` Florian Fainelli
@ 2011-03-02 18:11   ` Alex G.
  2011-03-02 19:36     ` David Miller
  0 siblings, 1 reply; 12+ messages in thread
From: Alex G. @ 2011-03-02 18:11 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: Roger Luethi, netdev, David S. Miller

On 03/02/2011 08:01 PM, Florian Fainelli wrote:
> 
> Why not generate a valid random ethernet address using random_ether_addr() 
> instead?

That would confuse ifcfg-* scripts that specify HWETHER, and probably
users as well. It might even stump udev.

An "ifconfig ethX hw ether xx:xx:xx:xx:xx:xx" in network scripts allows
udev to use the consistent "bad" MAC to assign a consistent ethX number,
and allows the networking scripts to use the consistent "new good" MAC.
It works as a stand-alone interface, as a WAN port hosting ppp0, and
even in bonding, all consistently.

Alex

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

* Re: [PATCH] via-rhine: do not abort due to invalid MAC address
  2011-03-02 18:11   ` Alex G.
@ 2011-03-02 19:36     ` David Miller
  2011-03-02 19:52       ` Roger Luethi
  0 siblings, 1 reply; 12+ messages in thread
From: David Miller @ 2011-03-02 19:36 UTC (permalink / raw)
  To: mr.nuke.me; +Cc: florian, rl, netdev

From: "Alex G." <mr.nuke.me@gmail.com>
Date: Wed, 02 Mar 2011 20:11:08 +0200

> On 03/02/2011 08:01 PM, Florian Fainelli wrote:
>> 
>> Why not generate a valid random ethernet address using random_ether_addr() 
>> instead?
> 
> That would confuse ifcfg-* scripts that specify HWETHER, and probably
> users as well. It might even stump udev.

If the ethernet address reported by the card is garbage, every single
ethernet driver that tried to handle this case uses a random ethernet
address.

There is no reason to choose a different strategy for this driver.

Also registering a network device with an invalid ethernet address can
cause all kinds of serious issues, let's say dhcp brings the device
up and emits DHCP requests with illegal MAC addresses in the header
because of this problem.

You therefore simply cannot leave crap in there.

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

* Re: [PATCH] via-rhine: do not abort due to invalid MAC address
  2011-03-02 19:52       ` Roger Luethi
@ 2011-03-02 19:49         ` Alex G.
  2011-03-02 20:06         ` Alex G.
  1 sibling, 0 replies; 12+ messages in thread
From: Alex G. @ 2011-03-02 19:49 UTC (permalink / raw)
  To: Roger Luethi; +Cc: David Miller, florian, netdev

On 03/02/2011 09:52 PM, Roger Luethi wrote:
> On Wed, 02 Mar 2011 11:36:10 -0800, David Miller wrote:
>> There is no reason to choose a different strategy for this driver.
> 
> Fair enough. I did not know other drivers had already dealt with the same
> problem. My bad. Alex, can you redo the patch accordingly?

Yes, no problem.

Alex

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

* Re: [PATCH] via-rhine: do not abort due to invalid MAC address
  2011-03-02 19:36     ` David Miller
@ 2011-03-02 19:52       ` Roger Luethi
  2011-03-02 19:49         ` Alex G.
  2011-03-02 20:06         ` Alex G.
  0 siblings, 2 replies; 12+ messages in thread
From: Roger Luethi @ 2011-03-02 19:52 UTC (permalink / raw)
  To: David Miller; +Cc: mr.nuke.me, florian, netdev

On Wed, 02 Mar 2011 11:36:10 -0800, David Miller wrote:
> From: "Alex G." <mr.nuke.me@gmail.com>
> Date: Wed, 02 Mar 2011 20:11:08 +0200
> 
> > On 03/02/2011 08:01 PM, Florian Fainelli wrote:
> >> 
> >> Why not generate a valid random ethernet address using random_ether_addr() 
> >> instead?
> > 
> > That would confuse ifcfg-* scripts that specify HWETHER, and probably
> > users as well. It might even stump udev.
> 
> If the ethernet address reported by the card is garbage, every single
> ethernet driver that tried to handle this case uses a random ethernet
> address.
>
> There is no reason to choose a different strategy for this driver.

Fair enough. I did not know other drivers had already dealt with the same
problem. My bad. Alex, can you redo the patch accordingly?

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

* Re: [PATCH] via-rhine: do not abort due to invalid MAC address
  2011-03-02 19:52       ` Roger Luethi
  2011-03-02 19:49         ` Alex G.
@ 2011-03-02 20:06         ` Alex G.
  2011-03-03 21:42           ` David Miller
  1 sibling, 1 reply; 12+ messages in thread
From: Alex G. @ 2011-03-02 20:06 UTC (permalink / raw)
  To: Roger Luethi; +Cc: David Miller, florian, netdev

[-- Attachment #1: Type: text/plain, Size: 1032 bytes --]

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---

On 03/02/2011 09:52 PM, Roger Luethi wrote:
> Alex, can you redo the patch accordingly?
Done and tested.


[root@nukeserv2 mrnuke]# rmmod via-rhine
[root@nukeserv2 mrnuke]# insmod via-rhine.ko
[28234.938404] via-rhine.c:v1.10-LK1.4.3 2007-03-06 Written by Donald Becker
[28234.945186] via-rhine: Invalid MAC address: ef:9f:e9:f7:f7:f7.
[28234.945518] via-rhine: Using randomly generated address:
02:0f:c4:98:a6:fd instead.
[28234.948793] eth1: VIA Rhine II at 0xfaa00000, 02:0f:c4:98:a6:fd, IRQ 23.
[28234.950223] eth1: MII PHY found at address 1, status 0x7849
advertising 01e1 Link 0000.
[root@nukeserv2 mrnuke]# ifconfig eth1
eth1      Link encap:Ethernet  HWaddr 02:0F:C4:98:A6:FD
          BROADCAST MULTICAST  MTU:1500  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000
          RX bytes:0 (0.0 b)  TX bytes:0 (0.0 b)
          Interrupt:23


[-- Attachment #2: rhine.patch --]
[-- Type: text/x-patch, Size: 987 bytes --]

diff --git a/linux-2.6.35.11/drivers/net/via-rhine.c b/via-rhine.c
index 4930f9d..4c1b9e7 100644
--- a/linux-2.6.35.11/drivers/net/via-rhine.c
+++ b/via-rhine.c
@@ -762,13 +762,16 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
 
 	for (i = 0; i < 6; i++)
 		dev->dev_addr[i] = ioread8(ioaddr + StationAddr + i);
-	memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
 
-	if (!is_valid_ether_addr(dev->perm_addr)) {
-		rc = -EIO;
-		printk(KERN_ERR "Invalid MAC address\n");
-		goto err_out_unmap;
+	if (!is_valid_ether_addr(dev->dev_addr)) {
+		printk(KERN_ERR "via-rhine: Invalid MAC address: %pM. \n",
+				dev->dev_addr);
+		/* The device may still be used normally if a valid MAC is configured */
+		random_ether_addr(dev->dev_addr);
+		printk(KERN_ERR "via-rhine: Using randomly generated address: %pM instead. \n",
+				dev->dev_addr);
 	}
+	memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
 
 	/* For Rhine-I/II, phy_id is loaded from EEPROM */
 	if (!phy_id)

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

* Re: [PATCH] via-rhine: do not abort due to invalid MAC address
  2011-03-02 20:06         ` Alex G.
@ 2011-03-03 21:42           ` David Miller
  2011-03-03 21:51             ` Alex G.
  0 siblings, 1 reply; 12+ messages in thread
From: David Miller @ 2011-03-03 21:42 UTC (permalink / raw)
  To: mr.nuke.me; +Cc: rl, florian, netdev

From: "Alex G." <mr.nuke.me@gmail.com>
Date: Wed, 02 Mar 2011 22:06:21 +0200

> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>

The file paths in your patch are incorrect, please read
Documentation/SubmittingPatches to learn how to generate
proper diffs.

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

* Re: [PATCH] via-rhine: do not abort due to invalid MAC address
  2011-03-03 21:42           ` David Miller
@ 2011-03-03 21:51             ` Alex G.
  2011-03-03 22:01               ` David Miller
  0 siblings, 1 reply; 12+ messages in thread
From: Alex G. @ 2011-03-03 21:51 UTC (permalink / raw)
  To: David Miller; +Cc: rl, florian, netdev

[-- Attachment #1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2: rhine.patch --]
[-- Type: text/x-patch, Size: 1330 bytes --]

via-rhine drops out of the init code if the hardware provides an invalid
MAC address. Roger Luethi has had several reports of Rhine NICs doing just
that. The hardware still works, though; assigning a random MAC address
allows the NIC to be used as usual. Tested as a standalone interface,
as carrier for ppp, and as bonding slave.

Signed-off-by Alexandru Gagniuc <mr.nuke.me@gmail.com>

--- drivers/net/via-rhine.c.orig	2011-02-06 21:04:07.000000000 +0200
+++ drivers/net/via-rhine.c	2011-03-02 22:03:13.000000000 +0200
@@ -762,13 +762,16 @@ static int __devinit rhine_init_one(stru
 
 	for (i = 0; i < 6; i++)
 		dev->dev_addr[i] = ioread8(ioaddr + StationAddr + i);
-	memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
 
-	if (!is_valid_ether_addr(dev->perm_addr)) {
-		rc = -EIO;
-		printk(KERN_ERR "Invalid MAC address\n");
-		goto err_out_unmap;
+	if (!is_valid_ether_addr(dev->dev_addr)) {
+		printk(KERN_ERR "via-rhine: Invalid MAC address: %pM. \n",
+				dev->dev_addr);
+		/* The device may still be used normally if a valid MAC is configured */
+		random_ether_addr(dev->dev_addr);
+		printk(KERN_ERR "via-rhine: Using randomly generated address: %pM instead. \n",
+				dev->dev_addr);
 	}
+	memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
 
 	/* For Rhine-I/II, phy_id is loaded from EEPROM */
 	if (!phy_id)

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

* Re: [PATCH] via-rhine: do not abort due to invalid MAC address
  2011-03-03 21:51             ` Alex G.
@ 2011-03-03 22:01               ` David Miller
  2011-03-03 22:08                 ` Alex G.
  0 siblings, 1 reply; 12+ messages in thread
From: David Miller @ 2011-03-03 22:01 UTC (permalink / raw)
  To: mr.nuke.me; +Cc: rl, florian, netdev


Still wrong, the patch needs to be "-p1" not "-p0" rooted.

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

* Re: [PATCH] via-rhine: do not abort due to invalid MAC address
  2011-03-03 22:01               ` David Miller
@ 2011-03-03 22:08                 ` Alex G.
  2011-03-09 21:30                   ` David Miller
  0 siblings, 1 reply; 12+ messages in thread
From: Alex G. @ 2011-03-03 22:08 UTC (permalink / raw)
  To: David Miller; +Cc: rl, florian, netdev

[-- Attachment #1: Type: text/plain, Size: 233 bytes --]

On 03/04/2011 12:01 AM, David Miller wrote:
> 
> Still wrong, the patch needs to be "-p1" not "-p0" rooted.

Documentation/SubmittingPatches seems to be outdated then. I used "git
diff"; I hope the format is correct this time

Alex


[-- Attachment #2: rhine.patch --]
[-- Type: text/x-patch, Size: 1377 bytes --]

via-rhine drops out of the init code if the hardware provides an invalid
MAC address. Roger Luethi has had several reports of Rhine NICs doing just
that. The hardware still works, though; assigning a random MAC address
allows the NIC to be used as usual. Tested as a standalone interface,
as carrier for ppp, and as bonding slave.

Signed-off-by Alexandru Gagniuc <mr.nuke.me@gmail.com>

diff --git a/drivers/net/via-rhine.c.orig b/drivers/net/via-rhine.c
index 4930f9d..4c1b9e7 100644
--- a/drivers/net/via-rhine.c.orig
+++ b/drivers/net/via-rhine.c
@@ -762,13 +762,16 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
 
 	for (i = 0; i < 6; i++)
 		dev->dev_addr[i] = ioread8(ioaddr + StationAddr + i);
-	memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
 
-	if (!is_valid_ether_addr(dev->perm_addr)) {
-		rc = -EIO;
-		printk(KERN_ERR "Invalid MAC address\n");
-		goto err_out_unmap;
+	if (!is_valid_ether_addr(dev->dev_addr)) {
+		printk(KERN_ERR "via-rhine: Invalid MAC address: %pM. \n",
+				dev->dev_addr);
+		/* The device may still be used normally if a valid MAC is configured */
+		random_ether_addr(dev->dev_addr);
+		printk(KERN_ERR "via-rhine: Using randomly generated address: %pM instead. \n",
+				dev->dev_addr);
 	}
+	memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
 
 	/* For Rhine-I/II, phy_id is loaded from EEPROM */
 	if (!phy_id)

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

* Re: [PATCH] via-rhine: do not abort due to invalid MAC address
  2011-03-03 22:08                 ` Alex G.
@ 2011-03-09 21:30                   ` David Miller
  0 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2011-03-09 21:30 UTC (permalink / raw)
  To: mr.nuke.me; +Cc: rl, florian, netdev

From: "Alex G." <mr.nuke.me@gmail.com>
Date: Fri, 04 Mar 2011 00:08:56 +0200

> On 03/04/2011 12:01 AM, David Miller wrote:
>> 
>> Still wrong, the patch needs to be "-p1" not "-p0" rooted.
> 
> Documentation/SubmittingPatches seems to be outdated then. I used "git
> diff"; I hope the format is correct this time

Please submit patches as fresh new list postings, rather than in reply
to another email, otherwise I have to do a lot of editing to get the
commit message to look right.

Also:

Signed-off-by Alexandru Gagniuc <mr.nuke.me@gmail.com>

You're missing a colon character at the end of "Signed-off-by"

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

end of thread, other threads:[~2011-03-09 21:29 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-02 16:32 [PATCH] via-rhine: do not abort due to invalid MAC address Roger Luethi
2011-03-02 18:01 ` Florian Fainelli
2011-03-02 18:11   ` Alex G.
2011-03-02 19:36     ` David Miller
2011-03-02 19:52       ` Roger Luethi
2011-03-02 19:49         ` Alex G.
2011-03-02 20:06         ` Alex G.
2011-03-03 21:42           ` David Miller
2011-03-03 21:51             ` Alex G.
2011-03-03 22:01               ` David Miller
2011-03-03 22:08                 ` Alex G.
2011-03-09 21:30                   ` 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).