[PATCH net-next 0/3] devlink: Add extack messages for reload and port split/unsplit

STALE2981d

Revision v1 of 2 in this series.

14 messages, 6 authors, 2018-06-05 · open the first message on its own page

[PATCH net-next 0/3] devlink: Add extack messages for reload and port split/unsplit

From: dsahern@kernel.org
Date: 2018-06-04 22:15:04 

From: David Ahern <redacted>

Patch 1 adds extack arg to reload, port_split and port_unsplit devlink
operations.

Patch 2 adds extack messages for reload operation in netdevsim.

Patch 3 adds extack messages to port split/unsplit in mlxsw driver.

David Ahern (3):
  devlink: Add extack to reload and port_{un,}split operations
  netdevsim: Add extack error message for devlink reload
  mlxsw: Add extack messages for port_{un,}split failures

 drivers/net/ethernet/mellanox/mlxsw/core.c       | 23 ++++++++++++++++-------
 drivers/net/ethernet/mellanox/mlxsw/core.h       |  5 +++--
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c   | 13 +++++++++++--
 drivers/net/ethernet/netronome/nfp/nfp_devlink.c |  5 +++--
 drivers/net/netdevsim/devlink.c                  |  7 ++++---
 drivers/net/netdevsim/fib.c                      |  9 ++++++---
 drivers/net/netdevsim/netdevsim.h                |  3 ++-
 include/net/devlink.h                            |  7 ++++---
 net/core/devlink.c                               | 18 ++++++++++--------
 9 files changed, 59 insertions(+), 31 deletions(-)

-- 
2.11.0

[PATCH net-next 1/3] devlink: Add extack to reload and port_{un,}split operations

From: dsahern@kernel.org
Date: 2018-06-04 22:15:04 

From: David Ahern <redacted>

Add extack argument to reload, port_split and port_unsplit operations.

Signed-off-by: David Ahern <redacted>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c       |  9 ++++++---
 drivers/net/ethernet/netronome/nfp/nfp_devlink.c |  5 +++--
 drivers/net/netdevsim/devlink.c                  |  3 ++-
 include/net/devlink.h                            |  7 ++++---
 net/core/devlink.c                               | 18 ++++++++++--------
 5 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 8a766fe28fa0..7ed38d80bc08 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -770,7 +770,8 @@ static void mlxsw_core_driver_put(const char *kind)
 
 static int mlxsw_devlink_port_split(struct devlink *devlink,
 				    unsigned int port_index,
-				    unsigned int count)
+				    unsigned int count,
+				    struct netlink_ext_ack *extack)
 {
 	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
 
@@ -782,7 +783,8 @@ static int mlxsw_devlink_port_split(struct devlink *devlink,
 }
 
 static int mlxsw_devlink_port_unsplit(struct devlink *devlink,
-				      unsigned int port_index)
+				      unsigned int port_index,
+				      struct netlink_ext_ack *extack)
 {
 	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
 
@@ -963,7 +965,8 @@ mlxsw_devlink_sb_occ_tc_port_bind_get(struct devlink_port *devlink_port,
 						     pool_type, p_cur, p_max);
 }
 
