[PATCH net-next 0/5] net: dsa: master and slave helpers

netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH net-next 0/5] net: dsa: master and slave helpers
@ 2017-10-12 22:51 Vivien Didelot
  2017-10-12 22:51 ` [PATCH net-next 1/5] net: dsa: use port's cpu_dp when creating a slave Vivien Didelot
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Vivien Didelot @ 2017-10-12 22:51 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

This patch series adds a few helpers to DSA core for clarity and
readability but brings no functional changes.

A dsa_slave_notify helper calls the DSA notifiers when (un)registering a
slave device.

Most of the DSA slave code only needs to access the dsa_port structure,
not the dsa_slave_priv (which only contains a few PHY-specific members).
Thus a dsa_slave_to_port helper returns a dsa_port structure of a slave
device.

A dsa_slave_get_master returns the master device of a slave device.

After that the netdev member of the dsa_port structure is split into two
explicit master and slave members to avoid confusion, even though it is
not planned to create a slave for DSA or CPU ports for the moment.

Vivien Didelot (5):
  net: dsa: use port's cpu_dp when creating a slave
  net: dsa: add slave notify helper
  net: dsa: add slave to port helper
  net: dsa: add slave get master helper
  net: dsa: split dsa_port's netdev member

 drivers/net/dsa/bcm_sf2.c        |   6 +-
 drivers/net/dsa/mt7530.c         |   2 +-
 drivers/net/dsa/mv88e6xxx/chip.c |   2 +-
 include/net/dsa.h                |   7 +-
 net/dsa/dsa.c                    |   6 +-
 net/dsa/dsa2.c                   |  22 ++--
 net/dsa/dsa_priv.h               |  22 ++--
 net/dsa/legacy.c                 |  20 ++--
 net/dsa/slave.c                  | 227 +++++++++++++++++++--------------------
 net/dsa/tag_brcm.c               |   9 +-
 net/dsa/tag_dsa.c                |  10 +-
 net/dsa/tag_edsa.c               |  10 +-
 net/dsa/tag_ksz.c                |   4 +-
 net/dsa/tag_lan9303.c            |   4 +-
 net/dsa/tag_mtk.c                |   4 +-
 net/dsa/tag_qca.c                |   5 +-
 net/dsa/tag_trailer.c            |   4 +-
 17 files changed, 183 insertions(+), 181 deletions(-)

-- 
2.14.2

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH net-next 1/5] net: dsa: use port's cpu_dp when creating a slave
  2017-10-12 22:51 [PATCH net-next 0/5] net: dsa: master and slave helpers Vivien Didelot
@ 2017-10-12 22:51 ` Vivien Didelot
  2017-10-12 22:53   ` Florian Fainelli
  2017-10-12 22:51 ` [PATCH net-next 2/5] net: dsa: add slave notify helper Vivien Didelot
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Vivien Didelot @ 2017-10-12 22:51 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

When dsa_slave_create is called, the related port already has a CPU port
assigned to it, available in its cpu_dp member. Use it instead of the
unique tree cpu_dp.

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

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 45f4ea845c07..c6f4829645bf 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1117,16 +1117,13 @@ int dsa_slave_resume(struct net_device *slave_dev)
 int dsa_slave_create(struct dsa_port *port, const char *name)
 {
 	struct dsa_notifier_register_info rinfo = { };
+	struct dsa_port *cpu_dp = port->cpu_dp;
+	struct net_device *master = cpu_dp->netdev;
 	struct dsa_switch *ds = port->ds;
-	struct net_device *master;
 	struct net_device *slave_dev;
 	struct dsa_slave_priv *p;
-	struct dsa_port *cpu_dp;
 	int ret;
 
-	cpu_dp = ds->dst->cpu_dp;
-	master = cpu_dp->netdev;
-
 	if (!ds->num_tx_queues)
 		ds->num_tx_queues = 1;
 
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next 1/5] net: dsa: use port's cpu_dp when creating a slave
  2017-10-12 22:51 ` [PATCH net-next 1/5] net: dsa: use port's cpu_dp when creating a slave Vivien Didelot
@ 2017-10-12 22:53   ` Florian Fainelli
  0 siblings, 0 replies; 18+ messages in thread
From: Florian Fainelli @ 2017-10-12 22:53 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn

On 10/12/2017 03:51 PM, Vivien Didelot wrote:
> When dsa_slave_create is called, the related port already has a CPU port
> assigned to it, available in its cpu_dp member. Use it instead of the
> unique tree cpu_dp.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH net-next 2/5] net: dsa: add slave notify helper
  2017-10-12 22:51 [PATCH net-next 0/5] net: dsa: master and slave helpers Vivien Didelot
  2017-10-12 22:51 ` [PATCH net-next 1/5] net: dsa: use port's cpu_dp when creating a slave Vivien Didelot
@ 2017-10-12 22:51 ` Vivien Didelot
  2017-10-12 22:55   ` Florian Fainelli
  2017-10-12 22:51 ` [PATCH net-next 3/5] net: dsa: add slave to port helper Vivien Didelot
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Vivien Didelot @ 2017-10-12 22:51 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

Both DSA slave create and destroy functions call call_dsa_notifiers with
respectively DSA_PORT_REGISTER and DSA_PORT_UNREGISTER and the same
dsa_notifier_register_info structure.

Wrap this in a dsa_slave_notify helper so prevent cluttering these
functions.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/slave.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index c6f4829645bf..f31737256f69 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1114,9 +1114,23 @@ int dsa_slave_resume(struct net_device *slave_dev)
 	return 0;
 }
 
+static void dsa_slave_notify(struct net_device *dev, unsigned long val)
+{
+	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct net_device *master = dsa_master_netdev(p);
+	struct dsa_port *dp = p->dp;
+	struct dsa_notifier_register_info rinfo = {
+		.switch_number = dp->ds->index,
+		.port_number = dp->index,
+		.master = master,
+		.info.dev = dev,
+	};
+
+	call_dsa_notifiers(val, dev, &rinfo.info);
+}
+
 int dsa_slave_create(struct dsa_port *port, const char *name)
 {
-	struct dsa_notifier_register_info rinfo = { };
 	struct dsa_port *cpu_dp = port->cpu_dp;
 	struct net_device *master = cpu_dp->netdev;
 	struct dsa_switch *ds = port->ds;
@@ -1175,11 +1189,7 @@ int dsa_slave_create(struct dsa_port *port, const char *name)
 		goto out_free;
 	}
 
-	rinfo.info.dev = slave_dev;
-	rinfo.master = master;
-	rinfo.port_number = p->dp->index;
-	rinfo.switch_number = p->dp->ds->index;
-	call_dsa_notifiers(DSA_PORT_REGISTER, slave_dev, &rinfo.info);
+	dsa_slave_notify(slave_dev, DSA_PORT_REGISTER);
 
 	ret = register_netdev(slave_dev);
 	if (ret) {
@@ -1204,7 +1214,6 @@ int dsa_slave_create(struct dsa_port *port, const char *name)
 void dsa_slave_destroy(struct net_device *slave_dev)
 {
 	struct dsa_slave_priv *p = netdev_priv(slave_dev);
-	struct dsa_notifier_register_info rinfo = { };
 	struct device_node *port_dn;
 
 	port_dn = p->dp->dn;
@@ -1216,11 +1225,7 @@ void dsa_slave_destroy(struct net_device *slave_dev)
 		if (of_phy_is_fixed_link(port_dn))
 			of_phy_deregister_fixed_link(port_dn);
 	}
-	rinfo.info.dev = slave_dev;
-	rinfo.master = p->dp->cpu_dp->netdev;
-	rinfo.port_number = p->dp->index;
-	rinfo.switch_number = p->dp->ds->index;
-	call_dsa_notifiers(DSA_PORT_UNREGISTER, slave_dev, &rinfo.info);
+	dsa_slave_notify(slave_dev, DSA_PORT_UNREGISTER);
 	unregister_netdev(slave_dev);
 	free_percpu(p->stats64);
 	free_netdev(slave_dev);
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next 2/5] net: dsa: add slave notify helper
  2017-10-12 22:51 ` [PATCH net-next 2/5] net: dsa: add slave notify helper Vivien Didelot
@ 2017-10-12 22:55   ` Florian Fainelli
  0 siblings, 0 replies; 18+ messages in thread
From: Florian Fainelli @ 2017-10-12 22:55 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn

On 10/12/2017 03:51 PM, Vivien Didelot wrote:
> Both DSA slave create and destroy functions call call_dsa_notifiers with
> respectively DSA_PORT_REGISTER and DSA_PORT_UNREGISTER and the same
> dsa_notifier_register_info structure.
> 
> Wrap this in a dsa_slave_notify helper so prevent cluttering these
> functions.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Much nicer, thanks!

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH net-next 3/5] net: dsa: add slave to port helper
  2017-10-12 22:51 [PATCH net-next 0/5] net: dsa: master and slave helpers Vivien Didelot
  2017-10-12 22:51 ` [PATCH net-next 1/5] net: dsa: use port's cpu_dp when creating a slave Vivien Didelot
  2017-10-12 22:51 ` [PATCH net-next 2/5] net: dsa: add slave notify helper Vivien Didelot
