[PATCH net-next 0/3] devlink: Add port width attribute

[PATCH net-next 0/3] devlink: Add port width attribute

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

Danielle says:

Currently, user space has no way of knowing if a port can be split and
into how many ports. This makes it impossible to write generic tests for
port split.

This patch set adds the port's width as an attribute of a devlink port
and exposes the information to user space using a new attribute.

Patch #1 prepares mlxsw to pass width information to devlink
Patch #2 changes device drivers to pass width information to devlink and
exposes it to user space
Patch #3 adds a port split test

Danielle Ratson (3):
  mlxsw: Set port width attribute in driver
  devlink: Add a new devlink port width attribute and pass to netlink
  selftests: net: Add port split test

 .../net/ethernet/broadcom/bnxt/bnxt_devlink.c |   2 +-
 drivers/net/ethernet/intel/ice/ice_devlink.c  |   2 +-
 .../ethernet/mellanox/mlx5/core/en/devlink.c  |   4 +-
 .../net/ethernet/mellanox/mlx5/core/en_rep.c  |   2 +-
 drivers/net/ethernet/mellanox/mlxsw/core.c    |   8 +-
 drivers/net/ethernet/mellanox/mlxsw/core.h    |   1 +
 drivers/net/ethernet/mellanox/mlxsw/minimal.c |   2 +-
 .../net/ethernet/mellanox/mlxsw/spectrum.c    |   1 +
 .../net/ethernet/mellanox/mlxsw/switchib.c    |   2 +-
 .../net/ethernet/mellanox/mlxsw/switchx2.c    |   2 +-
 .../net/ethernet/netronome/nfp/nfp_devlink.c  |   2 +-
 .../ethernet/pensando/ionic/ionic_devlink.c   |   2 +-
 drivers/net/netdevsim/dev.c                   |   2 +-
 include/net/devlink.h                         |   2 +
 include/uapi/linux/devlink.h                  |   2 +
 net/core/devlink.c                            |   7 +
 net/dsa/dsa2.c                                |   6 +-
 tools/testing/selftests/net/Makefile          |   1 +
 .../selftests/net/devlink_port_split.py       | 259 ++++++++++++++++++
 19 files changed, 292 insertions(+), 17 deletions(-)
 create mode 100755 tools/testing/selftests/net/devlink_port_split.py

-- 
2.26.2

[PATCH net-next 1/3] mlxsw: Set port width attribute in driver

[Ido Schimmel]

From: Danielle Ratson <danieller@mellanox.com>

Currently, port attributes like flavour, port number and whether the
port was split are set when initializing a port.

Set the width of the port as well so that it could be easily passed to
devlink in the next patch.

Signed-off-by: Danielle Ratson <danieller@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c     | 6 ++++--
 drivers/net/ethernet/mellanox/mlxsw/core.h     | 1 +
 drivers/net/ethernet/mellanox/mlxsw/minimal.c  | 2 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 1 +
 drivers/net/ethernet/mellanox/mlxsw/switchib.c | 2 +-
 drivers/net/ethernet/mellanox/mlxsw/switchx2.c | 2 +-
 6 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index e9ccd333f61d..8f1ef90c7f5a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -2122,6 +2122,7 @@ static int __mlxsw_core_port_init(struct mlxsw_core *mlxsw_core, u8 local_port,
 				  enum devlink_port_flavour flavour,
 				  u32 port_number, bool split,
 				  u32 split_port_subnumber,
+				  u32 width,
 				  const unsigned char *switch_id,
 				  unsigned char switch_id_len)
 {
@@ -2154,13 +2155,14 @@ static void __mlxsw_core_port_fini(struct mlxsw_core *mlxsw_core, u8 local_port)
 int mlxsw_core_port_init(struct mlxsw_core *mlxsw_core, u8 local_port,
 			 u32 port_number, bool split,
 			 u32 split_port_subnumber,
+			 u32 width,
 			 const unsigned char *switch_id,
 			 unsigned char switch_id_len)
 {
 	return __mlxsw_core_port_init(mlxsw_core, local_port,
 				      DEVLINK_PORT_FLAVOUR_PHYSICAL,
 				      port_number, split, split_port_subnumber,
-				      switch_id, switch_id_len);
+				      width, switch_id, switch_id_len);
 }
 EXPORT_SYMBOL(mlxsw_core_port_init);
 
