Skip to contentSkip to conversation

tg3: use random MAC address when tg3_get_device_address fails

PATCH Superseded Ivan Delalande 3 participants 4 messages Sep 17 – Sep 22

Subsystems (3)

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 tg3: use random MAC address when tg3_get_device_address fails

Some of the tg3 NICs we use (BCM57762) reset the SRAM MAC address to the placeholder address on link flaps, tg3_chip_reset, etc. We've typically fixed it from userspace, but since e4c00ba7274b ("tg3: replace placeholder MAC address with device property") was merged, tg3 just fails probe as we don't have a way to get it through the generic device_get_mac_address infrastructure as fallback on our systems.

Make the driver assign a random address in this condition instead of being fatal for probe.

Fixes e4c00ba7274b ("tg3: replace placeholder MAC address with device property")

---
v3: handle EPROBE_DEFER, set deferred_reason through dev_err_probe.
v2: https://lore.kernel.org/netdev/20260914230521.GA1400582@visor/t/
assign random address instead of changing the logic in tg3_get_device_address.
v1: https://lore.kernel.org/netdev/20260903233255.GA4109366@visor/t/

---
 drivers/net/ethernet/broadcom/tg3.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)
drivers/net/ethernet/broadcom/tg3.c +9 -4
index 73a4b569b03e..813181590bce 100644
@@ -17914,12 +17914,17 @@ static int tg3_init_one(struct pci_dev *pdev,
 	}
 
 	err = tg3_get_device_address(tp, addr);
