Re: [PATCH net v3] tg3: use random MAC address when tg3_get_device_address fails

[Ivan Delalande <colona@arista.com>]

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

On Mon, Sep 21, 2026 at 11:43:57PM +0000, netdev-bot+sashiko@kernel.org 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,

> 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?

-- 
Ivan Delalande - Arista Networks