[PATCH net-next 0/6] ethernet: add eth_hw_addr_gen() for switches

[PATCH net-next 0/6] ethernet: add eth_hw_addr_gen() for switches

[Jakub Kicinski]

While doing the last polishing of the drivers/ethernet
changes I realized we have a handful of drivers offsetting
some base MAC addr by an id. So I decided to add a helper
for it. The helper takes care of wrapping which is probably
not 100% necessary but seems like a good idea. And it saves
driver side LoC (the diffstat is actually negative if we
compare against the changes I'd have to make if I was to
convert all these drivers to not operate directly on
netdev->dev_addr).

Jakub Kicinski (6):
  ethernet: add a helper for assigning port addresses
  ethernet: ocelot: use eth_hw_addr_gen()
  ethernet: prestera: use eth_hw_addr_gen()
  ethernet: fec: use eth_hw_addr_gen()
  ethernet: mlxsw: use eth_hw_addr_gen()
  ethernet: sparx5: use eth_hw_addr_gen()

 drivers/net/ethernet/freescale/fec_main.c     |  5 +----
 .../ethernet/marvell/prestera/prestera_main.c |  7 +++++--
 drivers/net/ethernet/mellanox/mlxsw/minimal.c | 10 +++------
 .../net/ethernet/mellanox/mlxsw/spectrum.c    |  8 +++----
 .../ethernet/microchip/sparx5/sparx5_netdev.c |  4 +---
 drivers/net/ethernet/mscc/ocelot_net.c        |  3 +--
 include/linux/etherdevice.h                   | 21 +++++++++++++++++++
 7 files changed, 36 insertions(+), 22 deletions(-)

-- 
2.31.1

[PATCH net-next 1/6] ethernet: add a helper for assigning port addresses

[Jakub Kicinski]

We have 5 drivers which offset base MAC addr by port id.
Create a helper for them.

This helper takes care of overflows, which some drivers
did not do, please complain if that's going to break
anything!

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
--
 - eth_hw_addr_set_port() -> eth_hw_addr_gen()
 - id u8 -> unsigned int
---
 include/linux/etherdevice.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index 23681c3d3b8a..2ad71cc90b37 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -551,6 +551,27 @@ static inline unsigned long compare_ether_header(const void *a, const void *b)
 #endif
 }
 