-	if (err) {
-		dev_err(&pdev->dev,
-			"Could not obtain valid ethernet address, aborting\n");
+	if (err == -EPROBE_DEFER) {
+		dev_err_probe(&pdev->dev, err, "ethernet address provider not ready\n");
 		goto err_out_apeunmap;
 	}
-	eth_hw_addr_set(dev, addr);
+	if (err) {
+		dev_warn(&pdev->dev,
+			 "Could not obtain valid ethernet address, using a random address\n");
+		eth_hw_addr_random(dev);
+	} else {
+		eth_hw_addr_set(dev, addr);
+	}
 
 	intmbx = MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW;
 	rcvmbx = MAILBOX_RCVRET_CON_IDX_0 + TG3_64BIT_REG_LOW;
Signature · 1 line
2.43.0
#1 lore raw

drivers/net/ethernet/broadcom/tg3.c static int tg3_init_one(struct pci_dev *pdev, · line 17918

Quoted 2 lines
drivers/net/ethernet/broadcom/tg3.c +2 -0
+	if (err == -EPROBE_DEFER) {
+		dev_err_probe(&pdev->dev, err, "ethernet address provider not ready\n");

Please go read the documentation for dev_err_probe()

Andrew

---
pw-bot: cr

#2 lore raw
In reply to Ivan Delalande
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider.

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each issue of Medium severity or higher and explain why the AI is wrong. If any of them are legitimate and you plan to respin, please let patchwork know by including "pw-bot: cr" as a separate line at the end of your reply (one such reply per series is enough).

- [Medium] The new random-MAC fallback is taken on tg3_get_device_address()'s -EINVAL path, which returns *before* the…

--

tg3: use random MAC address when tg3_get_device_address fails

When tg3_get_device_address() cannot find a valid MAC address, the driver now assigns a random address and continues probing instead of failing. -EPROBE_DEFER from the address provider is still propagated as a probe deferral.

Quoted 13 lines
drivers/net/ethernet/broadcom/tg3.c +2 -3
index 73a4b569b03e3..813181590bce9 100644
@@ -17914,12 +17914,17 @@ static int tg3_init_one(struct pci_dev *pdev,
 	}
 
 	err = tg3_get_device_address(tp, addr);
-	if (err) {
-		dev_err(&pdev->dev,
-			"Could not obtain valid ethernet address, aborting\n");
+	if (err == -EPROBE_DEFER) {
+		dev_err_probe(&pdev->dev, err, "ethernet address provider not ready\n");

drivers/net/ethernet/broadcom/tg3.c static int tg3_init_one(struct pci_dev *pdev, · line 17927

Quoted 10 lines
drivers/net/ethernet/broadcom/tg3.c +7 -1
 		goto err_out_apeunmap;
 	}
-	eth_hw_addr_set(dev, addr);
+	if (err) {
+		dev_warn(&pdev->dev,
+			 "Could not obtain valid ethernet address, using a random address\n");
+		eth_hw_addr_random(dev);
+	} else {
+		eth_hw_addr_set(dev, addr);
+	}

[Severity: Medium]
Can this randomize the address on a system that does have a MAC available through the generic provider infrastructure?

The tail of tg3_get_device_address() looks like this:

	if (!is_valid_ether_addr(addr))
		return -EINVAL;

	if (tg3_is_default_mac_address(addr))
		return device_get_mac_address(&tp->pdev->dev, addr);

	return 0;

So the -EINVAL return happens before the only device_get_mac_address() call in the driver. If the SRAM mailbox has no 0x484b signature (or reports the zero address the comment in that function mentions as "Some old bootcode may report a 0 MAC address in SRAM"), and NVRAM plus MAC_ADDR_0_HIGH/MAC_ADDR_0_LOW also yield an invalid address, the function returns -EINVAL and the fwnode "mac-address"/"local-mac-address"/ "address" properties and the nvmem "mac-address" cell are never consulted.

tg3_init_one() then skips the new err == -EPROBE_DEFER test and calls eth_hw_addr_random(), so the device binds with NET_ADDR_RANDOM and only a dev_warn, even when device_get_mac_address() would have supplied the real address - or would have returned -EPROBE_DEFER because the nvmem provider is not registered yet.

Does the earlier call in tg3_get_device_address() cover that case?

	if (!eth_platform_get_mac_address(&tp->pdev->dev, addr))
		return 0;

eth_platform_get_mac_address() in net/ethernet/eth.c only looks at dev->of_node and arch_get_platform_mac_address(), and folds every failure into -ENODEV:

	ret = of_get_mac_address(dev->of_node, mac_addr);
	if (!ret)
		return 0;

	addr = arch_get_platform_mac_address();
	if (!addr)
		return -ENODEV;

while device_get_mac_address() is the one that reaches the non-OF fwnode properties and the device level nvmem cell:

	if (!fwnode_get_mac_address(dev_fwnode(dev), addr))
		return 0;

	return nvmem_get_mac_address(dev, addr);

Would it make sense to query device_get_mac_address() on the invalid address path too, before falling back to eth_hw_addr_random(), so that a firmware supplied MAC and a genuine -EPROBE_DEFER are not lost?

Signature · 1 line
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260917223416.GA245819%40visor
#3 lore raw

There is v4 at https://lore.kernel.org/r/20260918224715.GA654128@visor
by the way, which hopefully addresses what Andrew had in mind.

Quoted 3 lines · On Mon, Sep 21, 2026 at 11:43:57PM +0000, [email protected] wrote:

[Severity: Medium]
Can this randomize the address on a system that does have a MAC available through the generic provider infrastructure?

This point still stands in v4, and I can make that change inside tg3_get_device_address, but what do you think Pavan? It sounded like from the v1 discussion that you preferred we avoid changing and cleaning up tg3_get_device_address much further.

Thanks a lot,

Quoted 52 lines

The tail of tg3_get_device_address() looks like this:

	if (!is_valid_ether_addr(addr))
		return -EINVAL;

	if (tg3_is_default_mac_address(addr))
		return device_get_mac_address(&tp->pdev->dev, addr);

	return 0;

So the -EINVAL return happens before the only device_get_mac_address() call in the driver. If the SRAM mailbox has no 0x484b signature (or reports the zero address the comment in that function mentions as "Some old bootcode may report a 0 MAC address in SRAM"), and NVRAM plus MAC_ADDR_0_HIGH/MAC_ADDR_0_LOW also yield an invalid address, the function returns -EINVAL and the fwnode "mac-address"/"local-mac-address"/ "address" properties and the nvmem "mac-address" cell are never consulted.

tg3_init_one() then skips the new err == -EPROBE_DEFER test and calls eth_hw_addr_random(), so the device binds with NET_ADDR_RANDOM and only a dev_warn, even when device_get_mac_address() would have supplied the real address - or would have returned -EPROBE_DEFER because the nvmem provider is not registered yet.

Does the earlier call in tg3_get_device_address() cover that case?

	if (!eth_platform_get_mac_address(&tp->pdev->dev, addr))
		return 0;

eth_platform_get_mac_address() in net/ethernet/eth.c only looks at dev->of_node and arch_get_platform_mac_address(), and folds every failure into -ENODEV:

	ret = of_get_mac_address(dev->of_node, mac_addr);
	if (!ret)
		return 0;

	addr = arch_get_platform_mac_address();
	if (!addr)
		return -ENODEV;

while device_get_mac_address() is the one that reaches the non-OF fwnode properties and the device level nvmem cell:

	if (!fwnode_get_mac_address(dev_fwnode(dev), addr))
		return 0;

	return nvmem_get_mac_address(dev, addr);

Would it make sense to query device_get_mac_address() on the invalid address path too, before falling back to eth_hw_addr_random(), so that a firmware supplied MAC and a genuine -EPROBE_DEFER are not lost?

Signature · 1 line
Ivan Delalande - Arista Networks