[patch net-next 00/12] net: expose switch ID via devlink

[patch net-next 00/12] net: expose switch ID via devlink

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

To provide visibility of the ports, this patchset exposes switch ID
for devlink ports, which are part of a switch. The rest of the ports
if any (in case of sr-iov for example) do not set switch ID.

Jiri Pirko (12):
  net: devlink: convert devlink_port_attrs bools to bits
  net: devlink: extend port attrs for switch ID
  net: devlink: introduce devlink_compat_switch_id_get() helper
  mlxsw: Pass switch ID through devlink_port_attrs_set()
  mlxsw: Remove ndo_get_port_parent_id implementation
  bnxt: pass switch ID through devlink_port_attrs_set()
  bnxt: remove ndo_get_port_parent_id implementation for physical ports
  nfp: pass switch ID through devlink_port_attrs_set()
  nfp: remove ndo_get_port_parent_id implementation
  mlxsw: switch_ib: Pass valid HW id down to mlxsw_core_port_init()
  dsa: pass switch ID through devlink_port_attrs_set()
  net: devlink: add warning for ndo_get_port_parent_id set when not
    needed

 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  1 -
 .../net/ethernet/broadcom/bnxt/bnxt_devlink.c |  3 +-
 drivers/net/ethernet/mellanox/mlxsw/core.c    |  7 ++-
 drivers/net/ethernet/mellanox/mlxsw/core.h    |  4 +-
 drivers/net/ethernet/mellanox/mlxsw/minimal.c | 17 ++-----
 .../net/ethernet/mellanox/mlxsw/spectrum.c    | 17 ++-----
 .../net/ethernet/mellanox/mlxsw/switchib.c    | 22 ++++++++-
 .../net/ethernet/mellanox/mlxsw/switchx2.c    | 16 +-----
 .../net/ethernet/netronome/nfp/nfp_devlink.c  |  5 +-
 .../ethernet/netronome/nfp/nfp_net_common.c   |  1 -
 .../net/ethernet/netronome/nfp/nfp_net_repr.c |  1 -
 drivers/net/ethernet/netronome/nfp/nfp_port.c | 16 ------
 include/net/devlink.h                         | 19 +++++--
 include/uapi/linux/devlink.h                  |  2 +
 net/core/dev.c                                |  8 ++-
 net/core/devlink.c                            | 49 ++++++++++++++++---
 net/dsa/dsa2.c                                |  4 +-
 net/dsa/slave.c                               |  1 -
 18 files changed, 113 insertions(+), 80 deletions(-)

-- 
2.17.2

[patch net-next 01/12] net: devlink: convert devlink_port_attrs bools to bits

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

In order to save space in the struct, convert bools to bits.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 include/net/devlink.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 31d5cec4d06b..4a1e3452a4ce 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -41,10 +41,10 @@ struct devlink {
 };
 
 struct devlink_port_attrs {
-	bool set;
+	u8 set:1,
+	   split:1;
 	enum devlink_port_flavour flavour;
 	u32 port_number; /* same value as "split group" */
-	bool split;
 	u32 split_subport_number;
 };
 
-- 
2.17.2

[patch net-next 02/12] net: devlink: extend port attrs for switch ID

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Extend devlink_port_attrs_set() to pass switch ID for ports which are
part of switch and store it in port attrs. For other ports, this is
NULL. During dump to userspace only valid switch ID is filled up.
Note that this allows the driver to group devlink ports into one or more
switches according to the actual topology.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
rfc->v1:
- included bnxt bits
- rebased
---
 .../net/ethernet/broadcom/bnxt/bnxt_devlink.c |  2 +-
 drivers/net/ethernet/mellanox/mlxsw/core.c    |  3 +-
 .../net/ethernet/netronome/nfp/nfp_devlink.c  |  2 +-
 include/net/devlink.h                         |  8 +++--
 include/uapi/linux/devlink.h                  |  2 ++
 net/core/devlink.c                            | 29 +++++++++++++++----
 net/dsa/dsa2.c                                |  2 +-
 7 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index ab6fd05c462b..36ec4cb45276 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -230,7 +230,7 @@ int bnxt_dl_register(struct bnxt *bp)
 	}
 
 	devlink_port_attrs_set(&bp->dl_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
-			       bp->pf.port_id, false, 0);
+			       bp->pf.port_id, false, 0, NULL, 0);
 	rc = devlink_port_register(dl, &bp->dl_port, bp->pf.port_id);
 	if (rc) {
 		netdev_err(bp->dev, "devlink_port_register failed");
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index e55b4aa91e3b..d01bd9d71b90 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1730,7 +1730,8 @@ int mlxsw_core_port_init(struct mlxsw_core *mlxsw_core, u8 local_port,
 
 	mlxsw_core_port->local_port = local_port;
 	devlink_port_attrs_set(devlink_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
-			       port_number, split, split_port_subnumber);
+			       port_number, split, split_port_subnumber,
+			       NULL, 0);
 	err = devlink_port_register(devlink, devlink_port, local_port);
 	if (err)
 		memset(mlxsw_core_port, 0, sizeof(*mlxsw_core_port));
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index 919da0d84fb4..15c4d2e0c86e 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -364,7 +364,7 @@ int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port)
 
 	devlink_port_attrs_set(&port->dl_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
 			       eth_port.label_port, eth_port.is_split,
-			       eth_port.label_subport);
+			       eth_port.label_subport, NULL, 0);
 
 	devlink = priv_to_devlink(app->pf);
 
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 4a1e3452a4ce..0f7968761204 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -42,10 +42,12 @@ struct devlink {
 
 struct devlink_port_attrs {
 	u8 set:1,
-	   split:1;
+	   split:1,
+	   switch_port:1;
 	enum devlink_port_flavour flavour;
 	u32 port_number; /* same value as "split group" */
 	u32 split_subport_number;
+	struct netdev_phys_item_id switch_id;
 };
 
 struct devlink_port {
@@ -582,7 +584,9 @@ void devlink_port_type_clear(struct devlink_port *devlink_port);
 void devlink_port_attrs_set(struct devlink_port *devlink_port,
 			    enum devlink_port_flavour flavour,
 			    u32 port_number, bool split,
-			    u32 split_subport_number);
+			    u32 split_subport_number,
+			    const unsigned char *switch_id,
+			    unsigned char switch_id_len);
 int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
 			u32 size, u16 ingress_pools_count,
 			u16 egress_pools_count, u16 ingress_tc_count,
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 5bb4ea67d84f..daabe7baaf4c 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -332,6 +332,8 @@ enum devlink_attr {
 	DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME,	/* string */
 	DEVLINK_ATTR_FLASH_UPDATE_COMPONENT,	/* string */
 
+	DEVLINK_ATTR_PORT_SWITCH_ID,		/* binary */
+
 	/* add new attributes above here, update the policy in devlink.c */
 
 	__DEVLINK_ATTR_MAX,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index dc3a99148ee7..eb2424773813 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -519,12 +519,15 @@ static int devlink_nl_port_attrs_put(struct sk_buff *msg,
 		return -EMSGSIZE;
 	if (nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER, attrs->port_number))
 		return -EMSGSIZE;
-	if (!attrs->split)
-		return 0;
-	if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP, attrs->port_number))
+	if (attrs->split &&
+	    (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP,
+			 attrs->port_number) ||
+	     nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_SUBPORT_NUMBER,
+			 attrs->split_subport_number)))
 		return -EMSGSIZE;
-	if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_SUBPORT_NUMBER,
-			attrs->split_subport_number))
+	if (attrs->switch_port &&
+	    nla_put(msg, DEVLINK_ATTR_PORT_SWITCH_ID,
+		    attrs->switch_id.id_len, attrs->switch_id.id))
 		return -EMSGSIZE;
 	return 0;
 }
@@ -5414,11 +5417,16 @@ EXPORT_SYMBOL_GPL(devlink_port_type_clear);
  *	@split: indicates if this is split port
  *	@split_subport_number: if the port is split, this is the number
  *	                       of subport.