+/**
+ * eth_hw_addr_gen - Generate and assign Ethernet address to a port
+ * @dev: pointer to port's net_device structure
+ * @base_addr: base Ethernet address
+ * @id: offset to add to the base address
+ *
+ * Generate a MAC address using a base address and an offset and assign it
+ * to a net_device. Commonly used by switch drivers which need to compute
+ * addresses for all their ports. addr_assign_type is not changed.
+ */
+static inline void eth_hw_addr_gen(struct net_device *dev, const u8 *base_addr,
+				   unsigned int id)
+{
+	u64 u = ether_addr_to_u64(base_addr);
+	u8 addr[ETH_ALEN];
+
+	u += id;
+	u64_to_ether_addr(u, addr);
+	eth_hw_addr_set(dev, addr);
+}
+
 /**
  * eth_skb_pad - Pad buffer to mininum number of octets for Ethernet frame
  * @skb: Buffer to pad
-- 
2.31.1

Re: [PATCH net-next 1/6] ethernet: add a helper for assigning port addresses

[Vladimir Oltean]

On Mon, Oct 18, 2021 at 02:10:02PM -0700, Jakub Kicinski wrote:
> We have 5 drivers which offset base MAC addr by port id.
> Create a helper for them.
> 
> This helper takes care of overflows, which some drivers
> did not do, please complain if that's going to break
> anything!
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> --

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Re: [PATCH net-next 1/6] ethernet: add a helper for assigning port addresses

[Shannon Nelson]

On 10/18/21 2:10 PM, Jakub Kicinski wrote:
> We have 5 drivers which offset base MAC addr by port id.
> Create a helper for them.
>
> This helper takes care of overflows, which some drivers
> did not do, please complain if that's going to break
> anything!
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Shannon Nelson <snelson@pensando.io>


> --
>   - eth_hw_addr_set_port() -> eth_hw_addr_gen()
>   - id u8 -> unsigned int
> ---
>   include/linux/etherdevice.h | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
>
> diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
> index 23681c3d3b8a..2ad71cc90b37 100644
> --- a/include/linux/etherdevice.h
> +++ b/include/linux/etherdevice.h
> @@ -551,6 +551,27 @@ static inline unsigned long compare_ether_header(const void *a, const void *b)
>   #endif
>   }
>   
> +/**
> + * eth_hw_addr_gen - Generate and assign Ethernet address to a port
> + * @dev: pointer to port's net_device structure
> + * @base_addr: base Ethernet address
> + * @id: offset to add to the base address
> + *
> + * Generate a MAC address using a base address and an offset and assign it
> + * to a net_device. Commonly used by switch drivers which need to compute
> + * addresses for all their ports. addr_assign_type is not changed.
> + */
> +static inline void eth_hw_addr_gen(struct net_device *dev, const u8 *base_addr,
> +				   unsigned int id)
> +{
> +	u64 u = ether_addr_to_u64(base_addr);
> +	u8 addr[ETH_ALEN];
> +
> +	u += id;
> +	u64_to_ether_addr(u, addr);
> +	eth_hw_addr_set(dev, addr);
> +}
> +
>   /**
>    * eth_skb_pad - Pad buffer to mininum number of octets for Ethernet frame
>    * @skb: Buffer to pad

Re: [PATCH net-next 1/6] ethernet: add a helper for assigning port addresses

[Ido Schimmel]

On Mon, Oct 18, 2021 at 02:10:02PM -0700, Jakub Kicinski wrote:
> We have 5 drivers which offset base MAC addr by port id.
> Create a helper for them.
> 
> This helper takes care of overflows, which some drivers
> did not do, please complain if that's going to break
> anything!
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

> --
>  - eth_hw_addr_set_port() -> eth_hw_addr_gen()
>  - id u8 -> unsigned int

Thanks!

[PATCH net-next 2/6] ethernet: ocelot: use eth_hw_addr_gen()

[Jakub Kicinski]

Commit 406f42fa0d3c ("net-next: When a bond have a massive amount
of VLANs...") introduced a rbtree for faster Ethernet address look
up. To maintain netdev->dev_addr in this tree we need to make all
the writes to it got through appropriate helpers.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/mscc/ocelot_net.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index 9992bf06311d..affa9649f490 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -1705,8 +1705,7 @@ int ocelot_probe_port(struct ocelot *ocelot, int port, struct regmap *target,
 		NETIF_F_HW_TC;
 	dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_TC;
 
-	eth_hw_addr_set(dev, ocelot->base_mac);
-	dev->dev_addr[ETH_ALEN - 1] += port;
+	eth_hw_addr_gen(dev, ocelot->base_mac, port);
 	ocelot_mact_learn(ocelot, PGID_CPU, dev->dev_addr,
 			  ocelot_port->pvid_vlan.vid, ENTRYTYPE_LOCKED);
 
-- 
2.31.1

Re: [PATCH net-next 2/6] ethernet: ocelot: use eth_hw_addr_gen()

[Vladimir Oltean]

On Mon, Oct 18, 2021 at 02:10:03PM -0700, Jakub Kicinski wrote:
> Commit 406f42fa0d3c ("net-next: When a bond have a massive amount
> of VLANs...") introduced a rbtree for faster Ethernet address look
> up. To maintain netdev->dev_addr in this tree we need to make all
> the writes to it got through appropriate helpers.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>

[PATCH net-next 3/6] ethernet: prestera: use eth_hw_addr_gen()

[Jakub Kicinski]

Commit 406f42fa0d3c ("net-next: When a bond have a massive amount
of VLANs...") introduced a rbtree for faster Ethernet address look
up. To maintain netdev->dev_addr in this tree we need to make all
the writes to it got through appropriate helpers.

Vadym and Taras report that the current behavior of the driver
is not exactly expected and it's better to add the port id in
like other drivers do.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/marvell/prestera/prestera_main.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c
index b667f560b931..d0d5a229d19d 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;
 	}
 
+	eth_hw_addr_gen(dev, sw->base_mac, port->fp_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;
+	if (memcmp(dev->dev_addr, sw->base_mac, ETH_ALEN - 1)) {
+		dev_warn(prestera_dev(sw), "Port MAC address wraps 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) {
-- 
2.31.1

[PATCH net-next 4/6] ethernet: fec: use eth_hw_addr_gen()

[Jakub Kicinski]

Commit 406f42fa0d3c ("net-next: When a bond have a massive amount
of VLANs...") introduced a rbtree for faster Ethernet address look
up. To maintain netdev->dev_addr in this tree we need to make all
the writes to it got through appropriate helpers.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/freescale/fec_main.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 47a6fc702ac7..bc418b910999 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1768,11 +1768,8 @@ static int fec_get_mac(struct net_device *ndev)
 		return 0;
 	}
 
-	eth_hw_addr_set(ndev, iap);
-
 	/* Adjust MAC if using macaddr */
