[PATCH net-next 0/4] net: mvpp2: fix the mac address retrieval logic

[PATCH net-next 0/4] net: mvpp2: fix the mac address retrieval logic

[Antoine Tenart]

Hi all,

The MAC address retrieval logic was broken and when using the PPv2
driver on PPv2.2 engines I ended up using the same mac address on all
ports. This series of patches fixes this, and also tackle a possible bug
when defining the mac address in the device tree.

To fix this in a nice way I ended up using a dedicated function to
handle the mac retrieval logic. This can be hard to backport into stable
kernels. This is why I also made a quick fix which is easy to backport
(patch 1/14), to tackle down the PPv2.2 mac retrieval bug. Let me know
if this approach is the proper way to handle this or if I should do
something else.

Thanks!
Antoine

Fixes: 2697582144dd ("net: mvpp2: handle misc PPv2.1/PPv2.2 differences")

Antoine Tenart (4):
  net: mvpp2: fix the mac address used when using PPv2.2
  net: mvpp2: move the mac retrieval/copy logic into its own function
  net: mvpp2: fix use of the random mac address for PPv2.2
  net: mvpp2: fallback using h/w and random mac if the dt one isn't
    valid

 drivers/net/ethernet/marvell/mvpp2.c | 48 ++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 18 deletions(-)

-- 
2.13.5

[PATCH net-next 1/4] net: mvpp2: fix the mac address used when using PPv2.2

[Antoine Tenart]

The mac address is only retrieved from h/w when using PPv2.1. Otherwise
the variable holding it is still checked and used if it contains a valid
value. As the variable isn't initialized to an invalid mac address
value, we end up with random mac addresses which can be the same for all
the ports handled by this PPv2 driver.

Fixes this by initializing the h/w mac address variable to {0}, which is
an invalid mac address value. This way the random assignation fallback
is called and all ports end up with their own addresses.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Fixes: 2697582144dd ("net: mvpp2: handle misc PPv2.1/PPv2.2 differences")
---
 drivers/net/ethernet/marvell/mvpp2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 3f8cbc070dc4..50a0920d3282 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -7466,7 +7466,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	struct resource *res;
 	const char *dt_mac_addr;
 	const char *mac_from;
-	char hw_mac_addr[ETH_ALEN];
+	char hw_mac_addr[ETH_ALEN] = {0};
 	unsigned int ntxqs, nrxqs;
 	bool has_tx_irqs;
 	u32 id;
-- 
2.13.5

[PATCH net-next 2/4] net: mvpp2: move the mac retrieval/copy logic into its own function

[Antoine Tenart]

The MAC retrieval has a quite complicated logic (which is broken). Moves
it to its own function to prepare for patches fixing its logic, so that
reviews are easier.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 45 +++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 50a0920d3282..908e5b148fd7 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -7453,6 +7453,31 @@ static bool mvpp2_port_has_tx_irqs(struct mvpp2 *priv,
 	return true;
 }
 
+static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
+				     struct device_node *port_node,
+				     char **mac_from)
+{
+	struct mvpp2_port *port = netdev_priv(dev);
+	char hw_mac_addr[ETH_ALEN] = {0};
+	const char *dt_mac_addr;
+
+	dt_mac_addr = of_get_mac_address(port_node);
+	if (dt_mac_addr && is_valid_ether_addr(dt_mac_addr)) {
+		*mac_from = "device tree";
+		ether_addr_copy(dev->dev_addr, dt_mac_addr);
+	} else {
+		if (priv->hw_version == MVPP21)
+			mvpp21_get_mac_address(port, hw_mac_addr);
+		if (is_valid_ether_addr(hw_mac_addr)) {
+			*mac_from = "hardware";
+			ether_addr_copy(dev->dev_addr, hw_mac_addr);
+		} else {
+			*mac_from = "random";
+			eth_hw_addr_random(dev);
+		}
+	}
+}
+
 /* Ports initialization */
 static int mvpp2_port_probe(struct platform_device *pdev,
 			    struct device_node *port_node,
@@ -7464,9 +7489,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	struct mvpp2_port_pcpu *port_pcpu;
 	struct net_device *dev;
 	struct resource *res;
-	const char *dt_mac_addr;
-	const char *mac_from;
-	char hw_mac_addr[ETH_ALEN] = {0};
+	char *mac_from = "";
 	unsigned int ntxqs, nrxqs;
 	bool has_tx_irqs;
 	u32 id;
@@ -7575,21 +7598,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 		goto err_free_irq;
 	}
 
-	dt_mac_addr = of_get_mac_address(port_node);
-	if (dt_mac_addr && is_valid_ether_addr(dt_mac_addr)) {
-		mac_from = "device tree";
-		ether_addr_copy(dev->dev_addr, dt_mac_addr);
-	} else {
-		if (priv->hw_version == MVPP21)
-			mvpp21_get_mac_address(port, hw_mac_addr);
-		if (is_valid_ether_addr(hw_mac_addr)) {
-			mac_from = "hardware";
-			ether_addr_copy(dev->dev_addr, hw_mac_addr);
-		} else {
-			mac_from = "random";
-			eth_hw_addr_random(dev);
-		}
-	}
+	mvpp2_port_copy_mac_addr(dev, priv, port_node, &mac_from);
 
 	port->tx_ring_size = MVPP2_MAX_TXD;
 	port->rx_ring_size = MVPP2_MAX_RXD;
-- 
2.13.5

[PATCH net-next 3/4] net: mvpp2: fix use of the random mac address for PPv2.2