+ *	@switch_id: if the port is part of switch, this is buffer with ID,
+ *	            otwerwise this is NULL
+ *	@switch_id_len: length of the switch_id buffer
  */
 void devlink_port_attrs_set(struct devlink_port *devlink_port,
 			    enum devlink_port_flavour flavour,
 			    u32 port_number, bool split,
-			    u32 split_subport_number)
+			    u32 split_subport_number,
+			    const unsigned char *switch_id,
+			    unsigned char switch_id_len)
 {
 	struct devlink_port_attrs *attrs = &devlink_port->attrs;
 
@@ -5429,6 +5437,15 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
 	attrs->port_number = port_number;
 	attrs->split = split;
 	attrs->split_subport_number = split_subport_number;
+	if (switch_id) {
+		attrs->switch_port = true;
+		if (WARN_ON(switch_id_len > MAX_PHYS_ITEM_ID_LEN))
+			switch_id_len = MAX_PHYS_ITEM_ID_LEN;
+		memcpy(attrs->switch_id.id, switch_id, switch_id_len);
+		attrs->switch_id.id_len = switch_id_len;
+	} else {
+		attrs->switch_port = false;
+	}
 }
 EXPORT_SYMBOL_GPL(devlink_port_attrs_set);
 
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index fe0a6197db9c..514be0583642 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -285,7 +285,7 @@ static int dsa_port_setup(struct dsa_port *dp)
 	 * independent from front panel port numbers.
 	 */
 	devlink_port_attrs_set(&dp->devlink_port, flavour,
-			       dp->index, false, 0);
+			       dp->index, false, 0, NULL, 0);
 	err = devlink_port_register(ds->devlink, &dp->devlink_port,
 				    dp->index);
 	if (err)
-- 
2.17.2