@ 2017-10-12 22:51 ` Vivien Didelot
  2017-10-12 22:59   ` Florian Fainelli
  2017-10-12 22:51 ` [PATCH net-next 4/5] net: dsa: add slave get master helper Vivien Didelot
  2017-10-12 22:51 ` [PATCH net-next 5/5] net: dsa: split dsa_port's netdev member Vivien Didelot
  4 siblings, 1 reply; 18+ messages in thread
From: Vivien Didelot @ 2017-10-12 22:51 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

Many portions of DSA core code require to get the dsa_port structure
corresponding to a slave net_device. For this purpose, introduce a
dsa_slave_to_port() helper.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/dsa_priv.h    |   7 +++
 net/dsa/legacy.c      |   6 +-
 net/dsa/slave.c       | 167 +++++++++++++++++++++++++-------------------------
 net/dsa/tag_brcm.c    |   9 ++-
 net/dsa/tag_dsa.c     |  10 +--
 net/dsa/tag_edsa.c    |  10 +--
 net/dsa/tag_ksz.c     |   4 +-
 net/dsa/tag_lan9303.c |   4 +-
 net/dsa/tag_mtk.c     |   4 +-
 net/dsa/tag_qca.c     |   5 +-
 net/dsa/tag_trailer.c |   4 +-
 11 files changed, 115 insertions(+), 115 deletions(-)

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 2850077cc9cc..569a4929b4c9 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -169,6 +169,13 @@ int dsa_slave_resume(struct net_device *slave_dev);
 int dsa_slave_register_notifier(void);
 void dsa_slave_unregister_notifier(void);
 
+static inline struct dsa_port *dsa_slave_to_port(const struct net_device *dev)
+{
+	struct dsa_slave_priv *p = netdev_priv(dev);
+
+	return p->dp;
+}
+
 /* switch.c */
 int dsa_switch_register_notifier(struct dsa_switch *ds);
 void dsa_switch_unregister_notifier(struct dsa_switch *ds);
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index 19ff6e0a21dc..6f2254753859 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -746,8 +746,7 @@ int dsa_legacy_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
 		       const unsigned char *addr, u16 vid,
 		       u16 flags)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_port *dp = p->dp;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 
 	return dsa_port_fdb_add(dp, addr, vid);
 }
@@ -756,8 +755,7 @@ int dsa_legacy_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
 		       struct net_device *dev,
 		       const unsigned char *addr, u16 vid)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_port *dp = p->dp;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 
 	return dsa_port_fdb_del(dp, addr, vid);
 }
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index f31737256f69..894602c88b09 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -72,8 +72,8 @@ static int dsa_slave_get_iflink(const struct net_device *dev)
 static int dsa_slave_open(struct net_device *dev)
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_port *dp = p->dp;
 	struct net_device *master = dsa_master_netdev(p);
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	int err;
 
 	if (!(master->flags & IFF_UP))
@@ -122,7 +122,7 @@ static int dsa_slave_close(struct net_device *dev)
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
 	struct net_device *master = dsa_master_netdev(p);
-	struct dsa_port *dp = p->dp;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 
 	if (dev->phydev)
 		phy_stop(dev->phydev);