[Antoine Tenart]

The MAC retrieval logic is using a variable to store an h/w stored mac
address and checks this mac against invalid ones before using it. But
the mac address is only read from h/w when using PPv2.1. So when using
PPv2.2 it defaults to its init state.

This patches fixes the logic to only check if the h/w mac is valid when
actually retrieving a mac from h/w.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 908e5b148fd7..fe8309124a09 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -7466,15 +7466,17 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
 		*mac_from = "device tree";
 		ether_addr_copy(dev->dev_addr, dt_mac_addr);
 	} else {
-		if (priv->hw_version == MVPP21)
+		if (priv->hw_version == MVPP21) {
 			mvpp21_get_mac_address(port, hw_mac_addr);
-		if (is_valid_ether_addr(hw_mac_addr)) {
-			*mac_from = "hardware";
-			ether_addr_copy(dev->dev_addr, hw_mac_addr);
-		} else {
-			*mac_from = "random";
-			eth_hw_addr_random(dev);
+			if (is_valid_ether_addr(hw_mac_addr)) {
+				*mac_from = "hardware";
+				ether_addr_copy(dev->dev_addr, hw_mac_addr);
+				return;
+			}
 		}
+
+		*mac_from = "random";
+		eth_hw_addr_random(dev);
 	}
 }
 
-- 
2.13.5

[PATCH net-next 4/4] net: mvpp2: fallback using h/w and random mac if the dt one isn't valid

[Antoine Tenart]

When using a mac address described in the device tree, a check is made
to see if it is valid. When it's not, no fallback is defined. This
patches tries to get the mac address from h/w (or use a random one if
the h/w one isn't valid) when the dt mac address isn't valid.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index fe8309124a09..b53254ef7cae 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -7465,19 +7465,20 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
 	if (dt_mac_addr && is_valid_ether_addr(dt_mac_addr)) {
 		*mac_from = "device tree";
 		ether_addr_copy(dev->dev_addr, dt_mac_addr);
-	} else {
-		if (priv->hw_version == MVPP21) {
-			mvpp21_get_mac_address(port, hw_mac_addr);
-			if (is_valid_ether_addr(hw_mac_addr)) {
-				*mac_from = "hardware";
-				ether_addr_copy(dev->dev_addr, hw_mac_addr);
-				return;
-			}
-		}
+		return;
+	}
 
-		*mac_from = "random";
-		eth_hw_addr_random(dev);
+	if (priv->hw_version == MVPP21) {
+		mvpp21_get_mac_address(port, hw_mac_addr);
+		if (is_valid_ether_addr(hw_mac_addr)) {
+			*mac_from = "hardware";
+			ether_addr_copy(dev->dev_addr, hw_mac_addr);
+			return;
+		}
 	}
+
+	*mac_from = "random";
+	eth_hw_addr_random(dev);
 }
 
 /* Ports initialization */
-- 
2.13.5

Re: [PATCH net-next 0/4] net: mvpp2: fix the mac address retrieval logic

[David Miller]

From: Antoine Tenart <antoine.tenart@free-electrons.com>
Date: Thu, 24 Aug 2017 11:46:54 +0200

> The MAC address retrieval logic was broken and when using the PPv2
> driver on PPv2.2 engines I ended up using the same mac address on all
> ports. This series of patches fixes this, and also tackle a possible bug
> when defining the mac address in the device tree.
> 
> To fix this in a nice way I ended up using a dedicated function to
> handle the mac retrieval logic. This can be hard to backport into stable
> kernels. This is why I also made a quick fix which is easy to backport
> (patch 1/14), to tackle down the PPv2.2 mac retrieval bug. Let me know
> if this approach is the proper way to handle this or if I should do
> something else.

This patch series doesn't apply to any of my trees, that is the first
thing.

Secondly, this is a bug fix, and the bug exists in the 'net' tree.
Therefore this patch series should target the 'net' tree.

Please always target legitimate bug fixes at the 'net' tree, rather
than 'net-next'.

Thank you.

Re: [PATCH net-next 0/4] net: mvpp2: fix the mac address retrieval logic

[Antoine Tenart]

[-- Attachment #1: Type: text/plain, Size: 1529 bytes --]

Hi Dave,

On Thu, Aug 24, 2017 at 09:46:24PM -0700, David Miller wrote:
> From: Antoine Tenart <antoine.tenart@free-electrons.com>
> Date: Thu, 24 Aug 2017 11:46:54 +0200
> 
> > The MAC address retrieval logic was broken and when using the PPv2
> > driver on PPv2.2 engines I ended up using the same mac address on all
> > ports. This series of patches fixes this, and also tackle a possible bug
> > when defining the mac address in the device tree.
> > 
> > To fix this in a nice way I ended up using a dedicated function to
> > handle the mac retrieval logic. This can be hard to backport into stable
> > kernels. This is why I also made a quick fix which is easy to backport
> > (patch 1/14), to tackle down the PPv2.2 mac retrieval bug. Let me know
> > if this approach is the proper way to handle this or if I should do
> > something else.
> 
> This patch series doesn't apply to any of my trees, that is the first
> thing.

That is very strange, my patches were based on top of net-next. I'll
double check if they apply correctly before sending the v2.

> Secondly, this is a bug fix, and the bug exists in the 'net' tree.
> Therefore this patch series should target the 'net' tree.

OK, that's the question I was asking. I'll resent everything to net
then.

> Please always target legitimate bug fixes at the 'net' tree, rather
> than 'net-next'.

Sure, will do.

Thanks!
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]