Skip to contentSkip to conversation

pch_gbe: Do not abort probe on bad MAC

PATCH In review Darren Hart 2 participants 6 messages Jan 19, 2012 – Jan 24, 2012 Network drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 pch_gbe: Do not abort probe on bad MAC

Bruce, please apply to linux-yocto-3.0/yocto/standard/base

Upstream-Status: Accepted (Linux 3.3)

If the MAC is invalid or not implemented, do not abort the probe. Issue a warning and prevent bringing the interface up until a MAC is set manually (via ifconfig $IFACE hw ether $MAC).

Tested on two platforms, one with a valid MAC, the other without a MAC. The real MAC is used if present, the interface fails to come up until the MAC is set on the other. They successfully get an IP over DHCP and pass a simple ping and login over ssh test.

This is meant to allow the Inforce SYS940X development board:
http://www.inforcecomputing.com/SYS940X_ECX.html
(and others suffering from a missing MAC) to work with the mainline kernel. Without this patch, the probe will fail and the interface will not be created, preventing the user from configuring the MAC manually.

This does not make any attempt to address a missing or invalid MAC for the pch_phub driver.

CC Arjan van de Ven <[email protected]>
CC Alan Cox <[email protected]>
CC Tomoya MORINAGA <[email protected]>
CC Jeff Kirsher <[email protected]>
CC "David S. Miller" <[email protected]>
CC Paul Gortmaker <[email protected]>
CC Jon Mason <[email protected]>
CC Mark Brown <[email protected]>
CC David Laight <[email protected]>
CC Joe Perches <[email protected]>
---
 drivers/net/pch_gbe/pch_gbe_main.c |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 deletions(-)
drivers/net/pch_gbe/pch_gbe_main.c +14 -3
index eac3c5c..e7412f2 100644
@@ -1701,6 +1701,12 @@ int pch_gbe_up(struct pch_gbe_adapter *adapter)
 	struct pch_gbe_rx_ring *rx_ring = adapter->rx_ring;
 	int err;
 
+	/* Ensure we have a valid MAC */
+	if (!is_valid_ether_addr(adapter->hw.mac.addr)) {
+		pr_err("Error: Invalid MAC address\n");
+		return -EINVAL;
+	}
+
 	/* hardware has been reset, we need to reload some things */
 	pch_gbe_set_multi(netdev);
 