@@ -2181,7 +2183,7 @@ int mlxsw_core_cpu_port_init(struct mlxsw_core *mlxsw_core,
 
 	err = __mlxsw_core_port_init(mlxsw_core, MLXSW_PORT_CPU_PORT,
 				     DEVLINK_PORT_FLAVOUR_CPU,
-				     0, false, 0,
+				     0, false, 0, 0,
 				     switch_id, switch_id_len);
 	if (err)
 		return err;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index 22b0dfa7cfae..28f7b1c156b0 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -193,6 +193,7 @@ 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 width,
 			 const unsigned char *switch_id,
 			 unsigned char switch_id_len);
 void mlxsw_core_port_fini(struct mlxsw_core *mlxsw_core, u8 local_port);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/minimal.c b/drivers/net/ethernet/mellanox/mlxsw/minimal.c
index c4caeeadcba9..f9c9d7baf3be 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/minimal.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/minimal.c
@@ -164,7 +164,7 @@ 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, 0,
 				   mlxsw_m->base_mac,
 				   sizeof(mlxsw_m->base_mac));
 	if (err) {
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index f78bde8bc16e..52e1c8263963 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3250,6 +3250,7 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
 	err = mlxsw_core_port_init(mlxsw_sp->core, local_port,
 				   port_mapping->module + 1, split,
 				   port_mapping->lane / port_mapping->width,
+				   port_mapping->width,
 				   mlxsw_sp->base_mac,
 				   sizeof(mlxsw_sp->base_mac));
 	if (err) {
diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchib.c b/drivers/net/ethernet/mellanox/mlxsw/switchib.c
index 4ff1e623aa76..1b446e6071b2 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/switchib.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/switchib.c
@@ -281,7 +281,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, 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",
diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
index 90535820b559..5ec161409954 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
@@ -1107,7 +1107,7 @@ 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, 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",
-- 
2.26.2

[PATCH net-next 2/3] devlink: Add a new devlink port width attribute and pass to netlink

[Ido Schimmel]

From: Danielle Ratson <danieller@mellanox.com>

Add a new devlink port attribute that indicates the port's width.
Drivers are expected to set it via devlink_port_attrs_set(), before
registering the port.

The attribute is not passed to user space in case the width is invalid
(0).

Signed-off-by: Danielle Ratson <danieller@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c    | 2 +-
 drivers/net/ethernet/intel/ice/ice_devlink.c         | 2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c | 4 ++--
 drivers/net/ethernet/mellanox/mlx5/core/en_rep.c     | 2 +-
 drivers/net/ethernet/mellanox/mlxsw/core.c           | 2 +-
 drivers/net/ethernet/netronome/nfp/nfp_devlink.c     | 2 +-
 drivers/net/ethernet/pensando/ionic/ionic_devlink.c  | 2 +-
 drivers/net/netdevsim/dev.c                          | 2 +-
 include/net/devlink.h                                | 2 ++
 include/uapi/linux/devlink.h                         | 2 ++
 net/core/devlink.c                                   | 7 +++++++
 net/dsa/dsa2.c                                       | 6 +++---
 12 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index a812beb46325..25d577433dbf 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -714,7 +714,7 @@ int bnxt_dl_register(struct bnxt *bp)
 		return 0;
 
 	devlink_port_attrs_set(&bp->dl_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
-			       bp->pf.port_id, false, 0, bp->dsn,
+			       bp->pf.port_id, false, 0, 0, bp->dsn,
 			       sizeof(bp->dsn));
 	rc = devlink_port_register(dl, &bp->dl_port, bp->pf.port_id);
 	if (rc) {
diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
index c6833944b90a..a46ebeb249b8 100644
--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
@@ -297,7 +297,7 @@ int ice_devlink_create_port(struct ice_pf *pf)
 	}
 
 	devlink_port_attrs_set(&pf->devlink_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
-			       pf->hw.pf_id, false, 0, NULL, 0);
+			       pf->hw.pf_id, false, 0, 0, NULL, 0);
 	err = devlink_port_register(devlink, &pf->devlink_port, pf->hw.pf_id);
 	if (err) {
 		dev_err(dev, "devlink_port_register failed: %d\n", err);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c
index f8b2de4b04be..365f2df6d851 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c
@@ -11,12 +11,12 @@ int mlx5e_devlink_port_register(struct mlx5e_priv *priv)
 		devlink_port_attrs_set(&priv->dl_port,
 				       DEVLINK_PORT_FLAVOUR_PHYSICAL,
 				       PCI_FUNC(priv->mdev->pdev->devfn),
-				       false, 0,
+				       false, 0, 0,
 				       NULL, 0);
 	else
 		devlink_port_attrs_set(&priv->dl_port,
 				       DEVLINK_PORT_FLAVOUR_VIRTUAL,
-				       0, false, 0, NULL, 0);
+				       0, false, 0, 0, NULL, 0);
 
 	return devlink_port_register(devlink, &priv->dl_port, 1);
 }
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index 52351c105627..cf54c88a90d1 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -2050,7 +2050,7 @@ static int register_devlink_port(struct mlx5_core_dev *dev,
 	if (rep->vport == MLX5_VPORT_UPLINK)
 		devlink_port_attrs_set(&rpriv->dl_port,
 				       DEVLINK_PORT_FLAVOUR_PHYSICAL,
-				       pfnum, false, 0,
+				       pfnum, false, 0, 0,
 				       &ppid.id[0], ppid.id_len);
 	else if (rep->vport == MLX5_VPORT_PF)
 		devlink_port_attrs_pci_pf_set(&rpriv->dl_port,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 8f1ef90c7f5a..df011c1d0712 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -2134,7 +2134,7 @@ static 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, flavour, port_number,
-			       split, split_port_subnumber,
+			       split, split_port_subnumber, width,
 			       switch_id, switch_id_len);
 	err = devlink_port_register(devlink, devlink_port, local_port);
 	if (err)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index 07dbf4d72227..65ecd0bdc8be 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -368,7 +368,7 @@ int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port)
 	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, serial, serial_len);