[patch net-next 03/12] net: devlink: introduce devlink_compat_switch_id_get() helper

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Introduce devlink_compat_switch_id_get() helper which fills up switch_id
according to passed netdev pointer. Call it directly from
dev_get_port_parent_id() as a fallback when ndo_get_port_parent_id
is not defined for given netdev.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
rfc->v1:
- don't take devlink mutexes
- rebase
---
 include/net/devlink.h |  9 +++++++++
 net/core/dev.c        |  8 ++++++--
 net/core/devlink.c    | 19 +++++++++++++++++++
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 0f7968761204..70c7d1ac8344 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -743,6 +743,8 @@ void devlink_compat_running_version(struct net_device *dev,
 int devlink_compat_flash_update(struct net_device *dev, const char *file_name);
 int devlink_compat_phys_port_name_get(struct net_device *dev,
 				      char *name, size_t len);
+int devlink_compat_switch_id_get(struct net_device *dev,
+				 struct netdev_phys_item_id *ppid);
 
 #else
 
@@ -764,6 +766,13 @@ devlink_compat_phys_port_name_get(struct net_device *dev,
 	return -EOPNOTSUPP;
 }
 
+static inline int
+devlink_compat_switch_id_get(struct net_device *dev,
+			     struct netdev_phys_item_id *ppid)
+{
+	return -EOPNOTSUPP;
+}
+
 #endif
 
 #endif /* _NET_DEVLINK_H_ */
diff --git a/net/core/dev.c b/net/core/dev.c
index 9823b7713f79..022dc2d86a5b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7905,14 +7905,18 @@ int dev_get_port_parent_id(struct net_device *dev,
 	struct netdev_phys_item_id first = { };
 	struct net_device *lower_dev;
 	struct list_head *iter;
-	int err = -EOPNOTSUPP;
+	int err;
 
 	if (ops->ndo_get_port_parent_id)
 		return ops->ndo_get_port_parent_id(dev, ppid);
 
-	if (!recurse)
+	err = devlink_compat_switch_id_get(dev, ppid);
+	if (!err || err != -EOPNOTSUPP)
 		return err;
 
+	if (!recurse)
+		return -EOPNOTSUPP;
+
 	netdev_for_each_lower_dev(dev, lower_dev, iter) {
 		err = dev_get_port_parent_id(lower_dev, ppid, recurse);
 		if (err)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index eb2424773813..0935b73c2076 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6511,6 +6511,25 @@ int devlink_compat_phys_port_name_get(struct net_device *dev,
 	return __devlink_port_phys_port_name_get(devlink_port, name, len);
 }
 
+int devlink_compat_switch_id_get(struct net_device *dev,
+				 struct netdev_phys_item_id *ppid)
+{
+	struct devlink_port *devlink_port;
+
+	/* RTNL mutex is held here which ensures that devlink_port
+	 * instance cannot disappear in the middle. No need to take
+	 * any devlink lock as only permanent values are accessed.
+	 */
+	ASSERT_RTNL();
+	devlink_port = netdev_to_devlink_port(dev);
+	if (!devlink_port || !devlink_port->attrs.switch_port)
+		return -EOPNOTSUPP;
+
+	memcpy(ppid, &devlink_port->attrs.switch_id, sizeof(*ppid));
+
+	return 0;
+}
+
 static int __init devlink_init(void)
 {
 	return genl_register_family(&devlink_nl_family);
-- 
2.17.2

[patch net-next 04/12] mlxsw: Pass switch ID through devlink_port_attrs_set()

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Pass the switch ID down the to devlink through devlink_port_attrs_set()
so it can be used by devlink_compat_switch_id_get().

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c     | 6 ++++--
 drivers/net/ethernet/mellanox/mlxsw/core.h     | 4 +++-
 drivers/net/ethernet/mellanox/mlxsw/minimal.c  | 4 +++-
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 4 +++-
 drivers/net/ethernet/mellanox/mlxsw/switchib.c | 2 +-
 drivers/net/ethernet/mellanox/mlxsw/switchx2.c | 3 ++-
 6 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index d01bd9d71b90..027e393c7cb2 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1720,7 +1720,9 @@ EXPORT_SYMBOL(mlxsw_core_res_get);
 
 int mlxsw_core_port_init(struct mlxsw_core *mlxsw_core, u8 local_port,
 			 u32 port_number, bool split,
-			 u32 split_port_subnumber)
+			 u32 split_port_subnumber,
+			 const unsigned char *switch_id,
+			 unsigned char switch_id_len)
 {
 	struct devlink *devlink = priv_to_devlink(mlxsw_core);
 	struct mlxsw_core_port *mlxsw_core_port =
@@ -1731,7 +1733,7 @@ int mlxsw_core_port_init(struct mlxsw_core *mlxsw_core, u8 local_port,
 	mlxsw_core_port->local_port = local_port;
 	devlink_port_attrs_set(devlink_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
 			       port_number, split, split_port_subnumber,
-			       NULL, 0);
+			       switch_id, switch_id_len);
 	err = devlink_port_register(devlink, devlink_port, local_port);
 	if (err)
 		memset(mlxsw_core_port, 0, sizeof(*mlxsw_core_port));
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index e8c424da534c..d51dfc3560b6 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -166,7 +166,9 @@ void mlxsw_core_lag_mapping_clear(struct mlxsw_core *mlxsw_core,
 void *mlxsw_core_port_driver_priv(struct mlxsw_core_port *mlxsw_core_port);
 int mlxsw_core_port_init(struct mlxsw_core *mlxsw_core, u8 local_port,
 			 u32 port_number, bool split,
-			 u32 split_port_subnumber);
+			 u32 split_port_subnumber,
+			 const unsigned char *switch_id,
+			 unsigned char switch_id_len);
 void mlxsw_core_port_fini(struct mlxsw_core *mlxsw_core, u8 local_port);
 void mlxsw_core_port_eth_set(struct mlxsw_core *mlxsw_core, u8 local_port,
 			     void *port_driver_priv, struct net_device *dev);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/minimal.c b/drivers/net/ethernet/mellanox/mlxsw/minimal.c
index ec5f5a66b607..bd96211602a4 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/minimal.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/minimal.c
@@ -151,7 +151,9 @@ mlxsw_m_port_create(struct mlxsw_m *mlxsw_m, u8 local_port, u8 module)
 	int err;
 
 	err = mlxsw_core_port_init(mlxsw_m->core, local_port,
-				   module + 1, false, 0);
+				   module + 1, false, 0,
+				   mlxsw_m->base_mac,
+				   sizeof(mlxsw_m->base_mac));
 	if (err) {
 		dev_err(mlxsw_m->bus_info->dev, "Port %d: Failed to init core port\n",
 			local_port);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 8b9a6870dbc2..2dbcc8e5e130 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3392,7 +3392,9 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
 	int err;
 
 	err = mlxsw_core_port_init(mlxsw_sp->core, local_port,
-				   module + 1, split, lane / width);
+				   module + 1, split, lane / width,
+				   mlxsw_sp->base_mac,
+				   sizeof(mlxsw_sp->base_mac));
 	if (err) {
 		dev_err(mlxsw_sp->bus_info->dev, "Port %d: Failed to init core port\n",
 			local_port);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchib.c b/drivers/net/ethernet/mellanox/mlxsw/switchib.c
index e1e7e0dd808d..b22caa154310 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/switchib.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/switchib.c
@@ -268,7 +268,7 @@ static int mlxsw_sib_port_create(struct mlxsw_sib *mlxsw_sib, u8 local_port,
 	int err;
 
 	err = mlxsw_core_port_init(mlxsw_sib->core, local_port,
-				   module + 1, false, 0);
+				   module + 1, false, 0, NULL, 0);
 	if (err) {
 		dev_err(mlxsw_sib->bus_info->dev, "Port %d: Failed to init core port\n",
 			local_port);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
index 5312dc1f339b..5397616fcda8 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
@@ -1128,7 +1128,8 @@ static int mlxsw_sx_port_eth_create(struct mlxsw_sx *mlxsw_sx, u8 local_port,
 	int err;
 
 	err = mlxsw_core_port_init(mlxsw_sx->core, local_port,
-				   module + 1, false, 0);
+				   module + 1, false, 0,
+				   mlxsw_sx->hw_id, sizeof(mlxsw_sx->hw_id));
 	if (err) {
 		dev_err(mlxsw_sx->bus_info->dev, "Port %d: Failed to init core port\n",
 			local_port);
-- 
2.17.2

[patch net-next 05/12] mlxsw: Remove ndo_get_port_parent_id implementation

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Remove implementation of get_port_parent_id ndo and rely on core calling
into devlink for the information directly.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/minimal.c  | 13 -------------
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 13 -------------
 drivers/net/ethernet/mellanox/mlxsw/switchx2.c | 13 -------------
 3 files changed, 39 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/minimal.c b/drivers/net/ethernet/mellanox/mlxsw/minimal.c
index bd96211602a4..cf2114273b72 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/minimal.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/minimal.c
@@ -51,18 +51,6 @@ static int mlxsw_m_port_dummy_open_stop(struct net_device *dev)
 	return 0;
 }
 
-static int mlxsw_m_port_get_port_parent_id(struct net_device *dev,
-					   struct netdev_phys_item_id *ppid)
-{
-	struct mlxsw_m_port *mlxsw_m_port = netdev_priv(dev);
-	struct mlxsw_m *mlxsw_m = mlxsw_m_port->mlxsw_m;
-
-	ppid->id_len = sizeof(mlxsw_m->base_mac);
-	memcpy(&ppid->id, &mlxsw_m->base_mac, ppid->id_len);
-
-	return 0;
-}
-
 static struct devlink_port *
 mlxsw_m_port_get_devlink_port(struct net_device *dev)
 {
@@ -76,7 +64,6 @@ mlxsw_m_port_get_devlink_port(struct net_device *dev)
 static const struct net_device_ops mlxsw_m_port_netdev_ops = {
 	.ndo_open		= mlxsw_m_port_dummy_open_stop,
 	.ndo_stop		= mlxsw_m_port_dummy_open_stop,
-	.ndo_get_port_parent_id	= mlxsw_m_port_get_port_parent_id,
 	.ndo_get_devlink_port	= mlxsw_m_port_get_devlink_port,
 };
 
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 2dbcc8e5e130..fc325f1213fb 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -1704,18 +1704,6 @@ static int mlxsw_sp_set_features(struct net_device *dev,
 				       mlxsw_sp_feature_hw_tc);
 }
 
-static int mlxsw_sp_port_get_port_parent_id(struct net_device *dev,
-					    struct netdev_phys_item_id *ppid)
-{
-	struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev);
-	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
-
-	ppid->id_len = sizeof(mlxsw_sp->base_mac);
-	memcpy(&ppid->id, &mlxsw_sp->base_mac, ppid->id_len);
-
-	return 0;
-}
-
 static struct devlink_port *
 mlxsw_sp_port_get_devlink_port(struct net_device *dev)
 {
@@ -1740,7 +1728,6 @@ static const struct net_device_ops mlxsw_sp_port_netdev_ops = {
 	.ndo_vlan_rx_add_vid	= mlxsw_sp_port_add_vid,
 	.ndo_vlan_rx_kill_vid	= mlxsw_sp_port_kill_vid,
 	.ndo_set_features	= mlxsw_sp_set_features,
-	.ndo_get_port_parent_id	= mlxsw_sp_port_get_port_parent_id,
 	.ndo_get_devlink_port	= mlxsw_sp_port_get_devlink_port,
 };
 
diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
index 5397616fcda8..fc4f19167262 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
@@ -379,18 +379,6 @@ mlxsw_sx_port_get_stats64(struct net_device *dev,
 	stats->tx_dropped	= tx_dropped;
 }
 
-static int mlxsw_sx_port_get_port_parent_id(struct net_device *dev,
-					    struct netdev_phys_item_id *ppid)
-{
-	struct mlxsw_sx_port *mlxsw_sx_port = netdev_priv(dev);
-	struct mlxsw_sx *mlxsw_sx = mlxsw_sx_port->mlxsw_sx;
-
-	ppid->id_len = sizeof(mlxsw_sx->hw_id);
-	memcpy(&ppid->id, &mlxsw_sx->hw_id, ppid->id_len);
-
-	return 0;
-}
-
 static struct devlink_port *
 mlxsw_sx_port_get_devlink_port(struct net_device *dev)
 {
@@ -407,7 +395,6 @@ static const struct net_device_ops mlxsw_sx_port_netdev_ops = {
 	.ndo_start_xmit		= mlxsw_sx_port_xmit,
 	.ndo_change_mtu		= mlxsw_sx_port_change_mtu,
 	.ndo_get_stats64	= mlxsw_sx_port_get_stats64,
-	.ndo_get_port_parent_id	= mlxsw_sx_port_get_port_parent_id,
 	.ndo_get_devlink_port	= mlxsw_sx_port_get_devlink_port,
 };
 
-- 
2.17.2

[patch net-next 06/12] bnxt: pass switch ID through devlink_port_attrs_set()

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Pass the switch ID down the to devlink through devlink_port_attrs_set()
so it can be used by devlink_compat_switch_id_get().

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
rfc->v1:
- new patch
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 36ec4cb45276..549c90d3e465 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -230,7 +230,8 @@ int bnxt_dl_register(struct bnxt *bp)
 	}
 
 	devlink_port_attrs_set(&bp->dl_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
-			       bp->pf.port_id, false, 0, NULL, 0);
+			       bp->pf.port_id, false, 0,
+			       bp->switch_id, sizeof(bp->switch_id));
 	rc = devlink_port_register(dl, &bp->dl_port, bp->pf.port_id);
 	if (rc) {
 		netdev_err(bp->dev, "devlink_port_register failed");
-- 
2.17.2

[patch net-next 07/12] bnxt: remove ndo_get_port_parent_id implementation for physical ports

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Remove implementation of get_port_parent_id ndo and rely on core calling
into devlink for the information directly.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 35e34e23ba33..9b2adf1c6b2d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -10104,7 +10104,6 @@ static const struct net_device_ops bnxt_netdev_ops = {
 	.ndo_bpf		= bnxt_xdp,
 	.ndo_bridge_getlink	= bnxt_bridge_getlink,
 	.ndo_bridge_setlink	= bnxt_bridge_setlink,
-	.ndo_get_port_parent_id	= bnxt_get_port_parent_id,
 	.ndo_get_devlink_port	= bnxt_get_devlink_port,
 };
 
-- 
2.17.2

[patch net-next 08/12] nfp: pass switch ID through devlink_port_attrs_set()

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Pass the switch ID down the to devlink through devlink_port_attrs_set()
so it can be used by devlink_compat_switch_id_get().

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_devlink.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index 15c4d2e0c86e..8e7591241e7c 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -354,6 +354,8 @@ int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port)
 {
 	struct nfp_eth_table_port eth_port;
 	struct devlink *devlink;
+	const u8 *serial;
+	int serial_len;
 	int ret;
 
 	rtnl_lock();
@@ -362,9 +364,10 @@ int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port)
 	if (ret)
 		return ret;
 
+	serial_len = nfp_cpp_serial(port->app->cpp, &serial);
 	devlink_port_attrs_set(&port->dl_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
 			       eth_port.label_port, eth_port.is_split,
-			       eth_port.label_subport, NULL, 0);
+			       eth_port.label_subport, serial, serial_len);
 
 	devlink = priv_to_devlink(app->pf);
 
-- 
2.17.2

[patch net-next 09/12] nfp: remove ndo_get_port_parent_id implementation

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Remove implementation of get_port_parent_id ndo and rely on core calling
into devlink for the information directly.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/netronome/nfp/nfp_net_common.c  |  1 -
 .../net/ethernet/netronome/nfp/nfp_net_repr.c    |  1 -
 drivers/net/ethernet/netronome/nfp/nfp_port.c    | 16 ----------------
 3 files changed, 18 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 99200b5dac76..7abb0005ecea 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3533,7 +3533,6 @@ const struct net_device_ops nfp_net_netdev_ops = {
 	.ndo_udp_tunnel_add	= nfp_net_add_vxlan_port,
 	.ndo_udp_tunnel_del	= nfp_net_del_vxlan_port,
 	.ndo_bpf		= nfp_net_xdp,
-	.ndo_get_port_parent_id	= nfp_port_get_port_parent_id,
 	.ndo_get_devlink_port	= nfp_devlink_get_devlink_port,
 };
 
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
index bf621674f583..c3ad083d36c6 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
@@ -272,7 +272,6 @@ const struct net_device_ops nfp_repr_netdev_ops = {
 	.ndo_fix_features	= nfp_repr_fix_features,
 	.ndo_set_features	= nfp_port_set_features,
 	.ndo_set_mac_address    = eth_mac_addr,
-	.ndo_get_port_parent_id	= nfp_port_get_port_parent_id,
 	.ndo_get_devlink_port	= nfp_devlink_get_devlink_port,
 };
 
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_port.c b/drivers/net/ethernet/netronome/nfp/nfp_port.c
index 93c5bfc0510b..fcd16877e6e0 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_port.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_port.c
@@ -30,22 +30,6 @@ struct nfp_port *nfp_port_from_netdev(struct net_device *netdev)
 	return NULL;
 }
 
-int nfp_port_get_port_parent_id(struct net_device *netdev,
-				struct netdev_phys_item_id *ppid)
-{
-	struct nfp_port *port;
-	const u8 *serial;
-
-	port = nfp_port_from_netdev(netdev);
-	if (!port)
-		return -EOPNOTSUPP;
-
-	ppid->id_len = nfp_cpp_serial(port->app->cpp, &serial);
-	memcpy(&ppid->id, serial, ppid->id_len);
-
-	return 0;
-}
-
 int nfp_port_setup_tc(struct net_device *netdev, enum tc_setup_type type,
 		      void *type_data)
 {
-- 
2.17.2

[patch net-next 10/12] mlxsw: switch_ib: Pass valid HW id down to mlxsw_core_port_init()

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Obtain HW id and pass it down to mlxsw_core_port_init() as it would be
used as switch_id in devlink and exposed to user.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/switchib.c    | 22 ++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchib.c b/drivers/net/ethernet/mellanox/mlxsw/switchib.c
index b22caa154310..0d9356b3f65d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/switchib.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/switchib.c
@@ -30,6 +30,7 @@ struct mlxsw_sib {
 	struct mlxsw_sib_port **ports;
 	struct mlxsw_core *core;
 	const struct mlxsw_bus_info *bus_info;
+	u8 hw_id[ETH_ALEN];
 };
 
 struct mlxsw_sib_port {
@@ -102,6 +103,18 @@ mlxsw_sib_tx_v1_hdr_construct(struct sk_buff *skb,
 	mlxsw_tx_v1_hdr_type_set(txhdr, MLXSW_TXHDR_TYPE_CONTROL);
 }
 
+static int mlxsw_sib_hw_id_get(struct mlxsw_sib *mlxsw_sib)
+{
+	char spad_pl[MLXSW_REG_SPAD_LEN] = {0};
+	int err;
+
+	err = mlxsw_reg_query(mlxsw_sib->core, MLXSW_REG(spad), spad_pl);
+	if (err)
+		return err;
+	mlxsw_reg_spad_base_mac_memcpy_from(spad_pl, mlxsw_sib->hw_id);
+	return 0;
+}
+
 static int
 mlxsw_sib_port_admin_status_set(struct mlxsw_sib_port *mlxsw_sib_port,
 				bool is_up)
@@ -268,7 +281,8 @@ static int mlxsw_sib_port_create(struct mlxsw_sib *mlxsw_sib, u8 local_port,
 	int err;
 
 	err = mlxsw_core_port_init(mlxsw_sib->core, local_port,
-				   module + 1, false, 0, NULL, 0);
+				   module + 1, false, 0,
+				   mlxsw_sib->hw_id, sizeof(mlxsw_sib->hw_id));
 	if (err) {
 		dev_err(mlxsw_sib->bus_info->dev, "Port %d: Failed to init core port\n",
 			local_port);
@@ -440,6 +454,12 @@ static int mlxsw_sib_init(struct mlxsw_core *mlxsw_core,
 	mlxsw_sib->core = mlxsw_core;
 	mlxsw_sib->bus_info = mlxsw_bus_info;
 
+	err = mlxsw_sib_hw_id_get(mlxsw_sib);
+	if (err) {
+		dev_err(mlxsw_sib->bus_info->dev, "Failed to get switch HW ID\n");
+		return err;
+	}
+
 	err = mlxsw_sib_ports_create(mlxsw_sib);
 	if (err) {
 		dev_err(mlxsw_sib->bus_info->dev, "Failed to create ports\n");
-- 
2.17.2

[patch net-next 11/12] dsa: pass switch ID through devlink_port_attrs_set()

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Pass the switch ID down the to devlink through devlink_port_attrs_set()
so it can be used by devlink_compat_switch_id_get(). Leave
ndo_get_port_parent_id implementation only for legacy.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/dsa/dsa2.c  | 4 +++-
 net/dsa/slave.c | 1 -
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 514be0583642..410cb0da9def 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -260,6 +260,7 @@ static int dsa_port_setup(struct dsa_port *dp)
 {
 	enum devlink_port_flavour flavour;
 	struct dsa_switch *ds = dp->ds;
+	struct dsa_switch_tree *dst = ds->dst;
 	int err;
 
 	if (dp->type == DSA_PORT_TYPE_UNUSED)
@@ -285,7 +286,8 @@ static int dsa_port_setup(struct dsa_port *dp)
 	 * independent from front panel port numbers.
 	 */
 	devlink_port_attrs_set(&dp->devlink_port, flavour,
-			       dp->index, false, 0, NULL, 0);
+			       dp->index, false, 0,
+			       (const char *) &dst->index, sizeof(dst->index));
 	err = devlink_port_register(ds->devlink, &dp->devlink_port,
 				    dp->index);
 	if (err)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 80be8e86c82d..026a4003d520 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1130,7 +1130,6 @@ static const struct net_device_ops dsa_slave_netdev_ops = {
 	.ndo_get_phys_port_name	= dsa_slave_get_phys_port_name,
 	.ndo_setup_tc		= dsa_slave_setup_tc,
 	.ndo_get_stats64	= dsa_slave_get_stats64,
-	.ndo_get_port_parent_id	= dsa_slave_get_port_parent_id,
 	.ndo_vlan_rx_add_vid	= dsa_slave_vlan_rx_add_vid,
 	.ndo_vlan_rx_kill_vid	= dsa_slave_vlan_rx_kill_vid,
 	.ndo_get_devlink_port	= dsa_slave_get_devlink_port,
-- 
2.17.2

[patch net-next 12/12] net: devlink: add warning for ndo_get_port_parent_id set when not needed

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Currently if the driver registers devlink port instance, he should set
the devlink port attributes as well. Then the devlink core is able to
obtain switch id itself, no need for driver to implement the ndo.
Once all drivers will implement devlink port registration, this ndo
should be removed. This warning guides new drivers to do things as
they should be done.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
rfc->v1:
- new patch
---
 net/core/devlink.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 0935b73c2076..cccb8c120b88 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -5379,6 +5379,7 @@ void devlink_port_type_eth_set(struct devlink_port *devlink_port,
 		err = ops->ndo_get_phys_port_name(netdev, name, sizeof(name));
 		WARN_ON(err != -EOPNOTSUPP);
 	}
+	WARN_ON(netdev->netdev_ops->ndo_get_port_parent_id);
 	__devlink_port_type_set(devlink_port, DEVLINK_PORT_TYPE_ETH, netdev);
 }
 EXPORT_SYMBOL_GPL(devlink_port_type_eth_set);
-- 
2.17.2

Re: [patch net-next 00/12] net: expose switch ID via devlink

[Jakub Kicinski]

On Thu, 28 Mar 2019 22:12:42 +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> To provide visibility of the ports, this patchset exposes switch ID
> for devlink ports, which are part of a switch. The rest of the ports
> if any (in case of sr-iov for example) do not set switch ID.

I don't feel good about this patch set.  There is no visibility
provided here.  Should the port flavour should be a sufficient
indication of whether netdev associated with that port can be 
switched to or not?  CPU, DSA, and Host flavours can't be switched 
to.  And the switchid can be an attribute of the devlink instance,
if we want to expose it via devlink.

Re: [patch net-next 00/12] net: expose switch ID via devlink

[Jiri Pirko]

Thu, Mar 28, 2019 at 10:40:02PM CET, jakub.kicinski@netronome.com wrote:
>On Thu, 28 Mar 2019 22:12:42 +0100, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> To provide visibility of the ports, this patchset exposes switch ID
>> for devlink ports, which are part of a switch. The rest of the ports
>> if any (in case of sr-iov for example) do not set switch ID.
>
>I don't feel good about this patch set.  There is no visibility
>provided here.  Should the port flavour should be a sufficient

1) this patch is mainly about avoiding need to define the ndo and moving
   the switch id definition to devlink port attr.
2) along with that, switch id is added as attribute. It tells the user
   that some devlink port is part of a switch with certain id. If port
   is not part of a switch (like upcoming hostport, cpu, dsa, etc),
   switch id is not set on that port

>indication of whether netdev associated with that port can be 
>switched to or not?  CPU, DSA, and Host flavours can't be switched 
>to.  And the switchid can be an attribute of the devlink instance,
>if we want to expose it via devlink.

One devlink instance can have multiple switch ids in use as it may
contain multiple switches. Take mlx5 as an instance. Currently every PF
creates a separate devlink instance, however there are some features
shared. In this example, with proposed idea of aliasing, there would be
one devlink instance aliased between these 2 pf inctances, with 2
eswitches and 2 sets of switch ports each belonging to an eswitch -
distinguished by switch id.

Re: [patch net-next 00/12] net: expose switch ID via devlink

[Jakub Kicinski]

On Fri, 29 Mar 2019 07:49:05 +0100, Jiri Pirko wrote:
> Thu, Mar 28, 2019 at 10:40:02PM CET, jakub.kicinski@netronome.com wrote:
> >On Thu, 28 Mar 2019 22:12:42 +0100, Jiri Pirko wrote:  
> >> From: Jiri Pirko <jiri@mellanox.com>
> >> 
> >> To provide visibility of the ports, this patchset exposes switch ID
> >> for devlink ports, which are part of a switch. The rest of the ports
> >> if any (in case of sr-iov for example) do not set switch ID.  
> >
> >I don't feel good about this patch set.  There is no visibility
> >provided here.  Should the port flavour should be a sufficient  
> 
> 1) this patch is mainly about avoiding need to define the ndo and moving
>    the switch id definition to devlink port attr.

Sure, that you could achieve by putting the data in the netdevice
structure as well..

What is the guiding principle here?  I'm trying to argue for leaving
forwarding-related info in netdev code, and only have HW control in
devlink.  I just don't see switch id being useful at devlink level in
any way.

> 2) along with that, switch id is added as attribute. It tells the user
>    that some devlink port is part of a switch with certain id. If port
>    is not part of a switch (like upcoming hostport, cpu, dsa, etc),
>    switch id is not set on that port

If the flavour already gives that information, why crowd the attributes
for ports with switch id?

> >indication of whether netdev associated with that port can be 
> >switched to or not?  CPU, DSA, and Host flavours can't be switched 
> >to.  And the switchid can be an attribute of the devlink instance,
> >if we want to expose it via devlink.  
> 
> One devlink instance can have multiple switch ids in use as it may
> contain multiple switches. Take mlx5 as an instance. Currently every PF
> creates a separate devlink instance, however there are some features
> shared. In this example, with proposed idea of aliasing, there would be
> one devlink instance aliased between these 2 pf inctances, with 2
> eswitches and 2 sets of switch ports each belonging to an eswitch -
> distinguished by switch id.

Out of curiosity, what are the shared features?  It seems mlx5 drives 
a lot of our API design, it'd be good if the community had a better
understanding of it.

The situation with pipelined devices is somewhat murky.  Didn't Or add
some from of PCIe-side looped queue to forward between PFs?

Presumably DSA would lean the opposite way with multiple ASICs
reporting the same ID?

Re: [patch net-next 00/12] net: expose switch ID via devlink

[Jiri Pirko]

Fri, Mar 29, 2019 at 07:59:26PM CET, jakub.kicinski@netronome.com wrote:
>On Fri, 29 Mar 2019 07:49:05 +0100, Jiri Pirko wrote:
>> Thu, Mar 28, 2019 at 10:40:02PM CET, jakub.kicinski@netronome.com wrote:
>> >On Thu, 28 Mar 2019 22:12:42 +0100, Jiri Pirko wrote:  
>> >> From: Jiri Pirko <jiri@mellanox.com>
>> >> 
>> >> To provide visibility of the ports, this patchset exposes switch ID
>> >> for devlink ports, which are part of a switch. The rest of the ports
>> >> if any (in case of sr-iov for example) do not set switch ID.  
>> >
>> >I don't feel good about this patch set.  There is no visibility
>> >provided here.  Should the port flavour should be a sufficient  
>> 
>> 1) this patch is mainly about avoiding need to define the ndo and moving
>>    the switch id definition to devlink port attr.
>
>Sure, that you could achieve by putting the data in the netdevice
>structure as well..
>
>What is the guiding principle here?  I'm trying to argue for leaving
>forwarding-related info in netdev code, and only have HW control in
>devlink.  I just don't see switch id being useful at devlink level in
>any way.

Well we have switchib driver which does not have any netdevice and still
the ports are part of a switch. In other words, this is not
ethernet-specific attribute, therefore devlink is the right fit.

>
>> 2) along with that, switch id is added as attribute. It tells the user
>>    that some devlink port is part of a switch with certain id. If port
>>    is not part of a switch (like upcoming hostport, cpu, dsa, etc),
>>    switch id is not set on that port
>
>If the flavour already gives that information, why crowd the attributes
>for ports with switch id?

Hmm, we'll have multiple non-switch port flavours and once your vf/pf
patchset  hits the tree we'll have multipkle switch port flavours.
So makes sense to have switch id. Also, you can have multiple switches
within one asic.


>
>> >indication of whether netdev associated with that port can be 
>> >switched to or not?  CPU, DSA, and Host flavours can't be switched 
>> >to.  And the switchid can be an attribute of the devlink instance,
>> >if we want to expose it via devlink.  
>> 
>> One devlink instance can have multiple switch ids in use as it may
>> contain multiple switches. Take mlx5 as an instance. Currently every PF
>> creates a separate devlink instance, however there are some features
>> shared. In this example, with proposed idea of aliasing, there would be
>> one devlink instance aliased between these 2 pf inctances, with 2
>> eswitches and 2 sets of switch ports each belonging to an eswitch -
>> distinguished by switch id.
>
>Out of curiosity, what are the shared features?  It seems mlx5 drives 
>a lot of our API design, it'd be good if the community had a better
>understanding of it.

I have to gather that info. Not so many things are shared. There is one
extra switch to mix 2 pfs together. I know about some IB features that
also mix 2 pfs.

>
>The situation with pipelined devices is somewhat murky.  Didn't Or add
>some from of PCIe-side looped queue to forward between PFs?

I have no clue. Ccing Or. Or?


>
>Presumably DSA would lean the opposite way with multiple ASICs
>reporting the same ID?

Yes, sounds right.

Re: [patch net-next 00/12] net: expose switch ID via devlink

[Florian Fainelli]

On 3/29/19 11:59 AM, Jakub Kicinski wrote:
> On Fri, 29 Mar 2019 07:49:05 +0100, Jiri Pirko wrote:
>> Thu, Mar 28, 2019 at 10:40:02PM CET, jakub.kicinski@netronome.com wrote:
>>> On Thu, 28 Mar 2019 22:12:42 +0100, Jiri Pirko wrote:  
>>>> From: Jiri Pirko <jiri@mellanox.com>
>>>>
>>>> To provide visibility of the ports, this patchset exposes switch ID
>>>> for devlink ports, which are part of a switch. The rest of the ports
>>>> if any (in case of sr-iov for example) do not set switch ID.  
>>>
>>> I don't feel good about this patch set.  There is no visibility
>>> provided here.  Should the port flavour should be a sufficient  
>>
>> 1) this patch is mainly about avoiding need to define the ndo and moving
>>    the switch id definition to devlink port attr.
> 
> Sure, that you could achieve by putting the data in the netdevice
> structure as well..
> 
> What is the guiding principle here?  I'm trying to argue for leaving
> forwarding-related info in netdev code, and only have HW control in
> devlink.  I just don't see switch id being useful at devlink level in
> any way.
> 
>> 2) along with that, switch id is added as attribute. It tells the user
>>    that some devlink port is part of a switch with certain id. If port
>>    is not part of a switch (like upcoming hostport, cpu, dsa, etc),
>>    switch id is not set on that port
> 
> If the flavour already gives that information, why crowd the attributes
> for ports with switch id?
> 
>>> indication of whether netdev associated with that port can be 
>>> switched to or not?  CPU, DSA, and Host flavours can't be switched 
>>> to.  And the switchid can be an attribute of the devlink instance,
>>> if we want to expose it via devlink.  
>>
>> One devlink instance can have multiple switch ids in use as it may
>> contain multiple switches. Take mlx5 as an instance. Currently every PF
>> creates a separate devlink instance, however there are some features
>> shared. In this example, with proposed idea of aliasing, there would be
>> one devlink instance aliased between these 2 pf inctances, with 2
>> eswitches and 2 sets of switch ports each belonging to an eswitch -
>> distinguished by switch id.
> 
> Out of curiosity, what are the shared features?  It seems mlx5 drives 
> a lot of our API design, it'd be good if the community had a better
> understanding of it.
> 
> The situation with pipelined devices is somewhat murky.  Didn't Or add
> some from of PCIe-side looped queue to forward between PFs?
> 
> Presumably DSA would lean the opposite way with multiple ASICs
> reporting the same ID?

