[PATCH net-next 00/11] mlxsw: Various cleanups

[PATCH net-next 00/11] mlxsw: Various cleanups

[Ido Schimmel]

Hi,

The first 10 patches from Jiri perform small and unrelated cleanups. The
largest being the conversion of the KVD linear partitions from a list to
an array, which simplifies the code.

The last patch from Petr is a bug fix for a recent net-next commit that
prevented the "kvd" resource from being marked as the parent of its
various child resources (e.g., "/kvd/linear").

Jiri Pirko (10):
  mlxsw: spectrum_acl: Fix flex actions header ifndef define construct
  mlxsw: spectrum_kvdl: Fix handling of resource_size_param
  mlxsw: Constify devlink_resource_ops
  mlxsw: spectrum: Change KVD linear parts from list to array
  mlxsw: remove kvd_hash_granularity from config profile struct
  mlxsw: core: Fix arg name of MLXSW_CORE_RES_VALID and
    MLXSW_CORE_RES_GET
  mlxsw: Move "used_kvd_sizes" check to mlxsw_pci_config_profile
  mlxsw: Move "resources_query_enable" out of mlxsw_config_profile
  devlink: convert occ_get op to separate registration
  mlxsw: spectrum: Pass mlxsw_core as arg of
    mlxsw_sp_kvdl_resources_register()

Petr Machata (1):
  mlxsw: spectrum: Don't use resource ID of 0

 drivers/net/ethernet/mellanox/mlxsw/core.c         |   5 +-
 drivers/net/ethernet/mellanox/mlxsw/core.h         |  14 +-
 drivers/net/ethernet/mellanox/mlxsw/pci.c          |  11 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     |  38 +--
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |   5 +-
 .../mellanox/mlxsw/spectrum_acl_flex_actions.h     |   4 +-
 .../net/ethernet/mellanox/mlxsw/spectrum_kvdl.c    | 339 +++++++++------------
 drivers/net/ethernet/mellanox/mlxsw/switchib.c     |   1 -
 drivers/net/ethernet/mellanox/mlxsw/switchx2.c     |   1 -
 include/net/devlink.h                              |  40 ++-
 net/core/devlink.c                                 |  74 ++++-
 11 files changed, 258 insertions(+), 274 deletions(-)

-- 
2.14.3

[PATCH net-next 01/11] mlxsw: spectrum_acl: Fix flex actions header ifndef define construct

[Ido Schimmel]

From: Jiri Pirko <jiri@mellanox.com>

Fix copy&paste error in flex actions header ifndef define construct

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_flex_actions.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_flex_actions.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_flex_actions.h
index 2726192836ad..bd6d552d95b9 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_flex_actions.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_flex_actions.h
@@ -33,8 +33,8 @@
  * POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef _MLXSW_SPECTRUM_ACL_FLEX_KEYS_H
-#define _MLXSW_SPECTRUM_ACL_FLEX_KEYS_H
+#ifndef _MLXSW_SPECTRUM_ACL_FLEX_ACTIONS_H
+#define _MLXSW_SPECTRUM_ACL_FLEX_ACTIONS_H
 
 #include "spectrum.h"
 
-- 
2.14.3

[PATCH net-next 02/11] mlxsw: spectrum_kvdl: Fix handling of resource_size_param

[Ido Schimmel]

From: Jiri Pirko <jiri@mellanox.com>

Current code uses global variables, adjusts them and passes pointer down
to devlink. With every other mlxsw_core instance, the previously passed
pointer values are rewritten. Fix this by de-globalize the variables.

Fixes: 7f47b19bd744 ("mlxsw: spectrum_kvdl: Add support for per part occupancy")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Acked-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_kvdl.c    | 47 +++++++---------------
 1 file changed, 14 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
index 85503e93b93f..9e61518c4945 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
@@ -420,67 +420,48 @@ static struct devlink_resource_ops mlxsw_sp_kvdl_chunks_large_ops = {
 	.occ_get = mlxsw_sp_kvdl_large_chunks_occ_get,
 };
 
-static struct devlink_resource_size_params mlxsw_sp_kvdl_single_size_params = {
-	.size_min = 0,
-	.size_granularity = 1,
-	.unit = DEVLINK_RESOURCE_UNIT_ENTRY,
-};
-
-static struct devlink_resource_size_params mlxsw_sp_kvdl_chunks_size_params = {
-	.size_min = 0,
-	.size_granularity = MLXSW_SP_CHUNK_MAX,
-	.unit = DEVLINK_RESOURCE_UNIT_ENTRY,
-};
-
-static struct devlink_resource_size_params mlxsw_sp_kvdl_large_chunks_size_params = {
-	.size_min = 0,
-	.size_granularity = MLXSW_SP_LARGE_CHUNK_MAX,
-	.unit = DEVLINK_RESOURCE_UNIT_ENTRY,
-};
-
-static void
-mlxsw_sp_kvdl_resource_size_params_prepare(struct devlink *devlink)
+int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
 {
 	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
+	static struct devlink_resource_size_params size_params;
 	u32 kvdl_max_size;
+	int err;
 
 	kvdl_max_size = MLXSW_CORE_RES_GET(mlxsw_core, KVD_SIZE) -
 			MLXSW_CORE_RES_GET(mlxsw_core, KVD_SINGLE_MIN_SIZE) -
 			MLXSW_CORE_RES_GET(mlxsw_core, KVD_DOUBLE_MIN_SIZE);
 
-	mlxsw_sp_kvdl_single_size_params.size_max = kvdl_max_size;
-	mlxsw_sp_kvdl_chunks_size_params.size_max = kvdl_max_size;
-	mlxsw_sp_kvdl_large_chunks_size_params.size_max = kvdl_max_size;
-}
-
-int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
-{
-	int err;
-
-	mlxsw_sp_kvdl_resource_size_params_prepare(devlink);
+	devlink_resource_size_params_init(&size_params, 0, kvdl_max_size, 1,
+					  DEVLINK_RESOURCE_UNIT_ENTRY);
 	err = devlink_resource_register(devlink, MLXSW_SP_RESOURCE_NAME_KVD_LINEAR_SINGLES,
 					MLXSW_SP_KVDL_SINGLE_SIZE,
 					MLXSW_SP_RESOURCE_KVD_LINEAR_SINGLE,
 					MLXSW_SP_RESOURCE_KVD_LINEAR,
-					&mlxsw_sp_kvdl_single_size_params,
+					&size_params,
 					&mlxsw_sp_kvdl_single_ops);
 	if (err)
 		return err;
 
+	devlink_resource_size_params_init(&size_params, 0, kvdl_max_size,
+					  MLXSW_SP_CHUNK_MAX,
+					  DEVLINK_RESOURCE_UNIT_ENTRY);
 	err = devlink_resource_register(devlink, MLXSW_SP_RESOURCE_NAME_KVD_LINEAR_CHUNKS,
 					MLXSW_SP_KVDL_CHUNKS_SIZE,
 					MLXSW_SP_RESOURCE_KVD_LINEAR_CHUNKS,
 					MLXSW_SP_RESOURCE_KVD_LINEAR,
-					&mlxsw_sp_kvdl_chunks_size_params,
+					&size_params,
 					&mlxsw_sp_kvdl_chunks_ops);
 	if (err)
 		return err;
 
+	devlink_resource_size_params_init(&size_params, 0, kvdl_max_size,
+					  MLXSW_SP_LARGE_CHUNK_MAX,
+					  DEVLINK_RESOURCE_UNIT_ENTRY);
 	err = devlink_resource_register(devlink, MLXSW_SP_RESOURCE_NAME_KVD_LINEAR_LARGE_CHUNKS,
 					MLXSW_SP_KVDL_LARGE_CHUNKS_SIZE,
 					MLXSW_SP_RESOURCE_KVD_LINEAR_LARGE_CHUNKS,
 					MLXSW_SP_RESOURCE_KVD_LINEAR,
-					&mlxsw_sp_kvdl_large_chunks_size_params,
+					&size_params,
 					&mlxsw_sp_kvdl_chunks_large_ops);
 	return err;
 }
-- 
2.14.3

[PATCH net-next 03/11] mlxsw: Constify devlink_resource_ops

[Ido Schimmel]

From: Jiri Pirko <jiri@mellanox.com>

devlink_resource_ops should be const as the arg of register function is
also const.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c      | 2 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 4aa84442e357..0e9ed41ce8bc 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3815,7 +3815,7 @@ static u64 mlxsw_sp_resource_kvd_linear_occ_get(struct devlink *devlink)
 	return mlxsw_sp_kvdl_occ_get(mlxsw_sp);
 }
 
-static struct devlink_resource_ops mlxsw_sp_resource_kvd_linear_ops = {
+static const struct devlink_resource_ops mlxsw_sp_resource_kvd_linear_ops = {
 	.occ_get = mlxsw_sp_resource_kvd_linear_occ_get,
 };
 
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
index 9e61518c4945..201825c0019b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
@@ -408,15 +408,15 @@ static u64 mlxsw_sp_kvdl_large_chunks_occ_get(struct devlink *devlink)
 	return mlxsw_sp_kvdl_part_occ(part);
 }
 