+			       eth_port.label_subport, 0, serial, serial_len);
 
 	devlink = priv_to_devlink(app->pf);
 
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
index 273c889faaad..a21a10307ecc 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
@@ -82,7 +82,7 @@ int ionic_devlink_register(struct ionic *ionic)
 		return 0;
 
 	devlink_port_attrs_set(&ionic->dl_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
-			       0, false, 0, NULL, 0);
+			       0, false, 0, 0, NULL, 0);
 	err = devlink_port_register(dl, &ionic->dl_port, 0);
 	if (err)
 		dev_err(ionic->dev, "devlink_port_register failed: %d\n", err);
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 68668a22b9dd..75549640d113 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -893,7 +893,7 @@ static int __nsim_dev_port_add(struct nsim_dev *nsim_dev,
 
 	devlink_port = &nsim_dev_port->devlink_port;
 	devlink_port_attrs_set(devlink_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
-			       port_index + 1, 0, 0,
+			       port_index + 1, 0, 0, 0,
 			       nsim_dev->switch_id.id,
 			       nsim_dev->switch_id.id_len);
 	err = devlink_port_register(priv_to_devlink(nsim_dev), devlink_port,
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 8ffc1b5cd89b..de374d544671 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -68,6 +68,7 @@ struct devlink_port_attrs {
 	u8 set:1,
 	   split:1,
 	   switch_port:1;
+	u32 width;
 	enum devlink_port_flavour flavour;
 	struct netdev_phys_item_id switch_id;
 	union {
@@ -972,6 +973,7 @@ 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 width,
 			    const unsigned char *switch_id,
 			    unsigned char switch_id_len);
 void devlink_port_attrs_pci_pf_set(struct devlink_port *devlink_port,
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 1ae90e06c06d..69e914e000c4 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -442,6 +442,8 @@ enum devlink_attr {
 	DEVLINK_ATTR_TRAP_POLICER_RATE,			/* u64 */
 	DEVLINK_ATTR_TRAP_POLICER_BURST,		/* u64 */
 
+	DEVLINK_ATTR_PORT_WIDTH,		/* u32 */
+
 	/* 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 7b76e5fffc10..9887fba60a7a 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -526,6 +526,10 @@ static int devlink_nl_port_attrs_put(struct sk_buff *msg,
 
 	if (!attrs->set)
 		return 0;
+	if (attrs->width) {
+		if (nla_put_u32(msg, DEVLINK_ATTR_PORT_WIDTH, attrs->width))
+			return -EMSGSIZE;
+	}
 	if (nla_put_u16(msg, DEVLINK_ATTR_PORT_FLAVOUR, attrs->flavour))
 		return -EMSGSIZE;
 	switch (devlink_port->attrs.flavour) {
@@ -7408,6 +7412,7 @@ static int __devlink_port_attrs_set(struct devlink_port *devlink_port,
  *	@split: indicates if this is split port
  *	@split_subport_number: if the port is split, this is the number
  *	                       of subport.
+ *	@width: width of the port. 0 value is not passed to netlink.
  *	@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
@@ -7416,6 +7421,7 @@ 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 width,
 			    const unsigned char *switch_id,
 			    unsigned char switch_id_len)
 {
@@ -7427,6 +7433,7 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
 	if (ret)
 		return;
 	attrs->split = split;
+	attrs->width = width;
 	attrs->phys.port_number = port_number;
 	attrs->phys.split_subport_number = split_subport_number;
 }
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 076908fdd29b..5d9322cb5bf3 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -275,7 +275,7 @@ static int dsa_port_setup(struct dsa_port *dp)
 	case DSA_PORT_TYPE_CPU:
 		memset(dlp, 0, sizeof(*dlp));
 		devlink_port_attrs_set(dlp, DEVLINK_PORT_FLAVOUR_CPU,
-				       dp->index, false, 0, id, len);
+				       dp->index, false, 0, 0, id, len);
 		err = devlink_port_register(dl, dlp, dp->index);
 		if (err)
 			break;
@@ -295,7 +295,7 @@ static int dsa_port_setup(struct dsa_port *dp)
 	case DSA_PORT_TYPE_DSA:
 		memset(dlp, 0, sizeof(*dlp));
 		devlink_port_attrs_set(dlp, DEVLINK_PORT_FLAVOUR_DSA,
-				       dp->index, false, 0, id, len);
+				       dp->index, false, 0, 0, id, len);
 		err = devlink_port_register(dl, dlp, dp->index);
 		if (err)
 			break;
@@ -315,7 +315,7 @@ static int dsa_port_setup(struct dsa_port *dp)
 	case DSA_PORT_TYPE_USER:
 		memset(dlp, 0, sizeof(*dlp));
 		devlink_port_attrs_set(dlp, DEVLINK_PORT_FLAVOUR_PHYSICAL,
-				       dp->index, false, 0, id, len);
+				       dp->index, false, 0, 0, id, len);
 		err = devlink_port_register(dl, dlp, dp->index);
 		if (err)
 			break;
-- 
2.26.2

[PATCH net-next 3/3] selftests: net: Add port split test

[Ido Schimmel]

From: Danielle Ratson <danieller@mellanox.com>

Test port split configuration using previously added port's width
attribute.

Check that all the splittable ports are successfully split to their
width and below, and that those which are not splittable fail to be
split.

Test output example:

TEST: swp4 is unsplittable                                         [ OK ]
TEST: split port swp53 into 4                                      [ OK ]
TEST: Unsplit port pci/0000:03:00.0/25                             [ OK ]
TEST: split port swp53 into 2                                      [ OK ]
TEST: Unsplit port pci/0000:03:00.0/25                             [ OK ]

Signed-off-by: Danielle Ratson <danieller@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 tools/testing/selftests/net/Makefile          |   1 +
 .../selftests/net/devlink_port_split.py       | 259 ++++++++++++++++++
 2 files changed, 260 insertions(+)
 create mode 100755 tools/testing/selftests/net/devlink_port_split.py

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 895ec992b2f1..90fcf8ba9ed0 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -17,6 +17,7 @@ TEST_PROGS += route_localnet.sh
 TEST_PROGS += reuseaddr_ports_exhausted.sh
 TEST_PROGS += txtimestamp.sh
 TEST_PROGS += vrf-xfrm-tests.sh
+TEST_PROGS += devlink_port_split.py
 TEST_PROGS_EXTENDED := in_netns.sh
 TEST_GEN_FILES =  socket nettest
 TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy reuseport_addr_any
diff --git a/tools/testing/selftests/net/devlink_port_split.py b/tools/testing/selftests/net/devlink_port_split.py
new file mode 100755
index 000000000000..e5ce331df233
--- /dev/null
+++ b/tools/testing/selftests/net/devlink_port_split.py
@@ -0,0 +1,259 @@
+#!/usr/bin/python3
+# SPDX-License-Identifier: GPL-2.0
+
+from subprocess import PIPE, Popen
+import json
+import time
+import argparse
+import collections
+import sys
+
+#
+# Test port split configuration using devlink-port width attribute.
+# The test is skipped in case the attribute is not available.
+#
+# First, check that all the ports with a width of 1 fail to split.
+# Second, check that all the ports with a width larger than 1 can be split
+# to all valid configurations (e.g., split to 2, split to 4 etc.)
+#
+
+
+Port = collections.namedtuple('Port', 'bus_info name')
+
+
+def run_command(cmd, should_fail=False):
+    """
+    Run a command in subprocess.
+    Return: Tuple of (stdout, stderr).
+    """
+
+    p = Popen(cmd, stdout=PIPE, stderr=PIPE, shell=True)
+    stdout, stderr = p.communicate()
+    stdout, stderr = stdout.decode(), stderr.decode()
+
+    if stderr != "" and not should_fail:
+        print("Error sending command: %s" % cmd)
+        print(stdout)
+        print(stderr)
+    return stdout, stderr
+
+
+class devlink_ports(object):
+    """
+    Class that holds information on the devlink ports, required to the tests;
+    if_names: A list of interfaces in the devlink ports.
+    """
+
+    def get_if_names(dev):
+        """
+        Get a list of physical devlink ports.
+        Return: Array of tuples (bus_info/port, if_name).
+        """
+
+        arr = []
+
+        cmd = "devlink -j port show"
+        stdout, stderr = run_command(cmd)
+        assert stderr == ""
+        ports = json.loads(stdout)['port']
+
+        for port in ports:
+            if dev in port:
+                if ports[port]['flavour'] == 'physical':
+                    arr.append(Port(bus_info=port, name=ports[port]['netdev']))
+
+        return arr
+
+    def __init__(self, dev):
+        self.if_names = devlink_ports.get_if_names(dev)
+
+
+def get_width(port):
+    """
+    Get the width of $port.
+    Return: width, e.g. 1, 2, 4 and 8.
+    """
+
+    cmd = "devlink -j port show %s" % port
+    stdout, stderr = run_command(cmd)
+    assert stderr == ""
+    values = list(json.loads(stdout)['port'].values())[0]
+
+    if 'width' in values:
+        width = values['width']
+    else:
+        width = 0
+    return width
+
+
+def split(k, port, should_fail=False):
+    """
+    Split $port into $k ports.
+    If should_fail == True, the split should fail. Otherwise, should pass.
+    Return: Array of sub ports after splitting.
+            If the $port wasn't split, the array will be empty.
+    """
+
+    cmd = "devlink port split %s count %s" % (port.bus_info, k)
+    stdout, stderr = run_command(cmd, should_fail=should_fail)
+
+    if should_fail:
+        if not test(stderr != "", "%s is unsplittable" % port.name):
+            print("split an unsplittable port %s" % port.name)
+            return create_split_group(port, k)
+    else:
+        if stderr == "":
+            return create_split_group(port, k)
+        print("didn't split a splittable port %s" % port.name)
+
+    return []
+
+
+def unsplit(port):
+    """
+    Unsplit $port.
+    """
+
+    cmd = "devlink port unsplit %s" % port
+    stdout, stderr = run_command(cmd)
+    test(stderr == "", "Unsplit port %s" % port)
+
+
+def exists(port, dev):
+    """
+    Check if $port exists in the devlink ports.
+    Return: True is so, False otherwise.
+    """
+
+    return any(dev_port.name == port
+               for dev_port in devlink_ports.get_if_names(dev))
+
+
+def exists_and_width(ports, lanes, dev):
+    """
+    Check if every port in the list $ports exists in the devlink ports and has
+    $lanes number of lanes after splitting.
+    Return: True if both are True, False otherwise.
+    """
+
+    for port in ports:
+        width = get_width(port)
+        if not exists(port, dev):
+            print("port %s doesn't exist in devlink ports" % port)
+            return False
+        if width != lanes:
+            print("port %s has %d lanes, but %s were expected"
+                  % (port, lanes, width))
+            return False
+    return True
+
+
+def test(cond, msg):
+    """
+    Check $cond and print a message accordingly.
+    Return: True is pass, False otherwise.
+    """
+
+    if cond:
+        print("TEST: %-60s [ OK ]" % msg)
+    else:
+        print("TEST: %-60s [FAIL]" % msg)
+
+    return cond
+
+
+def create_split_group(port, k):
+    """
+    Create the split group for $port.
+    Return: Array with $k elements, which are the split port group.
+    """
+
+    return list(port.name + "s" + str(i) for i in range(k))
+
+
+def split_unsplittable_port(port, k):
+    """
+    Test that splitting of unsplittable port fails.
+    """
+
+    # split to max
+    new_split_group = split(k, port, should_fail=True)
+
+    if new_split_group != []:
+        unsplit(port.bus_info)
+
+
+def split_splittable_port(port, k, width, dev):
+    """
+    Test that splitting of splittable port passes correctly.
+    """
+
+    new_split_group = split(k, port)
+
+    # Once the split command ends, it takes some time to the sub ifaces'
+    # to get their names. Use udevadm to continue only when all current udev
+    # events are handled.
+    cmd = "udevadm settle"
+    stdout, stderr = run_command(cmd)
+    assert stderr == ""
+
+    if new_split_group != []:
+        test(exists_and_width(new_split_group, width/k, dev),
+             "split port %s into %s" % (port.name, k))
+
+    unsplit(port.bus_info)
+
+
+def make_parser():
+    parser = argparse.ArgumentParser(description='A test for port splitting.')
+    parser.add_argument('--dev',
+                        help='The devlink handle of the device under test. ' +
+                             'The default is the first registered devlink ' +
+                             'handle.')
+
+    return parser
+
+
+def main(cmdline=None):
+    parser = make_parser()
+    args = parser.parse_args(cmdline)
+
+    dev = args.dev
+    if not dev:
+        cmd = "devlink -j dev show"
+        stdout, stderr = run_command(cmd)
+        assert stderr == ""
+
+        devs = json.loads(stdout)['dev']
+        dev = list(devs.keys())[0]
+
+    cmd = "devlink dev show %s" % dev
+    stdout, stderr = run_command(cmd)
+    if stderr != "":
+        print("devlink device %s can not be found" % dev)
+        sys.exit(1)
+
+    ports = devlink_ports(dev)
+
+    for port in ports.if_names:
+        width = get_width(port.name)
+
+        # If width is 0, do not test port splitting at all
+        if width == 0:
+            continue
+
+        # If 1 lane, shouldn't be able to split
+        elif width == 1:
+            split_unsplittable_port(port, width)
+
+        # Else, splitting should pass and all the split ports should exist.
+        else:
+            lane = width
+            while lane > 1:
+                split_splittable_port(port, lane, width, dev)
+
+                lane //= 2
+
+
+if __name__ == "__main__":
+    main()
-- 
2.26.2

Re: [PATCH net-next 3/3] selftests: net: Add port split test

[Andrew Lunn]

> +# Test port split configuration using devlink-port width attribute.
> +# The test is skipped in case the attribute is not available.
> +#
> +# First, check that all the ports with a width of 1 fail to split.
> +# Second, check that all the ports with a width larger than 1 can be split
> +# to all valid configurations (e.g., split to 2, split to 4 etc.)

Hi Ido

I know very little about splitting ports. So these might be dumb
questions.

Is there a well defined meaning of width? Is it something which can be
found in an 802.3 standard?

Is it well defined that all splits of the for 2, 4, 8 have to be
supported? Must all 40Gbps ports with a width of 4, be splitable to 2x
20Mps? It seems like some hardware might only allow 4x 10G?

If 20Gbps is supported, can you then go recursive and split one of the
20G ports into 2x 10G, leaving the other as a 20G port?

	Andrew

Re: [PATCH net-next 3/3] selftests: net: Add port split test

[Ido Schimmel]

On Tue, May 19, 2020 at 04:15:41PM +0200, Andrew Lunn wrote:
> > +# Test port split configuration using devlink-port width attribute.
> > +# The test is skipped in case the attribute is not available.
> > +#
> > +# First, check that all the ports with a width of 1 fail to split.
> > +# Second, check that all the ports with a width larger than 1 can be split
> > +# to all valid configurations (e.g., split to 2, split to 4 etc.)
> 
> Hi Ido

Hi Andrew,

> 
> I know very little about splitting ports. So these might be dumb
> questions.
> 
> Is there a well defined meaning of width? Is it something which can be
> found in an 802.3 standard?

It's basically the number of lanes: If a port is a 100Gbps port and has
a width of 4, then every lane is running at 25Gbps. Splitting this port
to 4 (via 'devlink port split') allows you to get 4 ports each capable
of supporting speeds up to 25Gbps (the original netdev disappears).

Example splitter cable:
https://www.mellanox.com/related-docs/prod_cables/PB_MCP7F00-A0xxRyyz_100GbE_QSFP28_to_4x25GbE_4xSFP28_DAC_Splitter.pdf

Some documentation from mlxsw Wiki:
https://github.com/Mellanox/mlxsw/wiki/Switch-Port-Configuration#port-splitting

> Is it well defined that all splits of the for 2, 4, 8 have to be
> supported?

That I don't actually know. It is true for Mellanox and I can only
assume it holds for other vendors. So far beside mlxsw only nfp
implemented port_split() callback. I see it has this check:

```
        if (eth_port.is_split || eth_port.port_lanes % count) {
                ret = -EINVAL;
                goto out;
        }
```

So it seems to be consistent with mlxsw. Jakub will hopefully chime in
and keep me honest.

> Must all 40Gbps ports with a width of 4, be splitable to 2x
> 20Mps? It seems like some hardware might only allow 4x 10G?

Possible. There are many vendor-specific quirks in this area, as I'm
sure you know :)

> 
> If 20Gbps is supported, can you then go recursive and split one of the
> 20G ports into 2x 10G, leaving the other as a 20G port?

Quite certain this is not supported by any vendor.

I assume you're asking because you are trying to see if the test is not
making some vendor-specific assumptions? Personally, I think it's not.
We decided to put it under net/ rather than drivers/net/mlxsw because we
always prefer to write tests that can be shared with others. This is
what actually motivated this work. We had a very Mellanox-specific test
in our regression and we wanted to remove it, but it was not possible to
write such a test without this attribute.

Re: [PATCH net-next 2/3] devlink: Add a new devlink port width attribute and pass to netlink

[Shannon Nelson]

On 5/19/20 6:40 AM, Ido Schimmel wrote:
> From: Danielle Ratson <danieller@mellanox.com>
>
> Add a new devlink port attribute that indicates the port's width.
> Drivers are expected to set it via devlink_port_attrs_set(), before
> registering the port.
>
> The attribute is not passed to user space in case the width is invalid
> (0).
>
> Signed-off-by: Danielle Ratson <danieller@mellanox.com>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> ---

[...]

> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
> index 273c889faaad..a21a10307ecc 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
> @@ -82,7 +82,7 @@ int ionic_devlink_register(struct ionic *ionic)
>   		return 0;
>   
>   	devlink_port_attrs_set(&ionic->dl_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
> -			       0, false, 0, NULL, 0);
> +			       0, false, 0, 0, NULL, 0);
>   	err = devlink_port_register(dl, &ionic->dl_port, 0);
>   	if (err)
>   		dev_err(ionic->dev, "devlink_port_register failed: %d\n", err);
for ionic
Acked-by: Shannon Nelson <snelson@pensando.io>

[...]

> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index 7b76e5fffc10..9887fba60a7a 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -526,6 +526,10 @@ static int devlink_nl_port_attrs_put(struct sk_buff *msg,
>   
>   	if (!attrs->set)
>   		return 0;
> +	if (attrs->width) {
> +		if (nla_put_u32(msg, DEVLINK_ATTR_PORT_WIDTH, attrs->width))
> +			return -EMSGSIZE;
> +	}
>   	if (nla_put_u16(msg, DEVLINK_ATTR_PORT_FLAVOUR, attrs->flavour))
>   		return -EMSGSIZE;
>   	switch (devlink_port->attrs.flavour) {
> @@ -7408,6 +7412,7 @@ static int __devlink_port_attrs_set(struct devlink_port *devlink_port,
>    *	@split: indicates if this is split port
>    *	@split_subport_number: if the port is split, this is the number
>    *	                       of subport.
> + *	@width: width of the port. 0 value is not passed to netlink.

A little more explanation here would help - basically something like
     @width: number of ways the port can be split.  0 value is not 
passed to netlink.

Is this always going to be an even number, or a power-of-2?  If so, 
perhaps that should be noted here.


Thanks,
sln

Re: [PATCH net-next 3/3] selftests: net: Add port split test

[Andrew Lunn]

> It's basically the number of lanes

Then why not call it lanes? It makes it clearer how this maps to the
hardware?

> > Is it well defined that all splits of the for 2, 4, 8 have to be
> > supported?
> 
> That I don't actually know. It is true for Mellanox and I can only
> assume it holds for other vendors. So far beside mlxsw only nfp
> implemented port_split() callback. I see it has this check:
> 
> ```
>         if (eth_port.is_split || eth_port.port_lanes % count) {
>                 ret = -EINVAL;
>                 goto out;
>         }
> ```
> 
> So it seems to be consistent with mlxsw. Jakub will hopefully chime in
> and keep me honest.
> 
> > Must all 40Gbps ports with a width of 4, be splitable to 2x
> > 20Mps? It seems like some hardware might only allow 4x 10G?
> 
> Possible. There are many vendor-specific quirks in this area, as I'm
> sure you know :)

So this makes me wonder if the API is sufficient. Do we actually want
to enumerate what is possible, rather than leave the user to guess,
trial and error?

> I assume you're asking because you are trying to see if the test is not
> making some vendor-specific assumptions?

Not just the test, but also the API itself. Is the API generic enough?
Should we actually be able to indicate a 40G port cannot be used as 2x
20G? But 4x 10G is O.K?

The PDF you gave a link to actually says nothing about 2x 50G, or 2x
20G. There is a cable which does support 2x 50G. Does the firmware do
any sanity checking and return errors if you ask it to do something
which does not make sense with the cable currently inserted in the
SFP cage?

	Andrew

Re: [PATCH net-next 3/3] selftests: net: Add port split test

[Ido Schimmel]

On Tue, May 19, 2020 at 09:33:06PM +0200, Andrew Lunn wrote:
> > It's basically the number of lanes
> 
> Then why not call it lanes? It makes it clearer how this maps to the
> hardware?

I'm not against it. We discussed it and decided to go with width. Jiri /
Danielle, are you OK with changing to lanes?

> 
> > > Is it well defined that all splits of the for 2, 4, 8 have to be
> > > supported?
> > 
> > That I don't actually know. It is true for Mellanox and I can only
> > assume it holds for other vendors. So far beside mlxsw only nfp
> > implemented port_split() callback. I see it has this check:
> > 
> > ```
> >         if (eth_port.is_split || eth_port.port_lanes % count) {
> >                 ret = -EINVAL;
> >                 goto out;
> >         }
> > ```
> > 
> > So it seems to be consistent with mlxsw. Jakub will hopefully chime in
> > and keep me honest.
> > 
> > > Must all 40Gbps ports with a width of 4, be splitable to 2x
> > > 20Mps? It seems like some hardware might only allow 4x 10G?
> > 
> > Possible. There are many vendor-specific quirks in this area, as I'm
> > sure you know :)
> 
> So this makes me wonder if the API is sufficient. Do we actually want
> to enumerate what is possible, rather than leave the user to guess,
> trial and error?

The API is merely a read-only number which represents the number of
lanes. It's useful on its own without relation to split because it
allows you to debug issues such as "why port X does not support speed
Y". I actually wrote an ugly drgn script to pull this info from mlxsw
once:

```
#!/usr/bin/env drgn

import drgn
from drgn.helpers.linux import list_first_entry

devlink = list_first_entry(prog['devlink_list'].address_of_(),
                           'struct devlink', 'list')
mlxsw_core = drgn.reinterpret(prog.type("struct mlxsw_core"), devlink.priv)
mlxsw_sp = drgn.reinterpret(prog.type("struct mlxsw_sp"),
                            mlxsw_core.driver_priv)

max_ports = mlxsw_core.max_ports.value_() - 1
for i in range(0, max_ports):
    if mlxsw_sp.ports[i].value_() == 0:
        continue

    print("local_port=%d module=%d width=%d" %
          (mlxsw_sp.ports[i].local_port.value_(),
          mlxsw_sp.ports[i].mapping.module.value_(),
          mlxsw_sp.ports[i].mapping.width.value_()))
```

> 
> > I assume you're asking because you are trying to see if the test is not
> > making some vendor-specific assumptions?
> 
> Not just the test, but also the API itself. Is the API generic enough?
> Should we actually be able to indicate a 40G port cannot be used as 2x
> 20G? But 4x 10G is O.K?

We can do it, but I prefer to wait with new uAPI until we actually see a
need for it. We currently only expose width/lanes which is useful on its
own and infer from it how a port can be split. If this assumption turns
out to be wrong, we can add new attributes to answer the questions you
posed.

> The PDF you gave a link to actually says nothing about 2x 50G, or 2x
> 20G. There is a cable which does support 2x 50G. Does the firmware do
> any sanity checking and return errors if you ask it to do something
> which does not make sense with the cable currently inserted in the
> SFP cage?

Wait, I linked to a 4x splitter, not to 2x. This is a 2x:
https://www.mellanox.com/related-docs/prod_cables/PB_MCP7H00-G0xxxxxxx_100GbE_QSFP28_to_2x50GbE_2xQSFP28_DAC_Splitter.pdf

Complete list is here:
https://www.mellanox.com/products/interconnect/ethernet-copper-splitter

The firmware does not really care if it's a split or not. Software just
maps {module, width/lanes} -> local_port where each local_port is
represented by a netdev.

The supported speeds are set according to the width/lanes of the port.
If you requested a speed that is not supported or cannot be handled by
the cable you are using, then you will not be able to negotiate a link,
but it's the same regardless if the port is split or not.

Re: [PATCH net-next 3/3] selftests: net: Add port split test

[Jiri Pirko]

Wed, May 20, 2020 at 03:43:40PM CEST, idosch@idosch.org wrote:
>On Tue, May 19, 2020 at 09:33:06PM +0200, Andrew Lunn wrote:
>> > It's basically the number of lanes
>> 
>> Then why not call it lanes? It makes it clearer how this maps to the
>> hardware?
>
>I'm not against it. We discussed it and decided to go with width. Jiri /
>Danielle, are you OK with changing to lanes?

Sure, no problem.

Re: [PATCH net-next 3/3] selftests: net: Add port split test

[Danielle Ratson]

On 5/20/2020 4:53 PM, Jiri Pirko wrote:
> Wed, May 20, 2020 at 03:43:40PM CEST, idosch@idosch.org wrote:
>> On Tue, May 19, 2020 at 09:33:06PM +0200, Andrew Lunn wrote:
>>>> It's basically the number of lanes
>>> Then why not call it lanes? It makes it clearer how this maps to the
>>> hardware?
>> I'm not against it. We discussed it and decided to go with width. Jiri /
>> Danielle, are you OK with changing to lanes?
> Sure, no problem.


Hi Andrew,


Maximum number of lanes, is the maximum lanes capacity of the port. 

Besides that, there is the number of utilized lanes, which is the number of lanes that are actually used.


In that patch we expose the maximum number of lanes, and in my opinion "width" describes that better, and "lanes" on the other hand, can refer to both.

Thanks

Re: [PATCH net-next 0/3] devlink: Add port width attribute

[David Miller]

Honestly, width is completely ambiguous in this situation to me.

Width doesn't mean anything without an accompanying unit of measure
and in this situation it's one lane.