If you have multiple switches inter connected between each other to use
the "D" in DSA and form a fabric of switches, then you would expect each
port to be physically tied to a particular switch device/instance,
because, but how they will report the switch physical ID can be of the form:

<fabric>.<switch>

where fabric is dst->index and switch is ds->index (the switch within
the fabric).
-- 
Florian

Re: [patch net-next 00/12] net: expose switch ID via devlink

[Jakub Kicinski]

On Fri, 29 Mar 2019 14:29:11 -0700, Florian Fainelli wrote:
> > Out of curiosity, what are the shared features?  It seems mlx5 drives 
> > a lot of our API design, it'd be good if the community had a better
> > understanding of it.
> > 
> > The situation with pipelined devices is somewhat murky.  Didn't Or add
> > some from of PCIe-side looped queue to forward between PFs?
> > 
> > Presumably DSA would lean the opposite way with multiple ASICs
> > reporting the same ID?  
> 
> If you have multiple switches inter connected between each other to use
> the "D" in DSA and form a fabric of switches, then you would expect each
> port to be physically tied to a particular switch device/instance,
> because, but how they will report the switch physical ID can be of the form:
> 
> <fabric>.<switch>
> 
> where fabric is dst->index and switch is ds->index (the switch within
> the fabric).

Oh, I assumed you'd want the to all have the same switchid, and then
the "D" in DSA logic makes sure the flooding etc. works across the
ASICs..