-static struct devlink_resource_ops mlxsw_sp_kvdl_single_ops = {
+static const struct devlink_resource_ops mlxsw_sp_kvdl_single_ops = {
 	.occ_get = mlxsw_sp_kvdl_single_occ_get,
 };
 
-static struct devlink_resource_ops mlxsw_sp_kvdl_chunks_ops = {
+static const struct devlink_resource_ops mlxsw_sp_kvdl_chunks_ops = {
 	.occ_get = mlxsw_sp_kvdl_chunks_occ_get,
 };
 
-static struct devlink_resource_ops mlxsw_sp_kvdl_chunks_large_ops = {
+static const struct devlink_resource_ops mlxsw_sp_kvdl_chunks_large_ops = {
 	.occ_get = mlxsw_sp_kvdl_large_chunks_occ_get,
 };
 
-- 
2.14.3

[PATCH net-next 04/11] mlxsw: spectrum: Change KVD linear parts from list to array

[Ido Schimmel]

From: Jiri Pirko <jiri@mellanox.com>

The parts info is array. The parts copy this info array, yet they are a
list. So make the indexing according to the id and change the list of
parts into array of parts. This helps to eliminate lookups and
constructs like mlxsw_sp_kvdl_part_update() (took me some non-trivial
time to figure out what is going on there).
Alongside with that, introduce a helper macro to define the parts infos.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_kvdl.c    | 235 ++++++++-------------
 1 file changed, 92 insertions(+), 143 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
index 201825c0019b..7b28f65d6407 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
@@ -55,24 +55,47 @@
 #define MLXSW_SP_KVDL_LARGE_CHUNKS_END \
 	(MLXSW_SP_KVDL_LARGE_CHUNKS_SIZE + MLXSW_SP_KVDL_LARGE_CHUNKS_BASE - 1)
 
-#define MLXSW_SP_CHUNK_MAX 32
-#define MLXSW_SP_LARGE_CHUNK_MAX 512
+#define MLXSW_SP_KVDL_SINGLE_ALLOC_SIZE 1
+#define MLXSW_SP_KVDL_CHUNKS_ALLOC_SIZE 32
+#define MLXSW_SP_KVDL_LARGE_CHUNKS_ALLOC_SIZE 512
 
 struct mlxsw_sp_kvdl_part_info {
 	unsigned int part_index;
 	unsigned int start_index;
 	unsigned int end_index;
 	unsigned int alloc_size;
+	enum mlxsw_sp_resource_id resource_id;
+};
+
+enum mlxsw_sp_kvdl_part_id {
+	MLXSW_SP_KVDL_PART_ID_SINGLE,
+	MLXSW_SP_KVDL_PART_ID_CHUNKS,
+	MLXSW_SP_KVDL_PART_ID_LARGE_CHUNKS,
 };
 
+#define MLXSW_SP_KVDL_PART_INFO(id)				\
+[MLXSW_SP_KVDL_PART_ID_##id] = {				\
+	.start_index = MLXSW_SP_KVDL_##id##_BASE,		\
+	.end_index = MLXSW_SP_KVDL_##id##_END,			\
+	.alloc_size = MLXSW_SP_KVDL_##id##_ALLOC_SIZE,		\
+	.resource_id = MLXSW_SP_RESOURCE_KVD_LINEAR_##id,	\
+}
+
+static const struct mlxsw_sp_kvdl_part_info mlxsw_sp_kvdl_parts_info[] = {
+	MLXSW_SP_KVDL_PART_INFO(SINGLE),
+	MLXSW_SP_KVDL_PART_INFO(CHUNKS),
+	MLXSW_SP_KVDL_PART_INFO(LARGE_CHUNKS),
+};
+
+#define MLXSW_SP_KVDL_PARTS_INFO_LEN ARRAY_SIZE(mlxsw_sp_kvdl_parts_info)
+
 struct mlxsw_sp_kvdl_part {
-	struct list_head list;
-	struct mlxsw_sp_kvdl_part_info *info;
+	struct mlxsw_sp_kvdl_part_info info;
 	unsigned long usage[0];	/* Entries */
 };
 
 struct mlxsw_sp_kvdl {
-	struct list_head parts_list;
+	struct mlxsw_sp_kvdl_part *parts[MLXSW_SP_KVDL_PARTS_INFO_LEN];
 };
 
 static struct mlxsw_sp_kvdl_part *
@@ -80,11 +103,13 @@ mlxsw_sp_kvdl_alloc_size_part(struct mlxsw_sp_kvdl *kvdl,
 			      unsigned int alloc_size)
 {
 	struct mlxsw_sp_kvdl_part *part, *min_part = NULL;
+	int i;
 
-	list_for_each_entry(part, &kvdl->parts_list, list) {
-		if (alloc_size <= part->info->alloc_size &&
+	for (i = 0; i < MLXSW_SP_KVDL_PARTS_INFO_LEN; i++) {
+		part = kvdl->parts[i];
+		if (alloc_size <= part->info.alloc_size &&
 		    (!min_part ||
-		     part->info->alloc_size <= min_part->info->alloc_size))
+		     part->info.alloc_size <= min_part->info.alloc_size))
 			min_part = part;
 	}
 
@@ -95,10 +120,12 @@ static struct mlxsw_sp_kvdl_part *
 mlxsw_sp_kvdl_index_part(struct mlxsw_sp_kvdl *kvdl, u32 kvdl_index)
 {
 	struct mlxsw_sp_kvdl_part *part;
+	int i;
 
-	list_for_each_entry(part, &kvdl->parts_list, list) {
-		if (kvdl_index >= part->info->start_index &&
-		    kvdl_index <= part->info->end_index)
+	for (i = 0; i < MLXSW_SP_KVDL_PARTS_INFO_LEN; i++) {
+		part = kvdl->parts[i];
+		if (kvdl_index >= part->info.start_index &&
+		    kvdl_index <= part->info.end_index)
 			return part;
 	}
 
@@ -122,7 +149,7 @@ mlxsw_sp_kvdl_index_entry_index(const struct mlxsw_sp_kvdl_part_info *info,
 static int mlxsw_sp_kvdl_part_alloc(struct mlxsw_sp_kvdl_part *part,
 				    u32 *p_kvdl_index)
 {
-	const struct mlxsw_sp_kvdl_part_info *info = part->info;
+	const struct mlxsw_sp_kvdl_part_info *info = &part->info;
 	unsigned int entry_index, nr_entries;
 
 	nr_entries = (info->end_index - info->start_index + 1) /
@@ -132,8 +159,7 @@ static int mlxsw_sp_kvdl_part_alloc(struct mlxsw_sp_kvdl_part *part,
 		return -ENOBUFS;
 	__set_bit(entry_index, part->usage);
 
-	*p_kvdl_index = mlxsw_sp_entry_index_kvdl_index(part->info,
-							entry_index);
+	*p_kvdl_index = mlxsw_sp_entry_index_kvdl_index(info, entry_index);
 
 	return 0;
 }
@@ -141,10 +167,10 @@ static int mlxsw_sp_kvdl_part_alloc(struct mlxsw_sp_kvdl_part *part,
 static void mlxsw_sp_kvdl_part_free(struct mlxsw_sp_kvdl_part *part,
 				    u32 kvdl_index)
 {
+	const struct mlxsw_sp_kvdl_part_info *info = &part->info;
 	unsigned int entry_index;
 
-	entry_index = mlxsw_sp_kvdl_index_entry_index(part->info,
-						      kvdl_index);
+	entry_index = mlxsw_sp_kvdl_index_entry_index(info, kvdl_index);
 	__clear_bit(entry_index, part->usage);
 }
 
@@ -183,74 +209,30 @@ int mlxsw_sp_kvdl_alloc_size_query(struct mlxsw_sp *mlxsw_sp,
 	if (IS_ERR(part))
 		return PTR_ERR(part);
 
-	*p_alloc_size = part->info->alloc_size;
+	*p_alloc_size = part->info.alloc_size;
 
 	return 0;
 }
 
-enum mlxsw_sp_kvdl_part_id {
-	MLXSW_SP_KVDL_PART_SINGLE,
-	MLXSW_SP_KVDL_PART_CHUNKS,
-	MLXSW_SP_KVDL_PART_LARGE_CHUNKS,
-};
-
-static const struct mlxsw_sp_kvdl_part_info kvdl_parts_info[] = {
-	{
-		.part_index	= MLXSW_SP_KVDL_PART_SINGLE,
-		.start_index	= MLXSW_SP_KVDL_SINGLE_BASE,
-		.end_index	= MLXSW_SP_KVDL_SINGLE_END,
-		.alloc_size	= 1,
-	},
-	{
-		.part_index	= MLXSW_SP_KVDL_PART_CHUNKS,
-		.start_index	= MLXSW_SP_KVDL_CHUNKS_BASE,
-		.end_index	= MLXSW_SP_KVDL_CHUNKS_END,
-		.alloc_size	= MLXSW_SP_CHUNK_MAX,
-	},
-	{
-		.part_index	= MLXSW_SP_KVDL_PART_LARGE_CHUNKS,
-		.start_index	= MLXSW_SP_KVDL_LARGE_CHUNKS_BASE,
-		.end_index	= MLXSW_SP_KVDL_LARGE_CHUNKS_END,
-		.alloc_size	= MLXSW_SP_LARGE_CHUNK_MAX,
-	},
-};
-
-static struct mlxsw_sp_kvdl_part *
-mlxsw_sp_kvdl_part_find(struct mlxsw_sp *mlxsw_sp, unsigned int part_index)
-{
-	struct mlxsw_sp_kvdl_part *part;
-
-	list_for_each_entry(part, &mlxsw_sp->kvdl->parts_list, list) {
-		if (part->info->part_index == part_index)
-			return part;
-	}
-
-	return NULL;
-}
-
-static void
-mlxsw_sp_kvdl_part_update(struct mlxsw_sp *mlxsw_sp,
-			  struct mlxsw_sp_kvdl_part *part, unsigned int size)
+static void mlxsw_sp_kvdl_part_update(struct mlxsw_sp_kvdl_part *part,
+				      struct mlxsw_sp_kvdl_part *part_prev,
+				      unsigned int size)
 {
-	struct mlxsw_sp_kvdl_part_info *info = part->info;
-
-	if (list_is_last(&part->list, &mlxsw_sp->kvdl->parts_list)) {
-		info->end_index = size - 1;
-	} else  {
-		struct mlxsw_sp_kvdl_part *last_part;
 
-		last_part = list_next_entry(part, list);
-		info->start_index = last_part->info->end_index + 1;
-		info->end_index = info->start_index + size - 1;
+	if (!part_prev) {
+		part->info.end_index = size - 1;
+	} else {
+		part->info.start_index = part_prev->info.end_index + 1;
+		part->info.end_index = part->info.start_index + size - 1;
 	}
 }
 
-static int mlxsw_sp_kvdl_part_init(struct mlxsw_sp *mlxsw_sp,
-				   unsigned int part_index)
+static struct mlxsw_sp_kvdl_part *
+mlxsw_sp_kvdl_part_init(struct mlxsw_sp *mlxsw_sp,
+			const struct mlxsw_sp_kvdl_part_info *info,
+			struct mlxsw_sp_kvdl_part *part_prev)
 {
 	struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
-	const struct mlxsw_sp_kvdl_part_info *info;
-	enum mlxsw_sp_resource_id resource_id;
 	struct mlxsw_sp_kvdl_part *part;
 	bool need_update = true;
 	unsigned int nr_entries;
@@ -258,23 +240,8 @@ static int mlxsw_sp_kvdl_part_init(struct mlxsw_sp *mlxsw_sp,
 	u64 resource_size;
 	int err;
 
-	info = &kvdl_parts_info[part_index];
-
-	switch (part_index) {
-	case MLXSW_SP_KVDL_PART_SINGLE:
-		resource_id = MLXSW_SP_RESOURCE_KVD_LINEAR_SINGLE;
-		break;
-	case MLXSW_SP_KVDL_PART_CHUNKS:
-		resource_id = MLXSW_SP_RESOURCE_KVD_LINEAR_CHUNKS;
-		break;
-	case MLXSW_SP_KVDL_PART_LARGE_CHUNKS:
-		resource_id = MLXSW_SP_RESOURCE_KVD_LINEAR_LARGE_CHUNKS;
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	err = devlink_resource_size_get(devlink, resource_id, &resource_size);
+	err = devlink_resource_size_get(devlink, info->resource_id,
+					&resource_size);
 	if (err) {
 		need_update = false;
 		resource_size = info->end_index - info->start_index + 1;
@@ -284,86 +251,77 @@ static int mlxsw_sp_kvdl_part_init(struct mlxsw_sp *mlxsw_sp,
 	usage_size = BITS_TO_LONGS(nr_entries) * sizeof(unsigned long);
 	part = kzalloc(sizeof(*part) + usage_size, GFP_KERNEL);
 	if (!part)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
-	part->info = kmemdup(info, sizeof(*part->info), GFP_KERNEL);
-	if (!part->info)
-		goto err_part_info_alloc;
+	memcpy(&part->info, info, sizeof(part->info));
 
-	list_add(&part->list, &mlxsw_sp->kvdl->parts_list);
 	if (need_update)
-		mlxsw_sp_kvdl_part_update(mlxsw_sp, part, resource_size);
-	return 0;
-
-err_part_info_alloc:
-	kfree(part);
-	return -ENOMEM;
+		mlxsw_sp_kvdl_part_update(part, part_prev, resource_size);
+	return part;
 }
 
-static void mlxsw_sp_kvdl_part_fini(struct mlxsw_sp *mlxsw_sp,
-				    unsigned int part_index)
+static void mlxsw_sp_kvdl_part_fini(struct mlxsw_sp_kvdl_part *part)
 {
-	struct mlxsw_sp_kvdl_part *part;
-
-	part = mlxsw_sp_kvdl_part_find(mlxsw_sp, part_index);
-	if (!part)
-		return;
-
-	list_del(&part->list);
-	kfree(part->info);
 	kfree(part);
 }
 
 static int mlxsw_sp_kvdl_parts_init(struct mlxsw_sp *mlxsw_sp)
 {
+	struct mlxsw_sp_kvdl *kvdl = mlxsw_sp->kvdl;
+	const struct mlxsw_sp_kvdl_part_info *info;
+	struct mlxsw_sp_kvdl_part *part_prev = NULL;
 	int err, i;
 
-	INIT_LIST_HEAD(&mlxsw_sp->kvdl->parts_list);
-
-	for (i = 0; i < ARRAY_SIZE(kvdl_parts_info); i++) {
-		err = mlxsw_sp_kvdl_part_init(mlxsw_sp, i);
-		if (err)
+	for (i = 0; i < MLXSW_SP_KVDL_PARTS_INFO_LEN; i++) {
+		info = &mlxsw_sp_kvdl_parts_info[i];
+		kvdl->parts[i] = mlxsw_sp_kvdl_part_init(mlxsw_sp, info,
+							 part_prev);
+		if (IS_ERR(kvdl->parts[i])) {
+			err = PTR_ERR(kvdl->parts[i]);
 			goto err_kvdl_part_init;
+		}
+		part_prev = kvdl->parts[i];
 	}
-
 	return 0;
 
 err_kvdl_part_init:
 	for (i--; i >= 0; i--)
-		mlxsw_sp_kvdl_part_fini(mlxsw_sp, i);
+		mlxsw_sp_kvdl_part_fini(kvdl->parts[i]);
 	return err;
 }
 
 static void mlxsw_sp_kvdl_parts_fini(struct mlxsw_sp *mlxsw_sp)
 {
+	struct mlxsw_sp_kvdl *kvdl = mlxsw_sp->kvdl;
 	int i;
 
-	for (i = ARRAY_SIZE(kvdl_parts_info) - 1; i >= 0; i--)
-		mlxsw_sp_kvdl_part_fini(mlxsw_sp, i);
+	for (i = 0; i < MLXSW_SP_KVDL_PARTS_INFO_LEN; i++)
+		mlxsw_sp_kvdl_part_fini(kvdl->parts[i]);
 }
 
 static u64 mlxsw_sp_kvdl_part_occ(struct mlxsw_sp_kvdl_part *part)
 {
+	const struct mlxsw_sp_kvdl_part_info *info = &part->info;
 	unsigned int nr_entries;
 	int bit = -1;
 	u64 occ = 0;
 
-	nr_entries = (part->info->end_index -
-		      part->info->start_index + 1) /
-		      part->info->alloc_size;
+	nr_entries = (info->end_index -
+		      info->start_index + 1) /
+		      info->alloc_size;
 	while ((bit = find_next_bit(part->usage, nr_entries, bit + 1))
 		< nr_entries)
-		occ += part->info->alloc_size;
+		occ += info->alloc_size;
 	return occ;
 }
 
 u64 mlxsw_sp_kvdl_occ_get(const struct mlxsw_sp *mlxsw_sp)
 {
-	struct mlxsw_sp_kvdl_part *part;
 	u64 occ = 0;
+	int i;
 
-	list_for_each_entry(part, &mlxsw_sp->kvdl->parts_list, list)
-		occ += mlxsw_sp_kvdl_part_occ(part);
+	for (i = 0; i < MLXSW_SP_KVDL_PARTS_INFO_LEN; i++)
+		occ += mlxsw_sp_kvdl_part_occ(mlxsw_sp->kvdl->parts[i]);
 
 	return occ;
 }
@@ -374,10 +332,7 @@ static u64 mlxsw_sp_kvdl_single_occ_get(struct devlink *devlink)
 	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
 	struct mlxsw_sp_kvdl_part *part;
 
-	part = mlxsw_sp_kvdl_part_find(mlxsw_sp, MLXSW_SP_KVDL_PART_SINGLE);
-	if (!part)
-		return -EINVAL;
-
+	part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_SINGLE];
 	return mlxsw_sp_kvdl_part_occ(part);
 }
 
@@ -387,10 +342,7 @@ static u64 mlxsw_sp_kvdl_chunks_occ_get(struct devlink *devlink)
 	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
 	struct mlxsw_sp_kvdl_part *part;
 
-	part = mlxsw_sp_kvdl_part_find(mlxsw_sp, MLXSW_SP_KVDL_PART_CHUNKS);
-	if (!part)
-		return -EINVAL;
-
+	part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_CHUNKS];
 	return mlxsw_sp_kvdl_part_occ(part);
 }
 
@@ -400,11 +352,7 @@ static u64 mlxsw_sp_kvdl_large_chunks_occ_get(struct devlink *devlink)
 	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
 	struct mlxsw_sp_kvdl_part *part;
 
-	part = mlxsw_sp_kvdl_part_find(mlxsw_sp,
-				       MLXSW_SP_KVDL_PART_LARGE_CHUNKS);
-	if (!part)
-		return -EINVAL;
-
+	part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_LARGE_CHUNKS];
 	return mlxsw_sp_kvdl_part_occ(part);
 }
 
@@ -431,7 +379,8 @@ int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
 			MLXSW_CORE_RES_GET(mlxsw_core, KVD_SINGLE_MIN_SIZE) -
 			MLXSW_CORE_RES_GET(mlxsw_core, KVD_DOUBLE_MIN_SIZE);
 
-	devlink_resource_size_params_init(&size_params, 0, kvdl_max_size, 1,
+	devlink_resource_size_params_init(&size_params, 0, kvdl_max_size,
+					  MLXSW_SP_KVDL_SINGLE_ALLOC_SIZE,
 					  DEVLINK_RESOURCE_UNIT_ENTRY);
 	err = devlink_resource_register(devlink, MLXSW_SP_RESOURCE_NAME_KVD_LINEAR_SINGLES,
 					MLXSW_SP_KVDL_SINGLE_SIZE,
@@ -443,7 +392,7 @@ int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
 		return err;
 
 	devlink_resource_size_params_init(&size_params, 0, kvdl_max_size,
-					  MLXSW_SP_CHUNK_MAX,
+					  MLXSW_SP_KVDL_CHUNKS_ALLOC_SIZE,
 					  DEVLINK_RESOURCE_UNIT_ENTRY);
 	err = devlink_resource_register(devlink, MLXSW_SP_RESOURCE_NAME_KVD_LINEAR_CHUNKS,
 					MLXSW_SP_KVDL_CHUNKS_SIZE,
@@ -455,7 +404,7 @@ int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
 		return err;
 
 	devlink_resource_size_params_init(&size_params, 0, kvdl_max_size,
-					  MLXSW_SP_LARGE_CHUNK_MAX,
+					  MLXSW_SP_KVDL_LARGE_CHUNKS_ALLOC_SIZE,
 					  DEVLINK_RESOURCE_UNIT_ENTRY);
 	err = devlink_resource_register(devlink, MLXSW_SP_RESOURCE_NAME_KVD_LINEAR_LARGE_CHUNKS,
 					MLXSW_SP_KVDL_LARGE_CHUNKS_SIZE,
-- 
2.14.3

[PATCH net-next 05/11] mlxsw: remove kvd_hash_granularity from config profile struct

[Ido Schimmel]

From: Jiri Pirko <jiri@mellanox.com>

This should not be part of the struct, as the struct fields
are tightly coupled with the FW command payload of the same name.
Just use the "granularity" define directly, as in other places.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.h     | 1 -
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 5 ++---
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index 5ddafd74dc00..fd30eaf40475 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -256,7 +256,6 @@ struct mlxsw_config_profile {
 	u16	adaptive_routing_group_cap;
 	u8	arn;
 	u32	kvd_linear_size;
-	u16	kvd_hash_granularity;
 	u8	kvd_hash_single_parts;
 	u8	kvd_hash_double_parts;
 	u8	resource_query_enable;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 0e9ed41ce8bc..d503cdbeae29 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3794,7 +3794,6 @@ static const struct mlxsw_config_profile mlxsw_sp_config_profile = {
 	.used_max_pkey			= 1,
 	.max_pkey			= 0,
 	.used_kvd_split_data		= 1,
-	.kvd_hash_granularity		= MLXSW_SP_KVD_GRANULARITY,
 	.kvd_hash_single_parts		= 59,
 	.kvd_hash_double_parts		= 41,
 	.kvd_linear_size		= MLXSW_SP_KVD_LINEAR_SIZE,
@@ -3902,7 +3901,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
 	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);
+	double_size = rounddown(double_size, MLXSW_SP_KVD_GRANULARITY);
 	err = devlink_resource_register(devlink, MLXSW_SP_RESOURCE_NAME_KVD_HASH_DOUBLE,
 					double_size,
 					MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE,
@@ -3962,7 +3961,7 @@ static int mlxsw_sp_kvd_sizes_get(struct mlxsw_core *mlxsw_core,
 		double_size /= profile->kvd_hash_double_parts +
 			       profile->kvd_hash_single_parts;
 		*p_double_size = rounddown(double_size,
-					   profile->kvd_hash_granularity);
+					   MLXSW_SP_KVD_GRANULARITY);
 	}
 
 	err = devlink_resource_size_get(devlink,
-- 
2.14.3

[PATCH net-next 06/11] mlxsw: core: Fix arg name of MLXSW_CORE_RES_VALID and MLXSW_CORE_RES_GET

[Ido Schimmel]

From: Jiri Pirko <jiri@mellanox.com>

First arg of these helpers should be "mlxsw_core".

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index fd30eaf40475..0d6452699364 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -325,14 +325,14 @@ int mlxsw_core_kvd_sizes_get(struct mlxsw_core *mlxsw_core,
 bool mlxsw_core_res_valid(struct mlxsw_core *mlxsw_core,
 			  enum mlxsw_res_id res_id);
 
-#define MLXSW_CORE_RES_VALID(res, short_res_id)			\
-	mlxsw_core_res_valid(res, MLXSW_RES_ID_##short_res_id)
+#define MLXSW_CORE_RES_VALID(mlxsw_core, short_res_id)			\
+	mlxsw_core_res_valid(mlxsw_core, MLXSW_RES_ID_##short_res_id)
 
 u64 mlxsw_core_res_get(struct mlxsw_core *mlxsw_core,
 		       enum mlxsw_res_id res_id);
 
-#define MLXSW_CORE_RES_GET(res, short_res_id)			\
-	mlxsw_core_res_get(res, MLXSW_RES_ID_##short_res_id)
+#define MLXSW_CORE_RES_GET(mlxsw_core, short_res_id)			\
+	mlxsw_core_res_get(mlxsw_core, MLXSW_RES_ID_##short_res_id)
 
 #define MLXSW_BUS_F_TXRX	BIT(0)
 
-- 
2.14.3

[PATCH net-next 07/11] mlxsw: Move "used_kvd_sizes" check to mlxsw_pci_config_profile

[Ido Schimmel]

From: Jiri Pirko <jiri@mellanox.com>

The check should be done directly in mlxsw_pci_config_profile, as for
other profile items. Also, be consistent in naming with the rest and
rename to "used_kvd_sizes".

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.h     | 3 +--
 drivers/net/ethernet/mellanox/mlxsw/pci.c      | 2 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 5 ++---
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index 0d6452699364..ff9daa09341d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -235,8 +235,7 @@ struct mlxsw_config_profile {
 		used_max_pkey:1,
 		used_ar_sec:1,
 		used_adaptive_routing_group_cap:1,
-		used_kvd_split_data:1; /* indicate for the kvd's values */
-
+		used_kvd_sizes:1;
 	u8	max_vepa_channels;
 	u16	max_mid;
 	u16	max_pgt;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
index e30c6ce3dcb4..5ab068aec033 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c
@@ -1164,7 +1164,7 @@ static int mlxsw_pci_config_profile(struct mlxsw_pci *mlxsw_pci, char *mbox,
 		mlxsw_cmd_mbox_config_profile_adaptive_routing_group_cap_set(
 			mbox, profile->adaptive_routing_group_cap);
 	}
-	if (MLXSW_RES_VALID(res, KVD_SIZE)) {
+	if (profile->used_kvd_sizes && MLXSW_RES_VALID(res, KVD_SIZE)) {
 		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 d503cdbeae29..12062aab13c5 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3793,7 +3793,7 @@ static const struct mlxsw_config_profile mlxsw_sp_config_profile = {
 	.max_ib_mc			= 0,
 	.used_max_pkey			= 1,
 	.max_pkey			= 0,
-	.used_kvd_split_data		= 1,
+	.used_kvd_sizes			= 1,
 	.kvd_hash_single_parts		= 59,
 	.kvd_hash_double_parts		= 41,
 	.kvd_linear_size		= MLXSW_SP_KVD_LINEAR_SIZE,
@@ -3934,8 +3934,7 @@ static int mlxsw_sp_kvd_sizes_get(struct mlxsw_core *mlxsw_core,
 	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)
+	    !MLXSW_CORE_RES_VALID(mlxsw_core, KVD_DOUBLE_MIN_SIZE))
 		return -EIO;
 
 	/* The hash part is what left of the kvd without the
-- 
2.14.3

[PATCH net-next 08/11] mlxsw: Move "resources_query_enable" out of mlxsw_config_profile

[Ido Schimmel]

From: Jiri Pirko <jiri@mellanox.com>

As struct mlxsw_config_profile is mapped to the payload of the FW
command of the same name, resources_query_enable flag does not belong
there. Move it to struct mlxsw_driver.

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

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 3529b545675d..93ea56620a24 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1008,6 +1008,7 @@ int mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info,
 	const char *device_kind = mlxsw_bus_info->device_kind;
 	struct mlxsw_core *mlxsw_core;
 	struct mlxsw_driver *mlxsw_driver;
+	struct mlxsw_res *res;
 	size_t alloc_size;
 	int err;
 
@@ -1032,8 +1033,8 @@ int mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info,
 	mlxsw_core->bus_priv = bus_priv;
 	mlxsw_core->bus_info = mlxsw_bus_info;
 
-	err = mlxsw_bus->init(bus_priv, mlxsw_core, mlxsw_driver->profile,
-			      &mlxsw_core->res);
+	res = mlxsw_driver->res_query_enabled ? &mlxsw_core->res : NULL;
+	err = mlxsw_bus->init(bus_priv, mlxsw_core, mlxsw_driver->profile, res);
 	if (err)
 		goto err_bus_init;
 
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index ff9daa09341d..092d39399f3c 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -257,7 +257,6 @@ struct mlxsw_config_profile {
 	u32	kvd_linear_size;
 	u8	kvd_hash_single_parts;
 	u8	kvd_hash_double_parts;
-	u8	resource_query_enable;
 	struct mlxsw_swid_config swid_config[MLXSW_CONFIG_PROFILE_SWID_COUNT];
 };
 
@@ -314,6 +313,7 @@ struct mlxsw_driver {
 			     u64 *p_linear_size);
 	u8 txhdr_len;
 	const struct mlxsw_config_profile *profile;
+	bool res_query_enabled;
 };
 
 int mlxsw_core_kvd_sizes_get(struct mlxsw_core *mlxsw_core,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
index 5ab068aec033..3a9381977d6d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c
@@ -1015,16 +1015,14 @@ mlxsw_pci_config_profile_swid_config(struct mlxsw_pci *mlxsw_pci,
 }
 
 static int mlxsw_pci_resources_query(struct mlxsw_pci *mlxsw_pci, char *mbox,
-				     struct mlxsw_res *res,
-				     u8 query_enabled)
+				     struct mlxsw_res *res)
 {
 	int index, i;
 	u64 data;
 	u16 id;
 	int err;
 
-	/* Not all the versions support resources query */
-	if (!query_enabled)
+	if (!res)
 		return 0;
 
 	mlxsw_cmd_mbox_zero(mbox);
@@ -1376,8 +1374,7 @@ static int mlxsw_pci_init(void *bus_priv, struct mlxsw_core *mlxsw_core,
 	if (err)
 		goto err_boardinfo;
 
-	err = mlxsw_pci_resources_query(mlxsw_pci, mbox, res,
-					profile->resource_query_enable);
+	err = mlxsw_pci_resources_query(mlxsw_pci, mbox, res);
 	if (err)
 		goto err_query_resources;
 
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 12062aab13c5..b831af38e0a1 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3803,7 +3803,6 @@ static const struct mlxsw_config_profile mlxsw_sp_config_profile = {
 			.type		= MLXSW_PORT_SWID_TYPE_ETH,
 		}
 	},
-	.resource_query_enable		= 1,
 };
 
 static u64 mlxsw_sp_resource_kvd_linear_occ_get(struct devlink *devlink)
@@ -4002,6 +4001,7 @@ static struct mlxsw_driver mlxsw_sp_driver = {
 	.kvd_sizes_get			= mlxsw_sp_kvd_sizes_get,
 	.txhdr_len			= MLXSW_TXHDR_LEN,
 	.profile			= &mlxsw_sp_config_profile,
+	.res_query_enabled		= true,
 };
 
 bool mlxsw_sp_port_dev_check(const struct net_device *dev)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchib.c b/drivers/net/ethernet/mellanox/mlxsw/switchib.c
index ab7a29846bfa..c698ec4fd9d4 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/switchib.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/switchib.c
@@ -510,7 +510,6 @@ static const struct mlxsw_config_profile mlxsw_sib_config_profile = {
 			.type		= MLXSW_PORT_SWID_TYPE_IB,
 		}
 	},
-	.resource_query_enable		= 0,
 };
 
 static struct mlxsw_driver mlxsw_sib_driver = {
diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
index c87b0934a405..a655c5850aa6 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
@@ -1706,7 +1706,6 @@ static const struct mlxsw_config_profile mlxsw_sx_config_profile = {
 			.type		= MLXSW_PORT_SWID_TYPE_IB,
 		}
 	},
-	.resource_query_enable		= 0,
 };
 
 static struct mlxsw_driver mlxsw_sx_driver = {
-- 
2.14.3

[PATCH net-next 09/11] devlink: convert occ_get op to separate registration

[Ido Schimmel]

From: Jiri Pirko <jiri@mellanox.com>

This resolves race during initialization where the resources with
ops are registered before driver and the structures used by occ_get
op is initialized. So keep occ_get callbacks registered only when
all structs are initialized.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     | 24 ++-----
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |  1 -
 .../net/ethernet/mellanox/mlxsw/spectrum_kvdl.c    | 67 ++++++++++++--------
 include/net/devlink.h                              | 40 ++++++++----
 net/core/devlink.c                                 | 74 +++++++++++++++++++---
 5 files changed, 134 insertions(+), 72 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index b831af38e0a1..0d95d2cb73e3 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3805,18 +3805,6 @@ static const struct mlxsw_config_profile mlxsw_sp_config_profile = {
 	},
 };
 
-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 const struct devlink_resource_ops mlxsw_sp_resource_kvd_linear_ops = {
-	.occ_get = mlxsw_sp_resource_kvd_linear_occ_get,
-};
-
 static void
 mlxsw_sp_resource_size_params_prepare(struct mlxsw_core *mlxsw_core,
 				      struct devlink_resource_size_params *kvd_size_params,
@@ -3877,8 +3865,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
 	err = devlink_resource_register(devlink, MLXSW_SP_RESOURCE_NAME_KVD,
 					kvd_size, MLXSW_SP_RESOURCE_KVD,
 					DEVLINK_RESOURCE_ID_PARENT_TOP,
-					&kvd_size_params,
-					NULL);
+					&kvd_size_params);
 	if (err)
 		return err;
 
@@ -3887,8 +3874,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
 					linear_size,
 					MLXSW_SP_RESOURCE_KVD_LINEAR,
 					MLXSW_SP_RESOURCE_KVD,
-					&linear_size_params,
-					&mlxsw_sp_resource_kvd_linear_ops);
+					&linear_size_params);
 	if (err)
 		return err;
 
@@ -3905,8 +3891,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
 					double_size,
 					MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE,
 					MLXSW_SP_RESOURCE_KVD,
-					&hash_double_size_params,
-					NULL);
+					&hash_double_size_params);
 	if (err)
 		return err;
 
@@ -3915,8 +3900,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
 					single_size,
 					MLXSW_SP_RESOURCE_KVD_HASH_SINGLE,
 					MLXSW_SP_RESOURCE_KVD,
-					&hash_single_size_params,
-					NULL);
+					&hash_single_size_params);
 	if (err)
 		return err;
 
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 21bee8f19894..c59a0d7d81d5 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -442,7 +442,6 @@ 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);
 int mlxsw_sp_kvdl_resources_register(struct devlink *devlink);
 
 struct mlxsw_sp_acl_rule_info {
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
index 7b28f65d6407..1b7280168e6b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
@@ -315,8 +315,9 @@ static u64 mlxsw_sp_kvdl_part_occ(struct mlxsw_sp_kvdl_part *part)
 	return occ;
 }
 
-u64 mlxsw_sp_kvdl_occ_get(const struct mlxsw_sp *mlxsw_sp)
+static u64 mlxsw_sp_kvdl_occ_get(void *priv)
 {
+	const struct mlxsw_sp *mlxsw_sp = priv;
 	u64 occ = 0;
 	int i;
 
@@ -326,48 +327,33 @@ u64 mlxsw_sp_kvdl_occ_get(const struct mlxsw_sp *mlxsw_sp)
 	return occ;
 }
 
-static u64 mlxsw_sp_kvdl_single_occ_get(struct devlink *devlink)
+static u64 mlxsw_sp_kvdl_single_occ_get(void *priv)
 {
-	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
-	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
+	const struct mlxsw_sp *mlxsw_sp = priv;
 	struct mlxsw_sp_kvdl_part *part;
 
 	part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_SINGLE];
 	return mlxsw_sp_kvdl_part_occ(part);
 }
 
-static u64 mlxsw_sp_kvdl_chunks_occ_get(struct devlink *devlink)
+static u64 mlxsw_sp_kvdl_chunks_occ_get(void *priv)
 {
-	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
-	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
+	const struct mlxsw_sp *mlxsw_sp = priv;
 	struct mlxsw_sp_kvdl_part *part;
 
 	part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_CHUNKS];
 	return mlxsw_sp_kvdl_part_occ(part);
 }
 
-static u64 mlxsw_sp_kvdl_large_chunks_occ_get(struct devlink *devlink)
+static u64 mlxsw_sp_kvdl_large_chunks_occ_get(void *priv)
 {
-	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
-	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
+	const struct mlxsw_sp *mlxsw_sp = priv;
 	struct mlxsw_sp_kvdl_part *part;
 
 	part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_LARGE_CHUNKS];
 	return mlxsw_sp_kvdl_part_occ(part);
 }
 
-static const struct devlink_resource_ops mlxsw_sp_kvdl_single_ops = {
-	.occ_get = mlxsw_sp_kvdl_single_occ_get,
-};
-
-static const struct devlink_resource_ops mlxsw_sp_kvdl_chunks_ops = {
-	.occ_get = mlxsw_sp_kvdl_chunks_occ_get,
-};
-
-static const struct devlink_resource_ops mlxsw_sp_kvdl_chunks_large_ops = {
-	.occ_get = mlxsw_sp_kvdl_large_chunks_occ_get,
-};
-
 int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
 {
 	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
@@ -386,8 +372,7 @@ int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
 					MLXSW_SP_KVDL_SINGLE_SIZE,
 					MLXSW_SP_RESOURCE_KVD_LINEAR_SINGLE,
 					MLXSW_SP_RESOURCE_KVD_LINEAR,
-					&size_params,
-					&mlxsw_sp_kvdl_single_ops);
+					&size_params);
 	if (err)
 		return err;
 
@@ -398,8 +383,7 @@ int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
 					MLXSW_SP_KVDL_CHUNKS_SIZE,
 					MLXSW_SP_RESOURCE_KVD_LINEAR_CHUNKS,
 					MLXSW_SP_RESOURCE_KVD_LINEAR,
-					&size_params,
-					&mlxsw_sp_kvdl_chunks_ops);
+					&size_params);
 	if (err)
 		return err;
 
@@ -410,13 +394,13 @@ int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
 					MLXSW_SP_KVDL_LARGE_CHUNKS_SIZE,
 					MLXSW_SP_RESOURCE_KVD_LINEAR_LARGE_CHUNKS,
 					MLXSW_SP_RESOURCE_KVD_LINEAR,
-					&size_params,
-					&mlxsw_sp_kvdl_chunks_large_ops);
+					&size_params);
 	return err;
 }
 
 int mlxsw_sp_kvdl_init(struct mlxsw_sp *mlxsw_sp)
 {
+	struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
 	struct mlxsw_sp_kvdl *kvdl;
 	int err;
 
@@ -429,6 +413,23 @@ int mlxsw_sp_kvdl_init(struct mlxsw_sp *mlxsw_sp)
 	if (err)
 		goto err_kvdl_parts_init;
 
+	devlink_resource_occ_get_register(devlink,
+					  MLXSW_SP_RESOURCE_KVD_LINEAR,
+					  mlxsw_sp_kvdl_occ_get,
+					  mlxsw_sp);
+	devlink_resource_occ_get_register(devlink,
+					  MLXSW_SP_RESOURCE_KVD_LINEAR_SINGLE,
+					  mlxsw_sp_kvdl_single_occ_get,
+					  mlxsw_sp);
+	devlink_resource_occ_get_register(devlink,
+					  MLXSW_SP_RESOURCE_KVD_LINEAR_CHUNKS,
+					  mlxsw_sp_kvdl_chunks_occ_get,
+					  mlxsw_sp);
+	devlink_resource_occ_get_register(devlink,
+					  MLXSW_SP_RESOURCE_KVD_LINEAR_LARGE_CHUNKS,
+					  mlxsw_sp_kvdl_large_chunks_occ_get,
+					  mlxsw_sp);
+
 	return 0;
 
 err_kvdl_parts_init:
@@ -438,6 +439,16 @@ int mlxsw_sp_kvdl_init(struct mlxsw_sp *mlxsw_sp)
 
 void mlxsw_sp_kvdl_fini(struct mlxsw_sp *mlxsw_sp)
 {
+	struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
+
+	devlink_resource_occ_get_unregister(devlink,
+					    MLXSW_SP_RESOURCE_KVD_LINEAR_LARGE_CHUNKS);
+	devlink_resource_occ_get_unregister(devlink,
+					    MLXSW_SP_RESOURCE_KVD_LINEAR_CHUNKS);
+	devlink_resource_occ_get_unregister(devlink,
+					    MLXSW_SP_RESOURCE_KVD_LINEAR_SINGLE);
+	devlink_resource_occ_get_unregister(devlink,
+					    MLXSW_SP_RESOURCE_KVD_LINEAR);
 	mlxsw_sp_kvdl_parts_fini(mlxsw_sp);
 	kfree(mlxsw_sp->kvdl);
 }
diff --git a/include/net/devlink.h b/include/net/devlink.h
index e21d8cadd480..2e4f71e16e95 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -231,14 +231,6 @@ struct devlink_dpipe_headers {
 	unsigned int headers_count;
 };
 
-/**
- * struct devlink_resource_ops - resource ops
- * @occ_get: get the occupied size
- */
-struct devlink_resource_ops {
-	u64 (*occ_get)(struct devlink *devlink);
-};
-
 /**
  * struct devlink_resource_size_params - resource's size parameters
  * @size_min: minimum size which can be set
@@ -265,6 +257,8 @@ devlink_resource_size_params_init(struct devlink_resource_size_params *size_para
 	size_params->unit = unit;
 }
 
+typedef u64 devlink_resource_occ_get_t(void *priv);
+
 /**
  * struct devlink_resource - devlink resource
  * @name: name of the resource
@@ -277,7 +271,6 @@ devlink_resource_size_params_init(struct devlink_resource_size_params *size_para
  * @size_params: size parameters
  * @list: parent list
  * @resource_list: list of child resources
- * @resource_ops: resource ops
  */
 struct devlink_resource {
 	const char *name;
@@ -289,7 +282,8 @@ struct devlink_resource {
 	struct devlink_resource_size_params size_params;
 	struct list_head list;
 	struct list_head resource_list;
-	const struct devlink_resource_ops *resource_ops;
+	devlink_resource_occ_get_t *occ_get;
+	void *occ_get_priv;
 };
 
 #define DEVLINK_RESOURCE_ID_PARENT_TOP 0
@@ -409,8 +403,7 @@ int devlink_resource_register(struct devlink *devlink,
 			      u64 resource_size,
 			      u64 resource_id,
 			      u64 parent_resource_id,
-			      const struct devlink_resource_size_params *size_params,
-			      const struct devlink_resource_ops *resource_ops);
+			      const struct devlink_resource_size_params *size_params);
 void devlink_resources_unregister(struct devlink *devlink,
 				  struct devlink_resource *resource);
 int devlink_resource_size_get(struct devlink *devlink,
@@ -419,6 +412,12 @@ int devlink_resource_size_get(struct devlink *devlink,
 int devlink_dpipe_table_resource_set(struct devlink *devlink,
 				     const char *table_name, u64 resource_id,
 				     u64 resource_units);
+void devlink_resource_occ_get_register(struct devlink *devlink,
+				       u64 resource_id,
+				       devlink_resource_occ_get_t *occ_get,
+				       void *occ_get_priv);
+void devlink_resource_occ_get_unregister(struct devlink *devlink,
+					 u64 resource_id);
 
 #else
 
@@ -562,8 +561,7 @@ devlink_resource_register(struct devlink *devlink,
 			  u64 resource_size,
 			  u64 resource_id,
 			  u64 parent_resource_id,
-			  const struct devlink_resource_size_params *size_params,
-			  const struct devlink_resource_ops *resource_ops)
+			  const struct devlink_resource_size_params *size_params)
 {
 	return 0;
 }
@@ -589,6 +587,20 @@ devlink_dpipe_table_resource_set(struct devlink *devlink,
 	return -EOPNOTSUPP;
 }
 
+static inline void
+devlink_resource_occ_get_register(struct devlink *devlink,
+				  u64 resource_id,
+				  devlink_resource_occ_get_t *occ_get,
+				  void *occ_get_priv)
+{
+}
+
+static inline void
+devlink_resource_occ_get_unregister(struct devlink *devlink,
+				    u64 resource_id)
+{
+}
+
 #endif
 
 #endif /* _NET_DEVLINK_H_ */
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 9236e421bd62..ad1317376798 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2405,6 +2405,16 @@ devlink_resource_size_params_put(struct devlink_resource *resource,
 	return 0;
 }
 
+static int devlink_resource_occ_put(struct devlink_resource *resource,
+				    struct sk_buff *skb)
+{
+	if (!resource->occ_get)
+		return 0;
+	return nla_put_u64_64bit(skb, DEVLINK_ATTR_RESOURCE_OCC,
+				 resource->occ_get(resource->occ_get_priv),
+				 DEVLINK_ATTR_PAD);
+}
+
 static int devlink_resource_put(struct devlink *devlink, struct sk_buff *skb,
 				struct devlink_resource *resource)
 {
@@ -2425,11 +2435,8 @@ static int devlink_resource_put(struct devlink *devlink, struct sk_buff *skb,
 	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)
-		if (nla_put_u64_64bit(skb, DEVLINK_ATTR_RESOURCE_OCC,
-				      resource->resource_ops->occ_get(devlink),
-				      DEVLINK_ATTR_PAD))
-			goto nla_put_failure;
+	if (devlink_resource_occ_put(resource, skb))
+		goto nla_put_failure;
 	if (devlink_resource_size_params_put(resource, skb))
 		goto nla_put_failure;
 	if (list_empty(&resource->resource_list))
@@ -3162,15 +3169,13 @@ EXPORT_SYMBOL_GPL(devlink_dpipe_table_unregister);
  *	@resource_id: resource's id
  *	@parent_reosurce_id: resource's parent id
  *	@size params: size parameters
- *	@resource_ops: resource ops
  */
 int devlink_resource_register(struct devlink *devlink,
 			      const char *resource_name,
 			      u64 resource_size,
 			      u64 resource_id,
 			      u64 parent_resource_id,
-			      const struct devlink_resource_size_params *size_params,
-			      const struct devlink_resource_ops *resource_ops)
+			      const struct devlink_resource_size_params *size_params)
 {
 	struct devlink_resource *resource;
 	struct list_head *resource_list;
@@ -3213,7 +3218,6 @@ int devlink_resource_register(struct devlink *devlink,
 	resource->size = resource_size;
 	resource->size_new = resource_size;
 	resource->id = resource_id;
-	resource->resource_ops = resource_ops;
 	resource->size_valid = true;
 	memcpy(&resource->size_params, size_params,
 	       sizeof(resource->size_params));
@@ -3315,6 +3319,58 @@ int devlink_dpipe_table_resource_set(struct devlink *devlink,
 }
 EXPORT_SYMBOL_GPL(devlink_dpipe_table_resource_set);
 
+/**
+ *	devlink_resource_occ_get_register - register occupancy getter
+ *
+ *	@devlink: devlink
+ *	@resource_id: resource id
+ *	@occ_get: occupancy getter callback
+ *	@occ_get_priv: occupancy getter callback priv
+ */
+void devlink_resource_occ_get_register(struct devlink *devlink,
+				       u64 resource_id,
+				       devlink_resource_occ_get_t *occ_get,
+				       void *occ_get_priv)
+{
+	struct devlink_resource *resource;
+
+	mutex_lock(&devlink->lock);
+	resource = devlink_resource_find(devlink, NULL, resource_id);
+	if (WARN_ON(!resource))
+		goto out;
+	WARN_ON(resource->occ_get);
+
+	resource->occ_get = occ_get;
+	resource->occ_get_priv = occ_get_priv;
+out:
+	mutex_unlock(&devlink->lock);
+}
+EXPORT_SYMBOL_GPL(devlink_resource_occ_get_register);
+
+/**
+ *	devlink_resource_occ_get_unregister - unregister occupancy getter
+ *
+ *	@devlink: devlink
+ *	@resource_id: resource id
+ */
+void devlink_resource_occ_get_unregister(struct devlink *devlink,
+					 u64 resource_id)
+{
+	struct devlink_resource *resource;
+
+	mutex_lock(&devlink->lock);
+	resource = devlink_resource_find(devlink, NULL, resource_id);
+	if (WARN_ON(!resource))
+		goto out;
+	WARN_ON(!resource->occ_get);
+
+	resource->occ_get = NULL;
+	resource->occ_get_priv = NULL;
+out:
+	mutex_unlock(&devlink->lock);
+}
+EXPORT_SYMBOL_GPL(devlink_resource_occ_get_unregister);
+
 static int __init devlink_module_init(void)
 {
 	return genl_register_family(&devlink_nl_family);
-- 
2.14.3

Re: [PATCH net-next 09/11] devlink: convert occ_get op to separate registration

[David Ahern]

On 3/29/18 2:33 PM, Ido Schimmel wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> This resolves race during initialization where the resources with
> ops are registered before driver and the structures used by occ_get
> op is initialized. So keep occ_get callbacks registered only when
> all structs are initialized.

Why can't the occ_get handler look at some flag in an mlxsw struct to
know if the system has initialized?

Separate registration here is awkward. You register a resource and then
register its op later.

Also, this should be a standalone patch rather than embedded in a
'mlxsw: Various cleanups' set.


> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlxsw/spectrum.c     | 24 ++-----
>  drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |  1 -
>  .../net/ethernet/mellanox/mlxsw/spectrum_kvdl.c    | 67 ++++++++++++--------
>  include/net/devlink.h                              | 40 ++++++++----
>  net/core/devlink.c                                 | 74 +++++++++++++++++++---
>  5 files changed, 134 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> index b831af38e0a1..0d95d2cb73e3 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> @@ -3805,18 +3805,6 @@ static const struct mlxsw_config_profile mlxsw_sp_config_profile = {
>  	},
>  };
>  
> -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 const struct devlink_resource_ops mlxsw_sp_resource_kvd_linear_ops = {
> -	.occ_get = mlxsw_sp_resource_kvd_linear_occ_get,
> -};
> -
>  static void
>  mlxsw_sp_resource_size_params_prepare(struct mlxsw_core *mlxsw_core,
>  				      struct devlink_resource_size_params *kvd_size_params,
> @@ -3877,8 +3865,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
>  	err = devlink_resource_register(devlink, MLXSW_SP_RESOURCE_NAME_KVD,
>  					kvd_size, MLXSW_SP_RESOURCE_KVD,
>  					DEVLINK_RESOURCE_ID_PARENT_TOP,
> -					&kvd_size_params,
> -					NULL);
> +					&kvd_size_params);
>  	if (err)
>  		return err;
>  
> @@ -3887,8 +3874,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
>  					linear_size,
>  					MLXSW_SP_RESOURCE_KVD_LINEAR,
>  					MLXSW_SP_RESOURCE_KVD,
> -					&linear_size_params,
> -					&mlxsw_sp_resource_kvd_linear_ops);
> +					&linear_size_params);
>  	if (err)
>  		return err;
>  
> @@ -3905,8 +3891,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
>  					double_size,
>  					MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE,
>  					MLXSW_SP_RESOURCE_KVD,
> -					&hash_double_size_params,
> -					NULL);
> +					&hash_double_size_params);
>  	if (err)
>  		return err;
>  
> @@ -3915,8 +3900,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
>  					single_size,
>  					MLXSW_SP_RESOURCE_KVD_HASH_SINGLE,
>  					MLXSW_SP_RESOURCE_KVD,
> -					&hash_single_size_params,
> -					NULL);
> +					&hash_single_size_params);
>  	if (err)
>  		return err;
>  
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> index 21bee8f19894..c59a0d7d81d5 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> @@ -442,7 +442,6 @@ 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);
>  int mlxsw_sp_kvdl_resources_register(struct devlink *devlink);
>  
>  struct mlxsw_sp_acl_rule_info {
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
> index 7b28f65d6407..1b7280168e6b 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
> @@ -315,8 +315,9 @@ static u64 mlxsw_sp_kvdl_part_occ(struct mlxsw_sp_kvdl_part *part)
>  	return occ;
>  }
>  
> -u64 mlxsw_sp_kvdl_occ_get(const struct mlxsw_sp *mlxsw_sp)
> +static u64 mlxsw_sp_kvdl_occ_get(void *priv)
>  {
> +	const struct mlxsw_sp *mlxsw_sp = priv;
>  	u64 occ = 0;
>  	int i;
>  
> @@ -326,48 +327,33 @@ u64 mlxsw_sp_kvdl_occ_get(const struct mlxsw_sp *mlxsw_sp)
>  	return occ;
>  }
>  
> -static u64 mlxsw_sp_kvdl_single_occ_get(struct devlink *devlink)
> +static u64 mlxsw_sp_kvdl_single_occ_get(void *priv)
>  {
> -	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
> -	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
> +	const struct mlxsw_sp *mlxsw_sp = priv;
>  	struct mlxsw_sp_kvdl_part *part;
>  
>  	part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_SINGLE];
>  	return mlxsw_sp_kvdl_part_occ(part);
>  }
>  
> -static u64 mlxsw_sp_kvdl_chunks_occ_get(struct devlink *devlink)
> +static u64 mlxsw_sp_kvdl_chunks_occ_get(void *priv)
>  {
> -	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
> -	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
> +	const struct mlxsw_sp *mlxsw_sp = priv;
>  	struct mlxsw_sp_kvdl_part *part;
>  
>  	part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_CHUNKS];
>  	return mlxsw_sp_kvdl_part_occ(part);
>  }
>  
> -static u64 mlxsw_sp_kvdl_large_chunks_occ_get(struct devlink *devlink)
> +static u64 mlxsw_sp_kvdl_large_chunks_occ_get(void *priv)
>  {
> -	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
> -	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
> +	const struct mlxsw_sp *mlxsw_sp = priv;
>  	struct mlxsw_sp_kvdl_part *part;
>  
>  	part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_LARGE_CHUNKS];
>  	return mlxsw_sp_kvdl_part_occ(part);
>  }
>  
> -static const struct devlink_resource_ops mlxsw_sp_kvdl_single_ops = {
> -	.occ_get = mlxsw_sp_kvdl_single_occ_get,
> -};
> -
> -static const struct devlink_resource_ops mlxsw_sp_kvdl_chunks_ops = {
> -	.occ_get = mlxsw_sp_kvdl_chunks_occ_get,
> -};
> -
> -static const struct devlink_resource_ops mlxsw_sp_kvdl_chunks_large_ops = {
> -	.occ_get = mlxsw_sp_kvdl_large_chunks_occ_get,
> -};
> -
>  int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
>  {
>  	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
> @@ -386,8 +372,7 @@ int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
>  					MLXSW_SP_KVDL_SINGLE_SIZE,
>  					MLXSW_SP_RESOURCE_KVD_LINEAR_SINGLE,
>  					MLXSW_SP_RESOURCE_KVD_LINEAR,
> -					&size_params,
> -					&mlxsw_sp_kvdl_single_ops);
> +					&size_params);
>  	if (err)
>  		return err;
>  
> @@ -398,8 +383,7 @@ int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
>  					MLXSW_SP_KVDL_CHUNKS_SIZE,
>  					MLXSW_SP_RESOURCE_KVD_LINEAR_CHUNKS,
>  					MLXSW_SP_RESOURCE_KVD_LINEAR,
> -					&size_params,
> -					&mlxsw_sp_kvdl_chunks_ops);
> +					&size_params);
>  	if (err)
>  		return err;
>  
> @@ -410,13 +394,13 @@ int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
>  					MLXSW_SP_KVDL_LARGE_CHUNKS_SIZE,
>  					MLXSW_SP_RESOURCE_KVD_LINEAR_LARGE_CHUNKS,
>  					MLXSW_SP_RESOURCE_KVD_LINEAR,
> -					&size_params,
> -					&mlxsw_sp_kvdl_chunks_large_ops);
> +					&size_params);
>  	return err;
>  }
>  
>  int mlxsw_sp_kvdl_init(struct mlxsw_sp *mlxsw_sp)
>  {
> +	struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
>  	struct mlxsw_sp_kvdl *kvdl;
>  	int err;
>  
> @@ -429,6 +413,23 @@ int mlxsw_sp_kvdl_init(struct mlxsw_sp *mlxsw_sp)
>  	if (err)
>  		goto err_kvdl_parts_init;
>  
> +	devlink_resource_occ_get_register(devlink,
> +					  MLXSW_SP_RESOURCE_KVD_LINEAR,
> +					  mlxsw_sp_kvdl_occ_get,
> +					  mlxsw_sp);
> +	devlink_resource_occ_get_register(devlink,
> +					  MLXSW_SP_RESOURCE_KVD_LINEAR_SINGLE,
> +					  mlxsw_sp_kvdl_single_occ_get,
> +					  mlxsw_sp);
> +	devlink_resource_occ_get_register(devlink,
> +					  MLXSW_SP_RESOURCE_KVD_LINEAR_CHUNKS,
> +					  mlxsw_sp_kvdl_chunks_occ_get,
> +					  mlxsw_sp);
> +	devlink_resource_occ_get_register(devlink,
> +					  MLXSW_SP_RESOURCE_KVD_LINEAR_LARGE_CHUNKS,
> +					  mlxsw_sp_kvdl_large_chunks_occ_get,
> +					  mlxsw_sp);
> +
>  	return 0;
>  
>  err_kvdl_parts_init:
> @@ -438,6 +439,16 @@ int mlxsw_sp_kvdl_init(struct mlxsw_sp *mlxsw_sp)
>  
>  void mlxsw_sp_kvdl_fini(struct mlxsw_sp *mlxsw_sp)
>  {
> +	struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
> +
> +	devlink_resource_occ_get_unregister(devlink,
> +					    MLXSW_SP_RESOURCE_KVD_LINEAR_LARGE_CHUNKS);
> +	devlink_resource_occ_get_unregister(devlink,
> +					    MLXSW_SP_RESOURCE_KVD_LINEAR_CHUNKS);
> +	devlink_resource_occ_get_unregister(devlink,
> +					    MLXSW_SP_RESOURCE_KVD_LINEAR_SINGLE);
> +	devlink_resource_occ_get_unregister(devlink,
> +					    MLXSW_SP_RESOURCE_KVD_LINEAR);
>  	mlxsw_sp_kvdl_parts_fini(mlxsw_sp);
>  	kfree(mlxsw_sp->kvdl);
>  }
> diff --git a/include/net/devlink.h b/include/net/devlink.h
> index e21d8cadd480..2e4f71e16e95 100644
> --- a/include/net/devlink.h
> +++ b/include/net/devlink.h
> @@ -231,14 +231,6 @@ struct devlink_dpipe_headers {
>  	unsigned int headers_count;
>  };
>  
> -/**
> - * struct devlink_resource_ops - resource ops
> - * @occ_get: get the occupied size
> - */
> -struct devlink_resource_ops {
> -	u64 (*occ_get)(struct devlink *devlink);
> -};
> -
>  /**
>   * struct devlink_resource_size_params - resource's size parameters
>   * @size_min: minimum size which can be set
> @@ -265,6 +257,8 @@ devlink_resource_size_params_init(struct devlink_resource_size_params *size_para
>  	size_params->unit = unit;
>  }
>  
> +typedef u64 devlink_resource_occ_get_t(void *priv);
> +
>  /**
>   * struct devlink_resource - devlink resource
>   * @name: name of the resource
> @@ -277,7 +271,6 @@ devlink_resource_size_params_init(struct devlink_resource_size_params *size_para
>   * @size_params: size parameters
>   * @list: parent list
>   * @resource_list: list of child resources
> - * @resource_ops: resource ops
>   */
>  struct devlink_resource {
>  	const char *name;
> @@ -289,7 +282,8 @@ struct devlink_resource {
>  	struct devlink_resource_size_params size_params;
>  	struct list_head list;
>  	struct list_head resource_list;
> -	const struct devlink_resource_ops *resource_ops;
> +	devlink_resource_occ_get_t *occ_get;
> +	void *occ_get_priv;
>  };
>  
>  #define DEVLINK_RESOURCE_ID_PARENT_TOP 0
> @@ -409,8 +403,7 @@ int devlink_resource_register(struct devlink *devlink,
>  			      u64 resource_size,
>  			      u64 resource_id,
>  			      u64 parent_resource_id,
> -			      const struct devlink_resource_size_params *size_params,
> -			      const struct devlink_resource_ops *resource_ops);
> +			      const struct devlink_resource_size_params *size_params);
>  void devlink_resources_unregister(struct devlink *devlink,
>  				  struct devlink_resource *resource);
>  int devlink_resource_size_get(struct devlink *devlink,
> @@ -419,6 +412,12 @@ int devlink_resource_size_get(struct devlink *devlink,
>  int devlink_dpipe_table_resource_set(struct devlink *devlink,
>  				     const char *table_name, u64 resource_id,
>  				     u64 resource_units);
> +void devlink_resource_occ_get_register(struct devlink *devlink,
> +				       u64 resource_id,
> +				       devlink_resource_occ_get_t *occ_get,
> +				       void *occ_get_priv);
> +void devlink_resource_occ_get_unregister(struct devlink *devlink,
> +					 u64 resource_id);
>  
>  #else
>  
> @@ -562,8 +561,7 @@ devlink_resource_register(struct devlink *devlink,
>  			  u64 resource_size,
>  			  u64 resource_id,
>  			  u64 parent_resource_id,
> -			  const struct devlink_resource_size_params *size_params,
> -			  const struct devlink_resource_ops *resource_ops)
> +			  const struct devlink_resource_size_params *size_params)
>  {
>  	return 0;
>  }
> @@ -589,6 +587,20 @@ devlink_dpipe_table_resource_set(struct devlink *devlink,
>  	return -EOPNOTSUPP;
>  }
>  
> +static inline void
> +devlink_resource_occ_get_register(struct devlink *devlink,
> +				  u64 resource_id,
> +				  devlink_resource_occ_get_t *occ_get,
> +				  void *occ_get_priv)
> +{
> +}
> +
> +static inline void
> +devlink_resource_occ_get_unregister(struct devlink *devlink,
> +				    u64 resource_id)
> +{
> +}
> +
>  #endif
>  
>  #endif /* _NET_DEVLINK_H_ */
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index 9236e421bd62..ad1317376798 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -2405,6 +2405,16 @@ devlink_resource_size_params_put(struct devlink_resource *resource,
>  	return 0;
>  }
>  
> +static int devlink_resource_occ_put(struct devlink_resource *resource,
> +				    struct sk_buff *skb)
> +{
> +	if (!resource->occ_get)
> +		return 0;
> +	return nla_put_u64_64bit(skb, DEVLINK_ATTR_RESOURCE_OCC,
> +				 resource->occ_get(resource->occ_get_priv),
> +				 DEVLINK_ATTR_PAD);
> +}
> +
>  static int devlink_resource_put(struct devlink *devlink, struct sk_buff *skb,
>  				struct devlink_resource *resource)
>  {
> @@ -2425,11 +2435,8 @@ static int devlink_resource_put(struct devlink *devlink, struct sk_buff *skb,
>  	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)
> -		if (nla_put_u64_64bit(skb, DEVLINK_ATTR_RESOURCE_OCC,
> -				      resource->resource_ops->occ_get(devlink),
> -				      DEVLINK_ATTR_PAD))
> -			goto nla_put_failure;
> +	if (devlink_resource_occ_put(resource, skb))
> +		goto nla_put_failure;
>  	if (devlink_resource_size_params_put(resource, skb))
>  		goto nla_put_failure;
>  	if (list_empty(&resource->resource_list))
> @@ -3162,15 +3169,13 @@ EXPORT_SYMBOL_GPL(devlink_dpipe_table_unregister);
>   *	@resource_id: resource's id
>   *	@parent_reosurce_id: resource's parent id
>   *	@size params: size parameters
> - *	@resource_ops: resource ops
>   */
>  int devlink_resource_register(struct devlink *devlink,
>  			      const char *resource_name,
>  			      u64 resource_size,
>  			      u64 resource_id,
>  			      u64 parent_resource_id,
> -			      const struct devlink_resource_size_params *size_params,
> -			      const struct devlink_resource_ops *resource_ops)
> +			      const struct devlink_resource_size_params *size_params)
>  {
>  	struct devlink_resource *resource;
>  	struct list_head *resource_list;
> @@ -3213,7 +3218,6 @@ int devlink_resource_register(struct devlink *devlink,
>  	resource->size = resource_size;
>  	resource->size_new = resource_size;
>  	resource->id = resource_id;
> -	resource->resource_ops = resource_ops;
>  	resource->size_valid = true;
>  	memcpy(&resource->size_params, size_params,
>  	       sizeof(resource->size_params));
> @@ -3315,6 +3319,58 @@ int devlink_dpipe_table_resource_set(struct devlink *devlink,
>  }
>  EXPORT_SYMBOL_GPL(devlink_dpipe_table_resource_set);
>  
> +/**
> + *	devlink_resource_occ_get_register - register occupancy getter
> + *
> + *	@devlink: devlink
> + *	@resource_id: resource id
> + *	@occ_get: occupancy getter callback
> + *	@occ_get_priv: occupancy getter callback priv
> + */
> +void devlink_resource_occ_get_register(struct devlink *devlink,
> +				       u64 resource_id,
> +				       devlink_resource_occ_get_t *occ_get,
> +				       void *occ_get_priv)
> +{
> +	struct devlink_resource *resource;
> +
> +	mutex_lock(&devlink->lock);
> +	resource = devlink_resource_find(devlink, NULL, resource_id);
> +	if (WARN_ON(!resource))
> +		goto out;
> +	WARN_ON(resource->occ_get);
> +
> +	resource->occ_get = occ_get;
> +	resource->occ_get_priv = occ_get_priv;
> +out:
> +	mutex_unlock(&devlink->lock);
> +}
> +EXPORT_SYMBOL_GPL(devlink_resource_occ_get_register);
> +
> +/**
> + *	devlink_resource_occ_get_unregister - unregister occupancy getter
> + *
> + *	@devlink: devlink
> + *	@resource_id: resource id
> + */
> +void devlink_resource_occ_get_unregister(struct devlink *devlink,
> +					 u64 resource_id)
> +{
> +	struct devlink_resource *resource;
> +
> +	mutex_lock(&devlink->lock);
> +	resource = devlink_resource_find(devlink, NULL, resource_id);
> +	if (WARN_ON(!resource))
> +		goto out;
> +	WARN_ON(!resource->occ_get);
> +
> +	resource->occ_get = NULL;
> +	resource->occ_get_priv = NULL;
> +out:
> +	mutex_unlock(&devlink->lock);
> +}
> +EXPORT_SYMBOL_GPL(devlink_resource_occ_get_unregister);
> +
>  static int __init devlink_module_init(void)
>  {
>  	return genl_register_family(&devlink_nl_family);
> 

Re: [PATCH net-next 09/11] devlink: convert occ_get op to separate registration

[Ido Schimmel]

On Fri, Mar 30, 2018 at 08:45:50AM -0600, David Ahern wrote:
> On 3/29/18 2:33 PM, Ido Schimmel wrote:
> > From: Jiri Pirko <jiri@mellanox.com>
> > 
> > This resolves race during initialization where the resources with
> > ops are registered before driver and the structures used by occ_get
> > op is initialized. So keep occ_get callbacks registered only when
> > all structs are initialized.
> 
> Why can't the occ_get handler look at some flag in an mlxsw struct to
> know if the system has initialized?
> 
> Separate registration here is awkward. You register a resource and then
> register its op later.
> 
> Also, this should be a standalone patch rather than embedded in a
> 'mlxsw: Various cleanups' set.

I'll drop it from v2 and ask Jiri to post it as a standalone patch.

Re: [PATCH net-next 09/11] devlink: convert occ_get op to separate registration

[Jiri Pirko]

Fri, Mar 30, 2018 at 04:45:50PM CEST, dsahern@gmail.com wrote:
>On 3/29/18 2:33 PM, Ido Schimmel wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> This resolves race during initialization where the resources with
>> ops are registered before driver and the structures used by occ_get
>> op is initialized. So keep occ_get callbacks registered only when
>> all structs are initialized.
>
>Why can't the occ_get handler look at some flag in an mlxsw struct to
>know if the system has initialized?
>
>Separate registration here is awkward. You register a resource and then
>register its op later.

The separation is exactly why this patch is made. Note that devlink
resouce is registered by core way before the initialization is done and
the driver is actually able to perform the op. Also consider "reload"
case, when the resource is still registered and the driver unloads and
loads again. For that makes perfect sense to have that separated.
Flag would just make things odd. Also, the priv could not be used in
that case.



>
>Also, this should be a standalone patch rather than embedded in a
>'mlxsw: Various cleanups' set.
>
>
>> 
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
>> ---
>>  drivers/net/ethernet/mellanox/mlxsw/spectrum.c     | 24 ++-----
>>  drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |  1 -
>>  .../net/ethernet/mellanox/mlxsw/spectrum_kvdl.c    | 67 ++++++++++++--------
>>  include/net/devlink.h                              | 40 ++++++++----
>>  net/core/devlink.c                                 | 74 +++++++++++++++++++---
>>  5 files changed, 134 insertions(+), 72 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>> index b831af38e0a1..0d95d2cb73e3 100644
>> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>> @@ -3805,18 +3805,6 @@ static const struct mlxsw_config_profile mlxsw_sp_config_profile = {
>>  	},
>>  };
>>  
>> -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 const struct devlink_resource_ops mlxsw_sp_resource_kvd_linear_ops = {
>> -	.occ_get = mlxsw_sp_resource_kvd_linear_occ_get,
>> -};
>> -
>>  static void
>>  mlxsw_sp_resource_size_params_prepare(struct mlxsw_core *mlxsw_core,
>>  				      struct devlink_resource_size_params *kvd_size_params,
>> @@ -3877,8 +3865,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
>>  	err = devlink_resource_register(devlink, MLXSW_SP_RESOURCE_NAME_KVD,
>>  					kvd_size, MLXSW_SP_RESOURCE_KVD,
>>  					DEVLINK_RESOURCE_ID_PARENT_TOP,
>> -					&kvd_size_params,
>> -					NULL);
>> +					&kvd_size_params);
>>  	if (err)
>>  		return err;
>>  
>> @@ -3887,8 +3874,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
>>  					linear_size,
>>  					MLXSW_SP_RESOURCE_KVD_LINEAR,
>>  					MLXSW_SP_RESOURCE_KVD,
>> -					&linear_size_params,
>> -					&mlxsw_sp_resource_kvd_linear_ops);
>> +					&linear_size_params);
>>  	if (err)
>>  		return err;
>>  
>> @@ -3905,8 +3891,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
>>  					double_size,
>>  					MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE,
>>  					MLXSW_SP_RESOURCE_KVD,
>> -					&hash_double_size_params,
>> -					NULL);
>> +					&hash_double_size_params);
>>  	if (err)
>>  		return err;
>>  
>> @@ -3915,8 +3900,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
>>  					single_size,
>>  					MLXSW_SP_RESOURCE_KVD_HASH_SINGLE,
>>  					MLXSW_SP_RESOURCE_KVD,
>> -					&hash_single_size_params,
>> -					NULL);
>> +					&hash_single_size_params);
>>  	if (err)
>>  		return err;
>>  
>> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
>> index 21bee8f19894..c59a0d7d81d5 100644
>> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
>> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
>> @@ -442,7 +442,6 @@ 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);
>>  int mlxsw_sp_kvdl_resources_register(struct devlink *devlink);
>>  
>>  struct mlxsw_sp_acl_rule_info {
>> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
>> index 7b28f65d6407..1b7280168e6b 100644
>> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
>> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
>> @@ -315,8 +315,9 @@ static u64 mlxsw_sp_kvdl_part_occ(struct mlxsw_sp_kvdl_part *part)
>>  	return occ;
>>  }
>>  
>> -u64 mlxsw_sp_kvdl_occ_get(const struct mlxsw_sp *mlxsw_sp)
>> +static u64 mlxsw_sp_kvdl_occ_get(void *priv)
>>  {
>> +	const struct mlxsw_sp *mlxsw_sp = priv;
>>  	u64 occ = 0;
>>  	int i;
>>  
>> @@ -326,48 +327,33 @@ u64 mlxsw_sp_kvdl_occ_get(const struct mlxsw_sp *mlxsw_sp)
>>  	return occ;
>>  }
>>  
>> -static u64 mlxsw_sp_kvdl_single_occ_get(struct devlink *devlink)
>> +static u64 mlxsw_sp_kvdl_single_occ_get(void *priv)
>>  {
>> -	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
>> -	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
>> +	const struct mlxsw_sp *mlxsw_sp = priv;
>>  	struct mlxsw_sp_kvdl_part *part;
>>  
>>  	part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_SINGLE];
>>  	return mlxsw_sp_kvdl_part_occ(part);
>>  }
>>  
>> -static u64 mlxsw_sp_kvdl_chunks_occ_get(struct devlink *devlink)
>> +static u64 mlxsw_sp_kvdl_chunks_occ_get(void *priv)
>>  {
>> -	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
>> -	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
>> +	const struct mlxsw_sp *mlxsw_sp = priv;
>>  	struct mlxsw_sp_kvdl_part *part;
>>  
>>  	part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_CHUNKS];
>>  	return mlxsw_sp_kvdl_part_occ(part);
>>  }
>>  
>> -static u64 mlxsw_sp_kvdl_large_chunks_occ_get(struct devlink *devlink)
>> +static u64 mlxsw_sp_kvdl_large_chunks_occ_get(void *priv)
>>  {
>> -	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
>> -	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
>> +	const struct mlxsw_sp *mlxsw_sp = priv;
>>  	struct mlxsw_sp_kvdl_part *part;
>>  
>>  	part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_LARGE_CHUNKS];
>>  	return mlxsw_sp_kvdl_part_occ(part);
>>  }
>>  
>> -static const struct devlink_resource_ops mlxsw_sp_kvdl_single_ops = {
>> -	.occ_get = mlxsw_sp_kvdl_single_occ_get,
>> -};
>> -
>> -static const struct devlink_resource_ops mlxsw_sp_kvdl_chunks_ops = {
>> -	.occ_get = mlxsw_sp_kvdl_chunks_occ_get,
>> -};
>> -
>> -static const struct devlink_resource_ops mlxsw_sp_kvdl_chunks_large_ops = {
>> -	.occ_get = mlxsw_sp_kvdl_large_chunks_occ_get,
>> -};
>> -
>>  int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
>>  {
>>  	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
>> @@ -386,8 +372,7 @@ int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
>>  					MLXSW_SP_KVDL_SINGLE_SIZE,
>>  					MLXSW_SP_RESOURCE_KVD_LINEAR_SINGLE,
>>  					MLXSW_SP_RESOURCE_KVD_LINEAR,
>> -					&size_params,
>> -					&mlxsw_sp_kvdl_single_ops);
>> +					&size_params);
>>  	if (err)
>>  		return err;
>>  
>> @@ -398,8 +383,7 @@ int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
>>  					MLXSW_SP_KVDL_CHUNKS_SIZE,
>>  					MLXSW_SP_RESOURCE_KVD_LINEAR_CHUNKS,
>>  					MLXSW_SP_RESOURCE_KVD_LINEAR,
>> -					&size_params,
>> -					&mlxsw_sp_kvdl_chunks_ops);
>> +					&size_params);
>>  	if (err)
>>  		return err;
>>  
>> @@ -410,13 +394,13 @@ int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
>>  					MLXSW_SP_KVDL_LARGE_CHUNKS_SIZE,
>>  					MLXSW_SP_RESOURCE_KVD_LINEAR_LARGE_CHUNKS,
>>  					MLXSW_SP_RESOURCE_KVD_LINEAR,
>> -					&size_params,
>> -					&mlxsw_sp_kvdl_chunks_large_ops);
>> +					&size_params);
>>  	return err;
>>  }
>>  
>>  int mlxsw_sp_kvdl_init(struct mlxsw_sp *mlxsw_sp)
>>  {
>> +	struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
>>  	struct mlxsw_sp_kvdl *kvdl;
>>  	int err;
>>  
>> @@ -429,6 +413,23 @@ int mlxsw_sp_kvdl_init(struct mlxsw_sp *mlxsw_sp)
>>  	if (err)
>>  		goto err_kvdl_parts_init;
>>  
>> +	devlink_resource_occ_get_register(devlink,
>> +					  MLXSW_SP_RESOURCE_KVD_LINEAR,
>> +					  mlxsw_sp_kvdl_occ_get,
>> +					  mlxsw_sp);
>> +	devlink_resource_occ_get_register(devlink,
>> +					  MLXSW_SP_RESOURCE_KVD_LINEAR_SINGLE,
>> +					  mlxsw_sp_kvdl_single_occ_get,
>> +					  mlxsw_sp);
>> +	devlink_resource_occ_get_register(devlink,
>> +					  MLXSW_SP_RESOURCE_KVD_LINEAR_CHUNKS,
>> +					  mlxsw_sp_kvdl_chunks_occ_get,
>> +					  mlxsw_sp);
>> +	devlink_resource_occ_get_register(devlink,
>> +					  MLXSW_SP_RESOURCE_KVD_LINEAR_LARGE_CHUNKS,
>> +					  mlxsw_sp_kvdl_large_chunks_occ_get,
>> +					  mlxsw_sp);
>> +
>>  	return 0;
>>  
>>  err_kvdl_parts_init:
>> @@ -438,6 +439,16 @@ int mlxsw_sp_kvdl_init(struct mlxsw_sp *mlxsw_sp)
>>  
>>  void mlxsw_sp_kvdl_fini(struct mlxsw_sp *mlxsw_sp)
>>  {
>> +	struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
>> +
>> +	devlink_resource_occ_get_unregister(devlink,
>> +					    MLXSW_SP_RESOURCE_KVD_LINEAR_LARGE_CHUNKS);
>> +	devlink_resource_occ_get_unregister(devlink,
>> +					    MLXSW_SP_RESOURCE_KVD_LINEAR_CHUNKS);
>> +	devlink_resource_occ_get_unregister(devlink,
>> +					    MLXSW_SP_RESOURCE_KVD_LINEAR_SINGLE);
>> +	devlink_resource_occ_get_unregister(devlink,
>> +					    MLXSW_SP_RESOURCE_KVD_LINEAR);
>>  	mlxsw_sp_kvdl_parts_fini(mlxsw_sp);
>>  	kfree(mlxsw_sp->kvdl);
>>  }
>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>> index e21d8cadd480..2e4f71e16e95 100644
>> --- a/include/net/devlink.h
>> +++ b/include/net/devlink.h
>> @@ -231,14 +231,6 @@ struct devlink_dpipe_headers {
>>  	unsigned int headers_count;
>>  };
>>  
>> -/**
>> - * struct devlink_resource_ops - resource ops
>> - * @occ_get: get the occupied size
>> - */
>> -struct devlink_resource_ops {
>> -	u64 (*occ_get)(struct devlink *devlink);
>> -};
>> -
>>  /**
>>   * struct devlink_resource_size_params - resource's size parameters
>>   * @size_min: minimum size which can be set
>> @@ -265,6 +257,8 @@ devlink_resource_size_params_init(struct devlink_resource_size_params *size_para
>>  	size_params->unit = unit;
>>  }
>>  
>> +typedef u64 devlink_resource_occ_get_t(void *priv);
>> +
>>  /**
>>   * struct devlink_resource - devlink resource
>>   * @name: name of the resource
>> @@ -277,7 +271,6 @@ devlink_resource_size_params_init(struct devlink_resource_size_params *size_para
>>   * @size_params: size parameters
>>   * @list: parent list
>>   * @resource_list: list of child resources
>> - * @resource_ops: resource ops
>>   */
>>  struct devlink_resource {
>>  	const char *name;
>> @@ -289,7 +282,8 @@ struct devlink_resource {
>>  	struct devlink_resource_size_params size_params;
>>  	struct list_head list;
>>  	struct list_head resource_list;
>> -	const struct devlink_resource_ops *resource_ops;
>> +	devlink_resource_occ_get_t *occ_get;
>> +	void *occ_get_priv;
>>  };
>>  
>>  #define DEVLINK_RESOURCE_ID_PARENT_TOP 0
>> @@ -409,8 +403,7 @@ int devlink_resource_register(struct devlink *devlink,
>>  			      u64 resource_size,
>>  			      u64 resource_id,
>>  			      u64 parent_resource_id,
>> -			      const struct devlink_resource_size_params *size_params,
>> -			      const struct devlink_resource_ops *resource_ops);
>> +			      const struct devlink_resource_size_params *size_params);
>>  void devlink_resources_unregister(struct devlink *devlink,
>>  				  struct devlink_resource *resource);
>>  int devlink_resource_size_get(struct devlink *devlink,
>> @@ -419,6 +412,12 @@ int devlink_resource_size_get(struct devlink *devlink,
>>  int devlink_dpipe_table_resource_set(struct devlink *devlink,
>>  				     const char *table_name, u64 resource_id,
>>  				     u64 resource_units);
>> +void devlink_resource_occ_get_register(struct devlink *devlink,
>> +				       u64 resource_id,
>> +				       devlink_resource_occ_get_t *occ_get,
>> +				       void *occ_get_priv);
>> +void devlink_resource_occ_get_unregister(struct devlink *devlink,
>> +					 u64 resource_id);
>>  
>>  #else
>>  
>> @@ -562,8 +561,7 @@ devlink_resource_register(struct devlink *devlink,
>>  			  u64 resource_size,
>>  			  u64 resource_id,
>>  			  u64 parent_resource_id,
>> -			  const struct devlink_resource_size_params *size_params,
>> -			  const struct devlink_resource_ops *resource_ops)
>> +			  const struct devlink_resource_size_params *size_params)
>>  {
>>  	return 0;
>>  }
>> @@ -589,6 +587,20 @@ devlink_dpipe_table_resource_set(struct devlink *devlink,
>>  	return -EOPNOTSUPP;
>>  }
>>  
>> +static inline void
>> +devlink_resource_occ_get_register(struct devlink *devlink,
>> +				  u64 resource_id,
>> +				  devlink_resource_occ_get_t *occ_get,
>> +				  void *occ_get_priv)
>> +{
>> +}
>> +
>> +static inline void
>> +devlink_resource_occ_get_unregister(struct devlink *devlink,
>> +				    u64 resource_id)
>> +{
>> +}
>> +
>>  #endif
>>  
>>  #endif /* _NET_DEVLINK_H_ */
>> diff --git a/net/core/devlink.c b/net/core/devlink.c
>> index 9236e421bd62..ad1317376798 100644
>> --- a/net/core/devlink.c
>> +++ b/net/core/devlink.c
>> @@ -2405,6 +2405,16 @@ devlink_resource_size_params_put(struct devlink_resource *resource,
>>  	return 0;
>>  }
>>  
>> +static int devlink_resource_occ_put(struct devlink_resource *resource,
>> +				    struct sk_buff *skb)
>> +{
>> +	if (!resource->occ_get)
>> +		return 0;
>> +	return nla_put_u64_64bit(skb, DEVLINK_ATTR_RESOURCE_OCC,
>> +				 resource->occ_get(resource->occ_get_priv),
>> +				 DEVLINK_ATTR_PAD);
>> +}
>> +
>>  static int devlink_resource_put(struct devlink *devlink, struct sk_buff *skb,
>>  				struct devlink_resource *resource)
>>  {
>> @@ -2425,11 +2435,8 @@ static int devlink_resource_put(struct devlink *devlink, struct sk_buff *skb,
>>  	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)
>> -		if (nla_put_u64_64bit(skb, DEVLINK_ATTR_RESOURCE_OCC,
>> -				      resource->resource_ops->occ_get(devlink),
>> -				      DEVLINK_ATTR_PAD))
>> -			goto nla_put_failure;
>> +	if (devlink_resource_occ_put(resource, skb))
>> +		goto nla_put_failure;
>>  	if (devlink_resource_size_params_put(resource, skb))
>>  		goto nla_put_failure;
>>  	if (list_empty(&resource->resource_list))
>> @@ -3162,15 +3169,13 @@ EXPORT_SYMBOL_GPL(devlink_dpipe_table_unregister);
>>   *	@resource_id: resource's id
>>   *	@parent_reosurce_id: resource's parent id
>>   *	@size params: size parameters
>> - *	@resource_ops: resource ops
>>   */
>>  int devlink_resource_register(struct devlink *devlink,
>>  			      const char *resource_name,
>>  			      u64 resource_size,
>>  			      u64 resource_id,
>>  			      u64 parent_resource_id,
>> -			      const struct devlink_resource_size_params *size_params,
>> -			      const struct devlink_resource_ops *resource_ops)
>> +			      const struct devlink_resource_size_params *size_params)
>>  {
>>  	struct devlink_resource *resource;
>>  	struct list_head *resource_list;
>> @@ -3213,7 +3218,6 @@ int devlink_resource_register(struct devlink *devlink,
>>  	resource->size = resource_size;
>>  	resource->size_new = resource_size;
>>  	resource->id = resource_id;
>> -	resource->resource_ops = resource_ops;
>>  	resource->size_valid = true;
>>  	memcpy(&resource->size_params, size_params,
>>  	       sizeof(resource->size_params));
>> @@ -3315,6 +3319,58 @@ int devlink_dpipe_table_resource_set(struct devlink *devlink,
>>  }
>>  EXPORT_SYMBOL_GPL(devlink_dpipe_table_resource_set);
>>  
>> +/**
>> + *	devlink_resource_occ_get_register - register occupancy getter
>> + *
>> + *	@devlink: devlink
>> + *	@resource_id: resource id
>> + *	@occ_get: occupancy getter callback
>> + *	@occ_get_priv: occupancy getter callback priv
>> + */
>> +void devlink_resource_occ_get_register(struct devlink *devlink,
>> +				       u64 resource_id,
>> +				       devlink_resource_occ_get_t *occ_get,
>> +				       void *occ_get_priv)
>> +{
>> +	struct devlink_resource *resource;
>> +
>> +	mutex_lock(&devlink->lock);
>> +	resource = devlink_resource_find(devlink, NULL, resource_id);
>> +	if (WARN_ON(!resource))
>> +		goto out;
>> +	WARN_ON(resource->occ_get);
>> +
>> +	resource->occ_get = occ_get;
>> +	resource->occ_get_priv = occ_get_priv;
>> +out:
>> +	mutex_unlock(&devlink->lock);
>> +}
>> +EXPORT_SYMBOL_GPL(devlink_resource_occ_get_register);
>> +
>> +/**
>> + *	devlink_resource_occ_get_unregister - unregister occupancy getter
>> + *
>> + *	@devlink: devlink
>> + *	@resource_id: resource id
>> + */
>> +void devlink_resource_occ_get_unregister(struct devlink *devlink,
>> +					 u64 resource_id)
>> +{
>> +	struct devlink_resource *resource;
>> +
>> +	mutex_lock(&devlink->lock);
>> +	resource = devlink_resource_find(devlink, NULL, resource_id);
>> +	if (WARN_ON(!resource))
>> +		goto out;
>> +	WARN_ON(!resource->occ_get);
>> +
>> +	resource->occ_get = NULL;
>> +	resource->occ_get_priv = NULL;
>> +out:
>> +	mutex_unlock(&devlink->lock);
>> +}
>> +EXPORT_SYMBOL_GPL(devlink_resource_occ_get_unregister);
>> +
>>  static int __init devlink_module_init(void)
>>  {
>>  	return genl_register_family(&devlink_nl_family);
>> 
>

Re: [PATCH net-next 09/11] devlink: convert occ_get op to separate registration

[David Ahern]

On 4/3/18 1:32 AM, Jiri Pirko wrote:
> Fri, Mar 30, 2018 at 04:45:50PM CEST, dsahern@gmail.com wrote:
>> On 3/29/18 2:33 PM, Ido Schimmel wrote:
>>> From: Jiri Pirko <jiri@mellanox.com>
>>>
>>> This resolves race during initialization where the resources with
>>> ops are registered before driver and the structures used by occ_get
>>> op is initialized. So keep occ_get callbacks registered only when
>>> all structs are initialized.
>>
>> Why can't the occ_get handler look at some flag in an mlxsw struct to
>> know if the system has initialized?
>>
>> Separate registration here is awkward. You register a resource and then
>> register its op later.
> 
> The separation is exactly why this patch is made. Note that devlink
> resouce is registered by core way before the initialization is done and
> the driver is actually able to perform the op. Also consider "reload"

That's how you have chose to code it. I hit this problem adding devlink
to netdevsim; the solution was to fix the init order.

> case, when the resource is still registered and the driver unloads and
> loads again. For that makes perfect sense to have that separated.
> Flag would just make things odd. Also, the priv could not be used in
> that case.
> 

I am not aware of any other API where you invoked the register function
at point A and then later add the operations at point B. In every API
that comes to mind the ops are part of the register.

I am sure there are options for you to fix the init order of mlxsw
without making the devlink API awkward.

Re: [PATCH net-next 09/11] devlink: convert occ_get op to separate registration

[Jiri Pirko]

Tue, Apr 03, 2018 at 04:33:11PM CEST, dsahern@gmail.com wrote:
>On 4/3/18 1:32 AM, Jiri Pirko wrote:
>> Fri, Mar 30, 2018 at 04:45:50PM CEST, dsahern@gmail.com wrote:
>>> On 3/29/18 2:33 PM, Ido Schimmel wrote:
>>>> From: Jiri Pirko <jiri@mellanox.com>
>>>>
>>>> This resolves race during initialization where the resources with
>>>> ops are registered before driver and the structures used by occ_get
>>>> op is initialized. So keep occ_get callbacks registered only when
>>>> all structs are initialized.
>>>
>>> Why can't the occ_get handler look at some flag in an mlxsw struct to
>>> know if the system has initialized?
>>>
>>> Separate registration here is awkward. You register a resource and then
>>> register its op later.
>> 
>> The separation is exactly why this patch is made. Note that devlink
>> resouce is registered by core way before the initialization is done and
>> the driver is actually able to perform the op. Also consider "reload"
>
>That's how you have chose to code it. I hit this problem adding devlink
>to netdevsim; the solution was to fix the init order.

This is not about init order, at all. On reaload netdevs and internal
driver structures disappear and appear again. And in between currently,
the op is working with memory which is freed. That's the reason for this
patch.


>
>> case, when the resource is still registered and the driver unloads and
>> loads again. For that makes perfect sense to have that separated.
>> Flag would just make things odd. Also, the priv could not be used in
>> that case.
>> 
>
>I am not aware of any other API where you invoked the register function
>at point A and then later add the operations at point B. In every API
>that comes to mind the ops are part of the register.

I think that you just don't see any similar API.


>
>I am sure there are options for you to fix the init order of mlxsw
>without making the devlink API awkward.

Again, not about init order, at all. I have no clue why you think so...

Re: [PATCH net-next 09/11] devlink: convert occ_get op to separate registration

[David Ahern]

On 4/3/18 9:30 AM, Jiri Pirko wrote:
> Tue, Apr 03, 2018 at 04:33:11PM CEST, dsahern@gmail.com wrote:
>> On 4/3/18 1:32 AM, Jiri Pirko wrote:
>>> Fri, Mar 30, 2018 at 04:45:50PM CEST, dsahern@gmail.com wrote:
>>>> On 3/29/18 2:33 PM, Ido Schimmel wrote:
>>>>> From: Jiri Pirko <jiri@mellanox.com>
>>>>>
>>>>> This resolves race during initialization where the resources with
>>>>> ops are registered before driver and the structures used by occ_get
>>>>> op is initialized. So keep occ_get callbacks registered only when
>>>>> all structs are initialized.
>>>>
>>>> Why can't the occ_get handler look at some flag in an mlxsw struct to
>>>> know if the system has initialized?
>>>>
>>>> Separate registration here is awkward. You register a resource and then
>>>> register its op later.
>>>
>>> The separation is exactly why this patch is made. Note that devlink
>>> resouce is registered by core way before the initialization is done and
>>> the driver is actually able to perform the op. Also consider "reload"
>>
>> That's how you have chose to code it. I hit this problem adding devlink
>> to netdevsim; the solution was to fix the init order.
> 
> This is not about init order, at all. On reaload netdevs and internal
> driver structures disappear and appear again. And in between currently,
> the op is working with memory which is freed. That's the reason for this
> patch.
> 
> 
>>
>>> case, when the resource is still registered and the driver unloads and
>>> loads again. For that makes perfect sense to have that separated.
>>> Flag would just make things odd. Also, the priv could not be used in
>>> that case.
>>>
>>
>> I am not aware of any other API where you invoked the register function
>> at point A and then later add the operations at point B. In every API
>> that comes to mind the ops are part of the register.
> 
> I think that you just don't see any similar API.
> 
> 
>>
>> I am sure there are options for you to fix the init order of mlxsw
>> without making the devlink API awkward.
> 
> Again, not about init order, at all. I have no clue why you think so...
> 

Jiri, I am not aware of any other API where a driver registers with it
yet doesn't want the handler to be called so either waits to register
the handler later or unregisters the handler. That is an awkward design.
There are other options to have a sane API and handle the conditions you
need.

Re: [PATCH net-next 09/11] devlink: convert occ_get op to separate registration

[Jiri Pirko]

Wed, Apr 04, 2018 at 02:47:19AM CEST, dsahern@gmail.com wrote:
>On 4/3/18 9:30 AM, Jiri Pirko wrote:
>> Tue, Apr 03, 2018 at 04:33:11PM CEST, dsahern@gmail.com wrote:
>>> On 4/3/18 1:32 AM, Jiri Pirko wrote:
>>>> Fri, Mar 30, 2018 at 04:45:50PM CEST, dsahern@gmail.com wrote:
>>>>> On 3/29/18 2:33 PM, Ido Schimmel wrote:
>>>>>> From: Jiri Pirko <jiri@mellanox.com>
>>>>>>
>>>>>> This resolves race during initialization where the resources with
>>>>>> ops are registered before driver and the structures used by occ_get
>>>>>> op is initialized. So keep occ_get callbacks registered only when
>>>>>> all structs are initialized.
>>>>>
>>>>> Why can't the occ_get handler look at some flag in an mlxsw struct to
>>>>> know if the system has initialized?
>>>>>
>>>>> Separate registration here is awkward. You register a resource and then
>>>>> register its op later.
>>>>
>>>> The separation is exactly why this patch is made. Note that devlink
>>>> resouce is registered by core way before the initialization is done and
>>>> the driver is actually able to perform the op. Also consider "reload"
>>>
>>> That's how you have chose to code it. I hit this problem adding devlink
>>> to netdevsim; the solution was to fix the init order.
>> 
>> This is not about init order, at all. On reaload netdevs and internal
>> driver structures disappear and appear again. And in between currently,
>> the op is working with memory which is freed. That's the reason for this
>> patch.
>> 
>> 
>>>
>>>> case, when the resource is still registered and the driver unloads and
>>>> loads again. For that makes perfect sense to have that separated.
>>>> Flag would just make things odd. Also, the priv could not be used in
>>>> that case.
>>>>
>>>
>>> I am not aware of any other API where you invoked the register function
>>> at point A and then later add the operations at point B. In every API
>>> that comes to mind the ops are part of the register.
>> 
>> I think that you just don't see any similar API.
>> 
>> 
>>>
>>> I am sure there are options for you to fix the init order of mlxsw
>>> without making the devlink API awkward.
>> 
>> Again, not about init order, at all. I have no clue why you think so...
>> 
>
>Jiri, I am not aware of any other API where a driver registers with it
>yet doesn't want the handler to be called so either waits to register

Again, the thing is, this is kind of unusual because of the reload
thing. There is one instance registering the resources, another instance
fills up the ops. I spent some time to think this through, I did not
find the other good solution (all are more or less ugly).

>the handler later or unregisters the handler. That is an awkward design.
>There are other options to have a sane API and handle the conditions you
>need.

Also, this is in-kernel API so it is not set in stone and can be very
easily changed whenever we need to. Not sure why you are trying to be so
strict about this...
Okay, please propose some solution you consider "not awkward". Thanks!

Re: [PATCH net-next 09/11] devlink: convert occ_get op to separate registration

[Jakub Kicinski]

On Wed, 4 Apr 2018 08:25:11 +0200, Jiri Pirko wrote:
> >Jiri, I am not aware of any other API where a driver registers with it
> >yet doesn't want the handler to be called so either waits to register  
> 
> Again, the thing is, this is kind of unusual because of the reload
> thing. 

FWIW my knee jerk thought is that it's strange that devlink ops can
be executed at all while reload is happening (incl. another reload
request).  I'm probably missing the real issue..

Re: [PATCH net-next 09/11] devlink: convert occ_get op to separate registration

[David Ahern]

On 4/4/18 4:59 PM, Jakub Kicinski wrote:
> On Wed, 4 Apr 2018 08:25:11 +0200, Jiri Pirko wrote:
>>> Jiri, I am not aware of any other API where a driver registers with it
>>> yet doesn't want the handler to be called so either waits to register  
>>
>> Again, the thing is, this is kind of unusual because of the reload
>> thing. 
> 
> FWIW my knee jerk thought is that it's strange that devlink ops can
> be executed at all while reload is happening (incl. another reload
> request).  I'm probably missing the real issue..
> 

Just responding with the same question ...

Why are you not unregistering resources on a reload?

Re: [PATCH net-next 09/11] devlink: convert occ_get op to separate registration

[Jiri Pirko]

Thu, Apr 05, 2018 at 01:00:18AM CEST, dsahern@gmail.com wrote:
>On 4/4/18 4:59 PM, Jakub Kicinski wrote:
>> On Wed, 4 Apr 2018 08:25:11 +0200, Jiri Pirko wrote:
>>>> Jiri, I am not aware of any other API where a driver registers with it
>>>> yet doesn't want the handler to be called so either waits to register  
>>>
>>> Again, the thing is, this is kind of unusual because of the reload
>>> thing. 
>> 
>> FWIW my knee jerk thought is that it's strange that devlink ops can
>> be executed at all while reload is happening (incl. another reload
>> request).  I'm probably missing the real issue..
>> 
>
>Just responding with the same question ...
>
>Why are you not unregistering resources on a reload?

Because you need the use the values of course!

[PATCH net-next 10/11] mlxsw: spectrum: Pass mlxsw_core as arg of mlxsw_sp_kvdl_resources_register()

[Ido Schimmel]

From: Jiri Pirko <jiri@mellanox.com>

Pass struct mlxsw_core instead of devlink since it is nicer within mlxsw
code and we need both structs in mlxsw_sp_kvdl_resources_register()
anyway.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c      | 2 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h      | 2 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 0d95d2cb73e3..ca38a30fbe91 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3878,7 +3878,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
 	if (err)
 		return err;
 
-	err = mlxsw_sp_kvdl_resources_register(devlink);
+	err = mlxsw_sp_kvdl_resources_register(mlxsw_core);
 	if  (err)
 		return err;
 
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index c59a0d7d81d5..952d99decc23 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -442,7 +442,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);
-int mlxsw_sp_kvdl_resources_register(struct devlink *devlink);
+int mlxsw_sp_kvdl_resources_register(struct mlxsw_core *mlxsw_core);
 
 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 1b7280168e6b..fe4327f547d2 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
@@ -354,9 +354,9 @@ static u64 mlxsw_sp_kvdl_large_chunks_occ_get(void *priv)
 	return mlxsw_sp_kvdl_part_occ(part);
 }
 
-int mlxsw_sp_kvdl_resources_register(struct devlink *devlink)
+int mlxsw_sp_kvdl_resources_register(struct mlxsw_core *mlxsw_core)
 {
-	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
+	struct devlink *devlink = priv_to_devlink(mlxsw_core);
 	static struct devlink_resource_size_params size_params;
 	u32 kvdl_max_size;
 	int err;
-- 
2.14.3

[PATCH net-next 11/11] mlxsw: spectrum: Don't use resource ID of 0

[Ido Schimmel]

From: Petr Machata <petrm@mellanox.com>

In commit 145307460ba9 ("devlink: Remove top_hierarchy arg to
devlink_resource_register"), the "top_hierarchy" parameter to
devlink_resource_register() was removed in favor of using the parameter
"parent_resource_id" exclusively to determine who the parent is. The
root node's resource ID for this purpose is
DEVLINK_RESOURCE_ID_PARENT_TOP with the value 0. It is therefore
problematic that the resource MLXSW_SP_RESOURCE_KVD has also ID of 0.

Fix this by numbering driver-specific resources from 1.

Fixes: 145307460ba9 ("devlink: Remove top_hierarchy arg to devlink_resource_register")
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 952d99decc23..804d4d2c8031 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -75,7 +75,7 @@
 #define MLXSW_SP_RESOURCE_NAME_KVD_LINEAR_LARGE_CHUNKS "large_chunks"
 
 enum mlxsw_sp_resource_id {
-	MLXSW_SP_RESOURCE_KVD,
+	MLXSW_SP_RESOURCE_KVD = 1,
 	MLXSW_SP_RESOURCE_KVD_LINEAR,
 	MLXSW_SP_RESOURCE_KVD_HASH_SINGLE,
 	MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE,
-- 
2.14.3

Re: [PATCH net-next 00/11] mlxsw: Various cleanups

[David Miller]

From: Ido Schimmel <idosch@mellanox.com>
Date: Thu, 29 Mar 2018 23:33:23 +0300

> The first 10 patches from Jiri perform small and unrelated cleanups. The
> largest being the conversion of the KVD linear partitions from a list to
> an array, which simplifies the code.
> 
> The last patch from Petr is a bug fix for a recent net-next commit that
> prevented the "kvd" resource from being marked as the parent of its
> various child resources (e.g., "/kvd/linear").

Series applied, thanks.

Re: [PATCH net-next 00/11] mlxsw: Various cleanups

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Sat, 31 Mar 2018 21:54:50 -0400 (EDT)

> From: Ido Schimmel <idosch@mellanox.com>
> Date: Thu, 29 Mar 2018 23:33:23 +0300
> 
>> The first 10 patches from Jiri perform small and unrelated cleanups. The
>> largest being the conversion of the KVD linear partitions from a list to
>> an array, which simplifies the code.
>> 
>> The last patch from Petr is a bug fix for a recent net-next commit that
>> prevented the "kvd" resource from being marked as the parent of its
>> various child resources (e.g., "/kvd/linear").
> 
> Series applied, thanks.

I'm going to have to revert this.

Changing the signature of devlink_resource_register() requires an update to
the netdevsim driver, which you didn't do.

drivers/net/netdevsim/devlink.c:121:8: error: too many arguments to function ‘devlink_resource_register’
  err = devlink_resource_register(devlink, "IPv6", (u64)-1,
        ^~~~~~~~~~~~~~~~~~~~~~~~~

Re: [PATCH net-next 00/11] mlxsw: Various cleanups

[Ido Schimmel]

On Sat, Mar 31, 2018 at 10:14:51PM -0400, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Sat, 31 Mar 2018 21:54:50 -0400 (EDT)
> 
> > From: Ido Schimmel <idosch@mellanox.com>
> > Date: Thu, 29 Mar 2018 23:33:23 +0300
> > 
> >> The first 10 patches from Jiri perform small and unrelated cleanups. The
> >> largest being the conversion of the KVD linear partitions from a list to
> >> an array, which simplifies the code.
> >> 
> >> The last patch from Petr is a bug fix for a recent net-next commit that
> >> prevented the "kvd" resource from being marked as the parent of its
> >> various child resources (e.g., "/kvd/linear").
> > 
> > Series applied, thanks.
> 
> I'm going to have to revert this.
> 
> Changing the signature of devlink_resource_register() requires an update to
> the netdevsim driver, which you didn't do.

Right, thanks. Will fix in v2.