[patch net-next RFC 0/8] net: expose switch ID via devlink

[patch net-next RFC 0/8] net: expose switch ID via devlink

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

To provide visibility of the ports, as discussed this week, 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.

Note that I send this as RFC because I don't have the NFP bits correct.
The switch ID should be only set for the eswitch ports (representors).
Jakub, could you please help with this?

As a follow-up I also plan to add devlink_port_attrs_set to all users
of ndo_get_port_parent_id and ndo_get_phys_port_name and remove these
ndos completely. But that also depends on Jakub's patchset which
introduces PF and VF port flavours.

Jiri Pirko (8):
  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
  nfp: pass switch ID through devlink_port_attrs_set()
  nfp: register devlink port before netdev
  nfp: remove ndo_get_port_parent_id implementation

 drivers/net/ethernet/mellanox/mlxsw/core.c         |  7 ++-
 drivers/net/ethernet/mellanox/mlxsw/core.h         |  4 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     | 17 ++-----
 drivers/net/ethernet/mellanox/mlxsw/switchx2.c     | 16 +------
 drivers/net/ethernet/netronome/nfp/nfp_devlink.c   |  6 ++-
 .../net/ethernet/netronome/nfp/nfp_net_common.c    |  1 -
 drivers/net/ethernet/netronome/nfp/nfp_net_main.c  | 27 ++++++-----
 drivers/net/ethernet/netronome/nfp/nfp_net_repr.c  |  1 -
 drivers/net/ethernet/netronome/nfp/nfp_port.c      | 16 -------
 include/net/devlink.h                              | 23 +++++++--
 include/uapi/linux/devlink.h                       |  2 +
 net/core/dev.c                                     |  8 +++-
 net/core/devlink.c                                 | 54 +++++++++++++++++++---
 net/dsa/dsa2.c                                     |  6 +--
 14 files changed, 109 insertions(+), 79 deletions(-)

-- 
2.14.5

[patch net-next RFC 1/8] 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>
---
 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 227c453cc20e..084a8762f3e9 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -40,10 +40,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.14.5

Re: [patch net-next RFC 1/8] net: devlink: convert devlink_port_attrs bools to bits

[Florian Fainelli]

On 3/1/2019 8:05 AM, Jiri Pirko wrote:
> 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>
-- 
Florian

[patch net-next RFC 2/8] 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>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c       |  3 ++-
 drivers/net/ethernet/netronome/nfp/nfp_devlink.c |  2 +-
 include/net/devlink.h                            | 12 +++++++---
 include/uapi/linux/devlink.h                     |  2 ++
 net/core/devlink.c                               | 29 +++++++++++++++++++-----
 net/dsa/dsa2.c                                   |  6 ++---
 6 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index d10108e44438..a2fecfdec2a1 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1756,7 +1756,8 @@ void mlxsw_core_port_eth_set(struct mlxsw_core *mlxsw_core, u8 local_port,
 
 	mlxsw_core_port->port_driver_priv = port_driver_priv;
 	devlink_port_attrs_set(devlink_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
-			       port_number, split, split_port_subnumber);
+			       port_number, split, split_port_subnumber,
+			       NULL, 0);
 	devlink_port_type_eth_set(devlink_port, dev);
 }
 EXPORT_SYMBOL(mlxsw_core_port_eth_set);
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index f24a7fc11b9f..b042dd9b2200 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -365,7 +365,7 @@ int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port)
 	devlink_port_type_eth_set(&port->dl_port, port->netdev);
 	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 084a8762f3e9..1eebbb832cb3 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -41,10 +41,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 {
@@ -577,7 +579,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,
@@ -788,7 +792,9 @@ static inline void devlink_port_type_clear(struct devlink_port *devlink_port)
 static inline 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)
 {
 }
 
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 d90a745d8258..1a011431694a 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -518,12 +518,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;
 }
@@ -5402,11 +5405,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;
 
