diff mbox series

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

Message ID 20190301160542.6474-3-jiri@resnulli.us
State RFC
Delegated to: David Miller
Headers show
Series net: expose switch ID via devlink | expand

Commit Message

Jiri Pirko March 1, 2019, 4:05 p.m. UTC 
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(-)

Comments

Florian Fainelli March 2, 2019, 2:53 a.m. UTC | #1 
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?
Jiri Pirko March 2, 2019, 7:34 a.m. UTC | #2 
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
diff mbox series

Patch

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",