via-rhine: do not abort due to invalid MAC address

From: "Alex G." <[email protected]>

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.

index 4930f9d..a1189f4 100644
@@ -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 */

Hello,

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

From: "Alex G." <[email protected]>
Date: Wed, 02 Mar 2011 20:11:08 +0200

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.

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

---

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

index 4930f9d..4c1b9e7 100644
@@ -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)

From: "Alex G." <[email protected]>
Date: Wed, 02 Mar 2011 22:06:21 +0200

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

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 <[email protected]>

@@ -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)

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

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 <[email protected]>

index 4930f9d..4c1b9e7 100644
@@ -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)

From: "Alex G." <[email protected]>
Date: Fri, 04 Mar 2011 00:08:56 +0200

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 <[email protected]>

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