[PATCH net-next 0/4] net: dsa: move master ethtool code

[PATCH net-next 0/4] net: dsa: move master ethtool code

[Vivien Didelot]

The DSA core overrides the master device's ethtool_ops structure so that
it can inject statistics and such of its dedicated switch CPU port.

This ethtool code is currently called on unnecessary conditions or
before the master interface and its switch CPU port get wired up.
This patchset fixes this.

Similarly to slave.c where the DSA slave net_device is the entry point
of the dsa_slave_* functions, this patchset also isolates the master's
ethtool code in a new master.c file, where the DSA master net_device is
the entry point of the dsa_master_* functions.

This is a first step towards better control of the master device and
support for multiple CPU ports.

Vivien Didelot (4):
  net: dsa: remove copy of master ethtool_ops
  net: dsa: setup master ethtool unconditionally
  net: dsa: setup master ethtool after dsa_ptr
  net: dsa: move master ethtool code

 include/net/dsa.h  |   1 -
 net/dsa/Makefile   |   2 +-
 net/dsa/dsa.c      |  28 -------------
 net/dsa/dsa2.c     |  18 ++++----
 net/dsa/dsa_priv.h |   7 ++--
 net/dsa/legacy.c   |  10 ++---
 net/dsa/master.c   | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/dsa/slave.c    |  80 -----------------------------------
 8 files changed, 136 insertions(+), 130 deletions(-)
 create mode 100644 net/dsa/master.c

-- 
2.14.1

[PATCH net-next 1/4] net: dsa: remove copy of master ethtool_ops

[Vivien Didelot]

There is no need to store a copy of the master ethtool ops, storing the
original pointer in DSA and the new one in the master netdev itself is
enough.

In the meantime, set orig_ethtool_ops to NULL when restoring the master
ethtool ops and check the presence of the master original ethtool ops as
well as its needed functions before calling them.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 include/net/dsa.h |  1 -
 net/dsa/dsa.c     |  8 ++++----
 net/dsa/slave.c   | 19 +++++++++++--------
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index dd44d6ce1097..8dee216a5a9b 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -188,7 +188,6 @@ struct dsa_port {
 	/*
 	 * Original copy of the master netdev ethtool_ops
 	 */
-	struct ethtool_ops	ethtool_ops;
 	const struct ethtool_ops *orig_ethtool_ops;
 };
 
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 03c58b0eb082..abadf7b49236 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -124,11 +124,10 @@ int dsa_cpu_port_ethtool_setup(struct dsa_port *cpu_dp)
 	if (!cpu_ops)
 		return -ENOMEM;
 
-	memcpy(&cpu_dp->ethtool_ops, master->ethtool_ops,
-	       sizeof(struct ethtool_ops));
 	cpu_dp->orig_ethtool_ops = master->ethtool_ops;
-	memcpy(cpu_ops, &cpu_dp->ethtool_ops,
-	       sizeof(struct ethtool_ops));
+	if (cpu_dp->orig_ethtool_ops)
+		memcpy(cpu_ops, cpu_dp->orig_ethtool_ops, sizeof(*cpu_ops));
+
 	dsa_cpu_port_ethtool_init(cpu_ops);
 	master->ethtool_ops = cpu_ops;
 
@@ -138,6 +137,7 @@ int dsa_cpu_port_ethtool_setup(struct dsa_port *cpu_dp)
 void dsa_cpu_port_ethtool_restore(struct dsa_port *cpu_dp)
 {
 	cpu_dp->netdev->ethtool_ops = cpu_dp->orig_ethtool_ops;
+	cpu_dp->orig_ethtool_ops = NULL;
 }
 
 void dsa_cpu_dsa_destroy(struct dsa_port *port)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 2afa99506f8b..2ff4f907d137 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -574,12 +574,13 @@ static void dsa_cpu_port_get_ethtool_stats(struct net_device *dev,
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
 	struct dsa_port *cpu_dp = dsa_get_cpu_port(dst);
 	struct dsa_switch *ds = cpu_dp->ds;
+	const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops;
 	s8 cpu_port = cpu_dp->index;
 	int count = 0;
 
-	if (cpu_dp->ethtool_ops.get_sset_count) {
-		count = cpu_dp->ethtool_ops.get_sset_count(dev, ETH_SS_STATS);
-		cpu_dp->ethtool_ops.get_ethtool_stats(dev, stats, data);
+	if (ops && ops->get_sset_count && ops->get_ethtool_stats) {
+		count = ops->get_sset_count(dev, ETH_SS_STATS);
+		ops->get_ethtool_stats(dev, stats, data);
 	}
 
 	if (ds->ops->get_ethtool_stats)
@@ -591,10 +592,11 @@ static int dsa_cpu_port_get_sset_count(struct net_device *dev, int sset)
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
 	struct dsa_port *cpu_dp = dsa_get_cpu_port(dst);
 	struct dsa_switch *ds = cpu_dp->ds;
+	const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops;
 	int count = 0;
 
-	if (cpu_dp->ethtool_ops.get_sset_count)
-		count += cpu_dp->ethtool_ops.get_sset_count(dev, sset);
+	if (ops && ops->get_sset_count)
+		count += ops->get_sset_count(dev, sset);
 
 	if (sset == ETH_SS_STATS && ds->ops->get_sset_count)
 		count += ds->ops->get_sset_count(ds);
@@ -608,6 +610,7 @@ static void dsa_cpu_port_get_strings(struct net_device *dev,
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
 	struct dsa_port *cpu_dp = dsa_get_cpu_port(dst);
 	struct dsa_switch *ds = cpu_dp->ds;
+	const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops;
 	s8 cpu_port = cpu_dp->index;
 	int len = ETH_GSTRING_LEN;
 	int mcount = 0, count;
@@ -619,9 +622,9 @@ static void dsa_cpu_port_get_strings(struct net_device *dev,
 	/* We do not want to be NULL-terminated, since this is a prefix */
 	pfx[sizeof(pfx) - 1] = '_';
 
-	if (cpu_dp->ethtool_ops.get_sset_count) {
-		mcount = cpu_dp->ethtool_ops.get_sset_count(dev, ETH_SS_STATS);
-		cpu_dp->ethtool_ops.get_strings(dev, stringset, data);
+	if (ops && ops->get_sset_count && ops->get_strings) {
+		mcount = ops->get_sset_count(dev, ETH_SS_STATS);
+		ops->get_strings(dev, stringset, data);
 	}
 
 	if (stringset == ETH_SS_STATS && ds->ops->get_strings) {
-- 
2.14.1

Re: [PATCH net-next 1/4] net: dsa: remove copy of master ethtool_ops

[Florian Fainelli]

On 09/19/2017 08:56 AM, Vivien Didelot wrote:
> There is no need to store a copy of the master ethtool ops, storing the
> original pointer in DSA and the new one in the master netdev itself is
> enough.
> 
> In the meantime, set orig_ethtool_ops to NULL when restoring the master
> ethtool ops and check the presence of the master original ethtool ops as
> well as its needed functions before calling them.

I clearly like memcpy() too much, this looks good:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH net-next 2/4] net: dsa: setup master ethtool unconditionally

[Vivien Didelot]

When a DSA switch tree is meant to be applied, it already has a CPU
port. Thus remove the condition of dst->cpu_dp.

Moreover, the next lines access dst->cpu_dp unconditionally.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/dsa2.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 873af0108e24..bd19304f862f 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -433,11 +433,9 @@ static int dsa_dst_apply(struct dsa_switch_tree *dst)
 			return err;
 	}
 
-	if (dst->cpu_dp) {
-		err = dsa_cpu_port_ethtool_setup(dst->cpu_dp);
-		if (err)
-			return err;
-	}
+	err = dsa_cpu_port_ethtool_setup(dst->cpu_dp);
+	if (err)
+		return err;
 
 	/* If we use a tagging format that doesn't have an ethertype
 	 * field, make sure that all packets from this point on get
@@ -474,10 +472,8 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
 		dsa_ds_unapply(dst, ds);
 	}
 
-	if (dst->cpu_dp) {
-		dsa_cpu_port_ethtool_restore(dst->cpu_dp);
-		dst->cpu_dp = NULL;
-	}
+	dsa_cpu_port_ethtool_restore(dst->cpu_dp);
+	dst->cpu_dp = NULL;
 
 	pr_info("DSA: tree %d unapplied\n", dst->tree);
 	dst->applied = false;
-- 
2.14.1

Re: [PATCH net-next 2/4] net: dsa: setup master ethtool unconditionally

[Florian Fainelli]

On 09/19/2017 08:56 AM, Vivien Didelot wrote:
> When a DSA switch tree is meant to be applied, it already has a CPU
> port. Thus remove the condition of dst->cpu_dp.
> 
> Moreover, the next lines access dst->cpu_dp unconditionally.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH net-next 3/4] net: dsa: setup master ethtool after dsa_ptr

[Vivien Didelot]

DSA overrides the master's ethtool ops so that we can inject its CPU
port's statistics. Because of that, we need to setup the ethtool ops
after the master's dsa_ptr pointer has been assigned, not before.

This patch setups the ethtool ops after dsa_ptr is assigned, and
restores them before it gets cleared.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/dsa2.c   | 12 +++++++-----
 net/dsa/legacy.c | 10 +++-------
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index bd19304f862f..032f8bc3e788 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -433,16 +433,17 @@ static int dsa_dst_apply(struct dsa_switch_tree *dst)
 			return err;
 	}
 
-	err = dsa_cpu_port_ethtool_setup(dst->cpu_dp);
-	if (err)
-		return err;
-
 	/* If we use a tagging format that doesn't have an ethertype
 	 * field, make sure that all packets from this point on get
 	 * sent to the tag format's receive function.
 	 */
 	wmb();
 	dst->cpu_dp->netdev->dsa_ptr = dst;
+
+	err = dsa_cpu_port_ethtool_setup(dst->cpu_dp);
+	if (err)
+		return err;
+
 	dst->applied = true;
 
 	return 0;
@@ -456,6 +457,8 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
 	if (!dst->applied)
 		return;
 
+	dsa_cpu_port_ethtool_restore(dst->cpu_dp);
+
 	dst->cpu_dp->netdev->dsa_ptr = NULL;
 
 	/* If we used a tagging format that doesn't have an ethertype
@@ -472,7 +475,6 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
 		dsa_ds_unapply(dst, ds);
 	}
 
-	dsa_cpu_port_ethtool_restore(dst->cpu_dp);
 	dst->cpu_dp = NULL;
 
 	pr_info("DSA: tree %d unapplied\n", dst->tree);
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index 91e6f7981d39..163910699db7 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -206,10 +206,6 @@ static int dsa_switch_setup_one(struct dsa_switch *ds,
 		netdev_err(master, "[%d] : can't configure CPU and DSA ports\n",
 			   index);
 
-	ret = dsa_cpu_port_ethtool_setup(ds->dst->cpu_dp);
-	if (ret)
-		return ret;
-
 	return 0;
 }
 
@@ -606,7 +602,7 @@ static int dsa_setup_dst(struct dsa_switch_tree *dst, struct net_device *dev,
 	wmb();
 	dev->dsa_ptr = dst;
 
-	return 0;
+	return dsa_cpu_port_ethtool_setup(dst->cpu_dp);
 }
 
 static int dsa_probe(struct platform_device *pdev)
@@ -671,6 +667,8 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
 {
 	int i;
 
+	dsa_cpu_port_ethtool_restore(dst->cpu_dp);
+
 	dst->cpu_dp->netdev->dsa_ptr = NULL;
 
 	/* If we used a tagging format that doesn't have an ethertype
@@ -686,8 +684,6 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
 			dsa_switch_destroy(ds);
 	}
 
-	dsa_cpu_port_ethtool_restore(dst->cpu_dp);
-
 	dev_put(dst->cpu_dp->netdev);
 }
 
-- 
2.14.1

Re: [PATCH net-next 3/4] net: dsa: setup master ethtool after dsa_ptr

[Florian Fainelli]

On 09/19/2017 08:56 AM, Vivien Didelot wrote:
> DSA overrides the master's ethtool ops so that we can inject its CPU
> port's statistics. Because of that, we need to setup the ethtool ops
> after the master's dsa_ptr pointer has been assigned, not before.

Yes, good point, technically this is a bugfix, but since we have changed
this quite often and the race is tiny, I am not positive we could a)
trigger this in real life, and b) provide a proper Fixes tag.

> 
> This patch setups the ethtool ops after dsa_ptr is assigned, and
> restores them before it gets cleared.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH net-next 4/4] net: dsa: move master ethtool code

[Vivien Didelot]

DSA overrides the master device ethtool ops, so that it can inject stats
from its dedicated switch CPU port as well.

The related code is currently split in dsa.c and slave.c, but it only
scopes the master net device. Move it to a new master.c DSA core file.

This file will be later extented with master net device specific code.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/Makefile   |   2 +-
 net/dsa/dsa.c      |  28 -------------
 net/dsa/dsa2.c     |   4 +-
 net/dsa/dsa_priv.h |   7 ++--
 net/dsa/legacy.c   |   4 +-
 net/dsa/master.c   | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/dsa/slave.c    |  83 ------------------------------------
 7 files changed, 129 insertions(+), 119 deletions(-)
 create mode 100644 net/dsa/master.c

diff --git a/net/dsa/Makefile b/net/dsa/Makefile
index fcce25da937c..2e7ac8bab19d 100644
--- a/net/dsa/Makefile
+++ b/net/dsa/Makefile
@@ -1,6 +1,6 @@
 # the core
 obj-$(CONFIG_NET_DSA) += dsa_core.o
-dsa_core-y += dsa.o dsa2.o legacy.o port.o slave.o switch.o
+dsa_core-y += dsa.o dsa2.o legacy.o master.o port.o slave.o switch.o
 
 # tagging formats
 dsa_core-$(CONFIG_NET_DSA_TAG_BRCM) += tag_brcm.o
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index abadf7b49236..81c852e32821 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -112,34 +112,6 @@ const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol)
 	return ops;
 }
 
-int dsa_cpu_port_ethtool_setup(struct dsa_port *cpu_dp)
-{
-	struct dsa_switch *ds = cpu_dp->ds;
-	struct net_device *master;
-	struct ethtool_ops *cpu_ops;
-
-	master = cpu_dp->netdev;
-
-	cpu_ops = devm_kzalloc(ds->dev, sizeof(*cpu_ops), GFP_KERNEL);
-	if (!cpu_ops)
-		return -ENOMEM;
-
-	cpu_dp->orig_ethtool_ops = master->ethtool_ops;
-	if (cpu_dp->orig_ethtool_ops)
-		memcpy(cpu_ops, cpu_dp->orig_ethtool_ops, sizeof(*cpu_ops));
-
-	dsa_cpu_port_ethtool_init(cpu_ops);
-	master->ethtool_ops = cpu_ops;
-
-	return 0;
-}
-
-void dsa_cpu_port_ethtool_restore(struct dsa_port *cpu_dp)
-{
-	cpu_dp->netdev->ethtool_ops = cpu_dp->orig_ethtool_ops;
-	cpu_dp->orig_ethtool_ops = NULL;
-}
-
 void dsa_cpu_dsa_destroy(struct dsa_port *port)
 {
 	struct device_node *port_dn = port->dn;
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 032f8bc3e788..dcccaebde708 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -440,7 +440,7 @@ static int dsa_dst_apply(struct dsa_switch_tree *dst)
 	wmb();
 	dst->cpu_dp->netdev->dsa_ptr = dst;
 
-	err = dsa_cpu_port_ethtool_setup(dst->cpu_dp);
+	err = dsa_master_ethtool_setup(dst->cpu_dp->netdev);
 	if (err)
 		return err;
 
@@ -457,7 +457,7 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
 	if (!dst->applied)
 		return;
 
-	dsa_cpu_port_ethtool_restore(dst->cpu_dp);
+	dsa_master_ethtool_restore(dst->cpu_dp->netdev);
 
 	dst->cpu_dp->netdev->dsa_ptr = NULL;
 
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 9c3eeb72462d..f616b3444418 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -97,8 +97,6 @@ struct dsa_slave_priv {
 int dsa_cpu_dsa_setup(struct dsa_port *port);
 void dsa_cpu_dsa_destroy(struct dsa_port *dport);
 const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol);
-int dsa_cpu_port_ethtool_setup(struct dsa_port *cpu_dp);
-void dsa_cpu_port_ethtool_restore(struct dsa_port *cpu_dp);
 bool dsa_schedule_work(struct work_struct *work);
 
 /* legacy.c */
@@ -112,6 +110,10 @@ int dsa_legacy_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
 		       struct net_device *dev,
 		       const unsigned char *addr, u16 vid);
 
+/* master.c */
+int dsa_master_ethtool_setup(struct net_device *dev);
+void dsa_master_ethtool_restore(struct net_device *dev);
+
 /* port.c */
 int dsa_port_set_state(struct dsa_port *dp, u8 state,
 		       struct switchdev_trans *trans);
@@ -139,7 +141,6 @@ int dsa_port_vlan_del(struct dsa_port *dp,
 /* slave.c */
 extern const struct dsa_device_ops notag_netdev_ops;
 void dsa_slave_mii_bus_init(struct dsa_switch *ds);
-void dsa_cpu_port_ethtool_init(struct ethtool_ops *ops);
 int dsa_slave_create(struct dsa_port *port, const char *name);
 void dsa_slave_destroy(struct net_device *slave_dev);
 int dsa_slave_suspend(struct net_device *slave_dev);
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index 163910699db7..ae505d8e4417 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -602,7 +602,7 @@ static int dsa_setup_dst(struct dsa_switch_tree *dst, struct net_device *dev,
 	wmb();
 	dev->dsa_ptr = dst;
 
-	return dsa_cpu_port_ethtool_setup(dst->cpu_dp);
+	return dsa_master_ethtool_setup(dst->cpu_dp->netdev);
 }
 
 static int dsa_probe(struct platform_device *pdev)
@@ -667,7 +667,7 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
 {
 	int i;
 
-	dsa_cpu_port_ethtool_restore(dst->cpu_dp);
+	dsa_master_ethtool_restore(dst->cpu_dp->netdev);
 
 	dst->cpu_dp->netdev->dsa_ptr = NULL;
 
diff --git a/net/dsa/master.c b/net/dsa/master.c
new file mode 100644
index 000000000000..5e5147ec5a44
--- /dev/null
+++ b/net/dsa/master.c
@@ -0,0 +1,120 @@
+/*
+ * Handling of a master device, switching frames via its switch fabric CPU port
+ *
+ * Copyright (c) 2017 Savoir-faire Linux Inc.
+ *	Vivien Didelot <vivien.didelot@savoirfairelinux.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include "dsa_priv.h"
+
+static void dsa_master_get_ethtool_stats(struct net_device *dev,
+					 struct ethtool_stats *stats,
+					 uint64_t *data)
+{
+	struct dsa_switch_tree *dst = dev->dsa_ptr;
+	struct dsa_port *port = dst->cpu_dp;
+	struct dsa_switch *ds = port->ds;
+	const struct ethtool_ops *ops = port->orig_ethtool_ops;
+	int count = 0;
+
+	if (ops && ops->get_sset_count && ops->get_ethtool_stats) {
+		count = ops->get_sset_count(dev, ETH_SS_STATS);
+		ops->get_ethtool_stats(dev, stats, data);
+	}
+
+	if (ds->ops->get_ethtool_stats)
+		ds->ops->get_ethtool_stats(ds, port->index, data + count);
+}
+
+static int dsa_master_get_sset_count(struct net_device *dev, int sset)
+{
+	struct dsa_switch_tree *dst = dev->dsa_ptr;
+	struct dsa_port *port = dst->cpu_dp;
+	struct dsa_switch *ds = port->ds;
+	const struct ethtool_ops *ops = port->orig_ethtool_ops;
+	int count = 0;
+
+	if (ops && ops->get_sset_count)
+		count += ops->get_sset_count(dev, sset);
+
+	if (sset == ETH_SS_STATS && ds->ops->get_sset_count)
+		count += ds->ops->get_sset_count(ds);
+
+	return count;
+}
+
+static void dsa_master_get_strings(struct net_device *dev, uint32_t stringset,
+				   uint8_t *data)
+{
+	struct dsa_switch_tree *dst = dev->dsa_ptr;
+	struct dsa_port *port = dst->cpu_dp;
+	struct dsa_switch *ds = port->ds;
+	const struct ethtool_ops *ops = port->orig_ethtool_ops;
+	int len = ETH_GSTRING_LEN;
+	int mcount = 0, count;
+	unsigned int i;
+	uint8_t pfx[4];
+	uint8_t *ndata;
+
+	snprintf(pfx, sizeof(pfx), "p%.2d", port->index);
+	/* We do not want to be NULL-terminated, since this is a prefix */
+	pfx[sizeof(pfx) - 1] = '_';
+
+	if (ops && ops->get_sset_count && ops->get_strings) {
+		mcount = ops->get_sset_count(dev, ETH_SS_STATS);
+		ops->get_strings(dev, stringset, data);
+	}
+
+	if (stringset == ETH_SS_STATS && ds->ops->get_strings) {
+		ndata = data + mcount * len;
+		/* This function copies ETH_GSTRINGS_LEN bytes, we will mangle
+		 * the output after to prepend our CPU port prefix we
+		 * constructed earlier
+		 */
+		ds->ops->get_strings(ds, port->index, ndata);
+		count = ds->ops->get_sset_count(ds);
+		for (i = 0; i < count; i++) {
+			memmove(ndata + (i * len + sizeof(pfx)),
+				ndata + i * len, len - sizeof(pfx));
+			memcpy(ndata + i * len, pfx, sizeof(pfx));
+		}
+	}
+}
+
+int dsa_master_ethtool_setup(struct net_device *dev)
+{
+	struct dsa_switch_tree *dst = dev->dsa_ptr;
+	struct dsa_port *port = dst->cpu_dp;
+	struct dsa_switch *ds = port->ds;
+	struct ethtool_ops *ops;
+
+	ops = devm_kzalloc(ds->dev, sizeof(*ops), GFP_KERNEL);
+	if (!ops)
+		return -ENOMEM;
+
+	port->orig_ethtool_ops = dev->ethtool_ops;
+	if (port->orig_ethtool_ops)
+		memcpy(ops, port->orig_ethtool_ops, sizeof(*ops));
+
+	ops->get_sset_count = dsa_master_get_sset_count;
+	ops->get_ethtool_stats = dsa_master_get_ethtool_stats;
+	ops->get_strings = dsa_master_get_strings;
+
+	dev->ethtool_ops = ops;
+
+	return 0;
+}
+
+void dsa_master_ethtool_restore(struct net_device *dev)
+{
+	struct dsa_switch_tree *dst = dev->dsa_ptr;
+	struct dsa_port *port = dst->cpu_dp;
+
+	dev->ethtool_ops = port->orig_ethtool_ops;
+	port->orig_ethtool_ops = NULL;
+}
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 2ff4f907d137..d51b10450e1b 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -567,82 +567,6 @@ static void dsa_slave_get_strings(struct net_device *dev,
 	}
 }
 
-static void dsa_cpu_port_get_ethtool_stats(struct net_device *dev,
-					   struct ethtool_stats *stats,
-					   uint64_t *data)
-{
-	struct dsa_switch_tree *dst = dev->dsa_ptr;
-	struct dsa_port *cpu_dp = dsa_get_cpu_port(dst);
-	struct dsa_switch *ds = cpu_dp->ds;
-	const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops;
-	s8 cpu_port = cpu_dp->index;
-	int count = 0;
-
-	if (ops && ops->get_sset_count && ops->get_ethtool_stats) {
-		count = ops->get_sset_count(dev, ETH_SS_STATS);
-		ops->get_ethtool_stats(dev, stats, data);
-	}
-
-	if (ds->ops->get_ethtool_stats)
-		ds->ops->get_ethtool_stats(ds, cpu_port, data + count);
-}
-
-static int dsa_cpu_port_get_sset_count(struct net_device *dev, int sset)
-{
-	struct dsa_switch_tree *dst = dev->dsa_ptr;
-	struct dsa_port *cpu_dp = dsa_get_cpu_port(dst);
-	struct dsa_switch *ds = cpu_dp->ds;
-	const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops;
-	int count = 0;
-
-	if (ops && ops->get_sset_count)
-		count += ops->get_sset_count(dev, sset);
-
-	if (sset == ETH_SS_STATS && ds->ops->get_sset_count)
-		count += ds->ops->get_sset_count(ds);
-
-	return count;
-}
-
-static void dsa_cpu_port_get_strings(struct net_device *dev,
-				     uint32_t stringset, uint8_t *data)
-{
-	struct dsa_switch_tree *dst = dev->dsa_ptr;
-	struct dsa_port *cpu_dp = dsa_get_cpu_port(dst);
-	struct dsa_switch *ds = cpu_dp->ds;
-	const struct ethtool_ops *ops = cpu_dp->orig_ethtool_ops;
-	s8 cpu_port = cpu_dp->index;
-	int len = ETH_GSTRING_LEN;
-	int mcount = 0, count;
-	unsigned int i;
-	uint8_t pfx[4];
-	uint8_t *ndata;
-
-	snprintf(pfx, sizeof(pfx), "p%.2d", cpu_port);
-	/* We do not want to be NULL-terminated, since this is a prefix */
-	pfx[sizeof(pfx) - 1] = '_';
-
-	if (ops && ops->get_sset_count && ops->get_strings) {
-		mcount = ops->get_sset_count(dev, ETH_SS_STATS);
-		ops->get_strings(dev, stringset, data);
-	}
-
-	if (stringset == ETH_SS_STATS && ds->ops->get_strings) {
-		ndata = data + mcount * len;
-		/* This function copies ETH_GSTRINGS_LEN bytes, we will mangle
-		 * the output after to prepend our CPU port prefix we
-		 * constructed earlier
-		 */
-		ds->ops->get_strings(ds, cpu_port, ndata);
-		count = ds->ops->get_sset_count(ds);
-		for (i = 0; i < count; i++) {
-			memmove(ndata + (i * len + sizeof(pfx)),
-				ndata + i * len, len - sizeof(pfx));
-			memcpy(ndata + i * len, pfx, sizeof(pfx));
-		}
-	}
-}
-
 static void dsa_slave_get_ethtool_stats(struct net_device *dev,
 					struct ethtool_stats *stats,
 					uint64_t *data)
@@ -979,13 +903,6 @@ static void dsa_slave_get_stats64(struct net_device *dev,
 	}
 }
 
-void dsa_cpu_port_ethtool_init(struct ethtool_ops *ops)
-{
-	ops->get_sset_count = dsa_cpu_port_get_sset_count;
-	ops->get_ethtool_stats = dsa_cpu_port_get_ethtool_stats;
-	ops->get_strings = dsa_cpu_port_get_strings;
-}
-
 static int dsa_slave_get_rxnfc(struct net_device *dev,
 			       struct ethtool_rxnfc *nfc, u32 *rule_locs)
 {
-- 
2.14.1

Re: [PATCH net-next 4/4] net: dsa: move master ethtool code

[Florian Fainelli]

On 09/19/2017 08:57 AM, Vivien Didelot wrote:
> DSA overrides the master device ethtool ops, so that it can inject stats
> from its dedicated switch CPU port as well.
> 
> The related code is currently split in dsa.c and slave.c, but it only
> scopes the master net device. Move it to a new master.c DSA core file.
> 
> This file will be later extented with master net device specific code.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH net-next 0/4] net: dsa: move master ethtool code

[Florian Fainelli]

On 09/19/2017 08:56 AM, Vivien Didelot wrote:
> The DSA core overrides the master device's ethtool_ops structure so that
> it can inject statistics and such of its dedicated switch CPU port.
> 
> This ethtool code is currently called on unnecessary conditions or
> before the master interface and its switch CPU port get wired up.
> This patchset fixes this.
> 
> Similarly to slave.c where the DSA slave net_device is the entry point
> of the dsa_slave_* functions, this patchset also isolates the master's
> ethtool code in a new master.c file, where the DSA master net_device is
> the entry point of the dsa_master_* functions.
> 
> This is a first step towards better control of the master device and
> support for multiple CPU ports.

Tested-by: Florian Fainelli <f.fainelli@gmail.com>

* ethtool -S eth0 -> switch port CPU stats are still correctly overlayed
* ethtool -s gphy wol g -> both switch port and CPU port correctly
enable WoL
* ethtool -i eth0 -> driver still reports correct information

Thanks!

> 
> Vivien Didelot (4):
>   net: dsa: remove copy of master ethtool_ops
>   net: dsa: setup master ethtool unconditionally
>   net: dsa: setup master ethtool after dsa_ptr
>   net: dsa: move master ethtool code
> 
>  include/net/dsa.h  |   1 -
>  net/dsa/Makefile   |   2 +-
>  net/dsa/dsa.c      |  28 -------------
>  net/dsa/dsa2.c     |  18 ++++----
>  net/dsa/dsa_priv.h |   7 ++--
>  net/dsa/legacy.c   |  10 ++---
>  net/dsa/master.c   | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  net/dsa/slave.c    |  80 -----------------------------------
>  8 files changed, 136 insertions(+), 130 deletions(-)
>  create mode 100644 net/dsa/master.c
> 


-- 
Florian

Re: [PATCH net-next 0/4] net: dsa: move master ethtool code

[David Miller]

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Tue, 19 Sep 2017 13:04:56 -0700

> On 09/19/2017 08:56 AM, Vivien Didelot wrote:
>> The DSA core overrides the master device's ethtool_ops structure so that
>> it can inject statistics and such of its dedicated switch CPU port.
>> 
>> This ethtool code is currently called on unnecessary conditions or
>> before the master interface and its switch CPU port get wired up.
>> This patchset fixes this.
>> 
>> Similarly to slave.c where the DSA slave net_device is the entry point
>> of the dsa_slave_* functions, this patchset also isolates the master's
>> ethtool code in a new master.c file, where the DSA master net_device is
>> the entry point of the dsa_master_* functions.
>> 
>> This is a first step towards better control of the master device and
>> support for multiple CPU ports.
> 
> Tested-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> * ethtool -S eth0 -> switch port CPU stats are still correctly overlayed
> * ethtool -s gphy wol g -> both switch port and CPU port correctly
> enable WoL
> * ethtool -i eth0 -> driver still reports correct information

Series applied, thanks everyone.