[patch net-next RFC v2 00/11] Add support for resource abstraction

[patch net-next RFC v2 00/11] Add support for resource abstraction

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Arkadi says:

Many of the ASIC's internal resources are limited and are shared between
several hardware procedures. For example, unified hash-based memory can
be used for many lookup purposes, like FDB and LPM. In many cases the user
can provide a partitioning scheme for such a resource in order to perform
fine tuning for his application. In many cases after setting the
partitioning of the resource driver reload is needed. This patchset add
support for hot reset of the driver.

Such an abstraction can be coupled with devlink's dpipe interface, which
models the ASIC's pipeline as an graph of match/action tables. By modeling
the hardware resource object, and by coupling it to several dpipe tables,
further visibility can be achieved in order to debug ASIC-wide issues.

The proposed interface will provide the user the ability to understand the
limitations of the hardware, and receive notification regarding its occupancy.
Furthermore, monitoring the resource occupancy can be done in real-time and
can be useful in many cases.

Userspace part prototype can be found at https://github.com/arkadis/iproute2/
at resource_dev branch.

Arkadi Sharshevsky (11):
  devlink: Add per devlink instance lock
  devlink: Add support for resource abstraction
  devlink: Add support for reload
  devlink: Add relation between dpipe and resource
  mlxsw: pci: Add support for performing bus reset
  mlxsw: spectrum: Add "spectrum" prefix macro
  mlxsw: spectrum: Register KVD resources with devlink
  mlxsw: spectrum_dpipe: Connect dpipe tables to resources
  mlxsw: spectrum: Add support for getting kvdl occupancy
  mlxsw: pci: Add support for getting resource through devlink
  mlxsw: core: Add support for reload

 drivers/net/ethernet/mellanox/mlxsw/core.c         |  85 +++-
 drivers/net/ethernet/mellanox/mlxsw/core.h         |  16 +-
 drivers/net/ethernet/mellanox/mlxsw/i2c.c          |   5 +-
 drivers/net/ethernet/mellanox/mlxsw/pci.c          | 101 ++--
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     | 257 +++++++++-
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |  15 +
 .../net/ethernet/mellanox/mlxsw/spectrum_dpipe.c   |  72 ++-
 .../net/ethernet/mellanox/mlxsw/spectrum_kvdl.c    |  19 +
 include/net/devlink.h                              |  95 ++++
 include/uapi/linux/devlink.h                       |  16 +
 net/core/devlink.c                                 | 539 ++++++++++++++++++---
 11 files changed, 1077 insertions(+), 143 deletions(-)

-- 
2.9.5

[patch net-next RFC v2 01/11] devlink: Add per devlink instance lock

[Jiri Pirko]

From: Arkadi Sharshevsky <arkadis@mellanox.com>

This is a preparation before introducing resources and hot reload support.
Currently there is one global lock protecting all devlink access. This patch
adds per devlink instance lock which protects the internal members which are
the sb/dpipe/resource/ports. By introducing this lock the global devlink port
lock can be discarded.

Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/devlink.h |   1 +
 net/core/devlink.c    | 122 +++++++++++++++++++++++++++-----------------------
 2 files changed, 66 insertions(+), 57 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index b9654e1..4d2c6fc 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -30,6 +30,7 @@ struct devlink {
 	const struct devlink_ops *ops;
 	struct device *dev;
 	possible_net_t _net;
+	struct mutex lock;
 	char priv[0] __aligned(NETDEV_ALIGN);
 };
 
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 7d430c1..0114dfc 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -92,12 +92,6 @@ static LIST_HEAD(devlink_list);
  */
 static DEFINE_MUTEX(devlink_mutex);
 
-/* devlink_port_mutex
- *
- * Shared lock to guard lists of ports in all devlink devices.
- */
-static DEFINE_MUTEX(devlink_port_mutex);
-
 static struct net *devlink_net(const struct devlink *devlink)
 {
 	return read_pnet(&devlink->_net);
@@ -335,15 +329,12 @@ devlink_sb_tc_index_get_from_info(struct devlink_sb *devlink_sb,
 #define DEVLINK_NL_FLAG_NEED_DEVLINK	BIT(0)
 #define DEVLINK_NL_FLAG_NEED_PORT	BIT(1)
 #define DEVLINK_NL_FLAG_NEED_SB		BIT(2)
-#define DEVLINK_NL_FLAG_LOCK_PORTS	BIT(3)
-	/* port is not needed but we need to ensure they don't
-	 * change in the middle of command
-	 */
 
 static int devlink_nl_pre_doit(const struct genl_ops *ops,
 			       struct sk_buff *skb, struct genl_info *info)
 {
 	struct devlink *devlink;
+	int err;
 
 	mutex_lock(&devlink_mutex);
 	devlink = devlink_get_from_info(info);
@@ -351,44 +342,45 @@ static int devlink_nl_pre_doit(const struct genl_ops *ops,
 		mutex_unlock(&devlink_mutex);
 		return PTR_ERR(devlink);
 	}
+	mutex_lock(&devlink->lock);
 	if (ops->internal_flags & DEVLINK_NL_FLAG_NEED_DEVLINK) {
 		info->user_ptr[0] = devlink;
 	} else if (ops->internal_flags & DEVLINK_NL_FLAG_NEED_PORT) {
 		struct devlink_port *devlink_port;
 
-		mutex_lock(&devlink_port_mutex);
 		devlink_port = devlink_port_get_from_info(devlink, info);
 		if (IS_ERR(devlink_port)) {
-			mutex_unlock(&devlink_port_mutex);
-			mutex_unlock(&devlink_mutex);
-			return PTR_ERR(devlink_port);
+			err = PTR_ERR(devlink_port);
+			goto unlock;
 		}
 		info->user_ptr[0] = devlink_port;
 	}
-	if (ops->internal_flags & DEVLINK_NL_FLAG_LOCK_PORTS) {
-		mutex_lock(&devlink_port_mutex);
-	}
 	if (ops->internal_flags & DEVLINK_NL_FLAG_NEED_SB) {
 		struct devlink_sb *devlink_sb;
 
 		devlink_sb = devlink_sb_get_from_info(devlink, info);
 		if (IS_ERR(devlink_sb)) {
-			if (ops->internal_flags & DEVLINK_NL_FLAG_NEED_PORT)
-				mutex_unlock(&devlink_port_mutex);
-			mutex_unlock(&devlink_mutex);
-			return PTR_ERR(devlink_sb);
+			err = PTR_ERR(devlink_sb);
+			goto unlock;
 		}
 		info->user_ptr[1] = devlink_sb;
 	}
+
 	return 0;
+
+unlock:
+	mutex_unlock(&devlink->lock);
+	mutex_unlock(&devlink_mutex);
+	return err;
 }
 
 static void devlink_nl_post_doit(const struct genl_ops *ops,
 				 struct sk_buff *skb, struct genl_info *info)
 {
-	if (ops->internal_flags & DEVLINK_NL_FLAG_NEED_PORT ||
-	    ops->internal_flags & DEVLINK_NL_FLAG_LOCK_PORTS)
-		mutex_unlock(&devlink_port_mutex);
+	struct devlink *devlink;
+
+	devlink = devlink_get_from_info(info);
+	mutex_unlock(&devlink->lock);
 	mutex_unlock(&devlink_mutex);
 }
 
@@ -614,10 +606,10 @@ static int devlink_nl_cmd_port_get_dumpit(struct sk_buff *msg,
 	int err;
 
 	mutex_lock(&devlink_mutex);
-	mutex_lock(&devlink_port_mutex);
 	list_for_each_entry(devlink, &devlink_list, list) {
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
 			continue;
+		mutex_lock(&devlink->lock);
 		list_for_each_entry(devlink_port, &devlink->port_list, list) {
 			if (idx < start) {
 				idx++;
@@ -628,13 +620,15 @@ static int devlink_nl_cmd_port_get_dumpit(struct sk_buff *msg,
 						   NETLINK_CB(cb->skb).portid,
 						   cb->nlh->nlmsg_seq,
 						   NLM_F_MULTI);
-			if (err)
+			if (err) {
+				mutex_unlock(&devlink->lock);
 				goto out;
+			}
 			idx++;
 		}
+		mutex_unlock(&devlink->lock);
 	}
 out:
-	mutex_unlock(&devlink_port_mutex);
 	mutex_unlock(&devlink_mutex);
 
 	cb->args[0] = idx;
@@ -801,6 +795,7 @@ static int devlink_nl_cmd_sb_get_dumpit(struct sk_buff *msg,
 	list_for_each_entry(devlink, &devlink_list, list) {
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
 			continue;
+		mutex_lock(&devlink->lock);
 		list_for_each_entry(devlink_sb, &devlink->sb_list, list) {
 			if (idx < start) {
 				idx++;
@@ -811,10 +806,13 @@ static int devlink_nl_cmd_sb_get_dumpit(struct sk_buff *msg,
 						 NETLINK_CB(cb->skb).portid,
 						 cb->nlh->nlmsg_seq,
 						 NLM_F_MULTI);
-			if (err)
+			if (err) {
+				mutex_unlock(&devlink->lock);
 				goto out;
+			}
 			idx++;
 		}
+		mutex_unlock(&devlink->lock);
 	}
 out:
 	mutex_unlock(&devlink_mutex);
@@ -935,14 +933,18 @@ static int devlink_nl_cmd_sb_pool_get_dumpit(struct sk_buff *msg,
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)) ||
 		    !devlink->ops || !devlink->ops->sb_pool_get)
 			continue;
+		mutex_lock(&devlink->lock);
 		list_for_each_entry(devlink_sb, &devlink->sb_list, list) {
 			err = __sb_pool_get_dumpit(msg, start, &idx, devlink,
 						   devlink_sb,
 						   NETLINK_CB(cb->skb).portid,
 						   cb->nlh->nlmsg_seq);
-			if (err && err != -EOPNOTSUPP)
+			if (err && err != -EOPNOTSUPP) {
+				mutex_unlock(&devlink->lock);
 				goto out;
+			}
 		}
+		mutex_unlock(&devlink->lock);
 	}
 out:
 	mutex_unlock(&devlink_mutex);
@@ -1123,22 +1125,24 @@ static int devlink_nl_cmd_sb_port_pool_get_dumpit(struct sk_buff *msg,
 	int err;
 
 	mutex_lock(&devlink_mutex);
-	mutex_lock(&devlink_port_mutex);
 	list_for_each_entry(devlink, &devlink_list, list) {
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)) ||
 		    !devlink->ops || !devlink->ops->sb_port_pool_get)
 			continue;
+		mutex_lock(&devlink->lock);
 		list_for_each_entry(devlink_sb, &devlink->sb_list, list) {
 			err = __sb_port_pool_get_dumpit(msg, start, &idx,
 							devlink, devlink_sb,
 							NETLINK_CB(cb->skb).portid,
 							cb->nlh->nlmsg_seq);
-			if (err && err != -EOPNOTSUPP)
+			if (err && err != -EOPNOTSUPP) {
+				mutex_unlock(&devlink->lock);
 				goto out;
+			}
 		}
+		mutex_unlock(&devlink->lock);
 	}
 out:
-	mutex_unlock(&devlink_port_mutex);
 	mutex_unlock(&devlink_mutex);
 
 	cb->args[0] = idx;
@@ -1347,23 +1351,26 @@ devlink_nl_cmd_sb_tc_pool_bind_get_dumpit(struct sk_buff *msg,
 	int err;
 
 	mutex_lock(&devlink_mutex);
-	mutex_lock(&devlink_port_mutex);
 	list_for_each_entry(devlink, &devlink_list, list) {
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)) ||
 		    !devlink->ops || !devlink->ops->sb_tc_pool_bind_get)
 			continue;
+
+		mutex_lock(&devlink->lock);
 		list_for_each_entry(devlink_sb, &devlink->sb_list, list) {
 			err = __sb_tc_pool_bind_get_dumpit(msg, start, &idx,
 							   devlink,
 							   devlink_sb,
 							   NETLINK_CB(cb->skb).portid,
 							   cb->nlh->nlmsg_seq);
-			if (err && err != -EOPNOTSUPP)
+			if (err && err != -EOPNOTSUPP) {
+				mutex_unlock(&devlink->lock);
 				goto out;
+			}
 		}
+		mutex_unlock(&devlink->lock);
 	}
 out:
-	mutex_unlock(&devlink_port_mutex);
 	mutex_unlock(&devlink_mutex);
 
 	cb->args[0] = idx;
@@ -2397,8 +2404,7 @@ static const struct genl_ops devlink_nl_ops[] = {
 		.policy = devlink_nl_policy,
 		.flags = GENL_ADMIN_PERM,
 		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK |
-				  DEVLINK_NL_FLAG_NEED_SB |
-				  DEVLINK_NL_FLAG_LOCK_PORTS,
+				  DEVLINK_NL_FLAG_NEED_SB,
 	},
 	{
 		.cmd = DEVLINK_CMD_SB_OCC_MAX_CLEAR,
@@ -2406,8 +2412,7 @@ static const struct genl_ops devlink_nl_ops[] = {
 		.policy = devlink_nl_policy,
 		.flags = GENL_ADMIN_PERM,
 		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK |
-				  DEVLINK_NL_FLAG_NEED_SB |
-				  DEVLINK_NL_FLAG_LOCK_PORTS,
+				  DEVLINK_NL_FLAG_NEED_SB,
 	},
 	{
 		.cmd = DEVLINK_CMD_ESWITCH_GET,
@@ -2488,6 +2493,7 @@ struct devlink *devlink_alloc(const struct devlink_ops *ops, size_t priv_size)
 	INIT_LIST_HEAD(&devlink->port_list);
 	INIT_LIST_HEAD(&devlink->sb_list);
 	INIT_LIST_HEAD_RCU(&devlink->dpipe_table_list);
+	mutex_init(&devlink->lock);
 	return devlink;
 }
 EXPORT_SYMBOL_GPL(devlink_alloc);
@@ -2550,16 +2556,16 @@ int devlink_port_register(struct devlink *devlink,
 			  struct devlink_port *devlink_port,
 			  unsigned int port_index)
 {
-	mutex_lock(&devlink_port_mutex);
+	mutex_lock(&devlink->lock);
 	if (devlink_port_index_exists(devlink, port_index)) {
-		mutex_unlock(&devlink_port_mutex);
+		mutex_unlock(&devlink->lock);
 		return -EEXIST;
 	}
 	devlink_port->devlink = devlink;
 	devlink_port->index = port_index;
 	devlink_port->registered = true;
 	list_add_tail(&devlink_port->list, &devlink->port_list);
-	mutex_unlock(&devlink_port_mutex);
+	mutex_unlock(&devlink->lock);
 	devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
 	return 0;
 }
@@ -2572,10 +2578,12 @@ EXPORT_SYMBOL_GPL(devlink_port_register);
  */
 void devlink_port_unregister(struct devlink_port *devlink_port)
 {
+	struct devlink *devlink = devlink_port->devlink;
+
 	devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_DEL);
-	mutex_lock(&devlink_port_mutex);
+	mutex_lock(&devlink->lock);
 	list_del(&devlink_port->list);
-	mutex_unlock(&devlink_port_mutex);
+	mutex_unlock(&devlink->lock);
 }
 EXPORT_SYMBOL_GPL(devlink_port_unregister);
 
@@ -2651,7 +2659,7 @@ int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
 	struct devlink_sb *devlink_sb;
 	int err = 0;
 
-	mutex_lock(&devlink_mutex);
+	mutex_lock(&devlink->lock);
 	if (devlink_sb_index_exists(devlink, sb_index)) {
 		err = -EEXIST;
 		goto unlock;
@@ -2670,7 +2678,7 @@ int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
 	devlink_sb->egress_tc_count = egress_tc_count;
 	list_add_tail(&devlink_sb->list, &devlink->sb_list);
 unlock:
-	mutex_unlock(&devlink_mutex);
+	mutex_unlock(&devlink->lock);
 	return err;
 }
 EXPORT_SYMBOL_GPL(devlink_sb_register);
@@ -2679,11 +2687,11 @@ void devlink_sb_unregister(struct devlink *devlink, unsigned int sb_index)
 {
 	struct devlink_sb *devlink_sb;
 
-	mutex_lock(&devlink_mutex);
+	mutex_lock(&devlink->lock);
 	devlink_sb = devlink_sb_get_by_index(devlink, sb_index);
 	WARN_ON(!devlink_sb);
 	list_del(&devlink_sb->list);
-	mutex_unlock(&devlink_mutex);
+	mutex_unlock(&devlink->lock);
 	kfree(devlink_sb);
 }
 EXPORT_SYMBOL_GPL(devlink_sb_unregister);