@@ -2392,9 +2398,14 @@ static int pch_gbe_probe(struct pci_dev *pdev,
 
 	memcpy(netdev->dev_addr, adapter->hw.mac.addr, netdev->addr_len);
 	if (!is_valid_ether_addr(netdev->dev_addr)) {
-		dev_err(&pdev->dev, "Invalid MAC Address\n");
-		ret = -EIO;
-		goto err_free_adapter;
+		/*
+		 * If the MAC is invalid (or just missing), display a warning
+		 * but do not abort setting up the device. pch_gbe_up will
+		 * prevent the interface from being brought up until a valid MAC
+		 * is set.
+		 */
+		dev_err(&pdev->dev, "Invalid MAC address, "
+		                    "interface disabled.\n");
 	}
 	setup_timer(&adapter->watchdog_timer, pch_gbe_watchdog,
 		    (unsigned long)adapter);
Signature · 5 lines
1.7.6.5
-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel
#1 lore raw
Quoted 1 line · Darren Hart

Bruce, please apply to linux-yocto-3.0/yocto/standard/base

Queued.

Bruce

Quoted 72 lines

Upstream-Status: Accepted (Linux 3.3)

If the MAC is invalid or not implemented, do not abort the probe. Issue a warning and prevent bringing the interface up until a MAC is set manually (via ifconfig $IFACE hw ether $MAC).

Tested on two platforms, one with a valid MAC, the other without a MAC. The real MAC is used if present, the interface fails to come up until the MAC is set on the other. They successfully get an IP over DHCP and pass a simple ping and login over ssh test.

This is meant to allow the Inforce SYS940X development board:
http://www.inforcecomputing.com/SYS940X_ECX.html
(and others suffering from a missing MAC) to work with the mainline kernel. Without this patch, the probe will fail and the interface will not be created, preventing the user from configuring the MAC manually.

This does not make any attempt to address a missing or invalid MAC for the pch_phub driver.

CC Arjan van de Ven<[email protected]>
CC Alan Cox<[email protected]>
CC Tomoya MORINAGA<[email protected]>
CC Jeff Kirsher<[email protected]>
CC "David S. Miller"<[email protected]>
CC Paul Gortmaker<[email protected]>
CC Jon Mason<[email protected]>
CC Mark Brown<[email protected]>
CC David Laight<[email protected]>
CC Joe Perches<[email protected]>
---
  drivers/net/pch_gbe/pch_gbe_main.c |   17 ++++++++++++++---
  1 files changed, 14 insertions(+), 3 deletions(-)
drivers/net/pch_gbe/pch_gbe_main.c +14 -3
index eac3c5c..e7412f2 100644
@@ -1701,6 +1701,12 @@ int pch_gbe_up(struct pch_gbe_adapter *adapter)
  	struct pch_gbe_rx_ring *rx_ring = adapter->rx_ring;
  	int err;
 
+	/* Ensure we have a valid MAC */
+	if (!is_valid_ether_addr(adapter->hw.mac.addr)) {
+		pr_err("Error: Invalid MAC address\n");
+		return -EINVAL;
+	}
+
  	/* hardware has been reset, we need to reload some things */
  	pch_gbe_set_multi(netdev);
 
@@ -2392,9 +2398,14 @@ static int pch_gbe_probe(struct pci_dev *pdev,
 
  	memcpy(netdev->dev_addr, adapter->hw.mac.addr, netdev->addr_len);
  	if (!is_valid_ether_addr(netdev->dev_addr)) {
-		dev_err(&pdev->dev, "Invalid MAC Address\n");
-		ret = -EIO;
-		goto err_free_adapter;
+		/*
+		 * If the MAC is invalid (or just missing), display a warning
+		 * but do not abort setting up the device. pch_gbe_up will
+		 * prevent the interface from being brought up until a valid MAC
+		 * is set.
+		 */
+		dev_err(&pdev->dev, "Invalid MAC address, "
+		                    "interface disabled.\n");
  	}
  	setup_timer(&adapter->watchdog_timer, pch_gbe_watchdog,
  		    (unsigned long)adapter);
#2 lore raw
Quoted 4 lines · Bruce Ashfield
Quoted 1 line · Darren Hart

Bruce, please apply to linux-yocto-3.0/yocto/standard/base

Queued.

I don't see this in the repository yet, is it still pending? I'm trying to write a BSP that is dependent on it.

Thanks,

Darren

Quoted 80 lines

Bruce

Quoted 62 lines

Upstream-Status: Accepted (Linux 3.3)

If the MAC is invalid or not implemented, do not abort the probe. Issue a warning and prevent bringing the interface up until a MAC is set manually (via ifconfig $IFACE hw ether $MAC).

Tested on two platforms, one with a valid MAC, the other without a MAC. The real MAC is used if present, the interface fails to come up until the MAC is set on the other. They successfully get an IP over DHCP and pass a simple ping and login over ssh test.

This is meant to allow the Inforce SYS940X development board:
http://www.inforcecomputing.com/SYS940X_ECX.html
(and others suffering from a missing MAC) to work with the mainline kernel. Without this patch, the probe will fail and the interface will not be created, preventing the user from configuring the MAC manually.

This does not make any attempt to address a missing or invalid MAC for the pch_phub driver.

CC Arjan van de Ven<[email protected]>
CC Alan Cox<[email protected]>
CC Tomoya MORINAGA<[email protected]>
CC Jeff Kirsher<[email protected]>
CC "David S. Miller"<[email protected]>
CC Paul Gortmaker<[email protected]>
CC Jon Mason<[email protected]>
CC Mark Brown<[email protected]>
CC David Laight<[email protected]>
CC Joe Perches<[email protected]>
---
  drivers/net/pch_gbe/pch_gbe_main.c |   17 ++++++++++++++---
  1 files changed, 14 insertions(+), 3 deletions(-)
drivers/net/pch_gbe/pch_gbe_main.c +7 -3
index eac3c5c..e7412f2 100644
@@ -1701,6 +1701,12 @@ int pch_gbe_up(struct pch_gbe_adapter *adapter)
  	struct pch_gbe_rx_ring *rx_ring = adapter->rx_ring;
  	int err;
 
+	/* Ensure we have a valid MAC */
+	if (!is_valid_ether_addr(adapter->hw.mac.addr)) {
+		pr_err("Error: Invalid MAC address\n");
+		return -EINVAL;
+	}
+
  	/* hardware has been reset, we need to reload some things */
  	pch_gbe_set_multi(netdev);
 
@@ -2392,9 +2398,14 @@ static int pch_gbe_probe(struct pci_dev *pdev,
 
  	memcpy(netdev->dev_addr, adapter->hw.mac.addr, netdev->addr_len);
  	if (!is_valid_ether_addr(netdev->dev_addr)) {
-		dev_err(&pdev->dev, "Invalid MAC Address\n");
-		ret = -EIO;
-		goto err_free_adapter;
+		/*

drivers/net/pch_gbe/pch_gbe_main.c static int pch_gbe_probe(struct pci_dev *pdev, · line 2411

Quoted 10 lines
drivers/net/pch_gbe/pch_gbe_main.c +7 -0
+		 * If the MAC is invalid (or just missing), display a warning
+		 * but do not abort setting up the device. pch_gbe_up will
+		 * prevent the interface from being brought up until a valid MAC
+		 * is set.
+		 */
+		dev_err(&pdev->dev, "Invalid MAC address, "
+		                    "interface disabled.\n");
  	}
  	setup_timer(&adapter->watchdog_timer, pch_gbe_watchdog,
  		    (unsigned long)adapter);

_______________________________________________
yocto mailing list
[email protected]
https://lists.yoctoproject.org/listinfo/yocto

Signature · 3 lines
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel
#3 lore raw
Quoted 10 lines · Darren Hart
Quoted 4 lines · Bruce Ashfield
Quoted 1 line · Darren Hart

Bruce, please apply to linux-yocto-3.0/yocto/standard/base

Queued.

I don't see this in the repository yet, is it still pending? I'm trying to write a BSP that is dependent on it.'

It's queued with the rest of my 3.0 updates. I've been spending time on 3.2 and haven't pushed these out yet.

Bruce

Quoted 86 lines

Thanks,

Darren

Quoted 80 lines

Bruce

Quoted 62 lines

Upstream-Status: Accepted (Linux 3.3)

If the MAC is invalid or not implemented, do not abort the probe. Issue a warning and prevent bringing the interface up until a MAC is set manually (via ifconfig $IFACE hw ether $MAC).

Tested on two platforms, one with a valid MAC, the other without a MAC. The real MAC is used if present, the interface fails to come up until the MAC is set on the other. They successfully get an IP over DHCP and pass a simple ping and login over ssh test.

This is meant to allow the Inforce SYS940X development board:
http://www.inforcecomputing.com/SYS940X_ECX.html
(and others suffering from a missing MAC) to work with the mainline kernel. Without this patch, the probe will fail and the interface will not be created, preventing the user from configuring the MAC manually.

This does not make any attempt to address a missing or invalid MAC for the pch_phub driver.

CC Arjan van de Ven<[email protected]>
CC Alan Cox<[email protected]>
CC Tomoya MORINAGA<[email protected]>
CC Jeff Kirsher<[email protected]>
CC "David S. Miller"<[email protected]>
CC Paul Gortmaker<[email protected]>
CC Jon Mason<[email protected]>
CC Mark Brown<[email protected]>
CC David Laight<[email protected]>
CC Joe Perches<[email protected]>
---
   drivers/net/pch_gbe/pch_gbe_main.c |   17 ++++++++++++++---
   1 files changed, 14 insertions(+), 3 deletions(-)
drivers/net/pch_gbe/pch_gbe_main.c +7 -3
index eac3c5c..e7412f2 100644
@@ -1701,6 +1701,12 @@ int pch_gbe_up(struct pch_gbe_adapter *adapter)
   	struct pch_gbe_rx_ring *rx_ring = adapter->rx_ring;
   	int err;
 
+	/* Ensure we have a valid MAC */
+	if (!is_valid_ether_addr(adapter->hw.mac.addr)) {
+		pr_err("Error: Invalid MAC address\n");
+		return -EINVAL;
+	}
+
   	/* hardware has been reset, we need to reload some things */
   	pch_gbe_set_multi(netdev);
 
@@ -2392,9 +2398,14 @@ static int pch_gbe_probe(struct pci_dev *pdev,
 
   	memcpy(netdev->dev_addr, adapter->hw.mac.addr, netdev->addr_len);
   	if (!is_valid_ether_addr(netdev->dev_addr)) {
-		dev_err(&pdev->dev, "Invalid MAC Address\n");
-		ret = -EIO;
-		goto err_free_adapter;
+		/*

drivers/net/pch_gbe/pch_gbe_main.c static int pch_gbe_probe(struct pci_dev *pdev, · line 2411

Quoted 10 lines
drivers/net/pch_gbe/pch_gbe_main.c +7 -0
+		 * If the MAC is invalid (or just missing), display a warning
+		 * but do not abort setting up the device. pch_gbe_up will
+		 * prevent the interface from being brought up until a valid MAC
+		 * is set.
+		 */
+		dev_err(&pdev->dev, "Invalid MAC address, "
+		                    "interface disabled.\n");
   	}
   	setup_timer(&adapter->watchdog_timer, pch_gbe_watchdog,
   		    (unsigned long)adapter);

_______________________________________________
yocto mailing list
[email protected]
https://lists.yoctoproject.org/listinfo/yocto

#4 lore raw
Quoted 7 lines · Bruce Ashfield
Quoted 6 lines · Darren Hart
Quoted 3 lines · Bruce Ashfield
Quoted 1 line · Darren Hart

Bruce, please apply to linux-yocto-3.0/yocto/standard/base

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Queued.

I don't see this in the repository yet, is it still pending? I'm trying to write a BSP that is dependent on it.'

It's queued with the rest of my 3.0 updates. I've been spending time on 3.2 and haven't pushed these out yet.

FYI: You can see this in the 3.0 kernel repo now, I fetched davem's net repo and cherry-picked it in. The SRCREV update will follow once I've merged 3.0.18 (or .17 if .18 takes too long).

Cheers,

Bruce

Quoted 102 lines

Bruce

Quoted 94 lines

Thanks,

Darren

Quoted 88 lines

Bruce

Quoted 70 lines

Upstream-Status: Accepted (Linux 3.3)

If the MAC is invalid or not implemented, do not abort the probe. Issue a warning and prevent bringing the interface up until a MAC is set manually
(via ifconfig $IFACE hw ether $MAC).

Tested on two platforms, one with a valid MAC, the other without a MAC. The real
MAC is used if present, the interface fails to come up until the MAC is set on
the other. They successfully get an IP over DHCP and pass a simple ping and
login over ssh test.

This is meant to allow the Inforce SYS940X development board:
http://www.inforcecomputing.com/SYS940X_ECX.html
(and others suffering from a missing MAC) to work with the mainline kernel.
Without this patch, the probe will fail and the interface will not be created,
preventing the user from configuring the MAC manually.

This does not make any attempt to address a missing or invalid MAC for the
pch_phub driver.

CC Arjan van de Ven<[email protected]>
CC Alan Cox<[email protected]>
CC Tomoya MORINAGA<[email protected]>
CC Jeff Kirsher<[email protected]>
CC "David S. Miller"<[email protected]>
CC Paul Gortmaker<[email protected]>
CC Jon Mason<[email protected]>
CC Mark Brown<[email protected]>
CC David Laight<[email protected]>
CC Joe Perches<[email protected]>

---
drivers/net/pch_gbe/pch_gbe_main.c | 17 ++++++++++++++---

1 files changed, 14 insertions(+), 3 deletions(-)
drivers/net/pch_gbe/pch_gbe_main.c +7 -3
index eac3c5c..e7412f2 100644
@@ -1701,6 +1701,12 @@ int pch_gbe_up(struct pch_gbe_adapter *adapter)
 struct pch_gbe_rx_ring *rx_ring = adapter->rx_ring;
 int err;
 
+ /* Ensure we have a valid MAC */
+ if (!is_valid_ether_addr(adapter->hw.mac.addr)) {
+ pr_err("Error: Invalid MAC address\n");
+ return -EINVAL;
+ }
+
 /* hardware has been reset, we need to reload some things */
 pch_gbe_set_multi(netdev);
 
@@ -2392,9 +2398,14 @@ static int pch_gbe_probe(struct pci_dev *pdev,
 
 memcpy(netdev->dev_addr, adapter->hw.mac.addr, netdev->addr_len);
 if (!is_valid_ether_addr(netdev->dev_addr)) {
- dev_err(&pdev->dev, "Invalid MAC Address\n");
- ret = -EIO;
- goto err_free_adapter;
+ /*

drivers/net/pch_gbe/pch_gbe_main.c static int pch_gbe_probe(struct pci_dev *pdev, · line 2411

Quoted 10 lines
drivers/net/pch_gbe/pch_gbe_main.c +7 -0
+ * If the MAC is invalid (or just missing), display a warning
+ * but do not abort setting up the device. pch_gbe_up will
+ * prevent the interface from being brought up until a valid MAC
+ * is set.
+ */
+ dev_err(&pdev->dev, "Invalid MAC address, "
+ "interface disabled.\n");
 }
 setup_timer(&adapter->watchdog_timer, pch_gbe_watchdog,
 (unsigned long)adapter);

_______________________________________________
yocto mailing list
[email protected]
https://lists.yoctoproject.org/listinfo/yocto

_______________________________________________
yocto mailing list
[email protected]
https://lists.yoctoproject.org/listinfo/yocto

#5 lore raw
Quoted 13 lines · Bruce Ashfield
Quoted 12 lines · Bruce Ashfield
Quoted 10 lines · Darren Hart
Quoted 4 lines · Bruce Ashfield
Quoted 1 line · Darren Hart

Bruce, please apply to linux-yocto-3.0/yocto/standard/base

Queued.

I don't see this in the repository yet, is it still pending? I'm trying to write a BSP that is dependent on it.'

Quoted 6 lines
Quoted 2 lines

It's queued with the rest of my 3.0 updates. I've been spending time on 3.2 and haven't pushed these out yet.

FYI: You can see this in the 3.0 kernel repo now, I fetched davem's net repo and cherry-picked it in. The SRCREV update will follow once I've merged 3.0.18 (or .17 if .18 takes too long).

That's perfect, I can set SRCREV's in my meta-intel BSP in the interim, but when the patch isn't there, it's a messier process.

Thanks Bruce

Signature · 3 lines
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel