Re: [RFC net-next 3/6] ethernet: prestera: use eth_hw_addr_set_port()
From: Jakub Kicinski <kuba@kernel.org>
Date: 2021-10-18 17:01:44
Subsystem:
marvell prestera ethernet switch driver, networking drivers, the rest · Maintainers:
Elad Nachman, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
On Mon, 18 Oct 2021 16:54:00 +0000 Taras Chornyi [C] wrote:
quoted
@@ -341,8 +342,8 @@ static int prestera_port_create(struct prestera_switch *sw, u32 id) /* firmware requires that port's MAC address consist of the first * 5 bytes of the base MAC address */ - memcpy(dev->dev_addr, sw->base_mac, dev->addr_len - 1); - dev->dev_addr[dev->addr_len - 1] = port->fp_id; + memcpy(addr, sw->base_mac, dev->addr_len - 1);This code is a bit buggy. We do care about the last byte of the base mac address. For example if base mac is xx:xx:xx:xx:xx:10 first port mac should be xx:xx:xx:xx:xx:11
Thanks for the reply, does it mean we can assume base_mac will be valid or should we add a check like below?
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c
index b667f560b931..966f94c6c7a6 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_main.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c@@ -338,11 +338,14 @@ static int prestera_port_create(struct prestera_switch *sw, u32 id) goto err_port_init; } - /* firmware requires that port's MAC address consist of the first - * 5 bytes of the base MAC address - */ - memcpy(dev->dev_addr, sw->base_mac, dev->addr_len - 1); - dev->dev_addr[dev->addr_len - 1] = port->fp_id; + eth_hw_addr_set_port(dev, sw->base_mac, port->fp_id); + if (memcmp(dev->dev_addr, sw->base_mac, ETH_ALEN - 1)) { + /* firmware requires that port's MAC address consists + * of the first 5 bytes of the base MAC address + */ + dev_warn(prestera_dev(sw), "Port MAC address overflows the base for port(%u)\n", id); + dev_addr_mod(dev, 0, sw->base_mac, ETH_ALEN - 1); + } err = prestera_hw_port_mac_set(port, dev->dev_addr); if (err) {