Re: [patch net-next 11/12] dsa: pass switch ID through devlink_port_attrs_set()

[Florian Fainelli]

On 3/28/19 2:12 PM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Pass the switch ID down the to devlink through devlink_port_attrs_set()
> so it can be used by devlink_compat_switch_id_get(). Leave
> ndo_get_port_parent_id implementation only for legacy.

Nit you are passing the switch fabric id (dst->index), the switch id is
in ds->index.

> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  net/dsa/dsa2.c  | 4 +++-
>  net/dsa/slave.c | 1 -
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
> index 514be0583642..410cb0da9def 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -260,6 +260,7 @@ static int dsa_port_setup(struct dsa_port *dp)
>  {
>  	enum devlink_port_flavour flavour;
>  	struct dsa_switch *ds = dp->ds;
> +	struct dsa_switch_tree *dst = ds->dst;
>  	int err;
>  
>  	if (dp->type == DSA_PORT_TYPE_UNUSED)
> @@ -285,7 +286,8 @@ static int dsa_port_setup(struct dsa_port *dp)
>  	 * independent from front panel port numbers.
>  	 */
>  	devlink_port_attrs_set(&dp->devlink_port, flavour,
> -			       dp->index, false, 0, NULL, 0);
> +			       dp->index, false, 0,
> +			       (const char *) &dst->index, sizeof(dst->index));
>  	err = devlink_port_register(ds->devlink, &dp->devlink_port,
>  				    dp->index);
>  	if (err)
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 80be8e86c82d..026a4003d520 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -1130,7 +1130,6 @@ static const struct net_device_ops dsa_slave_netdev_ops = {
>  	.ndo_get_phys_port_name	= dsa_slave_get_phys_port_name,
>  	.ndo_setup_tc		= dsa_slave_setup_tc,
>  	.ndo_get_stats64	= dsa_slave_get_stats64,
> -	.ndo_get_port_parent_id	= dsa_slave_get_port_parent_id,
>  	.ndo_vlan_rx_add_vid	= dsa_slave_vlan_rx_add_vid,
>  	.ndo_vlan_rx_kill_vid	= dsa_slave_vlan_rx_kill_vid,
>  	.ndo_get_devlink_port	= dsa_slave_get_devlink_port,
> 


-- 
Florian

Re: [patch net-next 00/12] net: expose switch ID via devlink

[Florian Fainelli]

On 3/29/19 2:57 PM, Jakub Kicinski wrote:
> On Fri, 29 Mar 2019 14:29:11 -0700, Florian Fainelli wrote:
>>> Out of curiosity, what are the shared features?  It seems mlx5 drives 
>>> a lot of our API design, it'd be good if the community had a better
>>> understanding of it.
>>>
>>> The situation with pipelined devices is somewhat murky.  Didn't Or add
>>> some from of PCIe-side looped queue to forward between PFs?
>>>
>>> Presumably DSA would lean the opposite way with multiple ASICs
>>> reporting the same ID?  
>>
>> If you have multiple switches inter connected between each other to use
>> the "D" in DSA and form a fabric of switches, then you would expect each
>> port to be physically tied to a particular switch device/instance,
>> because, but how they will report the switch physical ID can be of the form:
>>
>> <fabric>.<switch>
>>
>> where fabric is dst->index and switch is ds->index (the switch within
>> the fabric).
> 
> Oh, I assumed you'd want the to all have the same switchid, and then
> the "D" in DSA logic makes sure the flooding etc. works across the
> ASICs..

That's a good point, they need to have the switch skb->fwd_offload_mark
to work, so yes, that should be the fabric's index then.
-- 
Florian

Re: [patch net-next 00/12] net: expose switch ID via devlink

[Jakub Kicinski]

On Fri, Mar 29, 2019 at 2:21 PM Jiri Pirko <jiri@resnulli.us> wrote:
> Fri, Mar 29, 2019 at 07:59:26PM CET, jakub.kicinski@netronome.com wrote:
> >On Fri, 29 Mar 2019 07:49:05 +0100, Jiri Pirko wrote:
> >> Thu, Mar 28, 2019 at 10:40:02PM CET, jakub.kicinski@netronome.com wrote:
> >> >On Thu, 28 Mar 2019 22:12:42 +0100, Jiri Pirko wrote:
> >> >> From: Jiri Pirko <jiri@mellanox.com>
> >> >>
> >> >> To provide visibility of the ports, this patchset exposes switch ID
> >> >> for devlink ports, which are part of a switch. The rest of the ports
> >> >> if any (in case of sr-iov for example) do not set switch ID.
> >> >
> >> >I don't feel good about this patch set.  There is no visibility
> >> >provided here.  Should the port flavour should be a sufficient
> >>
> >> 1) this patch is mainly about avoiding need to define the ndo and moving
> >>    the switch id definition to devlink port attr.
> >
> >Sure, that you could achieve by putting the data in the netdevice
> >structure as well..
> >
> >What is the guiding principle here?  I'm trying to argue for leaving
> >forwarding-related info in netdev code, and only have HW control in
> >devlink.  I just don't see switch id being useful at devlink level in
> >any way.
>
> Well we have switchib driver which does not have any netdevice and still
> the ports are part of a switch. In other words, this is not
> ethernet-specific attribute, therefore devlink is the right fit.