-static int mlxsw_devlink_core_bus_device_reload(struct devlink *devlink)
+static int mlxsw_devlink_core_bus_device_reload(struct devlink *devlink,
+						struct netlink_ext_ack *extack)
 {
 	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
 	int err;
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index 71c2edd83031..db463e20a876 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -92,7 +92,7 @@ nfp_devlink_set_lanes(struct nfp_pf *pf, unsigned int idx, unsigned int lanes)
 
 static int
 nfp_devlink_port_split(struct devlink *devlink, unsigned int port_index,
-		       unsigned int count)
+		       unsigned int count, struct netlink_ext_ack *extack)
 {
 	struct nfp_pf *pf = devlink_priv(devlink);
 	struct nfp_eth_table_port eth_port;
@@ -123,7 +123,8 @@ nfp_devlink_port_split(struct devlink *devlink, unsigned int port_index,
 }
 
 static int
-nfp_devlink_port_unsplit(struct devlink *devlink, unsigned int port_index)
+nfp_devlink_port_unsplit(struct devlink *devlink, unsigned int port_index,
+			 struct netlink_ext_ack *extack)
 {
 	struct nfp_pf *pf = devlink_priv(devlink);
 	struct nfp_eth_table_port eth_port;
diff --git a/drivers/net/netdevsim/devlink.c b/drivers/net/netdevsim/devlink.c
index bef7db5d129a..e8366cf372ff 100644
--- a/drivers/net/netdevsim/devlink.c
+++ b/drivers/net/netdevsim/devlink.c
@@ -147,7 +147,8 @@ static int devlink_resources_register(struct devlink *devlink)
 	return err;
 }
 
-static int nsim_devlink_reload(struct devlink *devlink)
+static int nsim_devlink_reload(struct devlink *devlink,
+			       struct netlink_ext_ack *extack)
 {
 	enum nsim_resource_id res_ids[] = {
 		NSIM_RESOURCE_IPV4_FIB, NSIM_RESOURCE_IPV4_FIB_RULES,
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 9686a1aa4ec9..e336ea9c73df 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -296,12 +296,13 @@ struct devlink_resource {
 #define DEVLINK_RESOURCE_ID_PARENT_TOP 0
 
 struct devlink_ops {
-	int (*reload)(struct devlink *devlink);
+	int (*reload)(struct devlink *devlink, struct netlink_ext_ack *extack);
 	int (*port_type_set)(struct devlink_port *devlink_port,
 			     enum devlink_port_type port_type);
 	int (*port_split)(struct devlink *devlink, unsigned int port_index,
-			  unsigned int count);
-	int (*port_unsplit)(struct devlink *devlink, unsigned int port_index);
+			  unsigned int count, struct netlink_ext_ack *extack);
+	int (*port_unsplit)(struct devlink *devlink, unsigned int port_index,
+			    struct netlink_ext_ack *extack);
 	int (*sb_pool_get)(struct devlink *devlink, unsigned int sb_index,
 			   u16 pool_index,
 			   struct devlink_sb_pool_info *pool_info);
diff --git a/net/core/devlink.c b/net/core/devlink.c
index f75ee022e6b2..22099705cc41 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -702,12 +702,13 @@ static int devlink_nl_cmd_port_set_doit(struct sk_buff *skb,
 	return 0;
 }
 
-static int devlink_port_split(struct devlink *devlink,
-			      u32 port_index, u32 count)
+static int devlink_port_split(struct devlink *devlink, u32 port_index,
+			      u32 count, struct netlink_ext_ack *extack)
 
 {
 	if (devlink->ops && devlink->ops->port_split)
-		return devlink->ops->port_split(devlink, port_index, count);
+		return devlink->ops->port_split(devlink, port_index, count,
+						extack);
 	return -EOPNOTSUPP;
 }
 
@@ -724,14 +725,15 @@ static int devlink_nl_cmd_port_split_doit(struct sk_buff *skb,
 
 	port_index = nla_get_u32(info->attrs[DEVLINK_ATTR_PORT_INDEX]);
 	count = nla_get_u32(info->attrs[DEVLINK_ATTR_PORT_SPLIT_COUNT]);
-	return devlink_port_split(devlink, port_index, count);
+	return devlink_port_split(devlink, port_index, count, info->extack);
 }
 
-static int devlink_port_unsplit(struct devlink *devlink, u32 port_index)
+static int devlink_port_unsplit(struct devlink *devlink, u32 port_index,
+				struct netlink_ext_ack *extack)
 
 {
 	if (devlink->ops && devlink->ops->port_unsplit)
-		return devlink->ops->port_unsplit(devlink, port_index);
+		return devlink->ops->port_unsplit(devlink, port_index, extack);
 	return -EOPNOTSUPP;
 }
 
@@ -745,7 +747,7 @@ static int devlink_nl_cmd_port_unsplit_doit(struct sk_buff *skb,
 		return -EINVAL;
 
 	port_index = nla_get_u32(info->attrs[DEVLINK_ATTR_PORT_INDEX]);
-	return devlink_port_unsplit(devlink, port_index);
+	return devlink_port_unsplit(devlink, port_index, info->extack);
 }
 
 static int devlink_nl_sb_fill(struct sk_buff *msg, struct devlink *devlink,
@@ -2599,7 +2601,7 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
 		NL_SET_ERR_MSG_MOD(info->extack, "resources size validation failed");
 		return err;
 	}
-	return devlink->ops->reload(devlink);
+	return devlink->ops->reload(devlink, info->extack);
 }
 
 static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
-- 
2.11.0

Re: [PATCH net-next 1/3] devlink: Add extack to reload and port_{un,}split operations

From: Jiri Pirko <jiri@resnulli.us>
Date: 2018-06-05 07:50:57 

Tue, Jun 05, 2018 at 12:15:01AM CEST, dsahern@kernel.org wrote:
From: David Ahern <redacted>

Add extack argument to reload, port_split and port_unsplit operations.

Signed-off-by: David Ahern <redacted>
Acked-by: Jiri Pirko <redacted>

[PATCH net-next 2/3] netdevsim: Add extack error message for devlink reload

From: dsahern@kernel.org
Date: 2018-06-04 22:15:05 

From: David Ahern <redacted>

devlink reset command can fail if a FIB resource limit is set to a value
lower than the current occupancy. Return a proper message indicating the
reason for the failure.

$ devlink resource sh netdevsim/netdevsim0
netdevsim/netdevsim0:
  name IPv4 size unlimited unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none
    resources:
      name fib size unlimited occ 43 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none
      name fib-rules size unlimited occ 4 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none
  name IPv6 size unlimited unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none
    resources:
      name fib size unlimited occ 54 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none
      name fib-rules size unlimited occ 3 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none

$ devlink resource set netdevsim/netdevsim0 path /IPv4/fib size 40

$ devlink dev  reload netdevsim/netdevsim0
Error: netdevsim: New size is less than current occupancy.
devlink answers: Invalid argument

Signed-off-by: David Ahern <redacted>
---
 drivers/net/netdevsim/devlink.c   | 4 ++--
 drivers/net/netdevsim/fib.c       | 9 ++++++---
 drivers/net/netdevsim/netdevsim.h | 3 ++-
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/netdevsim/devlink.c b/drivers/net/netdevsim/devlink.c
index e8366cf372ff..ba663e5af168 100644
--- a/drivers/net/netdevsim/devlink.c
+++ b/drivers/net/netdevsim/devlink.c
@@ -163,7 +163,7 @@ static int nsim_devlink_reload(struct devlink *devlink,
 
 		err = devlink_resource_size_get(devlink, res_ids[i], &val);
 		if (!err) {
-			err = nsim_fib_set_max(net, res_ids[i], val);
+			err = nsim_fib_set_max(net, res_ids[i], val, extack);
 			if (err)
 				return err;
 		}
@@ -181,7 +181,7 @@ static void nsim_devlink_net_reset(struct net *net)
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(res_ids); ++i) {
-		if (nsim_fib_set_max(net, res_ids[i], (u64)-1)) {
+		if (nsim_fib_set_max(net, res_ids[i], (u64)-1, NULL)) {
 			pr_err("Failed to reset limit for resource %u\n",
 			       res_ids[i]);
 		}
diff --git a/drivers/net/netdevsim/fib.c b/drivers/net/netdevsim/fib.c
index 9bfe9e151e13..f61d094746c0 100644
--- a/drivers/net/netdevsim/fib.c
+++ b/drivers/net/netdevsim/fib.c
@@ -64,7 +64,8 @@ u64 nsim_fib_get_val(struct net *net, enum nsim_resource_id res_id, bool max)
 	return max ? entry->max : entry->num;
 }
 
-int nsim_fib_set_max(struct net *net, enum nsim_resource_id res_id, u64 val)
+int nsim_fib_set_max(struct net *net, enum nsim_resource_id res_id, u64 val,
+		     struct netlink_ext_ack *extack)
 {
 	struct nsim_fib_data *fib_data = net_generic(net, nsim_fib_net_id);
 	struct nsim_fib_entry *entry;
@@ -90,10 +91,12 @@ int nsim_fib_set_max(struct net *net, enum nsim_resource_id res_id, u64 val)
 	/* not allowing a new max to be less than curren occupancy
 	 * --> no means of evicting entries
 	 */
-	if (val < entry->num)
+	if (val < entry->num) {
+		NL_SET_ERR_MSG_MOD(extack, "New size is less than current occupancy");
 		err = -EINVAL;
-	else
+	} else {
 		entry->max = val;
+	}
 
 	return err;
 }
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 3a8581af3b85..8ca50b72c328 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -126,7 +126,8 @@ void nsim_devlink_exit(void);
 int nsim_fib_init(void);
 void nsim_fib_exit(void);
 u64 nsim_fib_get_val(struct net *net, enum nsim_resource_id res_id, bool max);
-int nsim_fib_set_max(struct net *net, enum nsim_resource_id res_id, u64 val);
+int nsim_fib_set_max(struct net *net, enum nsim_resource_id res_id, u64 val,
+		     struct netlink_ext_ack *extack);
 #else
 static inline int nsim_devlink_setup(struct netdevsim *ns)
 {
-- 
2.11.0

Re: [PATCH net-next 2/3] netdevsim: Add extack error message for devlink reload

From: Jakub Kicinski <hidden>
Date: 2018-06-04 22:47:18 

On Mon,  4 Jun 2018 15:15:02 -0700, dsahern@kernel.org wrote:
From: David Ahern <redacted>

devlink reset command can fail if a FIB resource limit is set to a value
lower than the current occupancy. Return a proper message indicating the
reason for the failure.

$ devlink resource sh netdevsim/netdevsim0
netdevsim/netdevsim0:
  name IPv4 size unlimited unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none
    resources:
      name fib size unlimited occ 43 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none
      name fib-rules size unlimited occ 4 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none
  name IPv6 size unlimited unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none
    resources:
      name fib size unlimited occ 54 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none
      name fib-rules size unlimited occ 3 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none

$ devlink resource set netdevsim/netdevsim0 path /IPv4/fib size 40

$ devlink dev  reload netdevsim/netdevsim0
Error: netdevsim: New size is less than current occupancy.
devlink answers: Invalid argument

Signed-off-by: David Ahern <redacted>
Acked-by: Jakub Kicinski <redacted>

The entire set looks very useful, thanks!

[PATCH net-next 3/3] mlxsw: Add extack messages for port_{un,}split failures

From: dsahern@kernel.org
Date: 2018-06-04 22:15:05 

From: David Ahern <redacted>

Return messages in extack for port split/unsplit errors. e.g.,
    $ devlink port split swp1s1 count 4
    Error: mlxsw_spectrum: Port cannot be split further.
    devlink answers: Invalid argument

    $ devlink port unsplit swp4
    Error: mlxsw_spectrum: Port was not split.
    devlink answers: Invalid argument

Signed-off-by: David Ahern <redacted>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c     | 14 ++++++++++----
 drivers/net/ethernet/mellanox/mlxsw/core.h     |  5 +++--
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 13 +++++++++++--
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 7ed38d80bc08..f9c724752a32 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -775,11 +775,14 @@ static int mlxsw_devlink_port_split(struct devlink *devlink,
 {
 	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
 
-	if (port_index >= mlxsw_core->max_ports)
+	if (port_index >= mlxsw_core->max_ports) {
+		NL_SET_ERR_MSG_MOD(extack, "Port index exceeds maximum number of ports");
 		return -EINVAL;
+	}
 	if (!mlxsw_core->driver->port_split)
 		return -EOPNOTSUPP;
-	return mlxsw_core->driver->port_split(mlxsw_core, port_index, count);
+	return mlxsw_core->driver->port_split(mlxsw_core, port_index, count,
+					      extack);
 }
 
 static int mlxsw_devlink_port_unsplit(struct devlink *devlink,
@@ -788,11 +791,14 @@ static int mlxsw_devlink_port_unsplit(struct devlink *devlink,
 {
 	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
 
-	if (port_index >= mlxsw_core->max_ports)
+	if (port_index >= mlxsw_core->max_ports) {
+		NL_SET_ERR_MSG_MOD(extack, "Port index exceeds maximum number of ports");
 		return -EINVAL;
+	}
 	if (!mlxsw_core->driver->port_unsplit)
 		return -EOPNOTSUPP;
-	return mlxsw_core->driver->port_unsplit(mlxsw_core, port_index);
+	return mlxsw_core->driver->port_unsplit(mlxsw_core, port_index,
+						extack);
 }
 
 static int
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index 4a8d4c7f89d9..552cfa29c2f7 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -274,8 +274,9 @@ struct mlxsw_driver {
 	int (*port_type_set)(struct mlxsw_core *mlxsw_core, u8 local_port,
 			     enum devlink_port_type new_type);
 	int (*port_split)(struct mlxsw_core *mlxsw_core, u8 local_port,
-			  unsigned int count);
-	int (*port_unsplit)(struct mlxsw_core *mlxsw_core, u8 local_port);
+			  unsigned int count, struct netlink_ext_ack *extack);
+	int (*port_unsplit)(struct mlxsw_core *mlxsw_core, u8 local_port,
+			    struct netlink_ext_ack *extack);
 	int (*sb_pool_get)(struct mlxsw_core *mlxsw_core,
 			   unsigned int sb_index, u16 pool_index,
 			   struct devlink_sb_pool_info *pool_info);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index fc39f22e5c70..1b6d930e452d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3092,7 +3092,8 @@ static void mlxsw_sp_port_unsplit_create(struct mlxsw_sp *mlxsw_sp,
 }
 
 static int mlxsw_sp_port_split(struct mlxsw_core *mlxsw_core, u8 local_port,
-			       unsigned int count)
+			       unsigned int count,
+			       struct netlink_ext_ack *extack)
 {
 	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
 	struct mlxsw_sp_port *mlxsw_sp_port;
@@ -3104,6 +3105,7 @@ static int mlxsw_sp_port_split(struct mlxsw_core *mlxsw_core, u8 local_port,
 	if (!mlxsw_sp_port) {
 		dev_err(mlxsw_sp->bus_info->dev, "Port number \"%d\" does not exist\n",
 			local_port);
+		NL_SET_ERR_MSG_MOD(extack, "Port number does not exist");
 		return -EINVAL;
 	}
 
@@ -3112,11 +3114,13 @@ static int mlxsw_sp_port_split(struct mlxsw_core *mlxsw_core, u8 local_port,
 
 	if (count != 2 && count != 4) {
 		netdev_err(mlxsw_sp_port->dev, "Port can only be split into 2 or 4 ports\n");
+		NL_SET_ERR_MSG_MOD(extack, "Port can only be split into 2 or 4 ports");
 		return -EINVAL;
 	}
 
 	if (cur_width != MLXSW_PORT_MODULE_MAX_WIDTH) {
 		netdev_err(mlxsw_sp_port->dev, "Port cannot be split further\n");
+		NL_SET_ERR_MSG_MOD(extack, "Port cannot be split further");
 		return -EINVAL;
 	}
 
@@ -3125,6 +3129,7 @@ static int mlxsw_sp_port_split(struct mlxsw_core *mlxsw_core, u8 local_port,
 		base_port = local_port;
 		if (mlxsw_sp->ports[base_port + 1]) {
 			netdev_err(mlxsw_sp_port->dev, "Invalid split configuration\n");
+			NL_SET_ERR_MSG_MOD(extack, "Invalid split configuration");
 			return -EINVAL;
 		}
 	} else {
@@ -3132,6 +3137,7 @@ static int mlxsw_sp_port_split(struct mlxsw_core *mlxsw_core, u8 local_port,
 		if (mlxsw_sp->ports[base_port + 1] ||
 		    mlxsw_sp->ports[base_port + 3]) {
 			netdev_err(mlxsw_sp_port->dev, "Invalid split configuration\n");
+			NL_SET_ERR_MSG_MOD(extack, "Invalid split configuration");
 			return -EINVAL;
 		}
 	}
@@ -3153,7 +3159,8 @@ static int mlxsw_sp_port_split(struct mlxsw_core *mlxsw_core, u8 local_port,
 	return err;
 }
 
-static int mlxsw_sp_port_unsplit(struct mlxsw_core *mlxsw_core, u8 local_port)
+static int mlxsw_sp_port_unsplit(struct mlxsw_core *mlxsw_core, u8 local_port,
+				 struct netlink_ext_ack *extack)
 {
 	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
 	struct mlxsw_sp_port *mlxsw_sp_port;
@@ -3165,11 +3172,13 @@ static int mlxsw_sp_port_unsplit(struct mlxsw_core *mlxsw_core, u8 local_port)
 	if (!mlxsw_sp_port) {
 		dev_err(mlxsw_sp->bus_info->dev, "Port number \"%d\" does not exist\n",
 			local_port);
+		NL_SET_ERR_MSG_MOD(extack, "Port number does not exist");
 		return -EINVAL;
 	}
 
 	if (!mlxsw_sp_port->split) {
 		netdev_err(mlxsw_sp_port->dev, "Port wasn't split\n");
+		NL_SET_ERR_MSG_MOD(extack, "Port was not split");
 		return -EINVAL;
 	}
 
-- 
2.11.0

Re: [PATCH net-next 3/3] mlxsw: Add extack messages for port_{un,}split failures

From: Ido Schimmel <hidden>
Date: 2018-06-05 05:56:50 

On Mon, Jun 04, 2018 at 03:15:03PM -0700, dsahern@kernel.org wrote:
From: David Ahern <redacted>

Return messages in extack for port split/unsplit errors. e.g.,
    $ devlink port split swp1s1 count 4
    Error: mlxsw_spectrum: Port cannot be split further.
    devlink answers: Invalid argument

    $ devlink port unsplit swp4
    Error: mlxsw_spectrum: Port was not split.
    devlink answers: Invalid argument

Signed-off-by: David Ahern <redacted>
Reviewed-by: Ido Schimmel <redacted>

Thanks!

Re: [PATCH net-next 3/3] mlxsw: Add extack messages for port_{un,}split failures

From: Jiri Pirko <jiri@resnulli.us>
Date: 2018-06-05 07:53:06 

Tue, Jun 05, 2018 at 12:15:03AM CEST, dsahern@kernel.org wrote:
quoted hunk
From: David Ahern <redacted>

Return messages in extack for port split/unsplit errors. e.g.,
   $ devlink port split swp1s1 count 4
   Error: mlxsw_spectrum: Port cannot be split further.
   devlink answers: Invalid argument

   $ devlink port unsplit swp4
   Error: mlxsw_spectrum: Port was not split.
   devlink answers: Invalid argument

Signed-off-by: David Ahern <redacted>
---
drivers/net/ethernet/mellanox/mlxsw/core.c     | 14 ++++++++++----
drivers/net/ethernet/mellanox/mlxsw/core.h     |  5 +++--
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 13 +++++++++++--
3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 7ed38d80bc08..f9c724752a32 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -775,11 +775,14 @@ static int mlxsw_devlink_port_split(struct devlink *devlink,
{
	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);

-	if (port_index >= mlxsw_core->max_ports)
+	if (port_index >= mlxsw_core->max_ports) {
+		NL_SET_ERR_MSG_MOD(extack, "Port index exceeds maximum number of ports");
		return -EINVAL;
+	}
	if (!mlxsw_core->driver->port_split)
		return -EOPNOTSUPP;
-	return mlxsw_core->driver->port_split(mlxsw_core, port_index, count);
+	return mlxsw_core->driver->port_split(mlxsw_core, port_index, count,
+					      extack);
}

static int mlxsw_devlink_port_unsplit(struct devlink *devlink,
@@ -788,11 +791,14 @@ static int mlxsw_devlink_port_unsplit(struct devlink *devlink,
{
	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);

-	if (port_index >= mlxsw_core->max_ports)
+	if (port_index >= mlxsw_core->max_ports) {
+		NL_SET_ERR_MSG_MOD(extack, "Port index exceeds maximum number of ports");
		return -EINVAL;
+	}
	if (!mlxsw_core->driver->port_unsplit)
		return -EOPNOTSUPP;
-	return mlxsw_core->driver->port_unsplit(mlxsw_core, port_index);
+	return mlxsw_core->driver->port_unsplit(mlxsw_core, port_index,
+						extack);
}

static int
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index 4a8d4c7f89d9..552cfa29c2f7 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -274,8 +274,9 @@ struct mlxsw_driver {
	int (*port_type_set)(struct mlxsw_core *mlxsw_core, u8 local_port,
			     enum devlink_port_type new_type);
	int (*port_split)(struct mlxsw_core *mlxsw_core, u8 local_port,
-			  unsigned int count);
-	int (*port_unsplit)(struct mlxsw_core *mlxsw_core, u8 local_port);
+			  unsigned int count, struct netlink_ext_ack *extack);
+	int (*port_unsplit)(struct mlxsw_core *mlxsw_core, u8 local_port,
+			    struct netlink_ext_ack *extack);
	int (*sb_pool_get)(struct mlxsw_core *mlxsw_core,
			   unsigned int sb_index, u16 pool_index,
			   struct devlink_sb_pool_info *pool_info);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index fc39f22e5c70..1b6d930e452d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3092,7 +3092,8 @@ static void mlxsw_sp_port_unsplit_create(struct mlxsw_sp *mlxsw_sp,
}

static int mlxsw_sp_port_split(struct mlxsw_core *mlxsw_core, u8 local_port,
-			       unsigned int count)
+			       unsigned int count,
+			       struct netlink_ext_ack *extack)
{
	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
	struct mlxsw_sp_port *mlxsw_sp_port;
@@ -3104,6 +3105,7 @@ static int mlxsw_sp_port_split(struct mlxsw_core *mlxsw_core, u8 local_port,
	if (!mlxsw_sp_port) {
		dev_err(mlxsw_sp->bus_info->dev, "Port number \"%d\" does not exist\n",
			local_port);
+		NL_SET_ERR_MSG_MOD(extack, "Port number does not exist");
		return -EINVAL;
	}

@@ -3112,11 +3114,13 @@ static int mlxsw_sp_port_split(struct mlxsw_core *mlxsw_core, u8 local_port,

	if (count != 2 && count != 4) {
		netdev_err(mlxsw_sp_port->dev, "Port can only be split into 2 or 4 ports\n");
+		NL_SET_ERR_MSG_MOD(extack, "Port can only be split into 2 or 4 ports");
		return -EINVAL;
	}

	if (cur_width != MLXSW_PORT_MODULE_MAX_WIDTH) {
		netdev_err(mlxsw_sp_port->dev, "Port cannot be split further\n");
+		NL_SET_ERR_MSG_MOD(extack, "Port cannot be split further");
		return -EINVAL;
	}

@@ -3125,6 +3129,7 @@ static int mlxsw_sp_port_split(struct mlxsw_core *mlxsw_core, u8 local_port,
		base_port = local_port;
		if (mlxsw_sp->ports[base_port + 1]) {
			netdev_err(mlxsw_sp_port->dev, "Invalid split configuration\n");
+			NL_SET_ERR_MSG_MOD(extack, "Invalid split configuration");
			return -EINVAL;
		}
	} else {
@@ -3132,6 +3137,7 @@ static int mlxsw_sp_port_split(struct mlxsw_core *mlxsw_core, u8 local_port,
		if (mlxsw_sp->ports[base_port + 1] ||
		    mlxsw_sp->ports[base_port + 3]) {
			netdev_err(mlxsw_sp_port->dev, "Invalid split configuration\n");
+			NL_SET_ERR_MSG_MOD(extack, "Invalid split configuration");
			return -EINVAL;
		}
	}
@@ -3153,7 +3159,8 @@ static int mlxsw_sp_port_split(struct mlxsw_core *mlxsw_core, u8 local_port,
	return err;
}

-static int mlxsw_sp_port_unsplit(struct mlxsw_core *mlxsw_core, u8 local_port)
+static int mlxsw_sp_port_unsplit(struct mlxsw_core *mlxsw_core, u8 local_port,
+				 struct netlink_ext_ack *extack)
{
	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
	struct mlxsw_sp_port *mlxsw_sp_port;
@@ -3165,11 +3172,13 @@ static int mlxsw_sp_port_unsplit(struct mlxsw_core *mlxsw_core, u8 local_port)
	if (!mlxsw_sp_port) {
		dev_err(mlxsw_sp->bus_info->dev, "Port number \"%d\" does not exist\n",
			local_port);
+		NL_SET_ERR_MSG_MOD(extack, "Port number does not exist");
		return -EINVAL;
	}

	if (!mlxsw_sp_port->split) {
		netdev_err(mlxsw_sp_port->dev, "Port wasn't split\n");
+		NL_SET_ERR_MSG_MOD(extack, "Port was not split");
I wonder if we need the dmesg for these as well. Plus it is not the same
(wasn't/was not) which is maybe confusing. Any objection against the
original dmesg messages removal?


		return -EINVAL;
	}

-- 
2.11.0

Re: [PATCH net-next 3/3] mlxsw: Add extack messages for port_{un,}split failures?

From: Ido Schimmel <hidden>
Date: 2018-06-05 08:05:31 

On Tue, Jun 05, 2018 at 09:52:30AM +0200, Jiri Pirko wrote:
Tue, Jun 05, 2018 at 12:15:03AM CEST, dsahern@kernel.org wrote:
quoted
	if (!mlxsw_sp_port->split) {
		netdev_err(mlxsw_sp_port->dev, "Port wasn't split\n");
+		NL_SET_ERR_MSG_MOD(extack, "Port was not split");
I wonder if we need the dmesg for these as well. Plus it is not the same
(wasn't/was not) which is maybe confusing. Any objection against the
original dmesg messages removal?
We had this discussion about three months ago and decided to keep the
existing messages:
https://marc.info/?l=linux-netdev&m=151982813309466&w=2

Re: [PATCH net-next 3/3] mlxsw: Add extack messages for port_{un,}split failures?

From: Jiri Pirko <jiri@resnulli.us>
Date: 2018-06-05 08:19:12 

Tue, Jun 05, 2018 at 10:05:28AM CEST, idosch@idosch.org wrote:
On Tue, Jun 05, 2018 at 09:52:30AM +0200, Jiri Pirko wrote:
quoted
Tue, Jun 05, 2018 at 12:15:03AM CEST, dsahern@kernel.org wrote:
quoted
	if (!mlxsw_sp_port->split) {
		netdev_err(mlxsw_sp_port->dev, "Port wasn't split\n");
+		NL_SET_ERR_MSG_MOD(extack, "Port was not split");
I wonder if we need the dmesg for these as well. Plus it is not the same
(wasn't/was not) which is maybe confusing. Any objection against the
original dmesg messages removal?
We had this discussion about three months ago and decided to keep the
existing messages:
https://marc.info/?l=linux-netdev&m=151982813309466&w=2
I forgot. Thanks for reminding me. So could we at least have the
messages 100% same? Thanks.

Re: [PATCH net-next 3/3] mlxsw: Add extack messages for port_{un,}split failures?

From: David Miller <davem@davemloft.net>
Date: 2018-06-05 14:24:21 

From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 5 Jun 2018 10:18:36 +0200

Tue, Jun 05, 2018 at 10:05:28AM CEST, idosch@idosch.org wrote:
quoted
On Tue, Jun 05, 2018 at 09:52:30AM +0200, Jiri Pirko wrote:
quoted
Tue, Jun 05, 2018 at 12:15:03AM CEST, dsahern@kernel.org wrote:
quoted
	if (!mlxsw_sp_port->split) {
		netdev_err(mlxsw_sp_port->dev, "Port wasn't split\n");
+		NL_SET_ERR_MSG_MOD(extack, "Port was not split");
I wonder if we need the dmesg for these as well. Plus it is not the same
(wasn't/was not) which is maybe confusing. Any objection against the
original dmesg messages removal?
We had this discussion about three months ago and decided to keep the
existing messages:
https://marc.info/?l=linux-netdev&m=151982813309466&w=2
I forgot. Thanks for reminding me. So could we at least have the
messages 100% same? Thanks.
Seems like a reasonable request, David A.?

Re: [PATCH net-next 3/3] mlxsw: Add extack messages for port_{un,}split failures?

From: David Ahern <hidden>
Date: 2018-06-05 14:58:06 

On 6/5/18 1:18 AM, Jiri Pirko wrote:
Tue, Jun 05, 2018 at 10:05:28AM CEST, idosch@idosch.org wrote:
quoted
On Tue, Jun 05, 2018 at 09:52:30AM +0200, Jiri Pirko wrote:
quoted
Tue, Jun 05, 2018 at 12:15:03AM CEST, dsahern@kernel.org wrote:
quoted
	if (!mlxsw_sp_port->split) {
		netdev_err(mlxsw_sp_port->dev, "Port wasn't split\n");
+		NL_SET_ERR_MSG_MOD(extack, "Port was not split");
I wonder if we need the dmesg for these as well. Plus it is not the same
(wasn't/was not) which is maybe confusing. Any objection against the
original dmesg messages removal?
We had this discussion about three months ago and decided to keep the
existing messages:
https://marc.info/?l=linux-netdev&m=151982813309466&w=2
I forgot. Thanks for reminding me. So could we at least have the
messages 100% same? Thanks.
ok if I convert the current message to 'was not' and avoid the
contraction in messages?

Re: [PATCH net-next 3/3] mlxsw: Add extack messages for port_{un,}split failures?

From: David Miller <davem@davemloft.net>
Date: 2018-06-05 15:05:09 

From: David Ahern <redacted>
Date: Tue, 5 Jun 2018 07:58:44 -0700

On 6/5/18 1:18 AM, Jiri Pirko wrote:
quoted
Tue, Jun 05, 2018 at 10:05:28AM CEST, idosch@idosch.org wrote:
quoted
On Tue, Jun 05, 2018 at 09:52:30AM +0200, Jiri Pirko wrote:
quoted
Tue, Jun 05, 2018 at 12:15:03AM CEST, dsahern@kernel.org wrote:
quoted
	if (!mlxsw_sp_port->split) {
		netdev_err(mlxsw_sp_port->dev, "Port wasn't split\n");
+		NL_SET_ERR_MSG_MOD(extack, "Port was not split");
I wonder if we need the dmesg for these as well. Plus it is not the same
(wasn't/was not) which is maybe confusing. Any objection against the
original dmesg messages removal?
We had this discussion about three months ago and decided to keep the
existing messages:
https://marc.info/?l=linux-netdev&m=151982813309466&w=2
I forgot. Thanks for reminding me. So could we at least have the
messages 100% same? Thanks.
ok if I convert the current message to 'was not' and avoid the
contraction in messages?
Sure.

Re: [PATCH net-next 3/3] mlxsw: Add extack messages for port_{un,}split failures?

From: Jiri Pirko <jiri@resnulli.us>
Date: 2018-06-05 15:05:55 

Tue, Jun 05, 2018 at 04:58:44PM CEST, dsahern@gmail.com wrote:
On 6/5/18 1:18 AM, Jiri Pirko wrote:
quoted
Tue, Jun 05, 2018 at 10:05:28AM CEST, idosch@idosch.org wrote:
quoted
On Tue, Jun 05, 2018 at 09:52:30AM +0200, Jiri Pirko wrote:
quoted
Tue, Jun 05, 2018 at 12:15:03AM CEST, dsahern@kernel.org wrote:
quoted
	if (!mlxsw_sp_port->split) {
		netdev_err(mlxsw_sp_port->dev, "Port wasn't split\n");
+		NL_SET_ERR_MSG_MOD(extack, "Port was not split");
I wonder if we need the dmesg for these as well. Plus it is not the same
(wasn't/was not) which is maybe confusing. Any objection against the
original dmesg messages removal?
We had this discussion about three months ago and decided to keep the
existing messages:
https://marc.info/?l=linux-netdev&m=151982813309466&w=2
I forgot. Thanks for reminding me. So could we at least have the
messages 100% same? Thanks.
ok if I convert the current message to 'was not' and avoid the
contraction in messages?
Sure.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help