@@ -246,14 +246,13 @@ dsa_slave_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb,
 		   struct net_device *dev, struct net_device *filter_dev,
 		   int *idx)
 {
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	struct dsa_slave_dump_ctx dump = {
 		.dev = dev,
 		.skb = skb,
 		.cb = cb,
 		.idx = *idx,
 	};
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_port *dp = p->dp;
 	int err;
 
 	err = dsa_port_fdb_dump(dp, dsa_slave_port_fdb_do_dump, &dump);
@@ -274,8 +273,7 @@ static int dsa_slave_port_attr_set(struct net_device *dev,
 				   const struct switchdev_attr *attr,
 				   struct switchdev_trans *trans)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_port *dp = p->dp;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	int ret;
 
 	switch (attr->id) {
@@ -301,8 +299,7 @@ static int dsa_slave_port_obj_add(struct net_device *dev,
 				  const struct switchdev_obj *obj,
 				  struct switchdev_trans *trans)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_port *dp = p->dp;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	int err;
 
 	/* For the prepare phase, ensure the full set of changes is feasable in
@@ -329,8 +326,7 @@ static int dsa_slave_port_obj_add(struct net_device *dev,
 static int dsa_slave_port_obj_del(struct net_device *dev,
 				  const struct switchdev_obj *obj)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_port *dp = p->dp;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	int err;
 
 	switch (obj->id) {
@@ -351,8 +347,8 @@ static int dsa_slave_port_obj_del(struct net_device *dev,
 static int dsa_slave_port_attr_get(struct net_device *dev,
 				   struct switchdev_attr *attr)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_switch *ds = p->dp->ds;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
 
 	switch (attr->id) {
 	case SWITCHDEV_ATTR_ID_PORT_PARENT_ID:
@@ -431,11 +427,11 @@ static void dsa_slave_get_drvinfo(struct net_device *dev,
 
 static int dsa_slave_get_regs_len(struct net_device *dev)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_switch *ds = p->dp->ds;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
 
 	if (ds->ops->get_regs_len)
-		return ds->ops->get_regs_len(ds, p->dp->index);
+		return ds->ops->get_regs_len(ds, dp->index);
 
 	return -EOPNOTSUPP;
 }
@@ -443,11 +439,11 @@ static int dsa_slave_get_regs_len(struct net_device *dev)
 static void
 dsa_slave_get_regs(struct net_device *dev, struct ethtool_regs *regs, void *_p)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_switch *ds = p->dp->ds;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
 
 	if (ds->ops->get_regs)
-		ds->ops->get_regs(ds, p->dp->index, regs, _p);
+		ds->ops->get_regs(ds, dp->index, regs, _p);
 }
 
 static u32 dsa_slave_get_link(struct net_device *dev)
@@ -462,8 +458,8 @@ static u32 dsa_slave_get_link(struct net_device *dev)
 
 static int dsa_slave_get_eeprom_len(struct net_device *dev)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_switch *ds = p->dp->ds;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
 
 	if (ds->cd && ds->cd->eeprom_len)
 		return ds->cd->eeprom_len;
@@ -477,8 +473,8 @@ static int dsa_slave_get_eeprom_len(struct net_device *dev)
 static int dsa_slave_get_eeprom(struct net_device *dev,
 				struct ethtool_eeprom *eeprom, u8 *data)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_switch *ds = p->dp->ds;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
 
 	if (ds->ops->get_eeprom)
 		return ds->ops->get_eeprom(ds, eeprom, data);
@@ -489,8 +485,8 @@ static int dsa_slave_get_eeprom(struct net_device *dev,
 static int dsa_slave_set_eeprom(struct net_device *dev,
 				struct ethtool_eeprom *eeprom, u8 *data)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_switch *ds = p->dp->ds;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
 
 	if (ds->ops->set_eeprom)
 		return ds->ops->set_eeprom(ds, eeprom, data);
@@ -501,8 +497,8 @@ static int dsa_slave_set_eeprom(struct net_device *dev,
 static void dsa_slave_get_strings(struct net_device *dev,
 				  uint32_t stringset, uint8_t *data)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_switch *ds = p->dp->ds;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
 
 	if (stringset == ETH_SS_STATS) {
 		int len = ETH_GSTRING_LEN;
@@ -512,7 +508,7 @@ static void dsa_slave_get_strings(struct net_device *dev,
 		strncpy(data + 2 * len, "rx_packets", len);
 		strncpy(data + 3 * len, "rx_bytes", len);
 		if (ds->ops->get_strings)
-			ds->ops->get_strings(ds, p->dp->index, data + 4 * len);
+			ds->ops->get_strings(ds, dp->index, data + 4 * len);
 	}
 }
 
@@ -520,8 +516,9 @@ static void dsa_slave_get_ethtool_stats(struct net_device *dev,
 					struct ethtool_stats *stats,
 					uint64_t *data)
 {
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_switch *ds = p->dp->ds;
+	struct dsa_switch *ds = dp->ds;
 	struct pcpu_sw_netstats *s;
 	unsigned int start;
 	int i;
@@ -543,13 +540,13 @@ static void dsa_slave_get_ethtool_stats(struct net_device *dev,
 		data[3] += rx_bytes;
 	}
 	if (ds->ops->get_ethtool_stats)
-		ds->ops->get_ethtool_stats(ds, p->dp->index, data + 4);
+		ds->ops->get_ethtool_stats(ds, dp->index, data + 4);
 }
 
 static int dsa_slave_get_sset_count(struct net_device *dev, int sset)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_switch *ds = p->dp->ds;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
 
 	if (sset == ETH_SS_STATS) {
 		int count;
@@ -566,29 +563,29 @@ static int dsa_slave_get_sset_count(struct net_device *dev, int sset)
 
 static void dsa_slave_get_wol(struct net_device *dev, struct ethtool_wolinfo *w)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_switch *ds = p->dp->ds;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
 
 	if (ds->ops->get_wol)
-		ds->ops->get_wol(ds, p->dp->index, w);
+		ds->ops->get_wol(ds, dp->index, w);
 }
 
 static int dsa_slave_set_wol(struct net_device *dev, struct ethtool_wolinfo *w)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_switch *ds = p->dp->ds;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
 	int ret = -EOPNOTSUPP;
 
 	if (ds->ops->set_wol)
-		ret = ds->ops->set_wol(ds, p->dp->index, w);
+		ret = ds->ops->set_wol(ds, dp->index, w);
 
 	return ret;
 }
 
 static int dsa_slave_set_eee(struct net_device *dev, struct ethtool_eee *e)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_switch *ds = p->dp->ds;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
 	int ret;
 
 	/* Port's PHY and MAC both need to be EEE capable */
@@ -598,7 +595,7 @@ static int dsa_slave_set_eee(struct net_device *dev, struct ethtool_eee *e)
 	if (!ds->ops->set_mac_eee)
 		return -EOPNOTSUPP;
 
-	ret = ds->ops->set_mac_eee(ds, p->dp->index, e);
+	ret = ds->ops->set_mac_eee(ds, dp->index, e);
 	if (ret)
 		return ret;
 
@@ -613,8 +610,8 @@ static int dsa_slave_set_eee(struct net_device *dev, struct ethtool_eee *e)
 
 static int dsa_slave_get_eee(struct net_device *dev, struct ethtool_eee *e)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_switch *ds = p->dp->ds;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
 	int ret;
 
 	/* Port's PHY and MAC both need to be EEE capable */
@@ -624,7 +621,7 @@ static int dsa_slave_get_eee(struct net_device *dev, struct ethtool_eee *e)
 	if (!ds->ops->get_mac_eee)
 		return -EOPNOTSUPP;
 
-	ret = ds->ops->get_mac_eee(ds, p->dp->index, e);
+	ret = ds->ops->get_mac_eee(ds, dp->index, e);
 	if (ret)
 		return ret;
 
@@ -676,9 +673,9 @@ static void dsa_slave_poll_controller(struct net_device *dev)
 static int dsa_slave_get_phys_port_name(struct net_device *dev,
 					char *name, size_t len)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 
-	if (snprintf(name, len, "p%d", p->dp->index) >= len)
+	if (snprintf(name, len, "p%d", dp->index) >= len)
 		return -EINVAL;
 
 	return 0;
@@ -701,14 +698,15 @@ static int dsa_slave_add_cls_matchall(struct net_device *dev,
 				      struct tc_cls_matchall_offload *cls,
 				      bool ingress)
 {
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	struct dsa_slave_priv *p = netdev_priv(dev);
 	struct dsa_mall_tc_entry *mall_tc_entry;
 	__be16 protocol = cls->common.protocol;
-	struct dsa_switch *ds = p->dp->ds;
 	struct net *net = dev_net(dev);
-	struct dsa_slave_priv *to_p;
+	struct dsa_switch *ds = dp->ds;
 	struct net_device *to_dev;
 	const struct tc_action *a;
+	struct dsa_port *to_dp;
 	int err = -EOPNOTSUPP;
 	LIST_HEAD(actions);
 	int ifindex;
@@ -741,13 +739,12 @@ static int dsa_slave_add_cls_matchall(struct net_device *dev,
 		mall_tc_entry->type = DSA_PORT_MALL_MIRROR;
 		mirror = &mall_tc_entry->mirror;
 
-		to_p = netdev_priv(to_dev);
+		to_dp = dsa_slave_to_port(to_dev);
 
-		mirror->to_local_port = to_p->dp->index;
+		mirror->to_local_port = to_dp->index;
 		mirror->ingress = ingress;
 
-		err = ds->ops->port_mirror_add(ds, p->dp->index, mirror,
-					       ingress);
+		err = ds->ops->port_mirror_add(ds, dp->index, mirror, ingress);
 		if (err) {
 			kfree(mall_tc_entry);
 			return err;
@@ -762,9 +759,9 @@ static int dsa_slave_add_cls_matchall(struct net_device *dev,
 static void dsa_slave_del_cls_matchall(struct net_device *dev,
 				       struct tc_cls_matchall_offload *cls)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	struct dsa_mall_tc_entry *mall_tc_entry;
-	struct dsa_switch *ds = p->dp->ds;
+	struct dsa_switch *ds = dp->ds;
 
 	if (!ds->ops->port_mirror_del)
 		return;
@@ -777,8 +774,7 @@ static void dsa_slave_del_cls_matchall(struct net_device *dev,
 
 	switch (mall_tc_entry->type) {
 	case DSA_PORT_MALL_MIRROR:
-		ds->ops->port_mirror_del(ds, p->dp->index,
-					 &mall_tc_entry->mirror);
+		ds->ops->port_mirror_del(ds, dp->index, &mall_tc_entry->mirror);
 		break;
 	default:
 		WARN_ON(1);
@@ -855,25 +851,25 @@ static void dsa_slave_get_stats64(struct net_device *dev,
 static int dsa_slave_get_rxnfc(struct net_device *dev,
 			       struct ethtool_rxnfc *nfc, u32 *rule_locs)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_switch *ds = p->dp->ds;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
 
 	if (!ds->ops->get_rxnfc)
 		return -EOPNOTSUPP;
 
-	return ds->ops->get_rxnfc(ds, p->dp->index, nfc, rule_locs);
+	return ds->ops->get_rxnfc(ds, dp->index, nfc, rule_locs);
 }
 
 static int dsa_slave_set_rxnfc(struct net_device *dev,
 			       struct ethtool_rxnfc *nfc)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_switch *ds = p->dp->ds;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_switch *ds = dp->ds;
 
 	if (!ds->ops->set_rxnfc)
 		return -EOPNOTSUPP;
 
-	return ds->ops->set_rxnfc(ds, p->dp->index, nfc);
+	return ds->ops->set_rxnfc(ds, dp->index, nfc);
 }
 
 static const struct ethtool_ops dsa_slave_ethtool_ops = {
@@ -933,8 +929,9 @@ static struct device_type dsa_type = {
 
 static void dsa_slave_adjust_link(struct net_device *dev)
 {
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_switch *ds = p->dp->ds;
+	struct dsa_switch *ds = dp->ds;
 	unsigned int status_changed = 0;
 
 	if (p->old_link != dev->phydev->link) {
@@ -953,7 +950,7 @@ static void dsa_slave_adjust_link(struct net_device *dev)
 	}
 
 	if (ds->ops->adjust_link && status_changed)
-		ds->ops->adjust_link(ds, p->dp->index, dev->phydev);
+		ds->ops->adjust_link(ds, dp->index, dev->phydev);
 
 	if (status_changed)
 		phy_print_status(dev->phydev);
@@ -962,14 +959,14 @@ static void dsa_slave_adjust_link(struct net_device *dev)
 static int dsa_slave_fixed_link_update(struct net_device *dev,
 				       struct fixed_phy_status *status)
 {
-	struct dsa_slave_priv *p;
 	struct dsa_switch *ds;
+	struct dsa_port *dp;
 
 	if (dev) {
-		p = netdev_priv(dev);
-		ds = p->dp->ds;
+		dp = dsa_slave_to_port(dev);
+		ds = dp->ds;
 		if (ds->ops->fixed_link_update)
-			ds->ops->fixed_link_update(ds, p->dp->index, status);
+			ds->ops->fixed_link_update(ds, dp->index, status);
 	}
 
 	return 0;
@@ -978,8 +975,9 @@ static int dsa_slave_fixed_link_update(struct net_device *dev,
 /* slave device setup *******************************************************/
 static int dsa_slave_phy_connect(struct net_device *slave_dev, int addr)
 {
+	struct dsa_port *dp = dsa_slave_to_port(slave_dev);
 	struct dsa_slave_priv *p = netdev_priv(slave_dev);
-	struct dsa_switch *ds = p->dp->ds;
+	struct dsa_switch *ds = dp->ds;
 
 	slave_dev->phydev = mdiobus_get_phy(ds->slave_mii_bus, addr);
 	if (!slave_dev->phydev) {
@@ -997,14 +995,15 @@ static int dsa_slave_phy_connect(struct net_device *slave_dev, int addr)
 
 static int dsa_slave_phy_setup(struct net_device *slave_dev)
 {
+	struct dsa_port *dp = dsa_slave_to_port(slave_dev);
 	struct dsa_slave_priv *p = netdev_priv(slave_dev);
-	struct dsa_switch *ds = p->dp->ds;
-	struct device_node *phy_dn, *port_dn;
+	struct device_node *port_dn = dp->dn;
+	struct dsa_switch *ds = dp->ds;
+	struct device_node *phy_dn;
 	bool phy_is_fixed = false;
 	u32 phy_flags = 0;
 	int mode, ret;
 
-	port_dn = p->dp->dn;
 	mode = of_get_phy_mode(port_dn);
 	if (mode < 0)
 		mode = PHY_INTERFACE_MODE_NA;
@@ -1025,7 +1024,7 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev)
 	}
 
 	if (ds->ops->get_phy_flags)
-		phy_flags = ds->ops->get_phy_flags(ds, p->dp->index);
+		phy_flags = ds->ops->get_phy_flags(ds, dp->index);
 
 	if (phy_dn) {
 		int phy_id = of_mdio_parse_addr(&slave_dev->dev, phy_dn);
@@ -1061,10 +1060,10 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev)
 	 * MDIO bus instead
 	 */
 	if (!slave_dev->phydev) {
-		ret = dsa_slave_phy_connect(slave_dev, p->dp->index);
+		ret = dsa_slave_phy_connect(slave_dev, dp->index);
 		if (ret) {
 			netdev_err(slave_dev, "failed to connect to port %d: %d\n",
-				   p->dp->index, ret);
+				   dp->index, ret);
 			if (phy_is_fixed)
 				of_phy_deregister_fixed_link(port_dn);
 			return ret;
@@ -1118,7 +1117,7 @@ static void dsa_slave_notify(struct net_device *dev, unsigned long val)
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
 	struct net_device *master = dsa_master_netdev(p);
-	struct dsa_port *dp = p->dp;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	struct dsa_notifier_register_info rinfo = {
 		.switch_number = dp->ds->index,
 		.port_number = dp->index,
@@ -1202,8 +1201,8 @@ int dsa_slave_create(struct dsa_port *port, const char *name)
 
 out_phy:
 	phy_disconnect(slave_dev->phydev);
-	if (of_phy_is_fixed_link(p->dp->dn))
-		of_phy_deregister_fixed_link(p->dp->dn);
+	if (of_phy_is_fixed_link(port->dn))
+		of_phy_deregister_fixed_link(port->dn);
 out_free:
 	free_percpu(p->stats64);
 	free_netdev(slave_dev);
@@ -1213,10 +1212,9 @@ int dsa_slave_create(struct dsa_port *port, const char *name)
 
 void dsa_slave_destroy(struct net_device *slave_dev)
 {
+	struct dsa_port *dp = dsa_slave_to_port(slave_dev);
 	struct dsa_slave_priv *p = netdev_priv(slave_dev);
-	struct device_node *port_dn;
-
-	port_dn = p->dp->dn;
+	struct device_node *port_dn = dp->dn;
 
 	netif_carrier_off(slave_dev);
 	if (slave_dev->phydev) {
@@ -1239,8 +1237,7 @@ static bool dsa_slave_dev_check(struct net_device *dev)
 static int dsa_slave_changeupper(struct net_device *dev,
 				 struct netdev_notifier_changeupper_info *info)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_port *dp = p->dp;
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	int err = NOTIFY_DONE;
 
 	if (netif_is_bridge_master(info->upper_dev)) {
@@ -1283,14 +1280,14 @@ static void dsa_slave_switchdev_event_work(struct work_struct *work)
 		container_of(work, struct dsa_switchdev_event_work, work);
 	struct net_device *dev = switchdev_work->dev;
 	struct switchdev_notifier_fdb_info *fdb_info;
-	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	int err;
 
 	rtnl_lock();
 	switch (switchdev_work->event) {
 	case SWITCHDEV_FDB_ADD_TO_DEVICE:
 		fdb_info = &switchdev_work->fdb_info;
-		err = dsa_port_fdb_add(p->dp, fdb_info->addr, fdb_info->vid);
+		err = dsa_port_fdb_add(dp, fdb_info->addr, fdb_info->vid);
 		if (err) {
 			netdev_dbg(dev, "fdb add failed err=%d\n", err);
 			break;
@@ -1301,7 +1298,7 @@ static void dsa_slave_switchdev_event_work(struct work_struct *work)
 
 	case SWITCHDEV_FDB_DEL_TO_DEVICE:
 		fdb_info = &switchdev_work->fdb_info;
-		err = dsa_port_fdb_del(p->dp, fdb_info->addr, fdb_info->vid);
+		err = dsa_port_fdb_del(dp, fdb_info->addr, fdb_info->vid);
 		if (err) {
 			netdev_dbg(dev, "fdb del failed err=%d\n", err);
 			dev_close(dev);
diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
index cc4f472fbd77..2a0fa9f9f7ca 100644
--- a/net/dsa/tag_brcm.c
+++ b/net/dsa/tag_brcm.c
@@ -61,7 +61,7 @@
 
 static struct sk_buff *brcm_tag_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	u16 queue = skb_get_queue_mapping(skb);
 	u8 *brcm_tag;
 
@@ -82,15 +82,14 @@ static struct sk_buff *brcm_tag_xmit(struct sk_buff *skb, struct net_device *dev
 		       ((queue & BRCM_IG_TC_MASK) << BRCM_IG_TC_SHIFT);
 	brcm_tag[1] = 0;
 	brcm_tag[2] = 0;
-	if (p->dp->index == 8)
+	if (dp->index == 8)
 		brcm_tag[2] = BRCM_IG_DSTMAP2_MASK;
-	brcm_tag[3] = (1 << p->dp->index) & BRCM_IG_DSTMAP1_MASK;
+	brcm_tag[3] = (1 << dp->index) & BRCM_IG_DSTMAP1_MASK;
 
 	/* Now tell the master network device about the desired output queue
 	 * as well
 	 */
-	skb_set_queue_mapping(skb, BRCM_TAG_SET_PORT_QUEUE(p->dp->index,
-							   queue));
+	skb_set_queue_mapping(skb, BRCM_TAG_SET_PORT_QUEUE(dp->index, queue));
 
 	return skb;
 }
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index c77218f173d1..fe3c9700a8c8 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -18,7 +18,7 @@
 
 static struct sk_buff *dsa_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	u8 *dsa_header;
 
 	/*
@@ -34,8 +34,8 @@ static struct sk_buff *dsa_xmit(struct sk_buff *skb, struct net_device *dev)
 		 * Construct tagged FROM_CPU DSA tag from 802.1q tag.
 		 */
 		dsa_header = skb->data + 2 * ETH_ALEN;
-		dsa_header[0] = 0x60 | p->dp->ds->index;
-		dsa_header[1] = p->dp->index << 3;
+		dsa_header[0] = 0x60 | dp->ds->index;
+		dsa_header[1] = dp->index << 3;
 
 		/*
 		 * Move CFI field from byte 2 to byte 1.
@@ -55,8 +55,8 @@ static struct sk_buff *dsa_xmit(struct sk_buff *skb, struct net_device *dev)
 		 * Construct untagged FROM_CPU DSA tag.
 		 */
 		dsa_header = skb->data + 2 * ETH_ALEN;
-		dsa_header[0] = 0x40 | p->dp->ds->index;
-		dsa_header[1] = p->dp->index << 3;
+		dsa_header[0] = 0x40 | dp->ds->index;
+		dsa_header[1] = dp->index << 3;
 		dsa_header[2] = 0x00;
 		dsa_header[3] = 0x00;
 	}
diff --git a/net/dsa/tag_edsa.c b/net/dsa/tag_edsa.c
index 0b83cbe0c9e8..c683e240bafc 100644
--- a/net/dsa/tag_edsa.c
+++ b/net/dsa/tag_edsa.c
@@ -19,7 +19,7 @@
 
 static struct sk_buff *edsa_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	u8 *edsa_header;
 
 	/*
@@ -43,8 +43,8 @@ static struct sk_buff *edsa_xmit(struct sk_buff *skb, struct net_device *dev)
 		edsa_header[1] = ETH_P_EDSA & 0xff;
 		edsa_header[2] = 0x00;
 		edsa_header[3] = 0x00;
-		edsa_header[4] = 0x60 | p->dp->ds->index;
-		edsa_header[5] = p->dp->index << 3;
+		edsa_header[4] = 0x60 | dp->ds->index;
+		edsa_header[5] = dp->index << 3;
 
 		/*
 		 * Move CFI field from byte 6 to byte 5.
@@ -68,8 +68,8 @@ static struct sk_buff *edsa_xmit(struct sk_buff *skb, struct net_device *dev)
 		edsa_header[1] = ETH_P_EDSA & 0xff;
 		edsa_header[2] = 0x00;
 		edsa_header[3] = 0x00;
-		edsa_header[4] = 0x40 | p->dp->ds->index;
-		edsa_header[5] = p->dp->index << 3;
+		edsa_header[4] = 0x40 | dp->ds->index;
+		edsa_header[5] = dp->index << 3;
 		edsa_header[6] = 0x00;
 		edsa_header[7] = 0x00;
 	}
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index b241c990cde0..66e443fd7675 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -34,7 +34,7 @@
 
 static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	struct sk_buff *nskb;
 	int padlen;
 	u8 *tag;
@@ -72,7 +72,7 @@ static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	tag = skb_put(nskb, KSZ_INGRESS_TAG_LEN);
 	tag[0] = 0;
-	tag[1] = 1 << p->dp->index; /* destination port */
+	tag[1] = 1 << dp->index; /* destination port */
 
 	return nskb;
 }
diff --git a/net/dsa/tag_lan9303.c b/net/dsa/tag_lan9303.c
index 4f211e56c81f..c709adb5efd9 100644
--- a/net/dsa/tag_lan9303.c
+++ b/net/dsa/tag_lan9303.c
@@ -42,7 +42,7 @@
 
 static struct sk_buff *lan9303_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	u16 *lan9303_tag;
 
 	/* insert a special VLAN tag between the MAC addresses
@@ -62,7 +62,7 @@ static struct sk_buff *lan9303_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	lan9303_tag = (u16 *)(skb->data + 2 * ETH_ALEN);
 	lan9303_tag[0] = htons(ETH_P_8021Q);
-	lan9303_tag[1] = htons(p->dp->index | BIT(4));
+	lan9303_tag[1] = htons(dp->index | BIT(4));
 
 	return skb;
 }
diff --git a/net/dsa/tag_mtk.c b/net/dsa/tag_mtk.c
index 968586c5d40e..5abebbfd7060 100644
--- a/net/dsa/tag_mtk.c
+++ b/net/dsa/tag_mtk.c
@@ -23,7 +23,7 @@
 static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb,
 				    struct net_device *dev)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	u8 *mtk_tag;
 
 	if (skb_cow_head(skb, MTK_HDR_LEN) < 0)
@@ -36,7 +36,7 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb,
 	/* Build the tag after the MAC Source Address */
 	mtk_tag = skb->data + 2 * ETH_ALEN;
 	mtk_tag[0] = 0;
-	mtk_tag[1] = (1 << p->dp->index) & MTK_HDR_XMIT_DP_BIT_MASK;
+	mtk_tag[1] = (1 << dp->index) & MTK_HDR_XMIT_DP_BIT_MASK;
 	mtk_tag[2] = 0;
 	mtk_tag[3] = 0;
 
diff --git a/net/dsa/tag_qca.c b/net/dsa/tag_qca.c
index 8d33d9ebf910..b0274b6781cb 100644
--- a/net/dsa/tag_qca.c
+++ b/net/dsa/tag_qca.c
@@ -38,7 +38,7 @@
 
 static struct sk_buff *qca_tag_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	u16 *phdr, hdr;
 
 	dev->stats.tx_packets++;
@@ -54,8 +54,7 @@ static struct sk_buff *qca_tag_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	/* Set the version field, and set destination port information */
 	hdr = QCA_HDR_VERSION << QCA_HDR_XMIT_VERSION_S |
-		QCA_HDR_XMIT_FROM_CPU |
-		BIT(p->dp->index);
+		QCA_HDR_XMIT_FROM_CPU | BIT(dp->index);
 
 	*phdr = htons(hdr);
 
diff --git a/net/dsa/tag_trailer.c b/net/dsa/tag_trailer.c
index 61668be267f5..8b205bb5f521 100644
--- a/net/dsa/tag_trailer.c
+++ b/net/dsa/tag_trailer.c
@@ -16,7 +16,7 @@
 
 static struct sk_buff *trailer_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_port *dp = dsa_slave_to_port(dev);
 	struct sk_buff *nskb;
 	int padlen;
 	u8 *trailer;
@@ -48,7 +48,7 @@ static struct sk_buff *trailer_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	trailer = skb_put(nskb, 4);
 	trailer[0] = 0x80;
-	trailer[1] = 1 << p->dp->index;
+	trailer[1] = 1 << dp->index;
 	trailer[2] = 0x10;
 	trailer[3] = 0x00;
 
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next 3/5] net: dsa: add slave to port helper
  2017-10-12 22:51 ` [PATCH net-next 3/5] net: dsa: add slave to port helper Vivien Didelot
@ 2017-10-12 22:59   ` Florian Fainelli
  0 siblings, 0 replies; 18+ messages in thread
From: Florian Fainelli @ 2017-10-12 22:59 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn

On 10/12/2017 03:51 PM, Vivien Didelot wrote:
> Many portions of DSA core code require to get the dsa_port structure
> corresponding to a slave net_device. For this purpose, introduce a
> dsa_slave_to_port() helper.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH net-next 4/5] net: dsa: add slave get master helper
  2017-10-12 22:51 [PATCH net-next 0/5] net: dsa: master and slave helpers Vivien Didelot
                   ` (2 preceding siblings ...)
  2017-10-12 22:51 ` [PATCH net-next 3/5] net: dsa: add slave to port helper Vivien Didelot
@ 2017-10-12 22:51 ` Vivien Didelot
  2017-10-12 23:02   ` Florian Fainelli
  2017-10-12 22:51 ` [PATCH net-next 5/5] net: dsa: split dsa_port's netdev member Vivien Didelot
  4 siblings, 1 reply; 18+ messages in thread
From: Vivien Didelot @ 2017-10-12 22:51 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

Many part of the DSA slave code require to get the master device
assigned to a slave device. Remove dsa_master_netdev() in favor of a
dsa_slave_get_master() helper which does that.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/dsa_priv.h | 13 ++++++++-----
 net/dsa/slave.c    | 26 +++++++++-----------------
 2 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 569a4929b4c9..f1dc5a856fda 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -176,6 +176,14 @@ static inline struct dsa_port *dsa_slave_to_port(const struct net_device *dev)
 	return p->dp;
 }
 
+static inline struct net_device *
+dsa_slave_get_master(const struct net_device *dev)
+{
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+
+	return dp->cpu_dp->netdev;
+}
+
 /* switch.c */
 int dsa_switch_register_notifier(struct dsa_switch *ds);
 void dsa_switch_unregister_notifier(struct dsa_switch *ds);
@@ -204,9 +212,4 @@ extern const struct dsa_device_ops qca_netdev_ops;
 /* tag_trailer.c */
 extern const struct dsa_device_ops trailer_netdev_ops;
 
-static inline struct net_device *dsa_master_netdev(struct dsa_slave_priv *p)
-{
-	return p->dp->cpu_dp->netdev;
-}
-
 #endif
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 894602c88b09..d2c780f13d78 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -64,15 +64,12 @@ void dsa_slave_mii_bus_init(struct dsa_switch *ds)
 /* slave device handling ****************************************************/
 static int dsa_slave_get_iflink(const struct net_device *dev)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-
-	return dsa_master_netdev(p)->ifindex;
+	return dsa_slave_get_master(dev)->ifindex;
 }
 
 static int dsa_slave_open(struct net_device *dev)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct net_device *master = dsa_master_netdev(p);
+	struct net_device *master = dsa_slave_get_master(dev);
 	struct dsa_port *dp = dsa_slave_to_port(dev);
 	int err;
 
@@ -120,8 +117,7 @@ static int dsa_slave_open(struct net_device *dev)
 
 static int dsa_slave_close(struct net_device *dev)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct net_device *master = dsa_master_netdev(p);
+	struct net_device *master = dsa_slave_get_master(dev);
 	struct dsa_port *dp = dsa_slave_to_port(dev);
 
 	if (dev->phydev)
@@ -144,8 +140,7 @@ static int dsa_slave_close(struct net_device *dev)
 
 static void dsa_slave_change_rx_flags(struct net_device *dev, int change)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct net_device *master = dsa_master_netdev(p);
+	struct net_device *master = dsa_slave_get_master(dev);
 
 	if (change & IFF_ALLMULTI)
 		dev_set_allmulti(master, dev->flags & IFF_ALLMULTI ? 1 : -1);
@@ -155,8 +150,7 @@ static void dsa_slave_change_rx_flags(struct net_device *dev, int change)
 
 static void dsa_slave_set_rx_mode(struct net_device *dev)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct net_device *master = dsa_master_netdev(p);
+	struct net_device *master = dsa_slave_get_master(dev);
 
 	dev_mc_sync(master, dev);
 	dev_uc_sync(master, dev);
@@ -164,8 +158,7 @@ static void dsa_slave_set_rx_mode(struct net_device *dev)
 
 static int dsa_slave_set_mac_address(struct net_device *dev, void *a)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct net_device *master = dsa_master_netdev(p);
+	struct net_device *master = dsa_slave_get_master(dev);
 	struct sockaddr *addr = a;
 	int err;
 
@@ -409,7 +402,7 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev)
 	/* Queue the SKB for transmission on the parent interface, but
 	 * do not modify its EtherType
 	 */
-	nskb->dev = dsa_master_netdev(p);
+	nskb->dev = dsa_slave_get_master(dev);
 	dev_queue_xmit(nskb);
 
 	return NETDEV_TX_OK;
@@ -632,8 +625,8 @@ static int dsa_slave_get_eee(struct net_device *dev, struct ethtool_eee *e)
 static int dsa_slave_netpoll_setup(struct net_device *dev,
 				   struct netpoll_info *ni)
 {
+	struct net_device *master = dsa_slave_get_master(dev);
 	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct net_device *master = dsa_master_netdev(p);
 	struct netpoll *netpoll;
 	int err = 0;
 
@@ -1115,8 +1108,7 @@ int dsa_slave_resume(struct net_device *slave_dev)
 
 static void dsa_slave_notify(struct net_device *dev, unsigned long val)
 {
-	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct net_device *master = dsa_master_netdev(p);
+	struct net_device *master = dsa_slave_get_master(dev);
 	struct dsa_port *dp = dsa_slave_to_port(dev);
 	struct dsa_notifier_register_info rinfo = {
 		.switch_number = dp->ds->index,
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next 4/5] net: dsa: add slave get master helper
  2017-10-12 22:51 ` [PATCH net-next 4/5] net: dsa: add slave get master helper Vivien Didelot
@ 2017-10-12 23:02   ` Florian Fainelli
  2017-10-12 23:23     ` Vivien Didelot
  0 siblings, 1 reply; 18+ messages in thread
From: Florian Fainelli @ 2017-10-12 23:02 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn

On 10/12/2017 03:51 PM, Vivien Didelot wrote:
> Many part of the DSA slave code require to get the master device
> assigned to a slave device. Remove dsa_master_netdev() in favor of a
> dsa_slave_get_master() helper which does that.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---

> +static inline struct net_device *
> +dsa_slave_get_master(const struct net_device *dev)
> +{
> +	struct dsa_port *dp = dsa_slave_to_port(dev);
> +
> +	return dp->cpu_dp->netdev;
> +}

Nit: _get may convey the idea that a reference count may be incremented
when the function is called (balanced with a _put), so maybe name it
dsa_slave_to_master() which is more in line with dsa_slave_to_port() as
well? Other than that:

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next 4/5] net: dsa: add slave get master helper
  2017-10-12 23:02   ` Florian Fainelli
@ 2017-10-12 23:23     ` Vivien Didelot
  0 siblings, 0 replies; 18+ messages in thread
From: Vivien Didelot @ 2017-10-12 23:23 UTC (permalink / raw)
  To: Florian Fainelli, netdev
  Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn

Hi Florian,

Florian Fainelli <f.fainelli@gmail.com> writes:

> On 10/12/2017 03:51 PM, Vivien Didelot wrote:
>> Many part of the DSA slave code require to get the master device
>> assigned to a slave device. Remove dsa_master_netdev() in favor of a
>> dsa_slave_get_master() helper which does that.
>> 
>> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>> ---
>
>> +static inline struct net_device *
>> +dsa_slave_get_master(const struct net_device *dev)
>> +{
>> +	struct dsa_port *dp = dsa_slave_to_port(dev);
>> +
>> +	return dp->cpu_dp->netdev;
>> +}
>
> Nit: _get may convey the idea that a reference count may be incremented
> when the function is called (balanced with a _put), so maybe name it
> dsa_slave_to_master() which is more in line with dsa_slave_to_port() as
> well? Other than that:
>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Perfect, I was hesitating myself between the two, but I didn't have a
good argument for any of them. Now I have one! I'll change that for
dsa_slave_to_master and add your tag.


Thanks,

        Vivien

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH net-next 5/5] net: dsa: split dsa_port's netdev member
  2017-10-12 22:51 [PATCH net-next 0/5] net: dsa: master and slave helpers Vivien Didelot
                   ` (3 preceding siblings ...)
  2017-10-12 22:51 ` [PATCH net-next 4/5] net: dsa: add slave get master helper Vivien Didelot
@ 2017-10-12 22:51 ` Vivien Didelot
  2017-10-12 23:05   ` Florian Fainelli
  4 siblings, 1 reply; 18+ messages in thread
From: Vivien Didelot @ 2017-10-12 22:51 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

The dsa_port structure has a "netdev" member, which can be used for
either the master device, or the slave device, depending on its type.

It is true that today, CPU port are not exposed to userspace, thus the
port's netdev member can be used to point to its master interface.

But it is still slightly confusing, so split it into more explicit
"master" and "slave" members.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/bcm_sf2.c        |  6 +++---
 drivers/net/dsa/mt7530.c         |  2 +-
 drivers/net/dsa/mv88e6xxx/chip.c |  2 +-
 include/net/dsa.h                |  7 ++++++-
 net/dsa/dsa.c                    |  6 +++---
 net/dsa/dsa2.c                   | 22 +++++++++++-----------
 net/dsa/dsa_priv.h               |  4 ++--
 net/dsa/legacy.c                 | 14 +++++++-------
 net/dsa/slave.c                  |  6 +++---
 9 files changed, 37 insertions(+), 32 deletions(-)

diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 32025b990437..b43c063b9634 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -601,7 +601,7 @@ static void bcm_sf2_sw_fixed_link_update(struct dsa_switch *ds, int port,
 		 * state machine and make it go in PHY_FORCING state instead.
 		 */
 		if (!status->link)
-			netif_carrier_off(ds->ports[port].netdev);
+			netif_carrier_off(ds->ports[port].slave);
 		status->duplex = 1;
 	} else {
 		status->link = 1;
@@ -690,7 +690,7 @@ static int bcm_sf2_sw_resume(struct dsa_switch *ds)
 static void bcm_sf2_sw_get_wol(struct dsa_switch *ds, int port,
 			       struct ethtool_wolinfo *wol)
 {
-	struct net_device *p = ds->ports[port].cpu_dp->netdev;
+	struct net_device *p = ds->ports[port].cpu_dp->master;
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
 	struct ethtool_wolinfo pwol;
 
@@ -713,7 +713,7 @@ static void bcm_sf2_sw_get_wol(struct dsa_switch *ds, int port,
 static int bcm_sf2_sw_set_wol(struct dsa_switch *ds, int port,
 			      struct ethtool_wolinfo *wol)
 {
-	struct net_device *p = ds->ports[port].cpu_dp->netdev;
+	struct net_device *p = ds->ports[port].cpu_dp->master;
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
 	s8 cpu_port = ds->ports[port].cpu_dp->index;
 	struct ethtool_wolinfo pwol;
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 034241696ce2..fea2e665d0cb 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -933,7 +933,7 @@ mt7530_setup(struct dsa_switch *ds)
 	 * controller also is the container for two GMACs nodes representing
 	 * as two netdev instances.
 	 */
-	dn = ds->ports[MT7530_CPU_PORT].netdev->dev.of_node->parent;
+	dn = ds->ports[MT7530_CPU_PORT].master->dev.of_node->parent;
 	priv->ethernet = syscon_node_to_regmap(dn);
 	if (IS_ERR(priv->ethernet))
 		return PTR_ERR(priv->ethernet);
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index d74c7335c512..955f4e214191 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1124,7 +1124,7 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
 			if (dsa_is_dsa_port(ds, i) || dsa_is_cpu_port(ds, i))
 				continue;
 
-			if (!ds->ports[port].netdev)
+			if (!ds->ports[port].slave)
 				continue;
 
 			if (vlan.member[i] ==
diff --git a/include/net/dsa.h b/include/net/dsa.h
index ce1d622734d7..4c769bc8a8b5 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -164,6 +164,12 @@ struct dsa_mall_tc_entry {
 
 
 struct dsa_port {
+	/* Master device, physically connected if this is a CPU port */
+	struct net_device *master;
+
+	/* Slave device, if this port is exposed to userspace */
+	struct net_device *slave;
+
 	/* CPU port tagging operations used by master or slave devices */
 	const struct dsa_device_ops *tag_ops;
 
@@ -176,7 +182,6 @@ struct dsa_port {
 	unsigned int		index;
 	const char		*name;
 	struct dsa_port		*cpu_dp;
-	struct net_device	*netdev;
 	struct device_node	*dn;
 	unsigned int		ageing_time;
 	u8			stp_state;
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 832c659ff993..a3abf7a7b9a2 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -201,7 +201,7 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
 #ifdef CONFIG_PM_SLEEP
 static bool dsa_is_port_initialized(struct dsa_switch *ds, int p)
 {
-	return ds->enabled_port_mask & (1 << p) && ds->ports[p].netdev;
+	return ds->enabled_port_mask & (1 << p) && ds->ports[p].slave;
 }
 
 int dsa_switch_suspend(struct dsa_switch *ds)
@@ -213,7 +213,7 @@ int dsa_switch_suspend(struct dsa_switch *ds)
 		if (!dsa_is_port_initialized(ds, i))
 			continue;
 
-		ret = dsa_slave_suspend(ds->ports[i].netdev);
+		ret = dsa_slave_suspend(ds->ports[i].slave);
 		if (ret)
 			return ret;
 	}
@@ -240,7 +240,7 @@ int dsa_switch_resume(struct dsa_switch *ds)
 		if (!dsa_is_port_initialized(ds, i))
 			continue;
 
-		ret = dsa_slave_resume(ds->ports[i].netdev);
+		ret = dsa_slave_resume(ds->ports[i].slave);
 		if (ret)
 			return ret;
 	}
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 54ed054777bd..5ac78edd756f 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -279,7 +279,7 @@ static int dsa_user_port_apply(struct dsa_port *port)
 	if (err) {
 		dev_warn(ds->dev, "Failed to create slave %d: %d\n",
 			 port->index, err);
-		port->netdev = NULL;
+		port->slave = NULL;
 		return err;
 	}
 
@@ -289,7 +289,7 @@ static int dsa_user_port_apply(struct dsa_port *port)
 	if (err)
 		return err;
 
-	devlink_port_type_eth_set(&port->devlink_port, port->netdev);
+	devlink_port_type_eth_set(&port->devlink_port, port->slave);
 
 	return 0;
 }
@@ -297,9 +297,9 @@ static int dsa_user_port_apply(struct dsa_port *port)
 static void dsa_user_port_unapply(struct dsa_port *port)
 {
 	devlink_port_unregister(&port->devlink_port);
-	if (port->netdev) {
-		dsa_slave_destroy(port->netdev);
-		port->netdev = NULL;
+	if (port->slave) {
+		dsa_slave_destroy(port->slave);
+		port->slave = NULL;
 		port->ds->enabled_port_mask &= ~(1 << port->index);
 	}
 }
@@ -337,7 +337,7 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 		return err;
 
 	if (ds->ops->set_addr) {
-		err = ds->ops->set_addr(ds, dst->cpu_dp->netdev->dev_addr);
+		err = ds->ops->set_addr(ds, dst->cpu_dp->master->dev_addr);
 		if (err < 0)
 			return err;
 	}
@@ -438,9 +438,9 @@ static int dsa_dst_apply(struct dsa_switch_tree *dst)
 	 * sent to the tag format's receive function.
 	 */
 	wmb();
-	dst->cpu_dp->netdev->dsa_ptr = dst->cpu_dp;
+	dst->cpu_dp->master->dsa_ptr = dst->cpu_dp;
 
-	err = dsa_master_ethtool_setup(dst->cpu_dp->netdev);
+	err = dsa_master_ethtool_setup(dst->cpu_dp->master);
 	if (err)
 		return err;
 
@@ -457,9 +457,9 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
 	if (!dst->applied)
 		return;
 
-	dsa_master_ethtool_restore(dst->cpu_dp->netdev);
+	dsa_master_ethtool_restore(dst->cpu_dp->master);
 
-	dst->cpu_dp->netdev->dsa_ptr = NULL;
+	dst->cpu_dp->master->dsa_ptr = NULL;
 
 	/* If we used a tagging format that doesn't have an ethertype
 	 * field, make sure that all packets from this point get sent
@@ -505,7 +505,7 @@ static int dsa_cpu_parse(struct dsa_port *port, u32 index,
 
 	if (!dst->cpu_dp) {
 		dst->cpu_dp = port;
-		dst->cpu_dp->netdev = ethernet_dev;
+		dst->cpu_dp->master = ethernet_dev;
 	}
 
 	/* Initialize cpu_port_mask now for drv->setup()
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index f1dc5a856fda..b47c46fba376 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -130,7 +130,7 @@ static inline struct net_device *dsa_master_get_slave(struct net_device *dev,
 	if (port < 0 || port >= ds->num_ports)
 		return NULL;
 
-	return ds->ports[port].netdev;
+	return ds->ports[port].slave;
 }
 
 /* port.c */
@@ -181,7 +181,7 @@ dsa_slave_get_master(const struct net_device *dev)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
 
-	return dp->cpu_dp->netdev;
+	return dp->cpu_dp->master;
 }
 
 /* switch.c */
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index 6f2254753859..ce6bc80911d0 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -120,7 +120,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds,
 				return -EINVAL;
 			}
 			dst->cpu_dp = &ds->ports[i];
-			dst->cpu_dp->netdev = master;
+			dst->cpu_dp->master = master;
 			ds->cpu_port_mask |= 1 << i;
 		} else if (!strcmp(name, "dsa")) {
 			ds->dsa_port_mask |= 1 << i;
@@ -267,10 +267,10 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
 		if (!(ds->enabled_port_mask & (1 << port)))
 			continue;
 
-		if (!ds->ports[port].netdev)
+		if (!ds->ports[port].slave)
 			continue;
 
-		dsa_slave_destroy(ds->ports[port].netdev);
+		dsa_slave_destroy(ds->ports[port].slave);
 	}
 
 	/* Disable configuration of the CPU and DSA ports */
@@ -607,7 +607,7 @@ static int dsa_setup_dst(struct dsa_switch_tree *dst, struct net_device *dev,
 	wmb();
 	dev->dsa_ptr = dst->cpu_dp;
 
-	return dsa_master_ethtool_setup(dst->cpu_dp->netdev);
+	return dsa_master_ethtool_setup(dst->cpu_dp->master);
 }
 
 static int dsa_probe(struct platform_device *pdev)
@@ -672,9 +672,9 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
 {
 	int i;
 
-	dsa_master_ethtool_restore(dst->cpu_dp->netdev);
+	dsa_master_ethtool_restore(dst->cpu_dp->master);
 
-	dst->cpu_dp->netdev->dsa_ptr = NULL;
+	dst->cpu_dp->master->dsa_ptr = NULL;
 
 	/* If we used a tagging format that doesn't have an ethertype
 	 * field, make sure that all packets from this point get sent
@@ -689,7 +689,7 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
 			dsa_switch_destroy(ds);
 	}
 
-	dev_put(dst->cpu_dp->netdev);
+	dev_put(dst->cpu_dp->master);
 }
 
 static int dsa_remove(struct platform_device *pdev)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index d2c780f13d78..672977689e1b 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1123,7 +1123,7 @@ static void dsa_slave_notify(struct net_device *dev, unsigned long val)
 int dsa_slave_create(struct dsa_port *port, const char *name)
 {
 	struct dsa_port *cpu_dp = port->cpu_dp;
-	struct net_device *master = cpu_dp->netdev;
+	struct net_device *master = cpu_dp->master;
 	struct dsa_switch *ds = port->ds;
 	struct net_device *slave_dev;
 	struct dsa_slave_priv *p;
@@ -1170,7 +1170,7 @@ int dsa_slave_create(struct dsa_port *port, const char *name)
 	p->old_link = -1;
 	p->old_duplex = -1;
 
-	port->netdev = slave_dev;
+	port->slave = slave_dev;
 
 	netif_carrier_off(slave_dev);
 
@@ -1198,7 +1198,7 @@ int dsa_slave_create(struct dsa_port *port, const char *name)
 out_free:
 	free_percpu(p->stats64);
 	free_netdev(slave_dev);
-	port->netdev = NULL;
+	port->slave = NULL;
 	return ret;
 }
 
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next 5/5] net: dsa: split dsa_port's netdev member
  2017-10-12 22:51 ` [PATCH net-next 5/5] net: dsa: split dsa_port's netdev member Vivien Didelot
@ 2017-10-12 23:05   ` Florian Fainelli
  2017-10-12 23:21     ` Vivien Didelot
  2017-10-13 14:02     ` David Laight
  0 siblings, 2 replies; 18+ messages in thread
From: Florian Fainelli @ 2017-10-12 23:05 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn

On 10/12/2017 03:51 PM, Vivien Didelot wrote:
> The dsa_port structure has a "netdev" member, which can be used for
> either the master device, or the slave device, depending on its type.
> 
> It is true that today, CPU port are not exposed to userspace, thus the
> port's netdev member can be used to point to its master interface.
> 
> But it is still slightly confusing, so split it into more explicit
> "master" and "slave" members.

I do see some value in doing that, although I also see value in having
structure members be named after what they are, rather than their use
(oh well, it's all debatable anyway), see below for a suggestion on how
to reconcile the two:

>  struct dsa_port {
> +	/* Master device, physically connected if this is a CPU port */
> +	struct net_device *master;
> +
> +	/* Slave device, if this port is exposed to userspace */
> +	struct net_device *slave;
> +

How about using:

	union {
		struct net_device *master;
		struct net_device *slave;
	} netdev;

Such that this serves both purposes of clearly communicating what the
structure member is, and it can be either one of the two, but not both
at the same time?

-- 
Florian

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next 5/5] net: dsa: split dsa_port's netdev member
  2017-10-12 23:05   ` Florian Fainelli
@ 2017-10-12 23:21     ` Vivien Didelot
  2017-10-13 14:02     ` David Laight
  1 sibling, 0 replies; 18+ messages in thread
From: Vivien Didelot @ 2017-10-12 23:21 UTC (permalink / raw)
  To: Florian Fainelli, netdev
  Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn

Hi Florian,

Florian Fainelli <f.fainelli@gmail.com> writes:

> On 10/12/2017 03:51 PM, Vivien Didelot wrote:
>> The dsa_port structure has a "netdev" member, which can be used for
>> either the master device, or the slave device, depending on its type.
>> 
>> It is true that today, CPU port are not exposed to userspace, thus the
>> port's netdev member can be used to point to its master interface.
>> 
>> But it is still slightly confusing, so split it into more explicit
>> "master" and "slave" members.
>
> I do see some value in doing that, although I also see value in having
> structure members be named after what they are, rather than their use
> (oh well, it's all debatable anyway), see below for a suggestion on how
> to reconcile the two:
>
>>  struct dsa_port {
>> +	/* Master device, physically connected if this is a CPU port */
>> +	struct net_device *master;
>> +
>> +	/* Slave device, if this port is exposed to userspace */
>> +	struct net_device *slave;
>> +
>
> How about using:
>
> 	union {
> 		struct net_device *master;
> 		struct net_device *slave;
> 	} netdev;
>
> Such that this serves both purposes of clearly communicating what the
> structure member is, and it can be either one of the two, but not both
> at the same time?

I love that! It makes clear that master is not available for a non-CPU
port. Using this union is correct for the moment because DSA and CPU
ports don't have a slave device attached to them. If this becomes true
one day (unlikely), we'll remove the union.


Thanks,

        Vivien

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH net-next 5/5] net: dsa: split dsa_port's netdev member
  2017-10-12 23:05   ` Florian Fainelli
  2017-10-12 23:21     ` Vivien Didelot
@ 2017-10-13 14:02     ` David Laight
  2017-10-13 14:26       ` Vivien Didelot
  1 sibling, 1 reply; 18+ messages in thread
From: David Laight @ 2017-10-13 14:02 UTC (permalink / raw)
  To: 'Florian Fainelli', Vivien Didelot,
	netdev@vger.kernel.org
  Cc: linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com,
	David S. Miller, Andrew Lunn

From: Florian Fainelli
> Sent: 13 October 2017 00:05
...
> How about using:
> 
> 	union {
> 		struct net_device *master;
> 		struct net_device *slave;
> 	} netdev;
...

You can remove the 'netdev' all the compilers support unnamed unions.

	David


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH net-next 5/5] net: dsa: split dsa_port's netdev member
  2017-10-13 14:02     ` David Laight
@ 2017-10-13 14:26       ` Vivien Didelot
  2017-10-13 15:29         ` Vivien Didelot
  0 siblings, 1 reply; 18+ messages in thread
From: Vivien Didelot @ 2017-10-13 14:26 UTC (permalink / raw)
  To: David Laight, 'Florian Fainelli', netdev@vger.kernel.org
  Cc: linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com,
	David S. Miller, Andrew Lunn

Hi David,

David Laight <David.Laight@ACULAB.COM> writes:

> From: Florian Fainelli
>> Sent: 13 October 2017 00:05
> ...
>> How about using:
>> 
>> 	union {
>> 		struct net_device *master;
>> 		struct net_device *slave;
>> 	} netdev;
> ...
>
> You can remove the 'netdev' all the compilers support unnamed unions.

There are issues with older GCC versions, see the commit 42275bd8fcb3
("switchdev: don't use anonymous union on switchdev attr/obj structs")

That's why I kept it in the v2 I sent.


Thanks,

        Vivien

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH net-next 5/5] net: dsa: split dsa_port's netdev member
  2017-10-13 14:26       ` Vivien Didelot
@ 2017-10-13 15:29         ` Vivien Didelot
  2017-10-13 16:10           ` David Laight
  0 siblings, 1 reply; 18+ messages in thread
From: Vivien Didelot @ 2017-10-13 15:29 UTC (permalink / raw)
  To: David Laight, 'Florian Fainelli', netdev@vger.kernel.org
  Cc: Andrew Lunn, kernel@savoirfairelinux.com,
	linux-kernel@vger.kernel.org, David S. Miller

Hi again,

Vivien Didelot <vivien.didelot@savoirfairelinux.com> writes:

>>> How about using:
>>> 
>>> 	union {
>>> 		struct net_device *master;
>>> 		struct net_device *slave;
>>> 	} netdev;
>> ...
>>
>> You can remove the 'netdev' all the compilers support unnamed unions.
>
> There are issues with older GCC versions, see the commit 42275bd8fcb3
> ("switchdev: don't use anonymous union on switchdev attr/obj structs")
>
> That's why I kept it in the v2 I sent.

At the same time, I can see that struct sk_buff uses anonym union a lot.

It seems weird that one raised a compiler issue for switchdev but not
for skbuff.h... Do you think it is viable to drop the name here then?

I'd be happy to respin a v3 if this sounds safe.


Thanks,

        Vivien

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH net-next 5/5] net: dsa: split dsa_port's netdev member
  2017-10-13 15:29         ` Vivien Didelot
@ 2017-10-13 16:10           ` David Laight
  2017-10-13 16:50             ` Vivien Didelot
  0 siblings, 1 reply; 18+ messages in thread
From: David Laight @ 2017-10-13 16:10 UTC (permalink / raw)
  To: 'Vivien Didelot', 'Florian Fainelli',
	netdev@vger.kernel.org
  Cc: Andrew Lunn, kernel@savoirfairelinux.com,
	linux-kernel@vger.kernel.org, David S. Miller

From: Vivien Didelot
> Sent: 13 October 2017 16:29
> Vivien Didelot <vivien.didelot@savoirfairelinux.com> writes:
> 
> >>> How about using:
> >>>
> >>> 	union {
> >>> 		struct net_device *master;
> >>> 		struct net_device *slave;
> >>> 	} netdev;
> >> ...
> >>
> >> You can remove the 'netdev' all the compilers support unnamed unions.
> >
> > There are issues with older GCC versions, see the commit 42275bd8fcb3
> > ("switchdev: don't use anonymous union on switchdev attr/obj structs")
> >
> > That's why I kept it in the v2 I sent.
> 
> At the same time, I can see that struct sk_buff uses anonym union a lot.
> 
> It seems weird that one raised a compiler issue for switchdev but not
> for skbuff.h... Do you think it is viable to drop the name here then?

I believe the problem is with initialisers for static structures
that contain anonymous unions.

	David

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH net-next 5/5] net: dsa: split dsa_port's netdev member
  2017-10-13 16:10           ` David Laight
@ 2017-10-13 16:50             ` Vivien Didelot
  0 siblings, 0 replies; 18+ messages in thread
From: Vivien Didelot @ 2017-10-13 16:50 UTC (permalink / raw)
  To: David Laight, 'Florian Fainelli', netdev@vger.kernel.org
  Cc: Andrew Lunn, kernel@savoirfairelinux.com,
	linux-kernel@vger.kernel.org, David S. Miller

Hi David,

David Laight <David.Laight@ACULAB.COM> writes:

> From: Vivien Didelot
>> Sent: 13 October 2017 16:29
>> Vivien Didelot <vivien.didelot@savoirfairelinux.com> writes:
>> 
>> >>> How about using:
>> >>>
>> >>> 	union {
>> >>> 		struct net_device *master;
>> >>> 		struct net_device *slave;
>> >>> 	} netdev;
>> >> ...
>> >>
>> >> You can remove the 'netdev' all the compilers support unnamed unions.
>> >
>> > There are issues with older GCC versions, see the commit 42275bd8fcb3
>> > ("switchdev: don't use anonymous union on switchdev attr/obj structs")
>> >
>> > That's why I kept it in the v2 I sent.
>> 
>> At the same time, I can see that struct sk_buff uses anonym union a lot.
>> 
>> It seems weird that one raised a compiler issue for switchdev but not
>> for skbuff.h... Do you think it is viable to drop the name here then?
>
> I believe the problem is with initialisers for static structures
> that contain anonymous unions.

The dsa_port structures are dynamically allocated so this seems safe to
use an anonymous union here.

BTW v2 never left my computer in fact, so this will be fixed up in v2.


Thanks!

        Vivien

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2017-10-13 16:50 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-12 22:51 [PATCH net-next 0/5] net: dsa: master and slave helpers Vivien Didelot
2017-10-12 22:51 ` [PATCH net-next 1/5] net: dsa: use port's cpu_dp when creating a slave Vivien Didelot
2017-10-12 22:53   ` Florian Fainelli
2017-10-12 22:51 ` [PATCH net-next 2/5] net: dsa: add slave notify helper Vivien Didelot
2017-10-12 22:55   ` Florian Fainelli
2017-10-12 22:51 ` [PATCH net-next 3/5] net: dsa: add slave to port helper Vivien Didelot
2017-10-12 22:59   ` Florian Fainelli
2017-10-12 22:51 ` [PATCH net-next 4/5] net: dsa: add slave get master helper Vivien Didelot
2017-10-12 23:02   ` Florian Fainelli
2017-10-12 23:23     ` Vivien Didelot
2017-10-12 22:51 ` [PATCH net-next 5/5] net: dsa: split dsa_port's netdev member Vivien Didelot
2017-10-12 23:05   ` Florian Fainelli
2017-10-12 23:21     ` Vivien Didelot
2017-10-13 14:02     ` David Laight
2017-10-13 14:26       ` Vivien Didelot
2017-10-13 15:29         ` Vivien Didelot
2017-10-13 16:10           ` David Laight
2017-10-13 16:50             ` Vivien Didelot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).