Ack, but switch id is a switchdev thing, controlling which ports are
considered to be part of the same device _for_ _switchdev_.

Is switchib doing forwarding?  Not long ago Parav was convincing us
that switchdev mode for IB is pretty much meaningless.  Even though
there are apparently representors for IB (judging by the recent RDMA
patchset on netdev)...

> >> 2) along with that, switch id is added as attribute. It tells the user
> >>    that some devlink port is part of a switch with certain id. If port
> >>    is not part of a switch (like upcoming hostport, cpu, dsa, etc),
> >>    switch id is not set on that port
> >
> >If the flavour already gives that information, why crowd the attributes
> >for ports with switch id?
>
> Hmm, we'll have multiple non-switch port flavours and once your vf/pf
> patchset  hits the tree we'll have multipkle switch port flavours.
> So makes sense to have switch id.

It's probably a matter of preference then.  To me its very clear to say that
you can switch to physical/vf/pf ports and not cpu/dsa/host. I don't like
the duplication of dumpnig the same attribute both in rtnetlink and devlink.

> Also, you can have multiple switches within one asic.

That line is blurry.

> >> >indication of whether netdev associated with that port can be
> >> >switched to or not?  CPU, DSA, and Host flavours can't be switched
> >> >to.  And the switchid can be an attribute of the devlink instance,
> >> >if we want to expose it via devlink.
> >>
> >> One devlink instance can have multiple switch ids in use as it may
> >> contain multiple switches. Take mlx5 as an instance. Currently every PF
> >> creates a separate devlink instance, however there are some features
> >> shared. In this example, with proposed idea of aliasing, there would be
> >> one devlink instance aliased between these 2 pf inctances, with 2
> >> eswitches and 2 sets of switch ports each belonging to an eswitch -
> >> distinguished by switch id.
> >
> >Out of curiosity, what are the shared features?  It seems mlx5 drives
> >a lot of our API design, it'd be good if the community had a better
> >understanding of it.
>
> I have to gather that info. Not so many things are shared. There is one
> extra switch to mix 2 pfs together. I know about some IB features that
> also mix 2 pfs.