-	if (iap == macaddr)
-		 ndev->dev_addr[ETH_ALEN-1] = macaddr[ETH_ALEN-1] + fep->dev_id;
+	eth_hw_addr_gen(ndev, iap, iap == macaddr ? fep->dev_id : 0);
 
 	return 0;
 }
-- 
2.31.1

Re: [PATCH net-next 4/6] ethernet: fec: use eth_hw_addr_gen()

[Vladimir Oltean]

On Mon, Oct 18, 2021 at 02:10:05PM -0700, Jakub Kicinski wrote:
> Commit 406f42fa0d3c ("net-next: When a bond have a massive amount
> of VLANs...") introduced a rbtree for faster Ethernet address look
> up. To maintain netdev->dev_addr in this tree we need to make all
> the writes to it got through appropriate helpers.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>

[PATCH net-next 5/6] ethernet: mlxsw: use eth_hw_addr_gen()

[Jakub Kicinski]

Commit 406f42fa0d3c ("net-next: When a bond have a massive amount
of VLANs...") introduced a rbtree for faster Ethernet address look
up. To maintain netdev->dev_addr in this tree we need to make all
the writes to it got through appropriate helpers.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 - remove the dev temp variable as well
---
 drivers/net/ethernet/mellanox/mlxsw/minimal.c  | 10 +++-------
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c |  8 ++++----
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/minimal.c b/drivers/net/ethernet/mellanox/mlxsw/minimal.c
index e0892f259adf..5d4dfa5ddbb5 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/minimal.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/minimal.c
@@ -200,20 +200,16 @@ static int
 mlxsw_m_port_dev_addr_get(struct mlxsw_m_port *mlxsw_m_port)
 {
 	struct mlxsw_m *mlxsw_m = mlxsw_m_port->mlxsw_m;
-	struct net_device *dev = mlxsw_m_port->dev;
 	char ppad_pl[MLXSW_REG_PPAD_LEN];
+	u8 addr[ETH_ALEN];
 	int err;
 
 	mlxsw_reg_ppad_pack(ppad_pl, false, 0);
 	err = mlxsw_reg_query(mlxsw_m->core, MLXSW_REG(ppad), ppad_pl);
 	if (err)
 		return err;
-	mlxsw_reg_ppad_mac_memcpy_from(ppad_pl, dev->dev_addr);
-	/* The last byte value in base mac address is guaranteed
-	 * to be such it does not overflow when adding local_port
-	 * value.
-	 */
-	dev->dev_addr[ETH_ALEN - 1] += mlxsw_m_port->module + 1;
+	mlxsw_reg_ppad_mac_memcpy_from(ppad_pl, addr);
+	eth_hw_addr_gen(mlxsw_m_port->dev, addr, mlxsw_m_port->module + 1);
 	return 0;
 }
 
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index d05850ff3a77..66c346a86ec5 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -316,11 +316,11 @@ static int mlxsw_sp_port_dev_addr_set(struct mlxsw_sp_port *mlxsw_sp_port,
 static int mlxsw_sp_port_dev_addr_init(struct mlxsw_sp_port *mlxsw_sp_port)
 {
 	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
-	unsigned char *addr = mlxsw_sp_port->dev->dev_addr;
 
-	ether_addr_copy(addr, mlxsw_sp->base_mac);
-	addr[ETH_ALEN - 1] += mlxsw_sp_port->local_port;
-	return mlxsw_sp_port_dev_addr_set(mlxsw_sp_port, addr);
+	eth_hw_addr_gen(mlxsw_sp_port->dev, mlxsw_sp->base_mac,
+			mlxsw_sp_port->local_port);
+	return mlxsw_sp_port_dev_addr_set(mlxsw_sp_port,
+					  mlxsw_sp_port->dev->dev_addr);
 }
 
 static int mlxsw_sp_port_max_mtu_get(struct mlxsw_sp_port *mlxsw_sp_port, int *p_max_mtu)
-- 
2.31.1

Re: [PATCH net-next 5/6] ethernet: mlxsw: use eth_hw_addr_gen()

[Ido Schimmel]

On Mon, Oct 18, 2021 at 02:10:06PM -0700, Jakub Kicinski wrote:
> Commit 406f42fa0d3c ("net-next: When a bond have a massive amount
> of VLANs...") introduced a rbtree for faster Ethernet address look
> up. To maintain netdev->dev_addr in this tree we need to make all
> the writes to it got through appropriate helpers.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

Thanks!

[PATCH net-next 6/6] ethernet: sparx5: use eth_hw_addr_gen()

[Jakub Kicinski]

Commit 406f42fa0d3c ("net-next: When a bond have a massive amount
of VLANs...") introduced a rbtree for faster Ethernet address look
up. To maintain netdev->dev_addr in this tree we need to make all
the writes to it got through appropriate helpers.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c b/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
index b21ebaa32d7e..e042f117dc7a 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
@@ -200,7 +200,6 @@ struct net_device *sparx5_create_netdev(struct sparx5 *sparx5, u32 portno)
 {
 	struct sparx5_port *spx5_port;
 	struct net_device *ndev;
-	u64 val;
 
 	ndev = devm_alloc_etherdev(sparx5->dev, sizeof(struct sparx5_port));
 	if (!ndev)
@@ -216,8 +215,7 @@ struct net_device *sparx5_create_netdev(struct sparx5 *sparx5, u32 portno)
 	ndev->netdev_ops = &sparx5_port_netdev_ops;
 	ndev->ethtool_ops = &sparx5_ethtool_ops;
 
-	val = ether_addr_to_u64(sparx5->base_mac) + portno + 1;
-	u64_to_ether_addr(val, ndev->dev_addr);
+	eth_hw_addr_gen(ndev, sparx5->base_mac, portno + 1);
 
 	return ndev;
 }
-- 
2.31.1

Re: [PATCH net-next 0/6] ethernet: add eth_hw_addr_gen() for switches

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Mon, 18 Oct 2021 14:10:01 -0700 you wrote:
> While doing the last polishing of the drivers/ethernet
> changes I realized we have a handful of drivers offsetting
> some base MAC addr by an id. So I decided to add a helper
> for it. The helper takes care of wrapping which is probably
> not 100% necessary but seems like a good idea. And it saves
> driver side LoC (the diffstat is actually negative if we
> compare against the changes I'd have to make if I was to
> convert all these drivers to not operate directly on
> netdev->dev_addr).
> 
> [...]

Here is the summary with links:
  - [net-next,1/6] ethernet: add a helper for assigning port addresses
    https://git.kernel.org/netdev/net-next/c/e80094a473ee
  - [net-next,2/6] ethernet: ocelot: use eth_hw_addr_gen()
    https://git.kernel.org/netdev/net-next/c/53fdcce6ab93
  - [net-next,3/6] ethernet: prestera: use eth_hw_addr_gen()
    https://git.kernel.org/netdev/net-next/c/8eb8192ea291
  - [net-next,4/6] ethernet: fec: use eth_hw_addr_gen()
    https://git.kernel.org/netdev/net-next/c/ba3fdfe32bb9
  - [net-next,5/6] ethernet: mlxsw: use eth_hw_addr_gen()
    https://git.kernel.org/netdev/net-next/c/be7550549e26
  - [net-next,6/6] ethernet: sparx5: use eth_hw_addr_gen()
    https://git.kernel.org/netdev/net-next/c/07a7ec9bdafe

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html