@@ -2699,9 +2707,9 @@ EXPORT_SYMBOL_GPL(devlink_sb_unregister);
 int devlink_dpipe_headers_register(struct devlink *devlink,
 				   struct devlink_dpipe_headers *dpipe_headers)
 {
-	mutex_lock(&devlink_mutex);
+	mutex_lock(&devlink->lock);
 	devlink->dpipe_headers = dpipe_headers;
-	mutex_unlock(&devlink_mutex);
+	mutex_unlock(&devlink->lock);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(devlink_dpipe_headers_register);
@@ -2715,9 +2723,9 @@ EXPORT_SYMBOL_GPL(devlink_dpipe_headers_register);
  */
 void devlink_dpipe_headers_unregister(struct devlink *devlink)
 {
-	mutex_lock(&devlink_mutex);
+	mutex_lock(&devlink->lock);
 	devlink->dpipe_headers = NULL;
-	mutex_unlock(&devlink_mutex);
+	mutex_unlock(&devlink->lock);
 }
 EXPORT_SYMBOL_GPL(devlink_dpipe_headers_unregister);
 
@@ -2783,9 +2791,9 @@ int devlink_dpipe_table_register(struct devlink *devlink,
 	table->priv = priv;
 	table->counter_control_extern = counter_control_extern;
 
-	mutex_lock(&devlink_mutex);
+	mutex_lock(&devlink->lock);
 	list_add_tail_rcu(&table->list, &devlink->dpipe_table_list);
-	mutex_unlock(&devlink_mutex);
+	mutex_unlock(&devlink->lock);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(devlink_dpipe_table_register);
@@ -2801,17 +2809,17 @@ void devlink_dpipe_table_unregister(struct devlink *devlink,
 {
 	struct devlink_dpipe_table *table;
 
-	mutex_lock(&devlink_mutex);
+	mutex_lock(&devlink->lock);
 	table = devlink_dpipe_table_find(&devlink->dpipe_table_list,
 					 table_name);
 	if (!table)
 		goto unlock;
 	list_del_rcu(&table->list);
-	mutex_unlock(&devlink_mutex);
+	mutex_unlock(&devlink->lock);
 	kfree_rcu(table, rcu);
 	return;
 unlock:
-	mutex_unlock(&devlink_mutex);
+	mutex_unlock(&devlink->lock);
 }
 EXPORT_SYMBOL_GPL(devlink_dpipe_table_unregister);
 
-- 
2.9.5

[patch net-next RFC v2 02/11] devlink: Add support for resource abstraction

[Jiri Pirko]

From: Arkadi Sharshevsky <arkadis@mellanox.com>

Add support for hardware resource abstraction over devlink. Each resource
is identified via id, furthermore it contains information regarding its
size and its related sub resources. Each resource can also provide its
current occupancy.

In some cases the sizes of some resources can be changed, yet for those
changes to take place a hot driver reload may be needed. The reload
capability will be introduced in the next patch.

Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/devlink.h        |  80 +++++++++++
 include/uapi/linux/devlink.h |  10 ++
 net/core/devlink.c           | 330 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 420 insertions(+)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 4d2c6fc..960e80a 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -26,6 +26,7 @@ struct devlink {
 	struct list_head port_list;
 	struct list_head sb_list;
 	struct list_head dpipe_table_list;
+	struct list_head resource_list;
 	struct devlink_dpipe_headers *dpipe_headers;
 	const struct devlink_ops *ops;
 	struct device *dev;
@@ -224,6 +225,45 @@ struct devlink_dpipe_headers {
 	unsigned int headers_count;
 };
 
+/**
+ * struct devlink_resource_ops - resource ops
+ * @occ_get - get the occupied size
+ * @size_validate - validate the size of the resource before update, reload
+ *                  is needed for changes to take place
+ */
+struct devlink_resource_ops {
+	u64 (*occ_get)(struct devlink *devlink);
+	int (*size_validate)(struct devlink *devlink, u64 size,
+			     struct list_head *resource_list,
+			     struct netlink_ext_ack *extack);
+};
+
+/**
+ * struct devlink_resource - devlink resource
+ * @name - name of the resource
+ * @size - size of the resource
+ * @size_new - updated size of the resource, reload is needed
+ * @id - id, per devlink instance
+ * @reload_required - reload is required for new configuration to apply
+ * @parent - parent resource
+ * @list - parent list
+ * @resource_list - list of child resources
+ * @resource_ops - resource ops
+ */
+struct devlink_resource {
+	const char *name;
+	u64 size;
+	u64 size_new;
+	u64 id;
+	bool reload_required;
+	struct devlink_resource *parent;
+	struct list_head list;
+	struct list_head resource_list;
+	struct devlink_resource_ops *resource_ops;
+};
+
+#define DEVLINK_RESOURCE_ID_PARENT_TOP 0
+
 struct devlink_ops {
 	int (*port_type_set)(struct devlink_port *devlink_port,
 			     enum devlink_port_type port_type);
@@ -333,6 +373,20 @@ extern struct devlink_dpipe_header devlink_dpipe_header_ethernet;
 extern struct devlink_dpipe_header devlink_dpipe_header_ipv4;
 extern struct devlink_dpipe_header devlink_dpipe_header_ipv6;
 
+int devlink_resource_register(struct devlink *devlink,
+			      const char *resource_name,
+			      bool top_hierarchy,
+			      bool reload_required,
+			      u64 resource_size,
+			      u64 resource_id,
+			      u64 parent_resource_id,
+			      struct devlink_resource_ops *resource_ops);
+void devlink_resources_unregister(struct devlink *devlink,
+				  struct devlink_resource *resource);
+int devlink_resource_size_get(struct devlink *devlink,
+			      u64 resource_id,
+			      u64 *p_resource_size);
+
 #else
 
 static inline struct devlink *devlink_alloc(const struct devlink_ops *ops,
@@ -469,6 +523,32 @@ devlink_dpipe_match_put(struct sk_buff *skb,
 	return 0;
 }
 
+static inline int
+devlink_resource_register(struct devlink *devlink,
+			  const char *resource_name,
+			  bool top_hierarchy,
+			  bool reload_required,
+			  u64 resource_size,
+			  u64 resource_id,
+			  u64 parent_resource_id,
+			  struct devlink_resource_ops *resource_ops)
+{
+	return 0;
+}
+
+static inline void
+devlink_resources_unregister(struct devlink *devlink,
+			     struct devlink_resource *resource)
+{
+}
+
+static inline int
+devlink_resource_size_get(struct devlink *devlink, u64 resource_id,
+			  u64 *p_resource_size)
+{
+	return -EINVAL;
+}
+
 #endif
 
 #endif /* _NET_DEVLINK_H_ */
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 6665df6..1ee1c72 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -70,6 +70,8 @@ enum devlink_command {
 	DEVLINK_CMD_DPIPE_ENTRIES_GET,
 	DEVLINK_CMD_DPIPE_HEADERS_GET,
 	DEVLINK_CMD_DPIPE_TABLE_COUNTERS_SET,
+	DEVLINK_CMD_RESOURCE_SET,
+	DEVLINK_CMD_RESOURCE_DUMP,
 
 	/* add new commands above here */
 	__DEVLINK_CMD_MAX,
@@ -202,6 +204,14 @@ enum devlink_attr {
 	DEVLINK_ATTR_PAD,
 
 	DEVLINK_ATTR_ESWITCH_ENCAP_MODE,	/* u8 */
+	DEVLINK_ATTR_RESOURCE_LIST,		/* nested */
+	DEVLINK_ATTR_RESOURCE,			/* nested */
+	DEVLINK_ATTR_RESOURCE_NAME,		/* string */
+	DEVLINK_ATTR_RESOURCE_SIZE,		/* u64 */
+	DEVLINK_ATTR_RESOURCE_SIZE_NEW,		/* u64 */
+	DEVLINK_ATTR_RESOURCE_OCC,		/* u64 */
+	DEVLINK_ATTR_RESOURCE_ID,		/* u64 */
+	DEVLINK_ATTR_RESOURCE_RELOAD_REQUIRED,  /* u8  */
 
 	/* add new attributes above here, update the policy in devlink.c */
 
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 0114dfc..6ae644f 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2280,6 +2280,194 @@ static int devlink_nl_cmd_dpipe_table_counters_set(struct sk_buff *skb,
 						counters_enable);
 }
 
+struct devlink_resource *
+devlink_resource_find(struct devlink *devlink,
+		      struct devlink_resource *resource, u64 resource_id)
+{
+	struct list_head *resource_list;
+
+	if (resource)
+		resource_list = &resource->resource_list;
+	else
+		resource_list = &devlink->resource_list;
+
+	list_for_each_entry(resource, resource_list, list) {
+		struct devlink_resource *__resource;
+
+		if (resource->id == resource_id)
+			return resource;
+
+		__resource = devlink_resource_find(devlink, resource,
+						   resource_id);
+		if (__resource)
+			return __resource;
+	}
+	return NULL;
+}
+
+static int devlink_nl_cmd_resource_set(struct sk_buff *skb,
+				       struct genl_info *info)
+{
+	struct devlink *devlink = info->user_ptr[0];
+	struct devlink_resource *resource;
+	u64 resource_id;
+	u64 size;
+	int err;
+
+	if (!info->attrs[DEVLINK_ATTR_RESOURCE_ID] ||
+	    !info->attrs[DEVLINK_ATTR_RESOURCE_SIZE])
+		return -EINVAL;
+	resource_id = nla_get_u64(info->attrs[DEVLINK_ATTR_RESOURCE_ID]);
+
+	resource = devlink_resource_find(devlink, NULL, resource_id);
+	if (!resource)
+		return -EINVAL;
+
+	if (!resource->resource_ops->size_validate)
+		return -EINVAL;
+
+	size = nla_get_u64(info->attrs[DEVLINK_ATTR_RESOURCE_SIZE]);
+	err = resource->resource_ops->size_validate(devlink, size,
+						    &resource->resource_list,
+						    info->extack);
+	if (err)
+		return err;
+
+	resource->size_new = size;
+	return 0;
+}
+
+static int devlink_resource_put(struct devlink *devlink, struct sk_buff *skb,
+				struct devlink_resource *resource)
+{
+	struct devlink_resource *__resource;
+	struct nlattr *__resource_attr;
+	struct nlattr *resource_attr;
+
+	resource_attr = nla_nest_start(skb, DEVLINK_ATTR_RESOURCE);
+	if (!resource_attr)
+		return -EMSGSIZE;
+
+	if (nla_put_string(skb, DEVLINK_ATTR_RESOURCE_NAME, resource->name) ||
+	    nla_put_u8(skb, DEVLINK_ATTR_RESOURCE_RELOAD_REQUIRED,
+		       resource->reload_required) ||
+	    nla_put_u64_64bit(skb, DEVLINK_ATTR_RESOURCE_SIZE, resource->size,
+			      DEVLINK_ATTR_PAD) ||
+	    nla_put_u64_64bit(skb, DEVLINK_ATTR_RESOURCE_ID, resource->id,
+			      DEVLINK_ATTR_PAD))
+		goto nla_put_failure;
+	if (resource->size != resource->size_new)
+		nla_put_u64_64bit(skb, DEVLINK_ATTR_RESOURCE_SIZE_NEW,
+				  resource->size_new, DEVLINK_ATTR_PAD);
+	if (resource->resource_ops && resource->resource_ops->occ_get)
+		nla_put_u64_64bit(skb, DEVLINK_ATTR_RESOURCE_OCC,
+				  resource->resource_ops->occ_get(devlink),
+				  DEVLINK_ATTR_PAD);
+	if (list_empty(&resource->resource_list))
+		goto out;
+
+	__resource_attr = nla_nest_start(skb, DEVLINK_ATTR_RESOURCE_LIST);
+	if (!__resource_attr)
+		goto nla_put_failure;
+
+	list_for_each_entry(__resource, &resource->resource_list, list) {
+		if (devlink_resource_put(devlink, skb, __resource))
+			goto resource_put_failure;
+	}
+
+	nla_nest_end(skb, __resource_attr);
+out:
+	nla_nest_end(skb, resource_attr);
+	return 0;
+
+resource_put_failure:
+	nla_nest_cancel(skb, __resource_attr);
+nla_put_failure:
+	nla_nest_cancel(skb, resource_attr);
+	return -EMSGSIZE;
+}
+
+static int devlink_resource_fill(struct genl_info *info,
+				 enum devlink_command cmd, int flags)
+{
+	struct devlink *devlink = info->user_ptr[0];
+	struct devlink_resource *resource;
+	struct nlattr *resources_attr;
+	struct sk_buff *skb = NULL;
+	struct nlmsghdr *nlh;
+	bool incomplete;
+	void *hdr;
+	int i;
+	int err;
+
+	resource = list_first_entry(&devlink->resource_list,
+				    struct devlink_resource, list);
+start_again:
+	err = devlink_dpipe_send_and_alloc_skb(&skb, info);
+	if (err)
+		return err;
+
+	hdr = genlmsg_put(skb, info->snd_portid, info->snd_seq,
+			  &devlink_nl_family, NLM_F_MULTI, cmd);
+	if (!hdr) {
+		nlmsg_free(skb);
+		return -EMSGSIZE;
+	}
+
+	if (devlink_nl_put_handle(skb, devlink))
+		goto nla_put_failure;
+
+	resources_attr = nla_nest_start(skb, DEVLINK_ATTR_RESOURCE_LIST);
+	if (!resources_attr)
+		goto nla_put_failure;
+
+	incomplete = false;
+	i = 0;
+	list_for_each_entry_from(resource, &devlink->resource_list, list) {
+		err = devlink_resource_put(devlink, skb, resource);
+		if (err) {
+			if (!i)
+				goto err_resource_put;
+			incomplete = true;
+			break;
+		}
+		i++;
+	}
+	nla_nest_end(skb, resources_attr);
+	genlmsg_end(skb, hdr);
+	if (incomplete)
+		goto start_again;
+send_done:
+	nlh = nlmsg_put(skb, info->snd_portid, info->snd_seq,
+			NLMSG_DONE, 0, flags | NLM_F_MULTI);
+	if (!nlh) {
+		err = devlink_dpipe_send_and_alloc_skb(&skb, info);
+		if (err)
+			goto err_skb_send_alloc;
+		goto send_done;
+	}
+	return genlmsg_reply(skb, info);
+
+nla_put_failure:
+	err = -EMSGSIZE;
+err_resource_put:
+err_skb_send_alloc:
+	genlmsg_cancel(skb, hdr);
+	nlmsg_free(skb);
+	return err;
+}
+
+static int devlink_nl_cmd_resource_dump(struct sk_buff *skb,
+					struct genl_info *info)
+{
+	struct devlink *devlink = info->user_ptr[0];
+
+	if (list_empty(&devlink->resource_list))
+		return -EOPNOTSUPP;
+
+	return devlink_resource_fill(info, DEVLINK_CMD_RESOURCE_DUMP, 0);
+}
+
 static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
 	[DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING },
 	[DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING },
@@ -2456,6 +2644,20 @@ static const struct genl_ops devlink_nl_ops[] = {
 		.flags = GENL_ADMIN_PERM,
 		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
 	},
+	{
+		.cmd = DEVLINK_CMD_RESOURCE_SET,
+		.doit = devlink_nl_cmd_resource_set,
+		.policy = devlink_nl_policy,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
+	},
+	{
+		.cmd = DEVLINK_CMD_RESOURCE_DUMP,
+		.doit = devlink_nl_cmd_resource_dump,
+		.policy = devlink_nl_policy,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
+	},
 };
 
 static struct genl_family devlink_nl_family __ro_after_init = {
@@ -2493,6 +2695,7 @@ struct devlink *devlink_alloc(const struct devlink_ops *ops, size_t priv_size)
 	INIT_LIST_HEAD(&devlink->port_list);
 	INIT_LIST_HEAD(&devlink->sb_list);
 	INIT_LIST_HEAD_RCU(&devlink->dpipe_table_list);
+	INIT_LIST_HEAD(&devlink->resource_list);
 	mutex_init(&devlink->lock);
 	return devlink;
 }
@@ -2823,6 +3026,133 @@ void devlink_dpipe_table_unregister(struct devlink *devlink,
 }
 EXPORT_SYMBOL_GPL(devlink_dpipe_table_unregister);
 
+/**
+ *	devlink_resource_register - devlink resource register
+ *
+ *	@devlink: devlink
+ *	@resource_name: resource's name
+ *	@top_hierarchy: top hierarchy
+ *	@reload_required: reload is required for new configuration to
+ *			  apply
+ *	@resource_size: resource's size
+ *	@resource_id: resource's id
+ *	@parent_reosurce_id: resource's parent id
+ *	@resource_ops: resource ops
+ */
+int devlink_resource_register(struct devlink *devlink,
+			      const char *resource_name,
+			      bool top_hierarchy,
+			      bool reload_required,
+			      u64 resource_size,
+			      u64 resource_id,
+			      u64 parent_resource_id,
+			      struct devlink_resource_ops *resource_ops)
+{
+	struct devlink_resource *resource;
+	struct list_head *resource_list;
+	int err = 0;
+
+	mutex_lock(&devlink->lock);
+	resource = devlink_resource_find(devlink, NULL, resource_id);
+	if (resource) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	if (top_hierarchy) {
+		resource_list = &devlink->resource_list;
+	} else {
+		struct devlink_resource *parent_resource;
+
+		parent_resource = devlink_resource_find(devlink, NULL,
+							parent_resource_id);
+		if (parent_resource) {
+			resource_list = &parent_resource->resource_list;
+		} else {
+			err = -EINVAL;
+			goto out;
+		}
+	}
+
+	resource = kzalloc(sizeof(*resource), GFP_KERNEL);
+	if (!resource) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	resource->name = resource_name;
+	resource->size = resource_size;
+	resource->size_new = resource_size;
+	resource->id = resource_id;
+	resource->reload_required = reload_required;
+	resource->resource_ops = resource_ops;
+	INIT_LIST_HEAD(&resource->resource_list);
+	list_add_tail(&resource->list, resource_list);
+out:
+	mutex_unlock(&devlink->lock);
+	return err;
+}
+EXPORT_SYMBOL_GPL(devlink_resource_register);
+
+/**
+ *	devlink_resources_unregister - free all resources
+ *
+ *	@devlink: devlink
+ *	@resource: resource
+ */
+void devlink_resources_unregister(struct devlink *devlink,
+				  struct devlink_resource *resource)
+{
+	struct devlink_resource *tmp, *__resource;
+	struct list_head *resource_list;
+
+	if (resource)
+		resource_list = &resource->resource_list;
+	else
+		resource_list = &devlink->resource_list;
+
+	if (!resource)
+		mutex_lock(&devlink->lock);
+
+	list_for_each_entry_safe(__resource, tmp, resource_list, list) {
+		devlink_resources_unregister(devlink, __resource);
+		list_del(&__resource->list);
+		kfree(__resource);
+	}
+
+	if (!resource)
+		mutex_unlock(&devlink->lock);
+}
+EXPORT_SYMBOL_GPL(devlink_resources_unregister);
+
+/**
+ *	devlink_resource_size_get - get and update size
+ *
+ *	@devlink: devlink
+ *	@resource_id: the requested resource id
+ *	@p_resource_size: ptr to update
+ */
+int devlink_resource_size_get(struct devlink *devlink,
+			      u64 resource_id,
+			      u64 *p_resource_size)
+{
+	struct devlink_resource *resource;
+	int err = 0;
+
+	mutex_lock(&devlink->lock);
+	resource = devlink_resource_find(devlink, NULL, resource_id);
+	if (!resource) {
+		err = -EINVAL;
+		goto out;
+	}
+	*p_resource_size = resource->size_new;
+	resource->size = resource->size_new;
+out:
+	mutex_unlock(&devlink->lock);
+	return err;
+}
+EXPORT_SYMBOL_GPL(devlink_resource_size_get);
+
 static int __init devlink_module_init(void)
 {
 	return genl_register_family(&devlink_nl_family);
-- 
2.9.5

Re: [patch net-next RFC v2 02/11] devlink: Add support for resource abstraction

[Jakub Kicinski]

On Tue, 14 Nov 2017 17:18:43 +0100, Jiri Pirko wrote:
> From: Arkadi Sharshevsky <arkadis@mellanox.com>
> 
> Add support for hardware resource abstraction over devlink. Each resource
> is identified via id, furthermore it contains information regarding its
> size and its related sub resources. Each resource can also provide its
> current occupancy.
> 
> In some cases the sizes of some resources can be changed, yet for those
> changes to take place a hot driver reload may be needed. The reload
> capability will be introduced in the next patch.
> 
> Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  include/net/devlink.h        |  80 +++++++++++
>  include/uapi/linux/devlink.h |  10 ++
>  net/core/devlink.c           | 330 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 420 insertions(+)
> 
> diff --git a/include/net/devlink.h b/include/net/devlink.h
> index 4d2c6fc..960e80a 100644
> --- a/include/net/devlink.h
> +++ b/include/net/devlink.h
> @@ -224,6 +225,45 @@ struct devlink_dpipe_headers {
>  	unsigned int headers_count;
>  };
>  
> +/**
> + * struct devlink_resource_ops - resource ops
> + * @occ_get - get the occupied size
> + * @size_validate - validate the size of the resource before update, reload

nit:
@member: is more common and used throughout this file, rather than
@member - 

> + *                  is needed for changes to take place
> + */
> +struct devlink_resource_ops {
> +	u64 (*occ_get)(struct devlink *devlink);
> +	int (*size_validate)(struct devlink *devlink, u64 size,
> +			     struct list_head *resource_list,
> +			     struct netlink_ext_ack *extack);
> +};

Re: [patch net-next RFC v2 02/11] devlink: Add support for resource abstraction

[Arkadi Sharshevsky]

On 11/15/2017 09:59 AM, Jakub Kicinski wrote:
> On Tue, 14 Nov 2017 17:18:43 +0100, Jiri Pirko wrote:
>> From: Arkadi Sharshevsky <arkadis@mellanox.com>
>>
>> Add support for hardware resource abstraction over devlink. Each resource
>> is identified via id, furthermore it contains information regarding its
>> size and its related sub resources. Each resource can also provide its
>> current occupancy.
>>
>> In some cases the sizes of some resources can be changed, yet for those
>> changes to take place a hot driver reload may be needed. The reload
>> capability will be introduced in the next patch.
>>
>> Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>>  include/net/devlink.h        |  80 +++++++++++
>>  include/uapi/linux/devlink.h |  10 ++
>>  net/core/devlink.c           | 330 +++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 420 insertions(+)
>>
>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>> index 4d2c6fc..960e80a 100644
>> --- a/include/net/devlink.h
>> +++ b/include/net/devlink.h
>> @@ -224,6 +225,45 @@ struct devlink_dpipe_headers {
>>  	unsigned int headers_count;
>>  };
>>  
>> +/**
>> + * struct devlink_resource_ops - resource ops
>> + * @occ_get - get the occupied size
>> + * @size_validate - validate the size of the resource before update, reload
> 
> nit:
> @member: is more common and used throughout this file, rather than
> @member - 
> 

Will fix, thanks for the review.

>> + *                  is needed for changes to take place
>> + */
>> +struct devlink_resource_ops {
>> +	u64 (*occ_get)(struct devlink *devlink);
>> +	int (*size_validate)(struct devlink *devlink, u64 size,
>> +			     struct list_head *resource_list,
>> +			     struct netlink_ext_ack *extack);
>> +};

Re: [patch net-next RFC v2 02/11] devlink: Add support for resource abstraction

[David Ahern]

On 11/14/17 9:18 AM, Jiri Pirko wrote:
> diff --git a/include/net/devlink.h b/include/net/devlink.h
> index 4d2c6fc..960e80a 100644
> --- a/include/net/devlink.h
> +++ b/include/net/devlink.h
...

> @@ -469,6 +523,32 @@ devlink_dpipe_match_put(struct sk_buff *skb,
>  	return 0;
>  }
>  
> +static inline int
> +devlink_resource_register(struct devlink *devlink,
> +			  const char *resource_name,
> +			  bool top_hierarchy,
> +			  bool reload_required,
> +			  u64 resource_size,
> +			  u64 resource_id,
> +			  u64 parent_resource_id,
> +			  struct devlink_resource_ops *resource_ops)
> +{
> +	return 0;
> +}
> +
> +static inline void
> +devlink_resources_unregister(struct devlink *devlink,
> +			     struct devlink_resource *resource)
> +{
> +}
> +
> +static inline int
> +devlink_resource_size_get(struct devlink *devlink, u64 resource_id,
> +			  u64 *p_resource_size)
> +{
> +	return -EINVAL;

It's compiled out so -EOPNOTSUPP seems more appropriate.



> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index 0114dfc..6ae644f 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> +static int devlink_nl_cmd_resource_set(struct sk_buff *skb,
> +				       struct genl_info *info)
> +{
> +	struct devlink *devlink = info->user_ptr[0];
> +	struct devlink_resource *resource;
> +	u64 resource_id;
> +	u64 size;
> +	int err;
> +
> +	if (!info->attrs[DEVLINK_ATTR_RESOURCE_ID] ||
> +	    !info->attrs[DEVLINK_ATTR_RESOURCE_SIZE])
> +		return -EINVAL;

several of the of the DEVLINK_ATTR_RESOURCE attributes are kernel to
user only (e.g., DEVLINK_ATTR_RESOURCE_SIZE_NEW and
DEVLINK_ATTR_RESOURCE_RELOAD_REQUIRED), so if they are given by the user
that should be an error too right?


> +	resource_id = nla_get_u64(info->attrs[DEVLINK_ATTR_RESOURCE_ID]);

I don't see where these attributes are validated for proper size.

> +
> +	resource = devlink_resource_find(devlink, NULL, resource_id);
> +	if (!resource)
> +		return -EINVAL;
> +
> +	if (!resource->resource_ops->size_validate)
> +		return -EINVAL;

genl_info has extack; please add user messages for the above failures.


> +
> +	size = nla_get_u64(info->attrs[DEVLINK_ATTR_RESOURCE_SIZE]);
> +	err = resource->resource_ops->size_validate(devlink, size,
> +						    &resource->resource_list,
> +						    info->extack);
> +	if (err)
> +		return err;
> +
> +	resource->size_new = size;
> +	return 0;
> +}
> +

Re: [patch net-next RFC v2 02/11] devlink: Add support for resource abstraction

[Arkadi Sharshevsky]

On 11/18/2017 08:34 PM, David Ahern wrote:
> On 11/14/17 9:18 AM, Jiri Pirko wrote:
>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>> index 4d2c6fc..960e80a 100644
>> --- a/include/net/devlink.h
>> +++ b/include/net/devlink.h
> ...
> 
>> @@ -469,6 +523,32 @@ devlink_dpipe_match_put(struct sk_buff *skb,
>>  	return 0;
>>  }
>>  
>> +static inline int
>> +devlink_resource_register(struct devlink *devlink,
>> +			  const char *resource_name,
>> +			  bool top_hierarchy,
>> +			  bool reload_required,
>> +			  u64 resource_size,
>> +			  u64 resource_id,
>> +			  u64 parent_resource_id,
>> +			  struct devlink_resource_ops *resource_ops)
>> +{
>> +	return 0;
>> +}
>> +
>> +static inline void
>> +devlink_resources_unregister(struct devlink *devlink,
>> +			     struct devlink_resource *resource)
>> +{
>> +}
>> +
>> +static inline int
>> +devlink_resource_size_get(struct devlink *devlink, u64 resource_id,
>> +			  u64 *p_resource_size)
>> +{
>> +	return -EINVAL;
> 
> It's compiled out so -EOPNOTSUPP seems more appropriate.
> 

will fix

> 
> 
>> diff --git a/net/core/devlink.c b/net/core/devlink.c
>> index 0114dfc..6ae644f 100644
>> --- a/net/core/devlink.c
>> +++ b/net/core/devlink.c
>> +static int devlink_nl_cmd_resource_set(struct sk_buff *skb,
>> +				       struct genl_info *info)
>> +{
>> +	struct devlink *devlink = info->user_ptr[0];
>> +	struct devlink_resource *resource;
>> +	u64 resource_id;
>> +	u64 size;
>> +	int err;
>> +
>> +	if (!info->attrs[DEVLINK_ATTR_RESOURCE_ID] ||
>> +	    !info->attrs[DEVLINK_ATTR_RESOURCE_SIZE])
>> +		return -EINVAL;
> 
> several of the of the DEVLINK_ATTR_RESOURCE attributes are kernel to
> user only (e.g., DEVLINK_ATTR_RESOURCE_SIZE_NEW and
> DEVLINK_ATTR_RESOURCE_RELOAD_REQUIRED), so if they are given by the user
> that should be an error too right?
> 

Not sure I understood. As you see I only check for the mandatory
attributes, if the user provides not relevant data its ignored.

We use one single nla_policy for all the commands (devlink_nl_policy)

> 
>> +	resource_id = nla_get_u64(info->attrs[DEVLINK_ATTR_RESOURCE_ID]);
> 
> I don't see where these attributes are validated for proper size.
> 

right, forgot to update the policy.

>> +
>> +	resource = devlink_resource_find(devlink, NULL, resource_id);
>> +	if (!resource)
>> +		return -EINVAL;
>> +
>> +	if (!resource->resource_ops->size_validate)
>> +		return -EINVAL;
> 
> genl_info has extack; please add user messages for the above failures.
> 

Isn't EOPNOTSUPP enough ?

> 
>> +
>> +	size = nla_get_u64(info->attrs[DEVLINK_ATTR_RESOURCE_SIZE]);
>> +	err = resource->resource_ops->size_validate(devlink, size,
>> +						    &resource->resource_list,
>> +						    info->extack);
>> +	if (err)
>> +		return err;
>> +
>> +	resource->size_new = size;
>> +	return 0;
>> +}
>> +
> 

Re: [patch net-next RFC v2 02/11] devlink: Add support for resource abstraction

[David Ahern]

On 11/19/17 1:17 AM, Arkadi Sharshevsky wrote:
> 
> 
> On 11/18/2017 08:34 PM, David Ahern wrote:
>> On 11/14/17 9:18 AM, Jiri Pirko wrote:
>>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>>> index 4d2c6fc..960e80a 100644
>>> --- a/include/net/devlink.h
>>> +++ b/include/net/devlink.h
>> ...
>>
>>> @@ -469,6 +523,32 @@ devlink_dpipe_match_put(struct sk_buff *skb,
>>>  	return 0;
>>>  }
>>>  
>>> +static inline int
>>> +devlink_resource_register(struct devlink *devlink,
>>> +			  const char *resource_name,
>>> +			  bool top_hierarchy,
>>> +			  bool reload_required,
>>> +			  u64 resource_size,
>>> +			  u64 resource_id,
>>> +			  u64 parent_resource_id,
>>> +			  struct devlink_resource_ops *resource_ops)
>>> +{
>>> +	return 0;
>>> +}
>>> +
>>> +static inline void
>>> +devlink_resources_unregister(struct devlink *devlink,
>>> +			     struct devlink_resource *resource)
>>> +{
>>> +}
>>> +
>>> +static inline int
>>> +devlink_resource_size_get(struct devlink *devlink, u64 resource_id,
>>> +			  u64 *p_resource_size)
>>> +{
>>> +	return -EINVAL;
>>
>> It's compiled out so -EOPNOTSUPP seems more appropriate.
>>
> 
> will fix
> 
>>
>>
>>> diff --git a/net/core/devlink.c b/net/core/devlink.c
>>> index 0114dfc..6ae644f 100644
>>> --- a/net/core/devlink.c
>>> +++ b/net/core/devlink.c
>>> +static int devlink_nl_cmd_resource_set(struct sk_buff *skb,
>>> +				       struct genl_info *info)
>>> +{
>>> +	struct devlink *devlink = info->user_ptr[0];
>>> +	struct devlink_resource *resource;
>>> +	u64 resource_id;
>>> +	u64 size;
>>> +	int err;
>>> +
>>> +	if (!info->attrs[DEVLINK_ATTR_RESOURCE_ID] ||
>>> +	    !info->attrs[DEVLINK_ATTR_RESOURCE_SIZE])
>>> +		return -EINVAL;
>>
>> several of the of the DEVLINK_ATTR_RESOURCE attributes are kernel to
>> user only (e.g., DEVLINK_ATTR_RESOURCE_SIZE_NEW and
>> DEVLINK_ATTR_RESOURCE_RELOAD_REQUIRED), so if they are given by the user
>> that should be an error too right?
>>
> 
> Not sure I understood. As you see I only check for the mandatory
> attributes, if the user provides not relevant data its ignored.
> 
> We use one single nla_policy for all the commands (devlink_nl_policy)
> 
>>
>>> +	resource_id = nla_get_u64(info->attrs[DEVLINK_ATTR_RESOURCE_ID]);
>>
>> I don't see where these attributes are validated for proper size.
>>
> 
> right, forgot to update the policy.
> 
>>> +
>>> +	resource = devlink_resource_find(devlink, NULL, resource_id);
>>> +	if (!resource)
>>> +		return -EINVAL;
>>> +
>>> +	if (!resource->resource_ops->size_validate)
>>> +		return -EINVAL;
>>
>> genl_info has extack; please add user messages for the above failures.
>>
> 
> Isn't EOPNOTSUPP enough ?

No, I mean every failure above returns EINVAL. Add an extack message
telling the user what is wrong. e.g,

	resource = devlink_resource_find(devlink, NULL, resource_id);
	if (!resource) {
		NL_SET_ERR_MSG(extack, "Invalid resource id");
		return -EINVAL;
	}

similarly for the rest.

Re: [patch net-next RFC v2 02/11] devlink: Add support for resource abstraction

[Arkadi Sharshevsky]

[...]
>>>> +
>>>> +	resource = devlink_resource_find(devlink, NULL, resource_id);
>>>> +	if (!resource)
>>>> +		return -EINVAL;
>>>> +
>>>> +	if (!resource->resource_ops->size_validate)
>>>> +		return -EINVAL;
>>>
>>> genl_info has extack; please add user messages for the above failures.
>>>
>>
>> Isn't EOPNOTSUPP enough ?
> 
> No, I mean every failure above returns EINVAL. Add an extack message
> telling the user what is wrong. e.g,
> 
> 	resource = devlink_resource_find(devlink, NULL, resource_id);
> 	if (!resource) {
> 		NL_SET_ERR_MSG(extack, "Invalid resource id");
> 		return -EINVAL;
> 	}
> 
> similarly for the rest.
> 

I don't understand why actually, extended ack should be used when
typical errno is not enough, for example something driver specific
like "KVD overlapping is not supported".

But here if the user provided id for setting the resource, I think
EINVAL is enough if devlink cannot find it.

Re: [patch net-next RFC v2 02/11] devlink: Add support for resource abstraction

[David Ahern]

On 11/23/17 5:25 AM, Arkadi Sharshevsky wrote:
> [...]
>>>>> +
>>>>> +	resource = devlink_resource_find(devlink, NULL, resource_id);
>>>>> +	if (!resource)
>>>>> +		return -EINVAL;
>>>>> +
>>>>> +	if (!resource->resource_ops->size_validate)
>>>>> +		return -EINVAL;
>>>>
>>>> genl_info has extack; please add user messages for the above failures.
>>>>
>>>
>>> Isn't EOPNOTSUPP enough ?
>>
>> No, I mean every failure above returns EINVAL. Add an extack message
>> telling the user what is wrong. e.g,
>>
>> 	resource = devlink_resource_find(devlink, NULL, resource_id);
>> 	if (!resource) {
>> 		NL_SET_ERR_MSG(extack, "Invalid resource id");
>> 		return -EINVAL;
>> 	}
>>
>> similarly for the rest.
>>
> 
> I don't understand why actually, extended ack should be used when
> typical errno is not enough, for example something driver specific
> like "KVD overlapping is not supported".
> 
> But here if the user provided id for setting the resource, I think
> EINVAL is enough if devlink cannot find it.
> 

EINVAL is returned in multiple places, so an EINVAL response does not
uniquely tell you the problem.

Further, we have infrastructure to give users explicit text on why the
request fails. Let's use it and make Linux more user friendly.

[patch net-next RFC v2 03/11] devlink: Add support for reload

[Jiri Pirko]

From: Arkadi Sharshevsky <arkadis@mellanox.com>

Add support for performing driver hot reload.

Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/devlink.h        |  1 +
 include/uapi/linux/devlink.h |  5 ++++
 net/core/devlink.c           | 56 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 62 insertions(+)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 960e80a..a33bda4 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -265,6 +265,7 @@ struct devlink_resource {
 #define DEVLINK_RESOURCE_ID_PARENT_TOP 0
 
 struct devlink_ops {
+	int (*reload)(struct devlink *devlink);
 	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,
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 1ee1c72..ea4fa25 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -73,6 +73,11 @@ enum devlink_command {
 	DEVLINK_CMD_RESOURCE_SET,
 	DEVLINK_CMD_RESOURCE_DUMP,
 
+	/* Hot driver reload, used to query updated resources. The devlink
+	 * instance is not released during the process
+	 */
+	DEVLINK_CMD_RELOAD,
+
 	/* add new commands above here */
 	__DEVLINK_CMD_MAX,
 	DEVLINK_CMD_MAX = __DEVLINK_CMD_MAX - 1
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 6ae644f..d93f176 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2468,6 +2468,55 @@ static int devlink_nl_cmd_resource_dump(struct sk_buff *skb,
 	return devlink_resource_fill(info, DEVLINK_CMD_RESOURCE_DUMP, 0);
 }
 
+static int
+devlink_resources_validate(struct devlink *devlink,
+			   struct devlink_resource *resource,
+			   struct genl_info *info)
+{
+	struct list_head *resource_list;
+	int err = 0;
+
+	if (resource)
+		resource_list = &resource->resource_list;
+	else
+		resource_list = &devlink->resource_list;
+
+	list_for_each_entry(resource, resource_list, list) {
+		if (resource->resource_ops &&
+		    resource->resource_ops->size_validate) {
+			err = resource->resource_ops->size_validate(devlink,
+								    resource->size,
+								    &resource->resource_list,
+								    info->extack);
+			if (err)
+				return err;
+		}
+		err = devlink_resources_validate(devlink, resource, info);
+		if (err)
+			return err;
+	}
+	return err;
+}
+
+static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
+{
+	struct devlink *devlink = info->user_ptr[0];
+	int err;
+
+	if (!devlink->ops->reload)
+		return -EOPNOTSUPP;
+
+	err = devlink_resources_validate(devlink, NULL, info);
+	if (err)
+		return err;
+
+	mutex_unlock(&devlink->lock);
+	err = devlink->ops->reload(devlink);
+	mutex_lock(&devlink->lock);
+
+	return err;
+}
+
 static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
 	[DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING },
 	[DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING },
@@ -2658,6 +2707,13 @@ static const struct genl_ops devlink_nl_ops[] = {
 		.flags = GENL_ADMIN_PERM,
 		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
 	},
+	{
+		.cmd = DEVLINK_CMD_RELOAD,
+		.doit = devlink_nl_cmd_reload,
+		.policy = devlink_nl_policy,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
+	},
 };
 
 static struct genl_family devlink_nl_family __ro_after_init = {
-- 
2.9.5

Re: [patch net-next RFC v2 03/11] devlink: Add support for reload

[Jakub Kicinski]

On Tue, 14 Nov 2017 17:18:44 +0100, Jiri Pirko wrote:
> +static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
> +{
> +	struct devlink *devlink = info->user_ptr[0];
> +	int err;
> +
> +	if (!devlink->ops->reload)
> +		return -EOPNOTSUPP;
> +
> +	err = devlink_resources_validate(devlink, NULL, info);
> +	if (err)
> +		return err;
> +
> +	mutex_unlock(&devlink->lock);
> +	err = devlink->ops->reload(devlink);
> +	mutex_lock(&devlink->lock);
> +
> +	return err;
> +}

I'm a bit confused with the locking, why is devlink->lock not held
around the validation?

Re: [patch net-next RFC v2 03/11] devlink: Add support for reload

[Jiri Pirko]

Wed, Nov 15, 2017 at 09:03:59AM CET, jakub.kicinski@netronome.com wrote:
>On Tue, 14 Nov 2017 17:18:44 +0100, Jiri Pirko wrote:
>> +static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
>> +{
>> +	struct devlink *devlink = info->user_ptr[0];
>> +	int err;
>> +
>> +	if (!devlink->ops->reload)
>> +		return -EOPNOTSUPP;
>> +
>> +	err = devlink_resources_validate(devlink, NULL, info);
>> +	if (err)
>> +		return err;
>> +
>> +	mutex_unlock(&devlink->lock);
>> +	err = devlink->ops->reload(devlink);
>> +	mutex_lock(&devlink->lock);
>> +
>> +	return err;
>> +}
>
>I'm a bit confused with the locking, why is devlink->lock not held
>around the validation?

It is.

Re: [patch net-next RFC v2 03/11] devlink: Add support for reload

[Arkadi Sharshevsky]

On 11/15/2017 10:03 AM, Jakub Kicinski wrote:
> On Tue, 14 Nov 2017 17:18:44 +0100, Jiri Pirko wrote:
>> +static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
>> +{
>> +	struct devlink *devlink = info->user_ptr[0];
>> +	int err;
>> +
>> +	if (!devlink->ops->reload)
>> +		return -EOPNOTSUPP;
>> +
>> +	err = devlink_resources_validate(devlink, NULL, info);
>> +	if (err)
>> +		return err;
>> +
>> +	mutex_unlock(&devlink->lock);
>> +	err = devlink->ops->reload(devlink);
>> +	mutex_lock(&devlink->lock);
>> +
>> +	return err;
>> +}
> 
> I'm a bit confused with the locking, why is devlink->lock not held
> around the validation?
> 

As Jiri mentioned it is held. The per devlink instance lock is taken
by default for each doit operation in the pre_doit(), because it operates
on a specific devlink instance.

The lock is released before performing the reload itself because during
the reload the driver register/unregisters devlink objects like sb/dpipe
/ports, which require the lock again, so this is done in order to avoid
recursive locking.

[patch net-next RFC v2 04/11] devlink: Add relation between dpipe and resource

[Jiri Pirko]

From: Arkadi Sharshevsky <arkadis@mellanox.com>

The hardware processes which are modeled via dpipe commonly use some
internal hardware resources. Such relation can improve the understanding
of hardware limitations.

Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/devlink.h        | 13 +++++++++++++
 include/uapi/linux/devlink.h |  1 +
 net/core/devlink.c           | 31 +++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index a33bda4..6cb0621 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -183,6 +183,8 @@ struct devlink_dpipe_table_ops;
  * @counters_enabled: indicates if counters are active
  * @counter_control_extern: indicates if counter control is in dpipe or
  *			    external tool
+ * @resource_id: relative resource this table is related to
+ * @resource_valid: Indicate that the resource id is valid
  * @table_ops: table operations
  * @rcu: rcu
  */
@@ -192,6 +194,8 @@ struct devlink_dpipe_table {
 	const char *name;
 	bool counters_enabled;
 	bool counter_control_extern;
+	u64 resource_id;
+	bool resource_valid;
 	struct devlink_dpipe_table_ops *table_ops;
 	struct rcu_head rcu;
 };
@@ -387,6 +391,8 @@ void devlink_resources_unregister(struct devlink *devlink,
 int devlink_resource_size_get(struct devlink *devlink,
 			      u64 resource_id,
 			      u64 *p_resource_size);
+int devlink_dpipe_table_resource_set(struct devlink *devlink,
+				     const char *table_name, u64 resource_id);
 
 #else
 
@@ -550,6 +556,13 @@ devlink_resource_size_get(struct devlink *devlink, u64 resource_id,
 	return -EINVAL;
 }
 
+static inline int
+devlink_dpipe_table_resource_set(struct devlink *devlink,
+				 const char *table_name, u64 resource_id)
+{
+	return -EINVAL;
+}
+
 #endif
 
 #endif /* _NET_DEVLINK_H_ */
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index ea4fa25..80d5a1b 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -217,6 +217,7 @@ enum devlink_attr {
 	DEVLINK_ATTR_RESOURCE_OCC,		/* u64 */
 	DEVLINK_ATTR_RESOURCE_ID,		/* u64 */
 	DEVLINK_ATTR_RESOURCE_RELOAD_REQUIRED,  /* u8  */
+	DEVLINK_ATTR_DPIPE_TABLE_RESOURCE_ID,	/* u64 */
 
 	/* add new attributes above here, update the policy in devlink.c */
 
diff --git a/net/core/devlink.c b/net/core/devlink.c
index d93f176..a200e48 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1686,6 +1686,9 @@ static int devlink_dpipe_table_put(struct sk_buff *skb,
 		       table->counters_enabled))
 		goto nla_put_failure;
 
+	if (table->resource_valid)
+		nla_put_u64_64bit(skb, DEVLINK_ATTR_DPIPE_TABLE_RESOURCE_ID,
+				  table->resource_id, DEVLINK_ATTR_PAD);
 	if (devlink_dpipe_matches_put(table, skb))
 		goto nla_put_failure;
 
@@ -3209,6 +3212,34 @@ int devlink_resource_size_get(struct devlink *devlink,
 }
 EXPORT_SYMBOL_GPL(devlink_resource_size_get);
 
+/**
+ *	devlink_dpipe_table_resource_set - set the resource id
+ *
+ *	@devlink: devlink
+ *	@table_name: table name
+ *	@resource_id: resource id
+ */
+int devlink_dpipe_table_resource_set(struct devlink *devlink,
+				     const char *table_name, u64 resource_id)
+{
+	struct devlink_dpipe_table *table;
+	int err = 0;
+
+	mutex_lock(&devlink->lock);
+	table = devlink_dpipe_table_find(&devlink->dpipe_table_list,
+					 table_name);
+	if (!table) {
+		err = -EINVAL;
+		goto out;
+	}
+	table->resource_id = resource_id;
+	table->resource_valid = true;
+out:
+	mutex_unlock(&devlink->lock);
+	return err;
+}
+EXPORT_SYMBOL_GPL(devlink_dpipe_table_resource_set);
+
 static int __init devlink_module_init(void)
 {
 	return genl_register_family(&devlink_nl_family);
-- 
2.9.5

Re: [patch net-next RFC v2 04/11] devlink: Add relation between dpipe and resource

[David Ahern]

On 11/14/17 9:18 AM, Jiri Pirko wrote:
> @@ -192,6 +194,8 @@ struct devlink_dpipe_table {
>  	const char *name;
>  	bool counters_enabled;
>  	bool counter_control_extern;
> +	u64 resource_id;
> +	bool resource_valid;

from a space perspective, please reverse the order

>  	struct devlink_dpipe_table_ops *table_ops;
>  	struct rcu_head rcu;
>  };
> @@ -387,6 +391,8 @@ void devlink_resources_unregister(struct devlink *devlink,
>  int devlink_resource_size_get(struct devlink *devlink,
>  			      u64 resource_id,
>  			      u64 *p_resource_size);
> +int devlink_dpipe_table_resource_set(struct devlink *devlink,
> +				     const char *table_name, u64 resource_id);
>  
>  #else
>  
> @@ -550,6 +556,13 @@ devlink_resource_size_get(struct devlink *devlink, u64 resource_id,
>  	return -EINVAL;
>  }
>  
> +static inline int
> +devlink_dpipe_table_resource_set(struct devlink *devlink,
> +				 const char *table_name, u64 resource_id)
> +{
> +	return -EINVAL;

It's compiled out so -EOPNOTSUPP seems more appropriate.

[patch net-next RFC v2 05/11] mlxsw: pci: Add support for performing bus reset

[Jiri Pirko]

From: Arkadi Sharshevsky <arkadis@mellanox.com>

This is a preparation stage before introducing hot reload. During the
reload process the ASIC should be resetted by accessing the PCI BAR due
to unavailability of the mailbox/emad interfaces.

Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.h |  1 +
 drivers/net/ethernet/mellanox/mlxsw/pci.c  | 56 +++++++++++++++++++++++-------
 2 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index 6e966af..34dda96 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -332,6 +332,7 @@ struct mlxsw_bus {
 		    const struct mlxsw_config_profile *profile,
 		    struct mlxsw_res *res);
 	void (*fini)(void *bus_priv);
+	void (*reset)(void *bus_priv);
 	bool (*skb_transmit_busy)(void *bus_priv,
 				  const struct mlxsw_tx_info *tx_info);
 	int (*skb_transmit)(void *bus_priv, struct sk_buff *skb,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
index 23f7d82..00c9155 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c
@@ -154,6 +154,7 @@ struct mlxsw_pci {
 		} comp;
 	} cmd;
 	struct mlxsw_bus_info bus_info;
+	const struct pci_device_id *id;
 };
 
 static void mlxsw_pci_queue_tasklet_schedule(struct mlxsw_pci_queue *q)
@@ -1622,16 +1623,6 @@ static int mlxsw_pci_cmd_exec(void *bus_priv, u16 opcode, u8 opcode_mod,
 	return err;
 }
 
-static const struct mlxsw_bus mlxsw_pci_bus = {
-	.kind			= "pci",
-	.init			= mlxsw_pci_init,
-	.fini			= mlxsw_pci_fini,
-	.skb_transmit_busy	= mlxsw_pci_skb_transmit_busy,
-	.skb_transmit		= mlxsw_pci_skb_transmit,
-	.cmd_exec		= mlxsw_pci_cmd_exec,
-	.features		= MLXSW_BUS_F_TXRX,
-};
-
 static int mlxsw_pci_sw_reset(struct mlxsw_pci *mlxsw_pci,
 			      const struct pci_device_id *id)
 {
@@ -1655,6 +1646,44 @@ static int mlxsw_pci_sw_reset(struct mlxsw_pci *mlxsw_pci,
 	return 0;
 }
 
+static void mlxsw_pci_free_irq_vectors(struct mlxsw_pci *mlxsw_pci)
+{
+	pci_free_irq_vectors(mlxsw_pci->pdev);
+}
+
+static int mlxsw_pci_alloc_irq_vectors(struct mlxsw_pci *mlxsw_pci)
+{
+	int err;
+
+	err = pci_alloc_irq_vectors(mlxsw_pci->pdev, 1, 1, PCI_IRQ_MSIX);
+	if (err < 0) {
+		dev_err(&mlxsw_pci->pdev->dev, "MSI-X init failed\n");
+		return err;
+	}
+
+	return 0;
+}
+
+static void mlxsw_pci_reset(void *bus_priv)
+{
+	struct mlxsw_pci *mlxsw_pci = bus_priv;
+
+	mlxsw_pci_free_irq_vectors(mlxsw_pci);
+	mlxsw_pci_sw_reset(mlxsw_pci, mlxsw_pci->id);
+	mlxsw_pci_alloc_irq_vectors(mlxsw_pci);
+}
+
+static const struct mlxsw_bus mlxsw_pci_bus = {
+	.kind			= "pci",
+	.init			= mlxsw_pci_init,
+	.fini			= mlxsw_pci_fini,
+	.skb_transmit_busy	= mlxsw_pci_skb_transmit_busy,
+	.skb_transmit		= mlxsw_pci_skb_transmit,
+	.cmd_exec		= mlxsw_pci_cmd_exec,
+	.features		= MLXSW_BUS_F_TXRX,
+	.reset			= mlxsw_pci_reset,
+};
+
 static int mlxsw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	const char *driver_name = pdev->driver->name;
@@ -1716,7 +1745,7 @@ static int mlxsw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		goto err_sw_reset;
 	}
 
-	err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX);
+	err = mlxsw_pci_alloc_irq_vectors(mlxsw_pci);
 	if (err < 0) {
 		dev_err(&pdev->dev, "MSI-X init failed\n");
 		goto err_msix_init;
@@ -1725,6 +1754,7 @@ static int mlxsw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	mlxsw_pci->bus_info.device_kind = driver_name;
 	mlxsw_pci->bus_info.device_name = pci_name(mlxsw_pci->pdev);
 	mlxsw_pci->bus_info.dev = &pdev->dev;
+	mlxsw_pci->id = id;
 
 	err = mlxsw_core_bus_device_register(&mlxsw_pci->bus_info,
 					     &mlxsw_pci_bus, mlxsw_pci);
@@ -1736,7 +1766,7 @@ static int mlxsw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	return 0;
 
 err_bus_device_register:
-	pci_free_irq_vectors(mlxsw_pci->pdev);
+	mlxsw_pci_free_irq_vectors(mlxsw_pci);
 err_msix_init:
 err_sw_reset:
 	iounmap(mlxsw_pci->hw_addr);
@@ -1756,7 +1786,7 @@ static void mlxsw_pci_remove(struct pci_dev *pdev)
 	struct mlxsw_pci *mlxsw_pci = pci_get_drvdata(pdev);
 
 	mlxsw_core_bus_device_unregister(mlxsw_pci->core);
-	pci_free_irq_vectors(mlxsw_pci->pdev);
+	mlxsw_pci_free_irq_vectors(mlxsw_pci);
 	iounmap(mlxsw_pci->hw_addr);
 	pci_release_regions(mlxsw_pci->pdev);
 	pci_disable_device(mlxsw_pci->pdev);
-- 
2.9.5

[patch net-next RFC v2 06/11] mlxsw: spectrum: Add "spectrum" prefix macro

[Jiri Pirko]

From: Arkadi Sharshevsky <arkadis@mellanox.com>

Add "spectrum" string prefix macro for error strings.

Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 23 ++++++++++-------------
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h |  2 ++
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 2d46ec8..d02c130 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4116,13 +4116,11 @@ mlxsw_sp_master_lag_check(struct mlxsw_sp *mlxsw_sp,
 	u16 lag_id;
 
 	if (mlxsw_sp_lag_index_get(mlxsw_sp, lag_dev, &lag_id) != 0) {
-		NL_SET_ERR_MSG(extack,
-			       "spectrum: Exceeded number of supported LAG devices");
+		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "Exceeded number of supported LAG devices");
 		return false;
 	}
 	if (lag_upper_info->tx_type != NETDEV_LAG_TX_TYPE_HASH) {
-		NL_SET_ERR_MSG(extack,
-			       "spectrum: LAG device using unsupported Tx type");
+		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "LAG device using unsupported Tx type");
 		return false;
 	}
 	return true;
@@ -4346,15 +4344,14 @@ static int mlxsw_sp_netdevice_port_upper_event(struct net_device *lower_dev,
 		    !netif_is_lag_master(upper_dev) &&
 		    !netif_is_bridge_master(upper_dev) &&
 		    !netif_is_ovs_master(upper_dev)) {
-			NL_SET_ERR_MSG(extack,
-				       "spectrum: Unknown upper device type");
+			NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "Unknown upper device type");
 			return -EINVAL;
 		}
 		if (!info->linking)
 			break;
 		if (netdev_has_any_upper_dev(upper_dev)) {
 			NL_SET_ERR_MSG(extack,
-				       "spectrum: Enslaving a port to a device that already has an upper device is not supported");
+				       MLXSW_SP_PREFIX "Enslaving a port to a device that already has an upper device is not supported");
 			return -EINVAL;
 		}
 		if (netif_is_lag_master(upper_dev) &&
@@ -4363,23 +4360,23 @@ static int mlxsw_sp_netdevice_port_upper_event(struct net_device *lower_dev,
 			return -EINVAL;
 		if (netif_is_lag_master(upper_dev) && vlan_uses_dev(dev)) {
 			NL_SET_ERR_MSG(extack,
-				       "spectrum: Master device is a LAG master and this device has a VLAN");
+				       MLXSW_SP_PREFIX "Master device is a LAG master and this device has a VLAN");
 			return -EINVAL;
 		}
 		if (netif_is_lag_port(dev) && is_vlan_dev(upper_dev) &&
 		    !netif_is_lag_master(vlan_dev_real_dev(upper_dev))) {
 			NL_SET_ERR_MSG(extack,
-				       "spectrum: Can not put a VLAN on a LAG port");
+				       MLXSW_SP_PREFIX "Can not put a VLAN on a LAG port");
 			return -EINVAL;
 		}
 		if (netif_is_ovs_master(upper_dev) && vlan_uses_dev(dev)) {
 			NL_SET_ERR_MSG(extack,
-				       "spectrum: Master device is an OVS master and this device has a VLAN");
+				       MLXSW_SP_PREFIX "Master device is an OVS master and this device has a VLAN");
 			return -EINVAL;
 		}
 		if (netif_is_ovs_port(dev) && is_vlan_dev(upper_dev)) {
 			NL_SET_ERR_MSG(extack,
-				       "spectrum: Can not put a VLAN on an OVS port");
+				       MLXSW_SP_PREFIX "Can not put a VLAN on an OVS port");
 			return -EINVAL;
 		}
 		break;
@@ -4491,13 +4488,13 @@ static int mlxsw_sp_netdevice_port_vlan_event(struct net_device *vlan_dev,
 	case NETDEV_PRECHANGEUPPER:
 		upper_dev = info->upper_dev;
 		if (!netif_is_bridge_master(upper_dev)) {
-			NL_SET_ERR_MSG(extack, "spectrum: VLAN devices only support bridge and VRF uppers");
+			NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "VLAN devices only support bridge and VRF uppers");
 			return -EINVAL;
 		}
 		if (!info->linking)
 			break;
 		if (netdev_has_any_upper_dev(upper_dev)) {
-			NL_SET_ERR_MSG(extack, "spectrum: Enslaving a port to a device that already has an upper device is not supported");
+			NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "Enslaving a port to a device that already has an upper device is not supported");
 			return -EINVAL;
 		}
 		break;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 58cf222..f15dcca 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -66,6 +66,8 @@
 #define MLXSW_SP_KVD_LINEAR_SIZE 98304 /* entries */
 #define MLXSW_SP_KVD_GRANULARITY 128
 
+#define MLXSW_SP_PREFIX "spectrum: "
+
 struct mlxsw_sp_port;
 struct mlxsw_sp_rif;
 
-- 
2.9.5

[patch net-next RFC v2 07/11] mlxsw: spectrum: Register KVD resources with devlink

[Jiri Pirko]

From: Arkadi Sharshevsky <arkadis@mellanox.com>

Register the KVD resources with devlink. The KVD is a memory resource
which is subdivided into three partitions which are the linear, hash
single and hash double.

Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c     |   9 ++
 drivers/net/ethernet/mellanox/mlxsw/core.h     |   1 +
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 168 +++++++++++++++++++++++++
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h |  12 ++
 4 files changed, 190 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index f3315bc..2488662 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1012,6 +1012,12 @@ int mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info,
 	if (err)
 		goto err_bus_init;
 
+	if (mlxsw_driver->resources_register) {
+		err = mlxsw_driver->resources_register(mlxsw_core);
+		if (err)
+			goto err_register_resources;
+	}
+
 	err = mlxsw_ports_init(mlxsw_core);
 	if (err)
 		goto err_ports_init;
@@ -1067,6 +1073,8 @@ int mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info,
 err_ports_init:
 	mlxsw_bus->fini(bus_priv);
 err_bus_init:
+	devlink_resources_unregister(devlink, NULL);
+err_register_resources:
 	devlink_free(devlink);
 err_devlink_alloc:
 	mlxsw_core_driver_put(device_kind);
@@ -1086,6 +1094,7 @@ void mlxsw_core_bus_device_unregister(struct mlxsw_core *mlxsw_core)
 	mlxsw_emad_fini(mlxsw_core);
 	kfree(mlxsw_core->lag.mapping);
 	mlxsw_ports_fini(mlxsw_core);
+	devlink_resources_unregister(devlink, NULL);
 	mlxsw_core->bus->fini(mlxsw_core->bus_priv);
 	devlink_free(devlink);
 	mlxsw_core_driver_put(device_kind);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index 34dda96..e23f83b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -308,6 +308,7 @@ struct mlxsw_driver {
 				       u32 *p_cur, u32 *p_max);
 	void (*txhdr_construct)(struct sk_buff *skb,
 				const struct mlxsw_tx_info *tx_info);
+	int (*resources_register)(struct mlxsw_core *mlxsw_core);
 	u8 txhdr_len;
 	const struct mlxsw_config_profile *profile;
 };
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index d02c130..f0cbd67 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3927,6 +3927,173 @@ static const struct mlxsw_config_profile mlxsw_sp_config_profile = {
 	.resource_query_enable		= 1,
 };
 
+static bool
+mlxsw_sp_resource_kvd_granularity_validate(struct netlink_ext_ack *extack,
+					   u64 size)
+{
+	const struct mlxsw_config_profile *profile;
+
+	profile = &mlxsw_sp_config_profile;
+	if (size % profile->kvd_hash_granularity) {
+		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "resource set with wrong granularity");
+		return false;
+	}
+	return true;
+}
+
+static int
+mlxsw_sp_resource_kvd_size_validate(struct devlink *devlink, u64 size,
+				    struct list_head *resource_list,
+				    struct netlink_ext_ack *extack)
+{
+	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
+	u32 kvd_size, single_size, double_size, linear_size;
+	struct devlink_resource *resource;
+
+	kvd_size = MLXSW_CORE_RES_GET(mlxsw_core, KVD_SIZE);
+	if (kvd_size != size) {
+		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "kvd size cannot be chagned");
+		return -EINVAL;
+	}
+
+	list_for_each_entry(resource, resource_list, list) {
+		switch (resource->id) {
+		case MLXSW_SP_RESOURCE_KVD_LINEAR:
+			linear_size = resource->size_new;
+			break;
+		case MLXSW_SP_RESOURCE_KVD_HASH_SINGLE:
+			single_size = resource->size_new;
+			break;
+		case MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE:
+			double_size = resource->size_new;
+			break;
+		}
+	}
+
+	/* Overlap is not supported */
+	if (linear_size + single_size + double_size > kvd_size) {
+		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "Overlap is not supported");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int
+mlxsw_sp_resource_kvd_linear_size_validate(struct devlink *devlink, u64 size,
+					   struct list_head *resource_list,
+					   struct netlink_ext_ack *extack)
+{
+	if (!mlxsw_sp_resource_kvd_granularity_validate(extack, size))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int
+mlxsw_sp_resource_kvd_hash_single_size_validate(struct devlink *devlink, u64 size,
+						struct list_head *resource_list,
+						struct netlink_ext_ack *extack)
+{
+	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
+
+	if (!mlxsw_sp_resource_kvd_granularity_validate(extack, size))
+		return -EINVAL;
+
+	if (size < MLXSW_CORE_RES_GET(mlxsw_core, KVD_SINGLE_MIN_SIZE)) {
+		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "hash single size is smaller then min");
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int
+mlxsw_sp_resource_kvd_hash_double_size_validate(struct devlink *devlink, u64 size,
+						struct list_head *resource_list,
+						struct netlink_ext_ack *extack)
+{
+	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
+
+	if (!mlxsw_sp_resource_kvd_granularity_validate(extack, size))
+		return -EINVAL;
+
+	if (size < MLXSW_CORE_RES_GET(mlxsw_core, KVD_DOUBLE_MIN_SIZE)) {
+		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "hash double size is smaller then min");
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static struct devlink_resource_ops mlxsw_sp_resource_kvd_ops = {
+	.size_validate = mlxsw_sp_resource_kvd_size_validate,
+};
+
+static struct devlink_resource_ops mlxsw_sp_resource_kvd_linear_ops = {
+	.size_validate = mlxsw_sp_resource_kvd_linear_size_validate,
+};
+
+static struct devlink_resource_ops mlxsw_sp_resource_kvd_hash_single_ops = {
+	.size_validate = mlxsw_sp_resource_kvd_hash_single_size_validate,
+};
+
+static struct devlink_resource_ops mlxsw_sp_resource_kvd_hash_double_ops = {
+	.size_validate = mlxsw_sp_resource_kvd_hash_double_size_validate,
+};
+
+static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
+{
+	struct devlink *devlink = priv_to_devlink(mlxsw_core);
+	u32 kvd_size, single_size, double_size, linear_size;
+	const struct mlxsw_config_profile *profile;
+	int err;
+
+	profile = &mlxsw_sp_config_profile;
+	if (!MLXSW_CORE_RES_VALID(mlxsw_core, KVD_SIZE))
+		return -EIO;
+
+	kvd_size = MLXSW_CORE_RES_GET(mlxsw_core, KVD_SIZE);
+	err = devlink_resource_register(devlink, MLXSW_SP_RESOURCE_NAME_KVD,
+					true, false, kvd_size,
+					MLXSW_SP_RESOURCE_KVD,
+					DEVLINK_RESOURCE_ID_PARENT_TOP,
+					&mlxsw_sp_resource_kvd_ops);
+	if (err)
+		return err;
+
+	linear_size = profile->kvd_linear_size;
+	err = devlink_resource_register(devlink, MLXSW_SP_RESOURCE_NAME_KVD_LINEAR,
+					false, true, linear_size,
+					MLXSW_SP_RESOURCE_KVD_LINEAR,
+					MLXSW_SP_RESOURCE_KVD,
+					&mlxsw_sp_resource_kvd_linear_ops);
+	if (err)
+		return err;
+
+	double_size = kvd_size - linear_size;
+	double_size *= profile->kvd_hash_double_parts;
+	double_size /= profile->kvd_hash_double_parts +
+		       profile->kvd_hash_single_parts;
+	double_size = rounddown(double_size, profile->kvd_hash_granularity);
+	err = devlink_resource_register(devlink, MLXSW_SP_RESOURCE_NAME_KVD_HASH_DOUBLE,
+					false, true, double_size,
+					MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE,
+					MLXSW_SP_RESOURCE_KVD,
+					&mlxsw_sp_resource_kvd_hash_double_ops);
+	if (err)
+		return err;
+
+	single_size = kvd_size - double_size - linear_size;
+	err = devlink_resource_register(devlink, MLXSW_SP_RESOURCE_NAME_KVD_HASH_SINGLE,
+					false, true, single_size,
+					MLXSW_SP_RESOURCE_KVD_HASH_SINGLE,
+					MLXSW_SP_RESOURCE_KVD,
+					&mlxsw_sp_resource_kvd_hash_single_ops);
+	if (err)
+		return err;
+
+	return 0;
+}
+
 static struct mlxsw_driver mlxsw_sp_driver = {
 	.kind				= mlxsw_sp_driver_name,
 	.priv_size			= sizeof(struct mlxsw_sp),
@@ -3946,6 +4113,7 @@ static struct mlxsw_driver mlxsw_sp_driver = {
 	.sb_occ_port_pool_get		= mlxsw_sp_sb_occ_port_pool_get,
 	.sb_occ_tc_port_bind_get	= mlxsw_sp_sb_occ_tc_port_bind_get,
 	.txhdr_construct		= mlxsw_sp_txhdr_construct,
+	.resources_register		= mlxsw_sp_resources_register,
 	.txhdr_len			= MLXSW_TXHDR_LEN,
 	.profile			= &mlxsw_sp_config_profile,
 };
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index f15dcca..cbfeaf7 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -66,6 +66,18 @@
 #define MLXSW_SP_KVD_LINEAR_SIZE 98304 /* entries */
 #define MLXSW_SP_KVD_GRANULARITY 128
 
+#define MLXSW_SP_RESOURCE_NAME_KVD "kvd"
+#define MLXSW_SP_RESOURCE_NAME_KVD_LINEAR "linear"
+#define MLXSW_SP_RESOURCE_NAME_KVD_HASH_SINGLE "hash_single"
+#define MLXSW_SP_RESOURCE_NAME_KVD_HASH_DOUBLE "hash_double"
+
+enum mlxsw_sp_resource_id {
+	MLXSW_SP_RESOURCE_KVD,
+	MLXSW_SP_RESOURCE_KVD_LINEAR,
+	MLXSW_SP_RESOURCE_KVD_HASH_SINGLE,
+	MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE,
+};
+
 #define MLXSW_SP_PREFIX "spectrum: "
 
 struct mlxsw_sp_port;
-- 
2.9.5

Re: [patch net-next RFC v2 07/11] mlxsw: spectrum: Register KVD resources with devlink

[David Ahern]

On 11/14/17 9:18 AM, Jiri Pirko wrote:
> +static int
> +mlxsw_sp_resource_kvd_size_validate(struct devlink *devlink, u64 size,
> +				    struct list_head *resource_list,
> +				    struct netlink_ext_ack *extack)
> +{
> +	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
> +	u32 kvd_size, single_size, double_size, linear_size;
> +	struct devlink_resource *resource;
> +
> +	kvd_size = MLXSW_CORE_RES_GET(mlxsw_core, KVD_SIZE);
> +	if (kvd_size != size) {
> +		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "kvd size cannot be chagned");
> +		return -EINVAL;
> +	}
> +
> +	list_for_each_entry(resource, resource_list, list) {
> +		switch (resource->id) {
> +		case MLXSW_SP_RESOURCE_KVD_LINEAR:
> +			linear_size = resource->size_new;
> +			break;
> +		case MLXSW_SP_RESOURCE_KVD_HASH_SINGLE:
> +			single_size = resource->size_new;
> +			break;
> +		case MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE:
> +			double_size = resource->size_new;
> +			break;
> +		}
> +	}
> +
> +	/* Overlap is not supported */
> +	if (linear_size + single_size + double_size > kvd_size) {
> +		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "Overlap is not supported");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +

gcc warnings due to the above:

/home/dsa/kernel-3.git/drivers/net/ethernet/mellanox/mlxsw/spectrum.c:
In function ‘mlxsw_sp_resource_kvd_size_validate’:
/home/dsa/kernel-3.git/drivers/net/ethernet/mellanox/mlxsw/spectrum.c:3974:32:
warning: ‘linear_size’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
  if (linear_size + single_size + double_size > kvd_size) {
                                ^
/home/dsa/kernel-3.git/drivers/net/ethernet/mellanox/mlxsw/spectrum.c:3950:29:
warning: ‘double_size’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
  u32 kvd_size, single_size, double_size, linear_size;
                             ^
/home/dsa/kernel-3.git/drivers/net/ethernet/mellanox/mlxsw/spectrum.c:3950:16:
warning: ‘single_size’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
  u32 kvd_size, single_size, double_size, linear_size;
                ^

Re: [patch net-next RFC v2 07/11] mlxsw: spectrum: Register KVD resources with devlink

[Arkadi Sharshevsky]

On 11/15/2017 02:15 AM, David Ahern wrote:
> On 11/14/17 9:18 AM, Jiri Pirko wrote:
>> +static int
>> +mlxsw_sp_resource_kvd_size_validate(struct devlink *devlink, u64 size,
>> +				    struct list_head *resource_list,
>> +				    struct netlink_ext_ack *extack)
>> +{
>> +	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
>> +	u32 kvd_size, single_size, double_size, linear_size;
>> +	struct devlink_resource *resource;
>> +
>> +	kvd_size = MLXSW_CORE_RES_GET(mlxsw_core, KVD_SIZE);
>> +	if (kvd_size != size) {
>> +		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "kvd size cannot be chagned");
>> +		return -EINVAL;
>> +	}
>> +
>> +	list_for_each_entry(resource, resource_list, list) {
>> +		switch (resource->id) {
>> +		case MLXSW_SP_RESOURCE_KVD_LINEAR:
>> +			linear_size = resource->size_new;
>> +			break;
>> +		case MLXSW_SP_RESOURCE_KVD_HASH_SINGLE:
>> +			single_size = resource->size_new;
>> +			break;
>> +		case MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE:
>> +			double_size = resource->size_new;
>> +			break;
>> +		}
>> +	}
>> +
>> +	/* Overlap is not supported */
>> +	if (linear_size + single_size + double_size > kvd_size) {
>> +		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "Overlap is not supported");
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
> 
> gcc warnings due to the above:
> 
> /home/dsa/kernel-3.git/drivers/net/ethernet/mellanox/mlxsw/spectrum.c:
> In function ‘mlxsw_sp_resource_kvd_size_validate’:
> /home/dsa/kernel-3.git/drivers/net/ethernet/mellanox/mlxsw/spectrum.c:3974:32:
> warning: ‘linear_size’ may be used uninitialized in this function
> [-Wmaybe-uninitialized]
>   if (linear_size + single_size + double_size > kvd_size) {
>                                 ^
> /home/dsa/kernel-3.git/drivers/net/ethernet/mellanox/mlxsw/spectrum.c:3950:29:
> warning: ‘double_size’ may be used uninitialized in this function
> [-Wmaybe-uninitialized]
>   u32 kvd_size, single_size, double_size, linear_size;
>                              ^
> /home/dsa/kernel-3.git/drivers/net/ethernet/mellanox/mlxsw/spectrum.c:3950:16:
> warning: ‘single_size’ may be used uninitialized in this function
> [-Wmaybe-uninitialized]
>   u32 kvd_size, single_size, double_size, linear_size;
>                 ^
> 
Thanks, will fix

Re: [patch net-next RFC v2 07/11] mlxsw: spectrum: Register KVD resources with devlink

[David Ahern]

On 11/14/17 9:18 AM, Jiri Pirko wrote:
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> index d02c130..f0cbd67 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> @@ -3927,6 +3927,173 @@ static const struct mlxsw_config_profile mlxsw_sp_config_profile = {
>  	.resource_query_enable		= 1,
>  };
>  
> +static bool
> +mlxsw_sp_resource_kvd_granularity_validate(struct netlink_ext_ack *extack,
> +					   u64 size)
> +{
> +	const struct mlxsw_config_profile *profile;
> +
> +	profile = &mlxsw_sp_config_profile;
> +	if (size % profile->kvd_hash_granularity) {
> +		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "resource set with wrong granularity");
> +		return false;
> +	}
> +	return true;
> +}
> +
> +static int
> +mlxsw_sp_resource_kvd_size_validate(struct devlink *devlink, u64 size,
> +				    struct list_head *resource_list,
> +				    struct netlink_ext_ack *extack)
> +{
> +	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
> +	u32 kvd_size, single_size, double_size, linear_size;
> +	struct devlink_resource *resource;
> +
> +	kvd_size = MLXSW_CORE_RES_GET(mlxsw_core, KVD_SIZE);
> +	if (kvd_size != size) {
> +		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "kvd size cannot be chagned");

s/chagned/changed/

> +		return -EINVAL;
> +	}
> +
> +	list_for_each_entry(resource, resource_list, list) {
> +		switch (resource->id) {
> +		case MLXSW_SP_RESOURCE_KVD_LINEAR:
> +			linear_size = resource->size_new;
> +			break;
> +		case MLXSW_SP_RESOURCE_KVD_HASH_SINGLE:
> +			single_size = resource->size_new;
> +			break;
> +		case MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE:
> +			double_size = resource->size_new;
> +			break;
> +		}
> +	}
> +
> +	/* Overlap is not supported */
> +	if (linear_size + single_size + double_size > kvd_size) {
> +		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "Overlap is not supported");

Overlap? Isn't that sum of the partitions are greater than total size?


> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int
> +mlxsw_sp_resource_kvd_linear_size_validate(struct devlink *devlink, u64 size,
> +					   struct list_head *resource_list,
> +					   struct netlink_ext_ack *extack)
> +{
> +	if (!mlxsw_sp_resource_kvd_granularity_validate(extack, size))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int
> +mlxsw_sp_resource_kvd_hash_single_size_validate(struct devlink *devlink, u64 size,
> +						struct list_head *resource_list,
> +						struct netlink_ext_ack *extack)
> +{
> +	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
> +
> +	if (!mlxsw_sp_resource_kvd_granularity_validate(extack, size))
> +		return -EINVAL;
> +
> +	if (size < MLXSW_CORE_RES_GET(mlxsw_core, KVD_SINGLE_MIN_SIZE)) {
> +		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "hash single size is smaller then min");

s/then min/than minimium/

> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +static int
> +mlxsw_sp_resource_kvd_hash_double_size_validate(struct devlink *devlink, u64 size,
> +						struct list_head *resource_list,
> +						struct netlink_ext_ack *extack)
> +{
> +	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
> +
> +	if (!mlxsw_sp_resource_kvd_granularity_validate(extack, size))
> +		return -EINVAL;
> +
> +	if (size < MLXSW_CORE_RES_GET(mlxsw_core, KVD_DOUBLE_MIN_SIZE)) {
> +		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "hash double size is smaller then min");

s/then min/than minimium/

How does the user learn the minimum size and the granularity for the KVD
resources? Seems like those could be read-only attributes in the
resource dump to make it easier for the user.

Re: [patch net-next RFC v2 07/11] mlxsw: spectrum: Register KVD resources with devlink

[Arkadi Sharshevsky]

On 11/18/2017 09:18 PM, David Ahern wrote:
> On 11/14/17 9:18 AM, Jiri Pirko wrote:
>> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>> index d02c130..f0cbd67 100644
>> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>> @@ -3927,6 +3927,173 @@ static const struct mlxsw_config_profile mlxsw_sp_config_profile = {
>>  	.resource_query_enable		= 1,
>>  };
>>  
>> +static bool
>> +mlxsw_sp_resource_kvd_granularity_validate(struct netlink_ext_ack *extack,
>> +					   u64 size)
>> +{
>> +	const struct mlxsw_config_profile *profile;
>> +
>> +	profile = &mlxsw_sp_config_profile;
>> +	if (size % profile->kvd_hash_granularity) {
>> +		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "resource set with wrong granularity");
>> +		return false;
>> +	}
>> +	return true;
>> +}
>> +
>> +static int
>> +mlxsw_sp_resource_kvd_size_validate(struct devlink *devlink, u64 size,
>> +				    struct list_head *resource_list,
>> +				    struct netlink_ext_ack *extack)
>> +{
>> +	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
>> +	u32 kvd_size, single_size, double_size, linear_size;
>> +	struct devlink_resource *resource;
>> +
>> +	kvd_size = MLXSW_CORE_RES_GET(mlxsw_core, KVD_SIZE);
>> +	if (kvd_size != size) {
>> +		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "kvd size cannot be chagned");
> 
> s/chagned/changed/
> 
>> +		return -EINVAL;
>> +	}
>> +
>> +	list_for_each_entry(resource, resource_list, list) {
>> +		switch (resource->id) {
>> +		case MLXSW_SP_RESOURCE_KVD_LINEAR:
>> +			linear_size = resource->size_new;
>> +			break;
>> +		case MLXSW_SP_RESOURCE_KVD_HASH_SINGLE:
>> +			single_size = resource->size_new;
>> +			break;
>> +		case MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE:
>> +			double_size = resource->size_new;
>> +			break;
>> +		}
>> +	}
>> +
>> +	/* Overlap is not supported */
>> +	if (linear_size + single_size + double_size > kvd_size) {
>> +		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "Overlap is not supported");
> 
> Overlap? Isn't that sum of the partitions are greater than total size?
> 

In case sum of the partitions is greater than the kvd tot size, the
hash single/double will be set in an overlapping state, which we do
not support currently.

> 
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int
>> +mlxsw_sp_resource_kvd_linear_size_validate(struct devlink *devlink, u64 size,
>> +					   struct list_head *resource_list,
>> +					   struct netlink_ext_ack *extack)
>> +{
>> +	if (!mlxsw_sp_resource_kvd_granularity_validate(extack, size))
>> +		return -EINVAL;
>> +
>> +	return 0;
>> +}
>> +
>> +static int
>> +mlxsw_sp_resource_kvd_hash_single_size_validate(struct devlink *devlink, u64 size,
>> +						struct list_head *resource_list,
>> +						struct netlink_ext_ack *extack)
>> +{
>> +	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
>> +
>> +	if (!mlxsw_sp_resource_kvd_granularity_validate(extack, size))
>> +		return -EINVAL;
>> +
>> +	if (size < MLXSW_CORE_RES_GET(mlxsw_core, KVD_SINGLE_MIN_SIZE)) {
>> +		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "hash single size is smaller then min");
> 
> s/then min/than minimium/
> 
>> +		return -EINVAL;
>> +	}
>> +	return 0;
>> +}
>> +
>> +static int
>> +mlxsw_sp_resource_kvd_hash_double_size_validate(struct devlink *devlink, u64 size,
>> +						struct list_head *resource_list,
>> +						struct netlink_ext_ack *extack)
>> +{
>> +	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
>> +
>> +	if (!mlxsw_sp_resource_kvd_granularity_validate(extack, size))
>> +		return -EINVAL;
>> +
>> +	if (size < MLXSW_CORE_RES_GET(mlxsw_core, KVD_DOUBLE_MIN_SIZE)) {
>> +		NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "hash double size is smaller then min");
> 
> s/then min/than minimium/
> 
> How does the user learn the minimum size and the granularity for the KVD
> resources? Seems like those could be read-only attributes in the
> resource dump to make it easier for the user.
> 

This seems to me as too case specific and I didn't want to add
UAPI attributes for this stuff..

The resource shouldn't be define as only memory based hardware blocks.
I actually plane expose the rifs as resource as well.

I think that if the user try to configure and receives an such error
it is very clear what is the problem.


> 
> 

[patch net-next RFC v2 08/11] mlxsw: spectrum_dpipe: Connect dpipe tables to resources

[Jiri Pirko]

From: Arkadi Sharshevsky <arkadis@mellanox.com>

Connect current dpipe tables to resources. The tables are connected
in the following fashion:
1. IPv4 host - KVD hash single
2. IPv6 host - KVD hash double
3. Adjacency - KVD linear

Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_dpipe.c   | 72 ++++++++++++++++++----
 1 file changed, 60 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
index 96fdba7..282fe82 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
@@ -774,11 +774,27 @@ static struct devlink_dpipe_table_ops mlxsw_sp_host4_ops = {
 static int mlxsw_sp_dpipe_host4_table_init(struct mlxsw_sp *mlxsw_sp)
 {
 	struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
+	int err;
 
-	return devlink_dpipe_table_register(devlink,
-					    MLXSW_SP_DPIPE_TABLE_NAME_HOST4,
-					    &mlxsw_sp_host4_ops,
-					    mlxsw_sp, false);
+	err = devlink_dpipe_table_register(devlink,
+					   MLXSW_SP_DPIPE_TABLE_NAME_HOST4,
+					   &mlxsw_sp_host4_ops,
+					   mlxsw_sp, false);
+	if (err)
+		return err;
+
+	err = devlink_dpipe_table_resource_set(devlink,
+					       MLXSW_SP_DPIPE_TABLE_NAME_HOST4,
+					       MLXSW_SP_RESOURCE_KVD_HASH_SINGLE);
+	if (err)
+		goto err_resource_set;
+
+	return 0;
+
+err_resource_set:
+	devlink_dpipe_table_unregister(devlink,
+				       MLXSW_SP_DPIPE_TABLE_NAME_HOST4);
+	return err;
 }
 
 static void mlxsw_sp_dpipe_host4_table_fini(struct mlxsw_sp *mlxsw_sp)
@@ -832,11 +848,27 @@ static struct devlink_dpipe_table_ops mlxsw_sp_host6_ops = {
 static int mlxsw_sp_dpipe_host6_table_init(struct mlxsw_sp *mlxsw_sp)
 {
 	struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
+	int err;
 
-	return devlink_dpipe_table_register(devlink,
-					    MLXSW_SP_DPIPE_TABLE_NAME_HOST6,
-					    &mlxsw_sp_host6_ops,
-					    mlxsw_sp, false);
+	err = devlink_dpipe_table_register(devlink,
+					   MLXSW_SP_DPIPE_TABLE_NAME_HOST6,
+					   &mlxsw_sp_host6_ops,
+					   mlxsw_sp, false);
+	if (err)
+		return err;
+
+	err = devlink_dpipe_table_resource_set(devlink,
+					       MLXSW_SP_DPIPE_TABLE_NAME_HOST6,
+					       MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE);
+	if (err)
+		goto err_resource_set;
+
+	return 0;
+
+err_resource_set:
+	devlink_dpipe_table_unregister(devlink,
+				       MLXSW_SP_DPIPE_TABLE_NAME_HOST6);
+	return err;
 }
 
 static void mlxsw_sp_dpipe_host6_table_fini(struct mlxsw_sp *mlxsw_sp)
@@ -1216,11 +1248,27 @@ static struct devlink_dpipe_table_ops mlxsw_sp_dpipe_table_adj_ops = {
 static int mlxsw_sp_dpipe_adj_table_init(struct mlxsw_sp *mlxsw_sp)
 {
 	struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
+	int err;
 
-	return devlink_dpipe_table_register(devlink,
-					    MLXSW_SP_DPIPE_TABLE_NAME_ADJ,
-					    &mlxsw_sp_dpipe_table_adj_ops,
-					    mlxsw_sp, false);
+	err = devlink_dpipe_table_register(devlink,
+					   MLXSW_SP_DPIPE_TABLE_NAME_ADJ,
+					   &mlxsw_sp_dpipe_table_adj_ops,
+					   mlxsw_sp, false);
+	if (err)
+		return err;
+
+	err = devlink_dpipe_table_resource_set(devlink,
+					       MLXSW_SP_DPIPE_TABLE_NAME_ADJ,
+					       MLXSW_SP_RESOURCE_KVD_LINEAR);
+	if (err)
+		goto err_resource_set;
+
+	return 0;
+
+err_resource_set:
+	devlink_dpipe_table_unregister(devlink,
+				       MLXSW_SP_DPIPE_TABLE_NAME_ADJ);
+	return err;
 }
 
 static void mlxsw_sp_dpipe_adj_table_fini(struct mlxsw_sp *mlxsw_sp)
-- 
2.9.5

Re: [patch net-next RFC v2 08/11] mlxsw: spectrum_dpipe: Connect dpipe tables to resources

[David Ahern]

On 11/14/17 9:18 AM, Jiri Pirko wrote:
> From: Arkadi Sharshevsky <arkadis@mellanox.com>
> 
> Connect current dpipe tables to resources. The tables are connected
> in the following fashion:
> 1. IPv4 host - KVD hash single
> 2. IPv6 host - KVD hash double
> 3. Adjacency - KVD linear

Those descriptions would be helpful to the user. A description attribute
for the resources?

Re: [patch net-next RFC v2 08/11] mlxsw: spectrum_dpipe: Connect dpipe tables to resources

[Arkadi Sharshevsky]

On 11/18/2017 09:19 PM, David Ahern wrote:
> On 11/14/17 9:18 AM, Jiri Pirko wrote:
>> From: Arkadi Sharshevsky <arkadis@mellanox.com>
>>
>> Connect current dpipe tables to resources. The tables are connected
>> in the following fashion:
>> 1. IPv4 host - KVD hash single
>> 2. IPv6 host - KVD hash double
>> 3. Adjacency - KVD linear
> 
> Those descriptions would be helpful to the user. A description attribute
> for the resources?
> 

As described in the cover letter this resources are used by the
majority of the ASICs lookup processes. So currently there is one
to one mapping but is should increase as more tables are exposed,
so I don't think its a good idea to maintain such an attribute.

Re: [patch net-next RFC v2 08/11] mlxsw: spectrum_dpipe: Connect dpipe tables to resources

[David Ahern]

On 11/19/17 2:16 AM, Arkadi Sharshevsky wrote:
> 
> 
> On 11/18/2017 09:19 PM, David Ahern wrote:
>> On 11/14/17 9:18 AM, Jiri Pirko wrote:
>>> From: Arkadi Sharshevsky <arkadis@mellanox.com>
>>>
>>> Connect current dpipe tables to resources. The tables are connected
>>> in the following fashion:
>>> 1. IPv4 host - KVD hash single
>>> 2. IPv6 host - KVD hash double
>>> 3. Adjacency - KVD linear
>>
>> Those descriptions would be helpful to the user. A description attribute
>> for the resources?
>>
> 
> As described in the cover letter this resources are used by the
> majority of the ASICs lookup processes. So currently there is one
> to one mapping but is should increase as more tables are exposed,
> so I don't think its a good idea to maintain such an attribute.
> 

'IPv4 host' yes, but I mean the term 'KVD hash single'? Is it the same
across all h/w vendors? I have only seen that in the context of MLX. If
it is a MLX term then a description to the user that KVD hash single ==
IPv4 host is warranted.

Re: [patch net-next RFC v2 08/11] mlxsw: spectrum_dpipe: Connect dpipe tables to resources

[Arkadi Sharshevsky]

On 11/19/2017 05:58 PM, David Ahern wrote:
> On 11/19/17 2:16 AM, Arkadi Sharshevsky wrote:
>>
>>
>> On 11/18/2017 09:19 PM, David Ahern wrote:
>>> On 11/14/17 9:18 AM, Jiri Pirko wrote:
>>>> From: Arkadi Sharshevsky <arkadis@mellanox.com>
>>>>
>>>> Connect current dpipe tables to resources. The tables are connected
>>>> in the following fashion:
>>>> 1. IPv4 host - KVD hash single
>>>> 2. IPv6 host - KVD hash double
>>>> 3. Adjacency - KVD linear
>>>
>>> Those descriptions would be helpful to the user. A description attribute
>>> for the resources?
>>>
>>
>> As described in the cover letter this resources are used by the
>> majority of the ASICs lookup processes. So currently there is one
>> to one mapping but is should increase as more tables are exposed,
>> so I don't think its a good idea to maintain such an attribute.
>>
> 
> 'IPv4 host' yes, but I mean the term 'KVD hash single'? Is it the same
> across all h/w vendors? I have only seen that in the context of MLX. If
> it is a MLX term then a description to the user that KVD hash single ==
> IPv4 host is warranted.
> 

But this relation is wrong, there is no equality here. The LPM, FDB and
VID to FID mapping are all can be modeled as lookup tables (via dpipe)
that use KVD hash single resource.

This description string will grow very long. I dont think this is the
right place to document such thing, eitherway, the user can dump the
dpipe tables and see which is mapped to what resource.

Re: [patch net-next RFC v2 08/11] mlxsw: spectrum_dpipe: Connect dpipe tables to resources

[David Ahern]

On 11/23/17 6:40 AM, Arkadi Sharshevsky wrote:
> 
> 
> On 11/19/2017 05:58 PM, David Ahern wrote:
>> On 11/19/17 2:16 AM, Arkadi Sharshevsky wrote:
>>>
>>>
>>> On 11/18/2017 09:19 PM, David Ahern wrote:
>>>> On 11/14/17 9:18 AM, Jiri Pirko wrote:
>>>>> From: Arkadi Sharshevsky <arkadis@mellanox.com>
>>>>>
>>>>> Connect current dpipe tables to resources. The tables are connected
>>>>> in the following fashion:
>>>>> 1. IPv4 host - KVD hash single
>>>>> 2. IPv6 host - KVD hash double
>>>>> 3. Adjacency - KVD linear
>>>>
>>>> Those descriptions would be helpful to the user. A description attribute
>>>> for the resources?
>>>>
>>>
>>> As described in the cover letter this resources are used by the
>>> majority of the ASICs lookup processes. So currently there is one
>>> to one mapping but is should increase as more tables are exposed,
>>> so I don't think its a good idea to maintain such an attribute.
>>>
>>
>> 'IPv4 host' yes, but I mean the term 'KVD hash single'? Is it the same
>> across all h/w vendors? I have only seen that in the context of MLX. If
>> it is a MLX term then a description to the user that KVD hash single ==
>> IPv4 host is warranted.
>>
> 
> But this relation is wrong, there is no equality here. The LPM, FDB and
> VID to FID mapping are all can be modeled as lookup tables (via dpipe)
> that use KVD hash single resource.
> 
> This description string will grow very long. I dont think this is the
> right place to document such thing, eitherway, the user can dump the
> dpipe tables and see which is mapped to what resource.

Users should not have to find a PRM or user guide for *each version of
their hardware* to program something so fundamental. This is software.
We can make it user friendly. Use of vendor specific terms is fine --
allows correlation to vendor docs. But there should also be text to help
the user correlate vendor terms to generic industry terms.

Re: [patch net-next RFC v2 08/11] mlxsw: spectrum_dpipe: Connect dpipe tables to resources

[Arkadi Sharshevsky]

On 11/27/2017 06:12 PM, David Ahern wrote:
> On 11/23/17 6:40 AM, Arkadi Sharshevsky wrote:
>>
>>
>> On 11/19/2017 05:58 PM, David Ahern wrote:
>>> On 11/19/17 2:16 AM, Arkadi Sharshevsky wrote:
>>>>
>>>>
>>>> On 11/18/2017 09:19 PM, David Ahern wrote:
>>>>> On 11/14/17 9:18 AM, Jiri Pirko wrote:
>>>>>> From: Arkadi Sharshevsky <arkadis@mellanox.com>
>>>>>>
>>>>>> Connect current dpipe tables to resources. The tables are connected
>>>>>> in the following fashion:
>>>>>> 1. IPv4 host - KVD hash single
>>>>>> 2. IPv6 host - KVD hash double
>>>>>> 3. Adjacency - KVD linear
>>>>>
>>>>> Those descriptions would be helpful to the user. A description attribute
>>>>> for the resources?
>>>>>
>>>>
>>>> As described in the cover letter this resources are used by the
>>>> majority of the ASICs lookup processes. So currently there is one
>>>> to one mapping but is should increase as more tables are exposed,
>>>> so I don't think its a good idea to maintain such an attribute.
>>>>
>>>
>>> 'IPv4 host' yes, but I mean the term 'KVD hash single'? Is it the same
>>> across all h/w vendors? I have only seen that in the context of MLX. If
>>> it is a MLX term then a description to the user that KVD hash single ==
>>> IPv4 host is warranted.
>>>
>>
>> But this relation is wrong, there is no equality here. The LPM, FDB and
>> VID to FID mapping are all can be modeled as lookup tables (via dpipe)
>> that use KVD hash single resource.
>>
>> This description string will grow very long. I dont think this is the
>> right place to document such thing, eitherway, the user can dump the
>> dpipe tables and see which is mapped to what resource.
> 
> Users should not have to find a PRM or user guide for *each version of
> their hardware* to program something so fundamental. This is software.

I still don't understand, you can dump the dpipe table to see which
tables use which resource. Why I need this redundant documentation string
in the kernel?

> We can make it user friendly. Use of vendor specific terms is fine --
> allows correlation to vendor docs. But there should also be text to help

IMHO such documentation strings should not be in the kernel.

> the user correlate vendor terms to generic industry terms.
> 

Really you want to add DEVLINK_RESOURCE_DESCRIPTION string attributed?
"
Used by the following hardware tables
- VID-to-FID
- LPM
- FDB
- HOST
...
"

Again I think it is redundant, just dump those tables. No need to use
user-guides nor PRM.

Re: [patch net-next RFC v2 08/11] mlxsw: spectrum_dpipe: Connect dpipe tables to resources

[Jiri Pirko]

Mon, Nov 27, 2017 at 05:12:47PM CET, dsa@cumulusnetworks.com wrote:
>On 11/23/17 6:40 AM, Arkadi Sharshevsky wrote:
>> 
>> 
>> On 11/19/2017 05:58 PM, David Ahern wrote:
>>> On 11/19/17 2:16 AM, Arkadi Sharshevsky wrote:
>>>>
>>>>
>>>> On 11/18/2017 09:19 PM, David Ahern wrote:
>>>>> On 11/14/17 9:18 AM, Jiri Pirko wrote:
>>>>>> From: Arkadi Sharshevsky <arkadis@mellanox.com>
>>>>>>
>>>>>> Connect current dpipe tables to resources. The tables are connected
>>>>>> in the following fashion:
>>>>>> 1. IPv4 host - KVD hash single
>>>>>> 2. IPv6 host - KVD hash double
>>>>>> 3. Adjacency - KVD linear
>>>>>
>>>>> Those descriptions would be helpful to the user. A description attribute
>>>>> for the resources?
>>>>>
>>>>
>>>> As described in the cover letter this resources are used by the
>>>> majority of the ASICs lookup processes. So currently there is one
>>>> to one mapping but is should increase as more tables are exposed,
>>>> so I don't think its a good idea to maintain such an attribute.
>>>>
>>>
>>> 'IPv4 host' yes, but I mean the term 'KVD hash single'? Is it the same
>>> across all h/w vendors? I have only seen that in the context of MLX. If
>>> it is a MLX term then a description to the user that KVD hash single ==
>>> IPv4 host is warranted.
>>>
>> 
>> But this relation is wrong, there is no equality here. The LPM, FDB and
>> VID to FID mapping are all can be modeled as lookup tables (via dpipe)
>> that use KVD hash single resource.
>> 
>> This description string will grow very long. I dont think this is the
>> right place to document such thing, eitherway, the user can dump the
>> dpipe tables and see which is mapped to what resource.
>
>Users should not have to find a PRM or user guide for *each version of
>their hardware* to program something so fundamental. This is software.
>We can make it user friendly. Use of vendor specific terms is fine --
>allows correlation to vendor docs. But there should also be text to help
>the user correlate vendor terms to generic industry terms.

I have to be missing something. You can easily see the relation between
each dpipe table and resources already as a part of this patchset. The
string you suggest shows the same thing, therefore it is completely
redundant. What am I missing?

Re: [patch net-next RFC v2 08/11] mlxsw: spectrum_dpipe: Connect dpipe tables to resources

[David Ahern]

On 11/28/17 6:27 AM, Jiri Pirko wrote:
> 
> I have to be missing something. You can easily see the relation between
> each dpipe table and resources already as a part of this patchset. The
> string you suggest shows the same thing, therefore it is completely
> redundant. What am I missing?
> 

At this point let's see a working RFC v3 and we can discuss from there.

[patch net-next RFC v2 09/11] mlxsw: spectrum: Add support for getting kvdl occupancy

[Jiri Pirko]

From: Arkadi Sharshevsky <arkadis@mellanox.com>

Add support for getting the kvdl occupancy through the resource interface.

Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c      |  9 +++++++++
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h      |  1 +
 drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c | 19 +++++++++++++++++++
 3 files changed, 29 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index f0cbd67..2f954f5 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4024,12 +4024,21 @@ mlxsw_sp_resource_kvd_hash_double_size_validate(struct devlink *devlink, u64 siz
 	return 0;
 }
 
+static u64 mlxsw_sp_resource_kvd_linear_occ_get(struct devlink *devlink)
+{
+	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
+	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
+
+	return mlxsw_sp_kvdl_occ_get(mlxsw_sp);
+}
+
 static struct devlink_resource_ops mlxsw_sp_resource_kvd_ops = {
 	.size_validate = mlxsw_sp_resource_kvd_size_validate,
 };
 
 static struct devlink_resource_ops mlxsw_sp_resource_kvd_linear_ops = {
 	.size_validate = mlxsw_sp_resource_kvd_linear_size_validate,
+	.occ_get = mlxsw_sp_resource_kvd_linear_occ_get,
 };
 
 static struct devlink_resource_ops mlxsw_sp_resource_kvd_hash_single_ops = {
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index cbfeaf7..b2f76a8 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -470,6 +470,7 @@ void mlxsw_sp_kvdl_free(struct mlxsw_sp *mlxsw_sp, int entry_index);
 int mlxsw_sp_kvdl_alloc_size_query(struct mlxsw_sp *mlxsw_sp,
 				   unsigned int entry_count,
 				   unsigned int *p_alloc_size);
+u64 mlxsw_sp_kvdl_occ_get(const struct mlxsw_sp *mlxsw_sp);
 
 struct mlxsw_sp_acl_rule_info {
 	unsigned int priority;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
index 310c382..9f79eb5 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
@@ -286,6 +286,25 @@ static void mlxsw_sp_kvdl_parts_fini(struct mlxsw_sp *mlxsw_sp)
 		mlxsw_sp_kvdl_part_fini(mlxsw_sp, i);
 }
 
+u64 mlxsw_sp_kvdl_occ_get(const struct mlxsw_sp *mlxsw_sp)
+{
+	struct mlxsw_sp_kvdl_part *part;
+	u64 occ = 0;
+
+	list_for_each_entry(part, &mlxsw_sp->kvdl->parts_list, list) {
+		unsigned int nr_entries;
+		int bit = -1;
+
+		nr_entries = (part->info->end_index -
+			      part->info->start_index + 1) /
+			      part->info->alloc_size;
+		while ((bit = find_next_bit(part->usage, nr_entries, bit + 1))
+		       < nr_entries)
+			occ += part->info->alloc_size;
+	}
+	return occ;
+}
+
 int mlxsw_sp_kvdl_init(struct mlxsw_sp *mlxsw_sp)
 {
 	struct mlxsw_sp_kvdl *kvdl;
-- 
2.9.5

Re: [patch net-next RFC v2 09/11] mlxsw: spectrum: Add support for getting kvdl occupancy

[David Ahern]

On 11/14/17 9:18 AM, Jiri Pirko wrote:
> From: Arkadi Sharshevsky <arkadis@mellanox.com>
> 
> Add support for getting the kvdl occupancy through the resource interface.
> 

Do you intend to add occ_get for the other kvd partitions?

Re: [patch net-next RFC v2 09/11] mlxsw: spectrum: Add support for getting kvdl occupancy

[Arkadi Sharshevsky]

On 11/18/2017 09:21 PM, David Ahern wrote:
> On 11/14/17 9:18 AM, Jiri Pirko wrote:
>> From: Arkadi Sharshevsky <arkadis@mellanox.com>
>>
>> Add support for getting the kvdl occupancy through the resource interface.
>>
> 
> Do you intend to add occ_get for the other kvd partitions?
> 

Yes of course, its a separate patchset due to its complexity.

[patch net-next RFC v2 10/11] mlxsw: pci: Add support for getting resource through devlink

[Jiri Pirko]

From: Arkadi Sharshevsky <arkadis@mellanox.com>

Up until now the KVD partition was static. This patch introduces the
ability to get the resource sizes via devlink. In case the resource is not
available the default configuration is used.

Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c     | 16 ++++++++
 drivers/net/ethernet/mellanox/mlxsw/core.h     |  9 ++++
 drivers/net/ethernet/mellanox/mlxsw/pci.c      | 40 +++++-------------
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 57 ++++++++++++++++++++++++++
 4 files changed, 92 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 2488662..9fe25b1 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1800,6 +1800,22 @@ void mlxsw_core_flush_owq(void)
 }
 EXPORT_SYMBOL(mlxsw_core_flush_owq);
 
+int mlxsw_core_kvd_sizes_get(struct mlxsw_core *mlxsw_core,
+			     const struct mlxsw_config_profile *profile,
+			     u64 *p_single_size, u64 *p_double_size,
+			     u64 *p_linear_size)
+{
+	struct mlxsw_driver *driver =  mlxsw_core->driver;
+
+	if (!driver->kvd_sizes_get)
+		return -EINVAL;
+
+	return driver->kvd_sizes_get(mlxsw_core, profile,
+				     p_single_size, p_double_size,
+				     p_linear_size);
+}
+EXPORT_SYMBOL(mlxsw_core_kvd_sizes_get);
+
 static int __init mlxsw_core_module_init(void)
 {
 	int err;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index e23f83b..e44061d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -309,10 +309,19 @@ struct mlxsw_driver {
 	void (*txhdr_construct)(struct sk_buff *skb,
 				const struct mlxsw_tx_info *tx_info);
 	int (*resources_register)(struct mlxsw_core *mlxsw_core);
+	int (*kvd_sizes_get)(struct mlxsw_core *mlxsw_core,
+			     const struct mlxsw_config_profile *profile,
+			     u64 *p_single_size, u64 *p_double_size,
+			     u64 *p_linear_size);
 	u8 txhdr_len;
 	const struct mlxsw_config_profile *profile;
 };
 
+int mlxsw_core_kvd_sizes_get(struct mlxsw_core *mlxsw_core,
+			     const struct mlxsw_config_profile *profile,
+			     u64 *p_single_size, u64 *p_double_size,
+			     u64 *p_linear_size);
+
 bool mlxsw_core_res_valid(struct mlxsw_core *mlxsw_core,
 			  enum mlxsw_res_id res_id);
 
diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
index 00c9155..da08f08 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c
@@ -1053,38 +1053,18 @@ static int mlxsw_pci_resources_query(struct mlxsw_pci *mlxsw_pci, char *mbox,
 }
 
 static int
-mlxsw_pci_profile_get_kvd_sizes(const struct mlxsw_config_profile *profile,
+mlxsw_pci_profile_get_kvd_sizes(const struct mlxsw_pci *mlxsw_pci,
+				const struct mlxsw_config_profile *profile,
 				struct mlxsw_res *res)
 {
-	u32 single_size, double_size, linear_size;
-
-	if (!MLXSW_RES_VALID(res, KVD_SINGLE_MIN_SIZE) ||
-	    !MLXSW_RES_VALID(res, KVD_DOUBLE_MIN_SIZE) ||
-	    !profile->used_kvd_split_data)
-		return -EIO;
-
-	linear_size = profile->kvd_linear_size;
+	u64 single_size, double_size, linear_size;
+	int err;
 
-	/* The hash part is what left of the kvd without the
-	 * linear part. It is split to the single size and
-	 * double size by the parts ratio from the profile.
-	 * Both sizes must be a multiplications of the
-	 * granularity from the profile.
-	 */
-	double_size = MLXSW_RES_GET(res, KVD_SIZE) - linear_size;
-	double_size *= profile->kvd_hash_double_parts;
-	double_size /= profile->kvd_hash_double_parts +
-		       profile->kvd_hash_single_parts;
-	double_size /= profile->kvd_hash_granularity;
-	double_size *= profile->kvd_hash_granularity;
-	single_size = MLXSW_RES_GET(res, KVD_SIZE) - double_size -
-		      linear_size;
-
-	/* Check results are legal. */
-	if (single_size < MLXSW_RES_GET(res, KVD_SINGLE_MIN_SIZE) ||
-	    double_size < MLXSW_RES_GET(res, KVD_DOUBLE_MIN_SIZE) ||
-	    MLXSW_RES_GET(res, KVD_SIZE) < linear_size)
-		return -EIO;
+	err = mlxsw_core_kvd_sizes_get(mlxsw_pci->core, profile,
+				       &single_size, &double_size,
+				       &linear_size);
+	if (err)
+		return err;
 
 	MLXSW_RES_SET(res, KVD_SINGLE_SIZE, single_size);
 	MLXSW_RES_SET(res, KVD_DOUBLE_SIZE, double_size);
@@ -1185,7 +1165,7 @@ static int mlxsw_pci_config_profile(struct mlxsw_pci *mlxsw_pci, char *mbox,
 			mbox, profile->adaptive_routing_group_cap);
 	}
 	if (MLXSW_RES_VALID(res, KVD_SIZE)) {
-		err = mlxsw_pci_profile_get_kvd_sizes(profile, res);
+		err = mlxsw_pci_profile_get_kvd_sizes(mlxsw_pci, profile, res);
 		if (err)
 			return err;
 
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 2f954f5..6a1a02a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4103,6 +4103,62 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
 	return 0;
 }
 
+static int mlxsw_sp_kvd_sizes_get(struct mlxsw_core *mlxsw_core,
+				  const struct mlxsw_config_profile *profile,
+				  u64 *p_single_size, u64 *p_double_size,
+				  u64 *p_linear_size)
+{
+	struct devlink *devlink = priv_to_devlink(mlxsw_core);
+	u64 double_size;
+	int err;
+
+	if (!MLXSW_CORE_RES_VALID(mlxsw_core, KVD_SINGLE_MIN_SIZE) ||
+	    !MLXSW_CORE_RES_VALID(mlxsw_core, KVD_DOUBLE_MIN_SIZE) ||
+	    !profile->used_kvd_split_data)
+		return -EIO;
+
+	/* The hash part is what left of the kvd without the
+	 * linear part. It is split to the single size and
+	 * double size by the parts ratio from the profile.
+	 * Both sizes must be a multiplications of the
+	 * granularity from the profile. In case the user
+	 * provided the sizes they are obtained via devlink.
+	 */
+	err = devlink_resource_size_get(devlink,
+					MLXSW_SP_RESOURCE_KVD_LINEAR,
+					p_linear_size);
+	if (err)
+		*p_linear_size = profile->kvd_linear_size;
+
+	err = devlink_resource_size_get(devlink,
+					MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE,
+					p_double_size);
+	if (err) {
+		double_size = MLXSW_CORE_RES_GET(mlxsw_core, KVD_SIZE) -
+			      *p_linear_size;
+		double_size *= profile->kvd_hash_double_parts;
+		double_size /= profile->kvd_hash_double_parts +
+			       profile->kvd_hash_single_parts;
+		*p_double_size = rounddown(double_size,
+					   profile->kvd_hash_granularity);
+	}
+
+	err = devlink_resource_size_get(devlink,
+					MLXSW_SP_RESOURCE_KVD_HASH_SINGLE,
+					p_single_size);
+	if (err)
+		*p_single_size = MLXSW_CORE_RES_GET(mlxsw_core, KVD_SIZE) -
+				 *p_double_size - *p_linear_size;
+
+	/* Check results are legal. */
+	if (*p_single_size < MLXSW_CORE_RES_GET(mlxsw_core, KVD_SINGLE_MIN_SIZE) ||
+	    *p_double_size < MLXSW_CORE_RES_GET(mlxsw_core, KVD_DOUBLE_MIN_SIZE) ||
+	    MLXSW_CORE_RES_GET(mlxsw_core, KVD_SIZE) < *p_linear_size)
+		return -EIO;
+
+	return 0;
+}
+
 static struct mlxsw_driver mlxsw_sp_driver = {
 	.kind				= mlxsw_sp_driver_name,
 	.priv_size			= sizeof(struct mlxsw_sp),
@@ -4123,6 +4179,7 @@ static struct mlxsw_driver mlxsw_sp_driver = {
 	.sb_occ_tc_port_bind_get	= mlxsw_sp_sb_occ_tc_port_bind_get,
 	.txhdr_construct		= mlxsw_sp_txhdr_construct,
 	.resources_register		= mlxsw_sp_resources_register,
+	.kvd_sizes_get			= mlxsw_sp_kvd_sizes_get,
 	.txhdr_len			= MLXSW_TXHDR_LEN,
 	.profile			= &mlxsw_sp_config_profile,
 };
-- 
2.9.5

[patch net-next RFC v2 11/11] mlxsw: core: Add support for reload

[Jiri Pirko]

From: Arkadi Sharshevsky <arkadis@mellanox.com>

Add support for hot reload. First, all the driver/core resources are
released but the PCI and devlink instances, then reset is performed
through the PCI interface. Finally the driver performs initialization.

In case of reload failure the driver is left in a partially initialized
state. Special care is taken during the driver removal in order to
properly handle this state.

Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c | 64 +++++++++++++++++++++++-------
 drivers/net/ethernet/mellanox/mlxsw/core.h |  5 ++-
 drivers/net/ethernet/mellanox/mlxsw/i2c.c  |  5 ++-
 drivers/net/ethernet/mellanox/mlxsw/pci.c  |  5 ++-
 4 files changed, 59 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 9fe25b1..4b33919 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -113,6 +113,7 @@ struct mlxsw_core {
 	struct mlxsw_thermal *thermal;
 	struct mlxsw_core_port *ports;
 	unsigned int max_ports;
+	bool reload_fail;
 	unsigned long driver_priv[0];
 	/* driver_priv has to be always the last item */
 };
@@ -962,7 +963,28 @@ 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)
+{
+	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
+	const struct mlxsw_bus *mlxsw_bus = mlxsw_core->bus;
+	int err;
+
+	if (!mlxsw_bus->reset)
+		return -EOPNOTSUPP;
+
+	mlxsw_core_bus_device_unregister(mlxsw_core, true);
+	mlxsw_bus->reset(mlxsw_core->bus_priv);
+	err = mlxsw_core_bus_device_register(mlxsw_core->bus_info,
+					     mlxsw_core->bus,
+					     mlxsw_core->bus_priv, true,
+					     devlink);
+	if (err)
+		mlxsw_core->reload_fail = true;
+	return err;
+}
+
 static const struct devlink_ops mlxsw_devlink_ops = {
+	.reload				= mlxsw_devlink_core_bus_device_reload,
 	.port_type_set			= mlxsw_devlink_port_type_set,
 	.port_split			= mlxsw_devlink_port_split,
 	.port_unsplit			= mlxsw_devlink_port_unsplit,
@@ -980,23 +1002,26 @@ static const struct devlink_ops mlxsw_devlink_ops = {
 
 int mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info,
 				   const struct mlxsw_bus *mlxsw_bus,
-				   void *bus_priv)
+				   void *bus_priv, bool reload,
+				   struct devlink *devlink)
 {
 	const char *device_kind = mlxsw_bus_info->device_kind;
 	struct mlxsw_core *mlxsw_core;
 	struct mlxsw_driver *mlxsw_driver;
-	struct devlink *devlink;
 	size_t alloc_size;
 	int err;
 
 	mlxsw_driver = mlxsw_core_driver_get(device_kind);
 	if (!mlxsw_driver)
 		return -EINVAL;
-	alloc_size = sizeof(*mlxsw_core) + mlxsw_driver->priv_size;
-	devlink = devlink_alloc(&mlxsw_devlink_ops, alloc_size);
-	if (!devlink) {
-		err = -ENOMEM;
-		goto err_devlink_alloc;
+
+	if (!reload) {
+		alloc_size = sizeof(*mlxsw_core) + mlxsw_driver->priv_size;
+		devlink = devlink_alloc(&mlxsw_devlink_ops, alloc_size);
+		if (!devlink) {
+			err = -ENOMEM;
+			goto err_devlink_alloc;
+		}
 	}
 
 	mlxsw_core = devlink_priv(devlink);
@@ -1012,7 +1037,7 @@ int mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info,
 	if (err)
 		goto err_bus_init;
 
-	if (mlxsw_driver->resources_register) {
+	if (mlxsw_driver->resources_register && !reload) {
 		err = mlxsw_driver->resources_register(mlxsw_core);
 		if (err)
 			goto err_register_resources;
@@ -1038,9 +1063,11 @@ int mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info,
 	if (err)
 		goto err_emad_init;
 
-	err = devlink_register(devlink, mlxsw_bus_info->dev);
-	if (err)
-		goto err_devlink_register;
+	if (!reload) {
+		err = devlink_register(devlink, mlxsw_bus_info->dev);
+		if (err)
+			goto err_devlink_register;
+	}
 
 	err = mlxsw_hwmon_init(mlxsw_core, mlxsw_bus_info, &mlxsw_core->hwmon);
 	if (err)
@@ -1082,20 +1109,29 @@ int mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info,
 }
 EXPORT_SYMBOL(mlxsw_core_bus_device_register);
 
-void mlxsw_core_bus_device_unregister(struct mlxsw_core *mlxsw_core)
+void mlxsw_core_bus_device_unregister(struct mlxsw_core *mlxsw_core,
+				      bool reload)
 {
 	const char *device_kind = mlxsw_core->bus_info->device_kind;
 	struct devlink *devlink = priv_to_devlink(mlxsw_core);
 
+	if (mlxsw_core->reload_fail)
+		goto out;
+
 	if (mlxsw_core->driver->fini)
 		mlxsw_core->driver->fini(mlxsw_core);
 	mlxsw_thermal_fini(mlxsw_core->thermal);
-	devlink_unregister(devlink);
+	if (!reload)
+		devlink_unregister(devlink);
 	mlxsw_emad_fini(mlxsw_core);
 	kfree(mlxsw_core->lag.mapping);
 	mlxsw_ports_fini(mlxsw_core);
-	devlink_resources_unregister(devlink, NULL);
+	if (!reload)
+		devlink_resources_unregister(devlink, NULL);
 	mlxsw_core->bus->fini(mlxsw_core->bus_priv);
+	if (reload)
+		return;
+out:
 	devlink_free(devlink);
 	mlxsw_core_driver_put(device_kind);
 }
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index e44061d..5ddafd7 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -66,8 +66,9 @@ void mlxsw_core_driver_unregister(struct mlxsw_driver *mlxsw_driver);
 
 int mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info,
 				   const struct mlxsw_bus *mlxsw_bus,
-				   void *bus_priv);
-void mlxsw_core_bus_device_unregister(struct mlxsw_core *mlxsw_core);
+				   void *bus_priv, bool reload,
+				   struct devlink *devlink);
+void mlxsw_core_bus_device_unregister(struct mlxsw_core *mlxsw_core, bool reload);
 
 struct mlxsw_tx_info {
 	u8 local_port;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/i2c.c b/drivers/net/ethernet/mellanox/mlxsw/i2c.c
index c0dcfa0..25f9915 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/i2c.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/i2c.c
@@ -539,7 +539,8 @@ static int mlxsw_i2c_probe(struct i2c_client *client,
 	mlxsw_i2c->dev = &client->dev;
 
 	err = mlxsw_core_bus_device_register(&mlxsw_i2c->bus_info,
-					     &mlxsw_i2c_bus, mlxsw_i2c);
+					     &mlxsw_i2c_bus, mlxsw_i2c, false,
+					     NULL);
 	if (err) {
 		dev_err(&client->dev, "Fail to register core bus\n");
 		return err;
@@ -557,7 +558,7 @@ static int mlxsw_i2c_remove(struct i2c_client *client)
 {
 	struct mlxsw_i2c *mlxsw_i2c = i2c_get_clientdata(client);
 
-	mlxsw_core_bus_device_unregister(mlxsw_i2c->core);
+	mlxsw_core_bus_device_unregister(mlxsw_i2c->core, false);
 	mutex_destroy(&mlxsw_i2c->cmd.lock);
 
 	return 0;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
index da08f08..bb96620 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c
@@ -1737,7 +1737,8 @@ static int mlxsw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	mlxsw_pci->id = id;
 
 	err = mlxsw_core_bus_device_register(&mlxsw_pci->bus_info,
-					     &mlxsw_pci_bus, mlxsw_pci);
+					     &mlxsw_pci_bus, mlxsw_pci, false,
+					     NULL);
 	if (err) {
 		dev_err(&pdev->dev, "cannot register bus device\n");
 		goto err_bus_device_register;
@@ -1765,7 +1766,7 @@ static void mlxsw_pci_remove(struct pci_dev *pdev)
 {
 	struct mlxsw_pci *mlxsw_pci = pci_get_drvdata(pdev);
 
-	mlxsw_core_bus_device_unregister(mlxsw_pci->core);
+	mlxsw_core_bus_device_unregister(mlxsw_pci->core, false);
 	mlxsw_pci_free_irq_vectors(mlxsw_pci);
 	iounmap(mlxsw_pci->hw_addr);
 	pci_release_regions(mlxsw_pci->pdev);
-- 
2.9.5

Re: [patch net-next RFC v2 00/11] Add support for resource abstraction

[David Ahern]

On 11/14/17 9:18 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Arkadi says:
> 
> Many of the ASIC's internal resources are limited and are shared between
> several hardware procedures. For example, unified hash-based memory can
> be used for many lookup purposes, like FDB and LPM. In many cases the user
> can provide a partitioning scheme for such a resource in order to perform
> fine tuning for his application. In many cases after setting the
> partitioning of the resource driver reload is needed. This patchset add
> support for hot reset of the driver.
> 
> Such an abstraction can be coupled with devlink's dpipe interface, which
> models the ASIC's pipeline as an graph of match/action tables. By modeling
> the hardware resource object, and by coupling it to several dpipe tables,
> further visibility can be achieved in order to debug ASIC-wide issues.
> 
> The proposed interface will provide the user the ability to understand the
> limitations of the hardware, and receive notification regarding its occupancy.
> Furthermore, monitoring the resource occupancy can be done in real-time and
> can be useful in many cases.
> 
> Userspace part prototype can be found at https://github.com/arkadis/iproute2/
> at resource_dev branch.
> 

now that my firmware problem is fixed, I installed a build with this
patch set. Trying to run devlink to split a port hangs:

$ devlink port split swp1 count 4


[  615.373359] INFO: task devlink:804 blocked for more than 120 seconds.
[  615.379934]       Tainted: G        W       4.14.0+ #38
[  615.385238] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
disables this message.
[  615.393111] devlink         D    0   804    771 0x00000080
[  615.393115] Call Trace:
[  615.393126]  __schedule+0x1de/0x690
[  615.393130]  schedule+0x36/0x80
[  615.393139]  schedule_preempt_disabled+0xe/0x10
[  615.393146]  __mutex_lock.isra.4+0x211/0x530
[  615.393152]  __mutex_lock_slowpath+0x13/0x20
[  615.393155]  ? __mutex_lock_slowpath+0x13/0x20
[  615.393158]  mutex_lock+0x2f/0x40
[  615.393164]  devlink_port_unregister+0x29/0x60 [devlink]
[  615.393169]  mlxsw_core_port_fini+0x25/0x50 [mlxsw_core]
[  615.393179]  mlxsw_sp_port_remove+0xf0/0x100 [mlxsw_spectrum]
[  615.393186]  mlxsw_sp_port_split+0xdc/0x260 [mlxsw_spectrum]
[  615.393193]  ? _cond_resched+0x19/0x30
[  615.393200]  mlxsw_devlink_port_split+0x36/0x50 [mlxsw_core]
[  615.393206]  devlink_nl_cmd_port_split_doit+0x42/0x50 [devlink]
[  615.393212]  genl_family_rcv_msg+0x1c9/0x390
[  615.393217]  genl_rcv_msg+0x4c/0xa0
[  615.393220]  ? _cond_resched+0x19/0x30
[  615.393228]  ? genl_family_rcv_msg+0x390/0x390
[  615.393232]  netlink_rcv_skb+0xec/0x120
[  615.393235]  genl_rcv+0x28/0x40
[  615.393239]  netlink_unicast+0x170/0x230
[  615.393244]  netlink_sendmsg+0x28e/0x370
[  615.393251]  SYSC_sendto+0x10e/0x1b0
[  615.393258]  ? __audit_syscall_entry+0xc1/0x110
[  615.393261]  ? syscall_trace_enter+0x1c6/0x2d0
[  615.393264]  ? __do_page_fault+0x231/0x4b0
[  615.393268]  SyS_sendto+0xe/0x10
[  615.393272]  do_syscall_64+0x60/0x1f0
[  615.393277]  entry_SYSCALL64_slow_path+0x25/0x25
[  615.393280] RIP: 0033:0x7f4ef43c16f3
[  615.393284] RSP: 002b:00007fffb907fbc8 EFLAGS: 00000246 ORIG_RAX:
000000000000002c
[  615.393287] RAX: ffffffffffffffda RBX: 00000000013660e0 RCX:
00007f4ef43c16f3
[  615.393290] RDX: 0000000000000040 RSI: 0000000001366110 RDI:
0000000000000003
[  615.393291] RBP: 0000000000000000 R08: 00007f4ef4686d80 R09:
000000000000000c
[  615.393292] R10: 0000000000000000 R11: 0000000000000246 R12:
0000000000000000
[  615.393296] R13: 0000000000000004 R14: 0000000000000000 R15:
0000000000000000

Re: [patch net-next RFC v2 00/11] Add support for resource abstraction

[Arkadi Sharshevsky]

On 11/17/2017 09:58 PM, David Ahern wrote:
> On 11/14/17 9:18 AM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Arkadi says:
>>
>> Many of the ASIC's internal resources are limited and are shared between
>> several hardware procedures. For example, unified hash-based memory can
>> be used for many lookup purposes, like FDB and LPM. In many cases the user
>> can provide a partitioning scheme for such a resource in order to perform
>> fine tuning for his application. In many cases after setting the
>> partitioning of the resource driver reload is needed. This patchset add
>> support for hot reset of the driver.
>>
>> Such an abstraction can be coupled with devlink's dpipe interface, which
>> models the ASIC's pipeline as an graph of match/action tables. By modeling
>> the hardware resource object, and by coupling it to several dpipe tables,
>> further visibility can be achieved in order to debug ASIC-wide issues.
>>
>> The proposed interface will provide the user the ability to understand the
>> limitations of the hardware, and receive notification regarding its occupancy.
>> Furthermore, monitoring the resource occupancy can be done in real-time and
>> can be useful in many cases.
>>
>> Userspace part prototype can be found at https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Farkadis%2Fiproute2%2F&data=02%7C01%7Carkadis%40mellanox.com%7Cbf79bc51b9c641e1c3cc08d52df59f29%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636465455333867183&sdata=JHpevZLdZJH2Imk%2FLpEaEbRTlAGMYP6GYaxTsNWHaig%3D&reserved=0
>> at resource_dev branch.
>>
> 
> now that my firmware problem is fixed, I installed a build with this
> patch set. Trying to run devlink to split a port hangs:
> 
> $ devlink port split swp1 count 4
> 
> 
> [  615.373359] INFO: task devlink:804 blocked for more than 120 seconds.
> [  615.379934]       Tainted: G        W       4.14.0+ #38
> [  615.385238] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
> disables this message.
> [  615.393111] devlink         D    0   804    771 0x00000080
> [  615.393115] Call Trace:
> [  615.393126]  __schedule+0x1de/0x690
> [  615.393130]  schedule+0x36/0x80
> [  615.393139]  schedule_preempt_disabled+0xe/0x10
> [  615.393146]  __mutex_lock.isra.4+0x211/0x530
> [  615.393152]  __mutex_lock_slowpath+0x13/0x20
> [  615.393155]  ? __mutex_lock_slowpath+0x13/0x20
> [  615.393158]  mutex_lock+0x2f/0x40
> [  615.393164]  devlink_port_unregister+0x29/0x60 [devlink]
> [  615.393169]  mlxsw_core_port_fini+0x25/0x50 [mlxsw_core]
> [  615.393179]  mlxsw_sp_port_remove+0xf0/0x100 [mlxsw_spectrum]
> [  615.393186]  mlxsw_sp_port_split+0xdc/0x260 [mlxsw_spectrum]
> [  615.393193]  ? _cond_resched+0x19/0x30
> [  615.393200]  mlxsw_devlink_port_split+0x36/0x50 [mlxsw_core]
> [  615.393206]  devlink_nl_cmd_port_split_doit+0x42/0x50 [devlink]
> [  615.393212]  genl_family_rcv_msg+0x1c9/0x390
> [  615.393217]  genl_rcv_msg+0x4c/0xa0
> [  615.393220]  ? _cond_resched+0x19/0x30
> [  615.393228]  ? genl_family_rcv_msg+0x390/0x390
> [  615.393232]  netlink_rcv_skb+0xec/0x120
> [  615.393235]  genl_rcv+0x28/0x40
> [  615.393239]  netlink_unicast+0x170/0x230
> [  615.393244]  netlink_sendmsg+0x28e/0x370
> [  615.393251]  SYSC_sendto+0x10e/0x1b0
> [  615.393258]  ? __audit_syscall_entry+0xc1/0x110
> [  615.393261]  ? syscall_trace_enter+0x1c6/0x2d0
> [  615.393264]  ? __do_page_fault+0x231/0x4b0
> [  615.393268]  SyS_sendto+0xe/0x10
> [  615.393272]  do_syscall_64+0x60/0x1f0
> [  615.393277]  entry_SYSCALL64_slow_path+0x25/0x25
> [  615.393280] RIP: 0033:0x7f4ef43c16f3
> [  615.393284] RSP: 002b:00007fffb907fbc8 EFLAGS: 00000246 ORIG_RAX:
> 000000000000002c
> [  615.393287] RAX: ffffffffffffffda RBX: 00000000013660e0 RCX:
> 00007f4ef43c16f3
> [  615.393290] RDX: 0000000000000040 RSI: 0000000001366110 RDI:
> 0000000000000003
> [  615.393291] RBP: 0000000000000000 R08: 00007f4ef4686d80 R09:
> 000000000000000c
> [  615.393292] R10: 0000000000000000 R11: 0000000000000246 R12:
> 0000000000000000
> [  615.393296] R13: 0000000000000004 R14: 0000000000000000 R15:
> 0000000000000000
> 

Thanks, will fix

Re: [patch net-next RFC v2 00/11] Add support for resource abstraction

[David Ahern]

On 11/14/17 9:18 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Arkadi says:
> 
> Many of the ASIC's internal resources are limited and are shared between
> several hardware procedures. For example, unified hash-based memory can
> be used for many lookup purposes, like FDB and LPM. In many cases the user
> can provide a partitioning scheme for such a resource in order to perform
> fine tuning for his application. In many cases after setting the
> partitioning of the resource driver reload is needed. This patchset add
> support for hot reset of the driver.
> 
> Such an abstraction can be coupled with devlink's dpipe interface, which
> models the ASIC's pipeline as an graph of match/action tables. By modeling
> the hardware resource object, and by coupling it to several dpipe tables,
> further visibility can be achieved in order to debug ASIC-wide issues.
> 
> The proposed interface will provide the user the ability to understand the
> limitations of the hardware, and receive notification regarding its occupancy.
> Furthermore, monitoring the resource occupancy can be done in real-time and
> can be useful in many cases.
> 
> Userspace part prototype can be found at https://github.com/arkadis/iproute2/
> at resource_dev branch.
> 

Please add the example user commands from the above to this
cover-letter. Makes easier to understand the intentions of this patch
set from a user perspective.