I see, so a dual port IB card would somehow have only one whatever the
equivalent of a netdev in IB world is?

> >The situation with pipelined devices is somewhat murky.  Didn't Or add
> >some from of PCIe-side looped queue to forward between PFs?
>
> I have no clue. Ccing Or. Or?
>
> >Presumably DSA would lean the opposite way with multiple ASICs
> >reporting the same ID?
>
> Yes, sounds right.

Right, so to me switch ID really is a forwarding attribute.

Re: [patch net-next 00/12] net: expose switch ID via devlink

[Jiri Pirko]

Fri, Mar 29, 2019 at 11:34:14PM CET, jakub.kicinski@netronome.com wrote:
>On Fri, Mar 29, 2019 at 2:21 PM Jiri Pirko <jiri@resnulli.us> wrote:
>> Fri, Mar 29, 2019 at 07:59:26PM CET, jakub.kicinski@netronome.com wrote:
>> >On Fri, 29 Mar 2019 07:49:05 +0100, Jiri Pirko wrote:
>> >> Thu, Mar 28, 2019 at 10:40:02PM CET, jakub.kicinski@netronome.com wrote:
>> >> >On Thu, 28 Mar 2019 22:12:42 +0100, Jiri Pirko wrote:
>> >> >> From: Jiri Pirko <jiri@mellanox.com>
>> >> >>
>> >> >> To provide visibility of the ports, this patchset exposes switch ID
>> >> >> for devlink ports, which are part of a switch. The rest of the ports
>> >> >> if any (in case of sr-iov for example) do not set switch ID.
>> >> >
>> >> >I don't feel good about this patch set.  There is no visibility
>> >> >provided here.  Should the port flavour should be a sufficient
>> >>
>> >> 1) this patch is mainly about avoiding need to define the ndo and moving
>> >>    the switch id definition to devlink port attr.
>> >
>> >Sure, that you could achieve by putting the data in the netdevice
>> >structure as well..
>> >
>> >What is the guiding principle here?  I'm trying to argue for leaving
>> >forwarding-related info in netdev code, and only have HW control in
>> >devlink.  I just don't see switch id being useful at devlink level in
>> >any way.
>>
>> Well we have switchib driver which does not have any netdevice and still
>> the ports are part of a switch. In other words, this is not
>> ethernet-specific attribute, therefore devlink is the right fit.
>
>Ack, but switch id is a switchdev thing, controlling which ports are
>considered to be part of the same device _for_ _switchdev_.

"Switchdev" is a bridge offload. Nothing else.


>
>Is switchib doing forwarding?  Not long ago Parav was convincing us
>that switchdev mode for IB is pretty much meaningless.  Even though
>there are apparently representors for IB (judging by the recent RDMA
>patchset on netdev)...

Switch id is simply identifier for ports in general that belong to some
switch. No matter if ethernet or IB.


>
>> >> 2) along with that, switch id is added as attribute. It tells the user
>> >>    that some devlink port is part of a switch with certain id. If port
>> >>    is not part of a switch (like upcoming hostport, cpu, dsa, etc),
>> >>    switch id is not set on that port
>> >
>> >If the flavour already gives that information, why crowd the attributes
>> >for ports with switch id?
>>
>> Hmm, we'll have multiple non-switch port flavours and once your vf/pf
>> patchset  hits the tree we'll have multipkle switch port flavours.
>> So makes sense to have switch id.
>
>It's probably a matter of preference then.  To me its very clear to say that
>you can switch to physical/vf/pf ports and not cpu/dsa/host. I don't like
>the duplication of dumpnig the same attribute both in rtnetlink and devlink.

I want to remind you, that switch_id in rtnetlink was introduced before
devlink existed. Back than, if was the only place to put it. However, as
I explained, it is not ethernet speficic thing and rather belongs to
devlink. This patchset is doing that.


>
>> Also, you can have multiple switches within one asic.
>
>That line is blurry.