@@ -5415,6 +5423,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;
+	}
 	devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
 }
 EXPORT_SYMBOL_GPL(devlink_port_attrs_set);
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 8c431e0f3627..9df9cb55ce9d 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -278,7 +278,7 @@ static int dsa_port_setup(struct dsa_port *dp)
 		 */
 		devlink_port_attrs_set(&dp->devlink_port,
 				       DEVLINK_PORT_FLAVOUR_CPU,
-				       dp->index, false, 0);
+				       dp->index, false, 0, NULL, 0);
 		err = dsa_port_link_register_of(dp);
 		if (err) {
 			dev_err(ds->dev, "failed to setup link for port %d.%d\n",
@@ -293,7 +293,7 @@ static int dsa_port_setup(struct dsa_port *dp)
 		 */
 		devlink_port_attrs_set(&dp->devlink_port,
 				       DEVLINK_PORT_FLAVOUR_DSA,
-				       dp->index, false, 0);
+				       dp->index, false, 0, NULL, 0);
 		err = dsa_port_link_register_of(dp);
 		if (err) {
 			dev_err(ds->dev, "failed to setup link for port %d.%d\n",
@@ -304,7 +304,7 @@ static int dsa_port_setup(struct dsa_port *dp)
 	case DSA_PORT_TYPE_USER:
 		devlink_port_attrs_set(&dp->devlink_port,
 				       DEVLINK_PORT_FLAVOUR_PHYSICAL,
-				       dp->index, false, 0);
+				       dp->index, false, 0, NULL, 0);
 		err = dsa_slave_create(dp);
 		if (err)
 			dev_err(ds->dev, "failed to create slave for port %d.%d\n",
-- 
2.14.5

Re: [patch net-next RFC 2/8] net: devlink: extend port attrs for switch ID

[Florian Fainelli]

On 3/1/2019 8:05 AM, Jiri Pirko wrote:
> 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>
> ---

[snip]

> @@ -5415,6 +5423,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;

Would not switch_id.id_len != 0 be enough of an indicator that this is a
switch port?
-- 
Florian

Re: [patch net-next RFC 2/8] net: devlink: extend port attrs for switch ID

[Jiri Pirko]

Sat, Mar 02, 2019 at 03:53:47AM CET, f.fainelli@gmail.com wrote:
>
>
>On 3/1/2019 8:05 AM, Jiri Pirko wrote:
>> 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>
>> ---
>
>[snip]
>
>> @@ -5415,6 +5423,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;
>
>Would not switch_id.id_len != 0 be enough of an indicator that this is a
>switch port?

Might be, yes. On the other hand. attrs->switch_port is just one bit..

>-- 
>Florian

[patch net-next RFC 3/8] 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>
---
 include/net/devlink.h |  9 +++++++++
 net/core/dev.c        |  8 ++++++--
 net/core/devlink.c    | 25 +++++++++++++++++++++++++
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 1eebbb832cb3..34c78485b050 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -733,6 +733,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
 
@@ -1230,6 +1232,13 @@ devlink_compat_phys_port_name_get(struct net_device *dev,
 {
 	return -EOPNOTSUPP;
 }
+
+static inline int
+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 a614b26b0842..7401b45738f2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7894,14 +7894,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 1a011431694a..0964ee319af5 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6505,6 +6505,31 @@ int devlink_compat_phys_port_name_get(struct net_device *dev,
 	return err;
 }
 
+int devlink_compat_switch_id_get(struct net_device *dev,
+				 struct netdev_phys_item_id *ppid)
+{
+	struct devlink_port *devlink_port;
+	struct devlink_port_attrs *attrs;
+	int err = -EOPNOTSUPP;
+
+	mutex_lock(&devlink_mutex);
+	devlink_port = netdev_to_devlink_port(dev);
+	if (!devlink_port)
+		goto unlock_list;
+
+	mutex_lock(&devlink_port->devlink->lock);
+	attrs = &devlink_port->attrs;
+	if (attrs->switch_port) {
+		memcpy(ppid, &attrs->switch_id, sizeof(*ppid));
+		err = 0;
+	}
+	mutex_unlock(&devlink_port->devlink->lock);
+unlock_list:
+	mutex_unlock(&devlink_mutex);
+
+	return err;
+}
+
 static int __init devlink_init(void)
 {
 	return genl_register_family(&devlink_nl_family);
-- 
2.14.5

Re: [patch net-next RFC 3/8] net: devlink: introduce devlink_compat_switch_id_get() helper

[Jakub Kicinski]

On Fri,  1 Mar 2019 17:05:37 +0100, Jiri Pirko wrote:
> 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>

Here my locking concerns are even stronger :S  Do you want to introduce
some extra ordering rules regarding lifetime of devlink ports and their
netdevs?

Re: [patch net-next RFC 3/8] net: devlink: introduce devlink_compat_switch_id_get() helper

[Jiri Pirko]

Fri, Mar 01, 2019 at 05:45:15PM CET, jakub.kicinski@netronome.com wrote:
>On Fri,  1 Mar 2019 17:05:37 +0100, Jiri Pirko wrote:
>> 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>
>
>Here my locking concerns are even stronger :S  Do you want to introduce
>some extra ordering rules regarding lifetime of devlink ports and their
>netdevs?

Working on it.

[patch net-next RFC 4/8] 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/spectrum.c | 4 +++-
 drivers/net/ethernet/mellanox/mlxsw/switchx2.c | 3 ++-
 4 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index a2fecfdec2a1..b770fa711c40 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1748,7 +1748,9 @@ EXPORT_SYMBOL(mlxsw_core_port_fini);
 void mlxsw_core_port_eth_set(struct mlxsw_core *mlxsw_core, u8 local_port,
 			     void *port_driver_priv, struct net_device *dev,
 			     u32 port_number, bool split,
-			     u32 split_port_subnumber)
+			     u32 split_port_subnumber,
+			     const unsigned char *switch_id,
+			     unsigned char switch_id_len)
 {
 	struct mlxsw_core_port *mlxsw_core_port =
 					&mlxsw_core->ports[local_port];
@@ -1757,7 +1759,7 @@ void mlxsw_core_port_eth_set(struct mlxsw_core *mlxsw_core, u8 local_port,
 	mlxsw_core_port->port_driver_priv = port_driver_priv;
 	devlink_port_attrs_set(devlink_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
 			       port_number, split, split_port_subnumber,
-			       NULL, 0);
+			       switch_id, switch_id_len);
 	devlink_port_type_eth_set(devlink_port, dev);
 }
 EXPORT_SYMBOL(mlxsw_core_port_eth_set);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index 60b017a07cea..af28b6e894f4 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -169,7 +169,9 @@ 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,
 			     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_ib_set(struct mlxsw_core *mlxsw_core, u8 local_port,
 			    void *port_driver_priv);
 void mlxsw_core_port_clear(struct mlxsw_core *mlxsw_core, u8 local_port,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 5d0adec84858..75c5b8f65912 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3670,7 +3670,9 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
 
 	mlxsw_core_port_eth_set(mlxsw_sp->core, mlxsw_sp_port->local_port,
 				mlxsw_sp_port, dev, module + 1,
-				mlxsw_sp_port->split, lane / width);
+				mlxsw_sp_port->split, lane / width,
+				mlxsw_sp->base_mac,
+				sizeof(mlxsw_sp->base_mac));
 	mlxsw_core_schedule_dw(&mlxsw_sp_port->periodic_hw_stats.update_dw, 0);
 	return 0;
 
diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
index 9999d866f394..e6c7bf78f245 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
@@ -1091,7 +1091,8 @@ static int __mlxsw_sx_port_eth_create(struct mlxsw_sx *mlxsw_sx, u8 local_port,
 	}
 
 	mlxsw_core_port_eth_set(mlxsw_sx->core, mlxsw_sx_port->local_port,
-				mlxsw_sx_port, dev, module + 1, false, 0);
+				mlxsw_sx_port, dev, module + 1, false, 0,
+				mlxsw_sx->hw_id, sizeof(mlxsw_sx->hw_id));
 	mlxsw_sx->ports[local_port] = mlxsw_sx_port;
 	return 0;
 
-- 
2.14.5

[patch net-next RFC 5/8] 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/spectrum.c | 13 -------------
 drivers/net/ethernet/mellanox/mlxsw/switchx2.c | 13 -------------
 2 files changed, 26 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 75c5b8f65912..f277c0eb6fc8 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -1703,18 +1703,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)
 {
@@ -1739,7 +1727,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 e6c7bf78f245..db54335032b4 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
@@ -379,25 +379,12 @@ 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 const struct net_device_ops mlxsw_sx_port_netdev_ops = {
 	.ndo_open		= mlxsw_sx_port_open,
 	.ndo_stop		= mlxsw_sx_port_stop,
 	.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,
 };
 
 static void mlxsw_sx_port_get_drvinfo(struct net_device *dev,
-- 
2.14.5

Re: [patch net-next RFC 5/8] mlxsw: Remove ndo_get_port_parent_id implementation

[Florian Fainelli]

On 3/1/2019 8:05 AM, Jiri Pirko wrote:
> 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>

Don't you need a select/depends on DEVLINK for MLXSW_SWITCHX2 and
MLXSW_SPECTRUM otherwise you are going to get unexpected results from
net/bridge/br_switchdev.c while computing the offload_fwd_mark?
-- 
Florian

Re: [patch net-next RFC 5/8] mlxsw: Remove ndo_get_port_parent_id implementation

[Jiri Pirko]

Sat, Mar 02, 2019 at 03:57:07AM CET, f.fainelli@gmail.com wrote:
>
>
>On 3/1/2019 8:05 AM, Jiri Pirko wrote:
>> 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>
>
>Don't you need a select/depends on DEVLINK for MLXSW_SWITCHX2 and
>MLXSW_SPECTRUM otherwise you are going to get unexpected results from
>net/bridge/br_switchdev.c while computing the offload_fwd_mark?

You are probably right. That is what we loose after move to
devlink-buildin. I think that every driver using devlink should enforce
devlink=y

>-- 
>Florian

[patch net-next RFC 6/8] 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 | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index b042dd9b2200..e7c083512a61 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();
@@ -363,9 +365,11 @@ int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port)
 		return ret;
 
 	devlink_port_type_eth_set(&port->dl_port, port->netdev);
+
+	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.14.5

[patch net-next RFC 7/8] nfp: register devlink port before netdev

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Change the init/fini flow and register devlink port instance before
netdev. Now it is needed for correct behavior of switch/parent id
reporting, but in general it makes sense to register devlink port first.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net_main.c | 27 +++++++++++------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
index 08f5fdbd8e41..6ec680482b37 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
@@ -150,34 +150,33 @@ nfp_net_pf_init_vnic(struct nfp_pf *pf, struct nfp_net *nn, unsigned int id)
 
 	nn->id = id;
 
-	err = nfp_net_init(nn);
-	if (err)
-		return err;
-
-	nfp_net_debugfs_vnic_add(nn, pf->ddir);
-
 	if (nn->port) {
 		err = nfp_devlink_port_register(pf->app, nn->port);
 		if (err)
-			goto err_dfs_clean;
+			return err;
 	}
 
-	nfp_net_info(nn);
-
 	if (nfp_net_is_data_vnic(nn)) {
 		err = nfp_app_vnic_init(pf->app, nn);
 		if (err)
 			goto err_devlink_port_clean;
 	}
 
+	err = nfp_net_init(nn);
+	if (err)
+		goto err_app_vnic_free;
+
+	nfp_net_debugfs_vnic_add(nn, pf->ddir);
+	nfp_net_info(nn);
+
 	return 0;
 
+err_app_vnic_free:
+	if (nfp_net_is_data_vnic(nn))
+		nfp_app_vnic_clean(pf->app, nn);
 err_devlink_port_clean:
 	if (nn->port)
 		nfp_devlink_port_unregister(nn->port);
-err_dfs_clean:
-	nfp_net_debugfs_dir_clean(&nn->debugfs_dir);
-	nfp_net_clean(nn);
 	return err;
 }
 
@@ -218,12 +217,12 @@ nfp_net_pf_alloc_vnics(struct nfp_pf *pf, void __iomem *ctrl_bar,
 
 static void nfp_net_pf_clean_vnic(struct nfp_pf *pf, struct nfp_net *nn)
 {
+	nfp_net_debugfs_dir_clean(&nn->debugfs_dir);
+	nfp_net_clean(nn);
 	if (nfp_net_is_data_vnic(nn))
 		nfp_app_vnic_clean(pf->app, nn);
 	if (nn->port)
 		nfp_devlink_port_unregister(nn->port);
-	nfp_net_debugfs_dir_clean(&nn->debugfs_dir);
-	nfp_net_clean(nn);
 }
 
 static int nfp_net_pf_alloc_irqs(struct nfp_pf *pf)
-- 
2.14.5

[patch net-next RFC 8/8] 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>
---
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c |  1 -
 drivers/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 7dd6b3fbb394..a75b19f2534f 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3530,7 +3530,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.14.5

Re: [patch net-next RFC 8/8] nfp: remove ndo_get_port_parent_id implementation

[Florian Fainelli]

On 3/1/2019 8:05 AM, Jiri Pirko wrote:
> 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.

Likewise, this probably needs a depends/select DEVLINK now to make sure
the offload_fwd_mark is correctly calculated.
-- 
Florian

Re: [patch net-next RFC 0/8] net: expose switch ID via devlink

[Florian Fainelli]

On 3/1/2019 8:05 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> To provide visibility of the ports, as discussed this week, 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.
> 
> Note that I send this as RFC because I don't have the NFP bits correct.
> The switch ID should be only set for the eswitch ports (representors).
> Jakub, could you please help with this?
> 
> As a follow-up I also plan to add devlink_port_attrs_set to all users
> of ndo_get_port_parent_id and ndo_get_phys_port_name and remove these
> ndos completely. But that also depends on Jakub's patchset which
> introduces PF and VF port flavours.

Some comments in specific patches, but overall this looks great to me,
thanks Jiri!

> 
> Jiri Pirko (8):
>   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
>   nfp: pass switch ID through devlink_port_attrs_set()
>   nfp: register devlink port before netdev
>   nfp: remove ndo_get_port_parent_id implementation
> 
>  drivers/net/ethernet/mellanox/mlxsw/core.c         |  7 ++-
>  drivers/net/ethernet/mellanox/mlxsw/core.h         |  4 +-
>  drivers/net/ethernet/mellanox/mlxsw/spectrum.c     | 17 ++-----
>  drivers/net/ethernet/mellanox/mlxsw/switchx2.c     | 16 +------
>  drivers/net/ethernet/netronome/nfp/nfp_devlink.c   |  6 ++-
>  .../net/ethernet/netronome/nfp/nfp_net_common.c    |  1 -
>  drivers/net/ethernet/netronome/nfp/nfp_net_main.c  | 27 ++++++-----
>  drivers/net/ethernet/netronome/nfp/nfp_net_repr.c  |  1 -
>  drivers/net/ethernet/netronome/nfp/nfp_port.c      | 16 -------
>  include/net/devlink.h                              | 23 +++++++--
>  include/uapi/linux/devlink.h                       |  2 +
>  net/core/dev.c                                     |  8 +++-
>  net/core/devlink.c                                 | 54 +++++++++++++++++++---
>  net/dsa/dsa2.c                                     |  6 +--
>  14 files changed, 109 insertions(+), 79 deletions(-)
> 

-- 
Florian