It is. So far, it is mlx CX hw that have basically 2 switches, one per
PF.


>
>> >> >indication of whether netdev associated with that port can be
>> >> >switched to or not?  CPU, DSA, and Host flavours can't be switched
>> >> >to.  And the switchid can be an attribute of the devlink instance,
>> >> >if we want to expose it via devlink.
>> >>
>> >> One devlink instance can have multiple switch ids in use as it may
>> >> contain multiple switches. Take mlx5 as an instance. Currently every PF
>> >> creates a separate devlink instance, however there are some features
>> >> shared. In this example, with proposed idea of aliasing, there would be
>> >> one devlink instance aliased between these 2 pf inctances, with 2
>> >> eswitches and 2 sets of switch ports each belonging to an eswitch -
>> >> distinguished by switch id.
>> >
>> >Out of curiosity, what are the shared features?  It seems mlx5 drives
>> >a lot of our API design, it'd be good if the community had a better
>> >understanding of it.
>>
>> I have to gather that info. Not so many things are shared. There is one
>> extra switch to mix 2 pfs together. I know about some IB features that
>> also mix 2 pfs.
>
>I see, so a dual port IB card would somehow have only one whatever the
>equivalent of a netdev in IB world is?

Yes. We have a patchset internally to manage that.


>
>> >The situation with pipelined devices is somewhat murky.  Didn't Or add
>> >some from of PCIe-side looped queue to forward between PFs?
>>
>> I have no clue. Ccing Or. Or?
>>
>> >Presumably DSA would lean the opposite way with multiple ASICs
>> >reporting the same ID?
>>
>> Yes, sounds right.
>
>Right, so to me switch ID really is a forwarding attribute.

Re: [patch net-next 00/12] net: expose switch ID via devlink

[Jakub Kicinski]

On Sat, 30 Mar 2019 08:35:27 +0100, Jiri Pirko wrote:
>> Is switchib doing forwarding?  Not long ago Parav was convincing us
>> that switchdev mode for IB is pretty much meaningless.  Even though
>> there are apparently representors for IB (judging by the recent RDMA
>> patchset on netdev)...  
> 
> Switch id is simply identifier for ports in general that belong to some
> switch. No matter if ethernet or IB.

IIUC functionally there are two uses for switch ID:
 (1) bridging code and flower offloads making forwarding decisions;
 (2) naming the netdevs of the same switch with udev rules (sw$id$port).
Meta use:
 - logically grouping netdevs so that user space knows how (1) and (2)
   will behave.

Devlink ports are already grouped under the devlink instance, and they
not involved in forwarding decisions.

We should add functionality when we have evidence it's going to be
useful.  I don't see how devlink needs switch id, in any way.  Sure,
the internal code reshuffle is a matter of taste, but for exposing
switch id on devlink ports, I see _no_ reason.

I think this makes my position clear :)  I don't really enjoy
disagreeing with people, and neither do I care about this particularly
strongly, so please consider what I said, and if that doesn't convince
you feel free to add my review tag on the nfp patches :)

Re: [patch net-next 00/12] net: expose switch ID via devlink

[Jiri Pirko]

Sat, Mar 30, 2019 at 08:49:23PM CET, jakub.kicinski@netronome.com wrote:
>On Sat, 30 Mar 2019 08:35:27 +0100, Jiri Pirko wrote:
>>> Is switchib doing forwarding?  Not long ago Parav was convincing us
>>> that switchdev mode for IB is pretty much meaningless.  Even though
>>> there are apparently representors for IB (judging by the recent RDMA
>>> patchset on netdev)...  
>> 
>> Switch id is simply identifier for ports in general that belong to some
>> switch. No matter if ethernet or IB.
>
>IIUC functionally there are two uses for switch ID:
> (1) bridging code and flower offloads making forwarding decisions;
> (2) naming the netdevs of the same switch with udev rules (sw$id$port).
>Meta use:
> - logically grouping netdevs so that user space knows how (1) and (2)
>   will behave.
>
>Devlink ports are already grouped under the devlink instance, and they
>not involved in forwarding decisions.
>
>We should add functionality when we have evidence it's going to be
>useful.  I don't see how devlink needs switch id, in any way.  Sure,
>the internal code reshuffle is a matter of taste, but for exposing
>switch id on devlink ports, I see _no_ reason.
>
>I think this makes my position clear :)  I don't really enjoy
>disagreeing with people, and neither do I care about this particularly
>strongly, so please consider what I said, and if that doesn't convince
>you feel free to add my review tag on the nfp patches :)

I understand that from the functionality point of view, you are correct.
But for the visibility and better understanding about what's the
topology, I think it would be fine to expose this. Anyway, I'll remove
the patch to expose the devlink ATTR for now. Let's see how things are
going to look after we do the rest.

Re: [patch net-next 00/12] net: expose switch ID via devlink

[Jakub Kicinski]

On Sun, 31 Mar 2019 10:50:17 +0200, Jiri Pirko wrote:
> I understand that from the functionality point of view, you are correct.
> But for the visibility and better understanding about what's the
> topology, I think it would be fine to expose this. Anyway, I'll remove
> the patch to expose the devlink ATTR for now. Let's see how things are
> going to look after we do the rest.

Thanks!

Re: [patch net-next 11/12] dsa: pass switch ID through devlink_port_attrs_set()

[Jiri Pirko]

Fri, Mar 29, 2019 at 10:59:54PM CET, f.fainelli@gmail.com wrote:
>On 3/28/19 2:12 PM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Pass the switch ID down the to devlink through devlink_port_attrs_set()
>> so it can be used by devlink_compat_switch_id_get(). Leave
>> ndo_get_port_parent_id implementation only for legacy.
>
>Nit you are passing the switch fabric id (dst->index), the switch id is
>in ds->index.

I'm doing the same as dsa_slave_get_port_parent_id() does. If it is
not correct, please fix it there.


>
>> 
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>>  net/dsa/dsa2.c  | 4 +++-
>>  net/dsa/slave.c | 1 -
>>  2 files changed, 3 insertions(+), 2 deletions(-)
>> 
>> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
>> index 514be0583642..410cb0da9def 100644
>> --- a/net/dsa/dsa2.c
>> +++ b/net/dsa/dsa2.c
>> @@ -260,6 +260,7 @@ static int dsa_port_setup(struct dsa_port *dp)
>>  {
>>  	enum devlink_port_flavour flavour;
>>  	struct dsa_switch *ds = dp->ds;
>> +	struct dsa_switch_tree *dst = ds->dst;
>>  	int err;
>>  
>>  	if (dp->type == DSA_PORT_TYPE_UNUSED)
>> @@ -285,7 +286,8 @@ static int dsa_port_setup(struct dsa_port *dp)
>>  	 * independent from front panel port numbers.
>>  	 */
>>  	devlink_port_attrs_set(&dp->devlink_port, flavour,
>> -			       dp->index, false, 0, NULL, 0);
>> +			       dp->index, false, 0,
>> +			       (const char *) &dst->index, sizeof(dst->index));
>>  	err = devlink_port_register(ds->devlink, &dp->devlink_port,
>>  				    dp->index);
>>  	if (err)
>> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
>> index 80be8e86c82d..026a4003d520 100644
>> --- a/net/dsa/slave.c
>> +++ b/net/dsa/slave.c
>> @@ -1130,7 +1130,6 @@ static const struct net_device_ops dsa_slave_netdev_ops = {
>>  	.ndo_get_phys_port_name	= dsa_slave_get_phys_port_name,
>>  	.ndo_setup_tc		= dsa_slave_setup_tc,
>>  	.ndo_get_stats64	= dsa_slave_get_stats64,
>> -	.ndo_get_port_parent_id	= dsa_slave_get_port_parent_id,
>>  	.ndo_vlan_rx_add_vid	= dsa_slave_vlan_rx_add_vid,
>>  	.ndo_vlan_rx_kill_vid	= dsa_slave_vlan_rx_kill_vid,
>>  	.ndo_get_devlink_port	= dsa_slave_get_devlink_port,
>> 
>
>
>-- 
>Florian