netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/5] devlink: Preparations for trap policers support
@ 2020-03-22 18:48 Ido Schimmel
  2020-03-22 18:48 ` [PATCH net-next 1/5] devlink: Add API to register packet trap groups Ido Schimmel
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Ido Schimmel @ 2020-03-22 18:48 UTC (permalink / raw)
  To: netdev; +Cc: davem, jiri, kuba, mlxsw, Ido Schimmel

From: Ido Schimmel <idosch@mellanox.com>

This patch set prepares the code for devlink-trap policer support in a
follow-up patch set [1][2]. No functional changes intended.

Policers are going to be added as attributes of packet trap groups,
which are entities used to aggregate logically related packet traps.
This will allow users, for example, to limit all the packets that
encountered an exception during routing to 10Kpps.

However, currently, device drivers register their packet trap groups
implicitly when they register their packet traps via
devlink_traps_register(). This makes it difficult to pass additional
attributes for the groups. For example, the policer bound to the group.

Therefore, this patch set converts device drivers to explicitly register
their packet trap groups. This will later allow these drivers to
register the group with additional attributes, if any.

API today:
devlink_traps_register(traps)

API after this patch set:
devlink_trap_groups_register(groups)
devlink_traps_register(traps)

API after follow-up patch set:
devlink_trap_policers_register(policers)
devlink_trap_groups_register(groups)
devlink_traps_register(traps)

Patch set overview:
Patch #1 adds the new API to register packet trap groups
Patches #2-#3 convert mlxsw and netdevsim to use the new API
Patches #4-#5 remove the old API

Tested successfully with current devlink-trap selftests.

[1] https://github.com/idosch/linux/tree/trap-policers
[2] https://github.com/idosch/iproute2/tree/trap-policers

Ido Schimmel (5):
  devlink: Add API to register packet trap groups
  mlxsw: spectrum_trap: Explicitly register packet trap groups
  netdevsim: Explicitly register packet trap groups
  devlink: Stop reference counting packet trap groups
  devlink: Only pass packet trap group identifier in trap structure

 .../ethernet/mellanox/mlxsw/spectrum_trap.c   |  38 ++-
 drivers/net/netdevsim/dev.c                   |  27 +-
 include/net/devlink.h                         |  19 +-
 net/core/devlink.c                            | 233 ++++++++++--------
 4 files changed, 202 insertions(+), 115 deletions(-)

-- 
2.24.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next 1/5] devlink: Add API to register packet trap groups
  2020-03-22 18:48 [PATCH net-next 0/5] devlink: Preparations for trap policers support Ido Schimmel
@ 2020-03-22 18:48 ` Ido Schimmel
  2020-03-22 18:48 ` [PATCH net-next 2/5] mlxsw: spectrum_trap: Explicitly " Ido Schimmel
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Ido Schimmel @ 2020-03-22 18:48 UTC (permalink / raw)
  To: netdev; +Cc: davem, jiri, kuba, mlxsw, Ido Schimmel

From: Ido Schimmel <idosch@mellanox.com>

Currently, packet trap groups are implicitly registered by drivers upon
packet trap registration. When the traps are registered, each is
associated with a group and the group is created by devlink, if it does
not exist already.

This makes it difficult for drivers to pass additional attributes for
the groups.

Therefore, as a preparation for future patches that require passing
additional group attributes, add an API to explicitly register /
unregister these groups.

Next patches will convert existing drivers to use this API.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/devlink.h |   6 +++
 net/core/devlink.c    | 117 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 123 insertions(+)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index c9ca86b054bc..de3289217b9a 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1055,6 +1055,12 @@ void devlink_trap_report(struct devlink *devlink, struct sk_buff *skb,
 			 void *trap_ctx, struct devlink_port *in_devlink_port,
 			 const struct flow_action_cookie *fa_cookie);
 void *devlink_trap_ctx_priv(void *trap_ctx);
+int devlink_trap_groups_register(struct devlink *devlink,
+				 const struct devlink_trap_group *groups,
+				 size_t groups_count);
+void devlink_trap_groups_unregister(struct devlink *devlink,
+				    const struct devlink_trap_group *groups,
+				    size_t groups_count);
 
 #if IS_ENABLED(CONFIG_NET_DEVLINK)
 
diff --git a/net/core/devlink.c b/net/core/devlink.c
index f51bebc8c33f..089a220aabab 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -8278,6 +8278,123 @@ void *devlink_trap_ctx_priv(void *trap_ctx)
 }
 EXPORT_SYMBOL_GPL(devlink_trap_ctx_priv);
 
+static int
+devlink_trap_group_register(struct devlink *devlink,
+			    const struct devlink_trap_group *group)
+{
+	struct devlink_trap_group_item *group_item;
+	int err;
+
+	if (devlink_trap_group_item_lookup(devlink, group->name))
+		return -EEXIST;
+
+	group_item = kzalloc(sizeof(*group_item), GFP_KERNEL);
+	if (!group_item)
+		return -ENOMEM;
+
+	group_item->stats = netdev_alloc_pcpu_stats(struct devlink_stats);
+	if (!group_item->stats) {
+		err = -ENOMEM;
+		goto err_stats_alloc;
+	}
+
+	group_item->group = group;
+	refcount_set(&group_item->refcount, 1);
+
+	if (devlink->ops->trap_group_init) {
+		err = devlink->ops->trap_group_init(devlink, group);
+		if (err)
+			goto err_group_init;
+	}
+
+	list_add_tail(&group_item->list, &devlink->trap_group_list);
+	devlink_trap_group_notify(devlink, group_item,
+				  DEVLINK_CMD_TRAP_GROUP_NEW);
+
+	return 0;
+
+err_group_init:
+	free_percpu(group_item->stats);
+err_stats_alloc:
+	kfree(group_item);
+	return err;
+}
+
+static void
+devlink_trap_group_unregister(struct devlink *devlink,
+			      const struct devlink_trap_group *group)
+{
+	struct devlink_trap_group_item *group_item;
+
+	group_item = devlink_trap_group_item_lookup(devlink, group->name);
+	if (WARN_ON_ONCE(!group_item))
+		return;
+
+	devlink_trap_group_notify(devlink, group_item,
+				  DEVLINK_CMD_TRAP_GROUP_DEL);
+	list_del(&group_item->list);
+	free_percpu(group_item->stats);
+	kfree(group_item);
+}
+
+/**
+ * devlink_trap_groups_register - Register packet trap groups with devlink.
+ * @devlink: devlink.
+ * @groups: Packet trap groups.
+ * @groups_count: Count of provided packet trap groups.
+ *
+ * Return: Non-zero value on failure.
+ */
+int devlink_trap_groups_register(struct devlink *devlink,
+				 const struct devlink_trap_group *groups,
+				 size_t groups_count)
+{
+	int i, err;
+
+	mutex_lock(&devlink->lock);
+	for (i = 0; i < groups_count; i++) {
+		const struct devlink_trap_group *group = &groups[i];
+
+		err = devlink_trap_group_verify(group);
+		if (err)
+			goto err_trap_group_verify;
+
+		err = devlink_trap_group_register(devlink, group);
+		if (err)
+			goto err_trap_group_register;
+	}
+	mutex_unlock(&devlink->lock);
+
+	return 0;
+
+err_trap_group_register:
+err_trap_group_verify:
+	for (i--; i >= 0; i--)
+		devlink_trap_group_unregister(devlink, &groups[i]);
+	mutex_unlock(&devlink->lock);
+	return err;
+}
+EXPORT_SYMBOL_GPL(devlink_trap_groups_register);
+
+/**
+ * devlink_trap_groups_unregister - Unregister packet trap groups from devlink.
+ * @devlink: devlink.
+ * @groups: Packet trap groups.
+ * @groups_count: Count of provided packet trap groups.
+ */
+void devlink_trap_groups_unregister(struct devlink *devlink,
+				    const struct devlink_trap_group *groups,
+				    size_t groups_count)
+{
+	int i;
+
+	mutex_lock(&devlink->lock);
+	for (i = groups_count - 1; i >= 0; i--)
+		devlink_trap_group_unregister(devlink, &groups[i]);
+	mutex_unlock(&devlink->lock);
+}
+EXPORT_SYMBOL_GPL(devlink_trap_groups_unregister);
+
 static void __devlink_compat_running_version(struct devlink *devlink,
 					     char *buf, size_t len)
 {
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next 2/5] mlxsw: spectrum_trap: Explicitly register packet trap groups
  2020-03-22 18:48 [PATCH net-next 0/5] devlink: Preparations for trap policers support Ido Schimmel
  2020-03-22 18:48 ` [PATCH net-next 1/5] devlink: Add API to register packet trap groups Ido Schimmel
@ 2020-03-22 18:48 ` Ido Schimmel
  2020-03-22 18:48 ` [PATCH net-next 3/5] netdevsim: " Ido Schimmel
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Ido Schimmel @ 2020-03-22 18:48 UTC (permalink / raw)
  To: netdev; +Cc: davem, jiri, kuba, mlxsw, Ido Schimmel

From: Ido Schimmel <idosch@mellanox.com>

Use the previously added API to explicitly register / unregister
supported packet trap groups. This is in preparation for future patches
that will enable drivers to pass additional group attributes, such as
associated policer identifier.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 .../ethernet/mellanox/mlxsw/spectrum_trap.c   | 30 +++++++++++++++++--
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
index 9c300d625e04..cf3891609d5c 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
@@ -165,6 +165,13 @@ static void mlxsw_sp_rx_exception_listener(struct sk_buff *skb, u8 local_port,
 	MLXSW_RXL(mlxsw_sp_rx_exception_listener, _id,			      \
 		   _action, false, SP_##_group_id, SET_FW_DEFAULT)
 
+static const struct devlink_trap_group mlxsw_sp_trap_groups_arr[] = {
+	DEVLINK_TRAP_GROUP_GENERIC(L2_DROPS),
+	DEVLINK_TRAP_GROUP_GENERIC(L3_DROPS),
+	DEVLINK_TRAP_GROUP_GENERIC(TUNNEL_DROPS),
+	DEVLINK_TRAP_GROUP_GENERIC(ACL_DROPS),
+};
+
 static const struct devlink_trap mlxsw_sp_traps_arr[] = {
 	MLXSW_SP_TRAP_DROP(SMAC_MC, L2_DROPS),
 	MLXSW_SP_TRAP_DROP(VLAN_TAG_MISMATCH, L2_DROPS),
@@ -318,6 +325,7 @@ static int mlxsw_sp_trap_dummy_group_init(struct mlxsw_sp *mlxsw_sp)
 
 int mlxsw_sp_devlink_traps_init(struct mlxsw_sp *mlxsw_sp)
 {
+	size_t groups_count = ARRAY_SIZE(mlxsw_sp_trap_groups_arr);
 	struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
 	int err;
 
@@ -333,17 +341,33 @@ int mlxsw_sp_devlink_traps_init(struct mlxsw_sp *mlxsw_sp)
 		    ARRAY_SIZE(mlxsw_sp_listeners_arr)))
 		return -EINVAL;
 
-	return devlink_traps_register(devlink, mlxsw_sp_traps_arr,
-				      ARRAY_SIZE(mlxsw_sp_traps_arr),
-				      mlxsw_sp);
+	err = devlink_trap_groups_register(devlink, mlxsw_sp_trap_groups_arr,
+					   groups_count);
+	if (err)
+		return err;
+
+	err = devlink_traps_register(devlink, mlxsw_sp_traps_arr,
+				     ARRAY_SIZE(mlxsw_sp_traps_arr), mlxsw_sp);
+	if (err)
+		goto err_traps_register;
+
+	return 0;
+
+err_traps_register:
+	devlink_trap_groups_unregister(devlink, mlxsw_sp_trap_groups_arr,
+				       groups_count);
+	return err;
 }
 
 void mlxsw_sp_devlink_traps_fini(struct mlxsw_sp *mlxsw_sp)
 {
+	size_t groups_count = ARRAY_SIZE(mlxsw_sp_trap_groups_arr);
 	struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
 
 	devlink_traps_unregister(devlink, mlxsw_sp_traps_arr,
 				 ARRAY_SIZE(mlxsw_sp_traps_arr));
+	devlink_trap_groups_unregister(devlink, mlxsw_sp_trap_groups_arr,
+				       groups_count);
 }
 
 int mlxsw_sp_trap_init(struct mlxsw_core *mlxsw_core,
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next 3/5] netdevsim: Explicitly register packet trap groups
  2020-03-22 18:48 [PATCH net-next 0/5] devlink: Preparations for trap policers support Ido Schimmel
  2020-03-22 18:48 ` [PATCH net-next 1/5] devlink: Add API to register packet trap groups Ido Schimmel
  2020-03-22 18:48 ` [PATCH net-next 2/5] mlxsw: spectrum_trap: Explicitly " Ido Schimmel
@ 2020-03-22 18:48 ` Ido Schimmel
  2020-03-22 18:48 ` [PATCH net-next 4/5] devlink: Stop reference counting " Ido Schimmel
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Ido Schimmel @ 2020-03-22 18:48 UTC (permalink / raw)
  To: netdev; +Cc: davem, jiri, kuba, mlxsw, Ido Schimmel

From: Ido Schimmel <idosch@mellanox.com>

Use the previously added API to explicitly register / unregister
supported packet trap groups. This is in preparation for future patches
that will enable drivers to pass additional group attributes, such as
associated policer identifier.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/netdevsim/dev.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index f81c47377f32..edeb61ddc8bc 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -397,6 +397,13 @@ enum {
 			    DEVLINK_TRAP_GROUP_GENERIC(_group_id),	      \
 			    NSIM_TRAP_METADATA)
 
+static const struct devlink_trap_group nsim_trap_groups_arr[] = {
+	DEVLINK_TRAP_GROUP_GENERIC(L2_DROPS),
+	DEVLINK_TRAP_GROUP_GENERIC(L3_DROPS),
+	DEVLINK_TRAP_GROUP_GENERIC(BUFFER_DROPS),
+	DEVLINK_TRAP_GROUP_GENERIC(ACL_DROPS),
+};
+
 static const struct devlink_trap nsim_traps_arr[] = {
 	NSIM_TRAP_DROP(SMAC_MC, L2_DROPS),
 	NSIM_TRAP_DROP(VLAN_TAG_MISMATCH, L2_DROPS),
@@ -556,10 +563,15 @@ static int nsim_dev_traps_init(struct devlink *devlink)
 	nsim_trap_data->nsim_dev = nsim_dev;
 	nsim_dev->trap_data = nsim_trap_data;
 
+	err = devlink_trap_groups_register(devlink, nsim_trap_groups_arr,
+					   ARRAY_SIZE(nsim_trap_groups_arr));
+	if (err)
+		goto err_trap_items_free;
+
 	err = devlink_traps_register(devlink, nsim_traps_arr,
 				     ARRAY_SIZE(nsim_traps_arr), NULL);
 	if (err)
-		goto err_trap_items_free;
+		goto err_trap_groups_unregister;
 
 	INIT_DELAYED_WORK(&nsim_dev->trap_data->trap_report_dw,
 			  nsim_dev_trap_report_work);
@@ -568,6 +580,9 @@ static int nsim_dev_traps_init(struct devlink *devlink)
 
 	return 0;
 
+err_trap_groups_unregister:
+	devlink_trap_groups_unregister(devlink, nsim_trap_groups_arr,
+				       ARRAY_SIZE(nsim_trap_groups_arr));
 err_trap_items_free:
 	kfree(nsim_trap_data->trap_items_arr);
 err_trap_data_free:
@@ -582,6 +597,8 @@ static void nsim_dev_traps_exit(struct devlink *devlink)
 	cancel_delayed_work_sync(&nsim_dev->trap_data->trap_report_dw);
 	devlink_traps_unregister(devlink, nsim_traps_arr,
 				 ARRAY_SIZE(nsim_traps_arr));
+	devlink_trap_groups_unregister(devlink, nsim_trap_groups_arr,
+				       ARRAY_SIZE(nsim_trap_groups_arr));
 	kfree(nsim_dev->trap_data->trap_items_arr);
 	kfree(nsim_dev->trap_data);
 }
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next 4/5] devlink: Stop reference counting packet trap groups
  2020-03-22 18:48 [PATCH net-next 0/5] devlink: Preparations for trap policers support Ido Schimmel
                   ` (2 preceding siblings ...)
  2020-03-22 18:48 ` [PATCH net-next 3/5] netdevsim: " Ido Schimmel
@ 2020-03-22 18:48 ` Ido Schimmel
  2020-03-22 18:48 ` [PATCH net-next 5/5] devlink: Only pass packet trap group identifier in trap structure Ido Schimmel
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Ido Schimmel @ 2020-03-22 18:48 UTC (permalink / raw)
  To: netdev; +Cc: davem, jiri, kuba, mlxsw, Ido Schimmel

From: Ido Schimmel <idosch@mellanox.com>

Now that drivers explicitly register their supported packet trap groups
there is no for devlink to create them on-demand and destroy them when
their reference count reaches zero.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 net/core/devlink.c | 101 +++------------------------------------------
 1 file changed, 5 insertions(+), 96 deletions(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 089a220aabab..a35285a48b02 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -5456,16 +5456,14 @@ struct devlink_stats {
 /**
  * struct devlink_trap_group_item - Packet trap group attributes.
  * @group: Immutable packet trap group attributes.
- * @refcount: Number of trap items using the group.
  * @list: trap_group_list member.
  * @stats: Trap group statistics.
  *
  * Describes packet trap group attributes. Created by devlink during trap
- * registration.
+ * group registration.
  */
 struct devlink_trap_group_item {
 	const struct devlink_trap_group *group;
-	refcount_t refcount;
 	struct list_head list;
 	struct devlink_stats __percpu *stats;
 };
@@ -7937,108 +7935,22 @@ devlink_trap_group_notify(struct devlink *devlink,
 				msg, 0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
 }
 
-static struct devlink_trap_group_item *
-devlink_trap_group_item_create(struct devlink *devlink,
-			       const struct devlink_trap_group *group)
-{
-	struct devlink_trap_group_item *group_item;
-	int err;
-
-	err = devlink_trap_group_verify(group);
-	if (err)
-		return ERR_PTR(err);
-
-	group_item = kzalloc(sizeof(*group_item), GFP_KERNEL);
-	if (!group_item)
-		return ERR_PTR(-ENOMEM);
-
-	group_item->stats = netdev_alloc_pcpu_stats(struct devlink_stats);
-	if (!group_item->stats) {
-		err = -ENOMEM;
-		goto err_stats_alloc;
-	}
-
-	group_item->group = group;
-	refcount_set(&group_item->refcount, 1);
-
-	if (devlink->ops->trap_group_init) {
-		err = devlink->ops->trap_group_init(devlink, group);
-		if (err)
-			goto err_group_init;
-	}
-
-	list_add_tail(&group_item->list, &devlink->trap_group_list);
-	devlink_trap_group_notify(devlink, group_item,
-				  DEVLINK_CMD_TRAP_GROUP_NEW);
-
-	return group_item;
-
-err_group_init:
-	free_percpu(group_item->stats);
-err_stats_alloc:
-	kfree(group_item);
-	return ERR_PTR(err);
-}
-
-static void
-devlink_trap_group_item_destroy(struct devlink *devlink,
-				struct devlink_trap_group_item *group_item)
-{
-	devlink_trap_group_notify(devlink, group_item,
-				  DEVLINK_CMD_TRAP_GROUP_DEL);
-	list_del(&group_item->list);
-	free_percpu(group_item->stats);
-	kfree(group_item);
-}
-
-static struct devlink_trap_group_item *
-devlink_trap_group_item_get(struct devlink *devlink,
-			    const struct devlink_trap_group *group)
-{
-	struct devlink_trap_group_item *group_item;
-
-	group_item = devlink_trap_group_item_lookup(devlink, group->name);
-	if (group_item) {
-		refcount_inc(&group_item->refcount);
-		return group_item;
-	}
-
-	return devlink_trap_group_item_create(devlink, group);
-}
-
-static void
-devlink_trap_group_item_put(struct devlink *devlink,
-			    struct devlink_trap_group_item *group_item)
-{
-	if (!refcount_dec_and_test(&group_item->refcount))
-		return;
-
-	devlink_trap_group_item_destroy(devlink, group_item);
-}
-
 static int
 devlink_trap_item_group_link(struct devlink *devlink,
 			     struct devlink_trap_item *trap_item)
 {
+	const struct devlink_trap *trap = trap_item->trap;
 	struct devlink_trap_group_item *group_item;
 
-	group_item = devlink_trap_group_item_get(devlink,
-						 &trap_item->trap->group);
-	if (IS_ERR(group_item))
-		return PTR_ERR(group_item);
+	group_item = devlink_trap_group_item_lookup(devlink, trap->group.name);
+	if (WARN_ON_ONCE(!group_item))
+		return -EINVAL;
 
 	trap_item->group_item = group_item;
 
 	return 0;
 }
 
-static void
-devlink_trap_item_group_unlink(struct devlink *devlink,
-			       struct devlink_trap_item *trap_item)
-{
-	devlink_trap_group_item_put(devlink, trap_item->group_item);
-}
-
 static void devlink_trap_notify(struct devlink *devlink,
 				const struct devlink_trap_item *trap_item,
 				enum devlink_command cmd)
@@ -8101,7 +8013,6 @@ devlink_trap_register(struct devlink *devlink,
 	return 0;
 
 err_trap_init:
-	devlink_trap_item_group_unlink(devlink, trap_item);
 err_group_link:
 	free_percpu(trap_item->stats);
 err_stats_alloc:
@@ -8122,7 +8033,6 @@ static void devlink_trap_unregister(struct devlink *devlink,
 	list_del(&trap_item->list);
 	if (devlink->ops->trap_fini)
 		devlink->ops->trap_fini(devlink, trap, trap_item);
-	devlink_trap_item_group_unlink(devlink, trap_item);
 	free_percpu(trap_item->stats);
 	kfree(trap_item);
 }
@@ -8299,7 +8209,6 @@ devlink_trap_group_register(struct devlink *devlink,
 	}
 
 	group_item->group = group;
-	refcount_set(&group_item->refcount, 1);
 
 	if (devlink->ops->trap_group_init) {
 		err = devlink->ops->trap_group_init(devlink, group);
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next 5/5] devlink: Only pass packet trap group identifier in trap structure
  2020-03-22 18:48 [PATCH net-next 0/5] devlink: Preparations for trap policers support Ido Schimmel
                   ` (3 preceding siblings ...)
  2020-03-22 18:48 ` [PATCH net-next 4/5] devlink: Stop reference counting " Ido Schimmel
@ 2020-03-22 18:48 ` Ido Schimmel
  2020-03-23 18:46 ` [PATCH net-next 0/5] devlink: Preparations for trap policers support Jakub Kicinski
  2020-03-24  4:41 ` David Miller
  6 siblings, 0 replies; 8+ messages in thread
From: Ido Schimmel @ 2020-03-22 18:48 UTC (permalink / raw)
  To: netdev; +Cc: davem, jiri, kuba, mlxsw, Ido Schimmel

From: Ido Schimmel <idosch@mellanox.com>

Packet trap groups are now explicitly registered by drivers and not
implicitly registered when the packet traps are registered. Therefore,
there is no need to encode entire group structure the trap is associated
with inside the trap structure.

Instead, only pass the group identifier. Refer to it as initial group
identifier, as future patches will allow user space to move traps
between groups.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 .../ethernet/mellanox/mlxsw/spectrum_trap.c   |  8 +++----
 drivers/net/netdevsim/dev.c                   |  8 +++----
 include/net/devlink.h                         | 13 ++++++------
 net/core/devlink.c                            | 21 +++++++++++++++----
 4 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
index cf3891609d5c..727f6ef243df 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
@@ -132,23 +132,23 @@ static void mlxsw_sp_rx_exception_listener(struct sk_buff *skb, u8 local_port,
 
 #define MLXSW_SP_TRAP_DROP(_id, _group_id)				      \
 	DEVLINK_TRAP_GENERIC(DROP, DROP, _id,				      \
-			     DEVLINK_TRAP_GROUP_GENERIC(_group_id),	      \
+			     DEVLINK_TRAP_GROUP_GENERIC_ID_##_group_id,	      \
 			     MLXSW_SP_TRAP_METADATA)
 
 #define MLXSW_SP_TRAP_DROP_EXT(_id, _group_id, _metadata)		      \
 	DEVLINK_TRAP_GENERIC(DROP, DROP, _id,				      \
-			     DEVLINK_TRAP_GROUP_GENERIC(_group_id),	      \
+			     DEVLINK_TRAP_GROUP_GENERIC_ID_##_group_id,	      \
 			     MLXSW_SP_TRAP_METADATA | (_metadata))
 
 #define MLXSW_SP_TRAP_DRIVER_DROP(_id, _group_id)			      \
 	DEVLINK_TRAP_DRIVER(DROP, DROP, DEVLINK_MLXSW_TRAP_ID_##_id,	      \
 			    DEVLINK_MLXSW_TRAP_NAME_##_id,		      \
-			    DEVLINK_TRAP_GROUP_GENERIC(_group_id),	      \
+			    DEVLINK_TRAP_GROUP_GENERIC_ID_##_group_id,	      \
 			    MLXSW_SP_TRAP_METADATA)
 
 #define MLXSW_SP_TRAP_EXCEPTION(_id, _group_id)		      \
 	DEVLINK_TRAP_GENERIC(EXCEPTION, TRAP, _id,			      \
-			     DEVLINK_TRAP_GROUP_GENERIC(_group_id),	      \
+			     DEVLINK_TRAP_GROUP_GENERIC_ID_##_group_id,	      \
 			     MLXSW_SP_TRAP_METADATA)
 
 #define MLXSW_SP_RXL_DISCARD(_id, _group_id)				      \
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index edeb61ddc8bc..7bfd0622cef1 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -381,20 +381,20 @@ enum {
 
 #define NSIM_TRAP_DROP(_id, _group_id)					      \
 	DEVLINK_TRAP_GENERIC(DROP, DROP, _id,				      \
-			     DEVLINK_TRAP_GROUP_GENERIC(_group_id),	      \
+			     DEVLINK_TRAP_GROUP_GENERIC_ID_##_group_id,	      \
 			     NSIM_TRAP_METADATA)
 #define NSIM_TRAP_DROP_EXT(_id, _group_id, _metadata)			      \
 	DEVLINK_TRAP_GENERIC(DROP, DROP, _id,				      \
-			     DEVLINK_TRAP_GROUP_GENERIC(_group_id),	      \
+			     DEVLINK_TRAP_GROUP_GENERIC_ID_##_group_id,	      \
 			     NSIM_TRAP_METADATA | (_metadata))
 #define NSIM_TRAP_EXCEPTION(_id, _group_id)				      \
 	DEVLINK_TRAP_GENERIC(EXCEPTION, TRAP, _id,			      \
-			     DEVLINK_TRAP_GROUP_GENERIC(_group_id),	      \
+			     DEVLINK_TRAP_GROUP_GENERIC_ID_##_group_id,	      \
 			     NSIM_TRAP_METADATA)
 #define NSIM_TRAP_DRIVER_EXCEPTION(_id, _group_id)			      \
 	DEVLINK_TRAP_DRIVER(EXCEPTION, TRAP, NSIM_TRAP_ID_##_id,	      \
 			    NSIM_TRAP_NAME_##_id,			      \
-			    DEVLINK_TRAP_GROUP_GENERIC(_group_id),	      \
+			    DEVLINK_TRAP_GROUP_GENERIC_ID_##_group_id,	      \
 			    NSIM_TRAP_METADATA)
 
 static const struct devlink_trap_group nsim_trap_groups_arr[] = {
diff --git a/include/net/devlink.h b/include/net/devlink.h
index de3289217b9a..f3eda246fe32 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -551,7 +551,7 @@ struct devlink_trap_group {
  * @generic: Whether the trap is generic or not.
  * @id: Trap identifier.
  * @name: Trap name.
- * @group: Immutable packet trap group attributes.
+ * @init_group_id: Initial group identifier.
  * @metadata_cap: Metadata types that can be provided by the trap.
  *
  * Describes immutable attributes of packet traps that drivers register with
@@ -563,7 +563,7 @@ struct devlink_trap {
 	bool generic;
 	u16 id;
 	const char *name;
-	struct devlink_trap_group group;
+	u16 init_group_id;
 	u32 metadata_cap;
 };
 
@@ -692,18 +692,19 @@ enum devlink_trap_group_generic_id {
 #define DEVLINK_TRAP_GROUP_GENERIC_NAME_ACL_DROPS \
 	"acl_drops"
 
-#define DEVLINK_TRAP_GENERIC(_type, _init_action, _id, _group, _metadata_cap) \
+#define DEVLINK_TRAP_GENERIC(_type, _init_action, _id, _group_id,	      \
+			     _metadata_cap)				      \
 	{								      \
 		.type = DEVLINK_TRAP_TYPE_##_type,			      \
 		.init_action = DEVLINK_TRAP_ACTION_##_init_action,	      \
 		.generic = true,					      \
 		.id = DEVLINK_TRAP_GENERIC_ID_##_id,			      \
 		.name = DEVLINK_TRAP_GENERIC_NAME_##_id,		      \
-		.group = _group,					      \
+		.init_group_id = _group_id,				      \
 		.metadata_cap = _metadata_cap,				      \
 	}
 
-#define DEVLINK_TRAP_DRIVER(_type, _init_action, _id, _name, _group,	      \
+#define DEVLINK_TRAP_DRIVER(_type, _init_action, _id, _name, _group_id,	      \
 			    _metadata_cap)				      \
 	{								      \
 		.type = DEVLINK_TRAP_TYPE_##_type,			      \
@@ -711,7 +712,7 @@ enum devlink_trap_group_generic_id {
 		.generic = false,					      \
 		.id = _id,						      \
 		.name = _name,						      \
-		.group = _group,					      \
+		.init_group_id = _group_id,				      \
 		.metadata_cap = _metadata_cap,				      \
 	}
 
diff --git a/net/core/devlink.c b/net/core/devlink.c
index a35285a48b02..73bb8fbe3393 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -5815,6 +5815,19 @@ devlink_trap_group_item_lookup(struct devlink *devlink, const char *name)
 	return NULL;
 }
 
+static struct devlink_trap_group_item *
+devlink_trap_group_item_lookup_by_id(struct devlink *devlink, u16 id)
+{
+	struct devlink_trap_group_item *group_item;
+
+	list_for_each_entry(group_item, &devlink->trap_group_list, list) {
+		if (group_item->group->id == id)
+			return group_item;
+	}
+
+	return NULL;
+}
+
 static struct devlink_trap_group_item *
 devlink_trap_group_item_get_from_info(struct devlink *devlink,
 				      struct genl_info *info)
@@ -5953,7 +5966,7 @@ __devlink_trap_group_action_set(struct devlink *devlink,
 	int err;
 
 	list_for_each_entry(trap_item, &devlink->trap_list, list) {
-		if (strcmp(trap_item->trap->group.name, group_name))
+		if (strcmp(trap_item->group_item->group->name, group_name))
 			continue;
 		err = __devlink_trap_action_set(devlink, trap_item,
 						trap_action, extack);
@@ -7864,7 +7877,7 @@ static int devlink_trap_driver_verify(const struct devlink_trap *trap)
 
 static int devlink_trap_verify(const struct devlink_trap *trap)
 {
-	if (!trap || !trap->name || !trap->group.name)
+	if (!trap || !trap->name)
 		return -EINVAL;
 
 	if (trap->generic)
@@ -7939,10 +7952,10 @@ static int
 devlink_trap_item_group_link(struct devlink *devlink,
 			     struct devlink_trap_item *trap_item)
 {
-	const struct devlink_trap *trap = trap_item->trap;
+	u16 group_id = trap_item->trap->init_group_id;
 	struct devlink_trap_group_item *group_item;
 
-	group_item = devlink_trap_group_item_lookup(devlink, trap->group.name);
+	group_item = devlink_trap_group_item_lookup_by_id(devlink, group_id);
 	if (WARN_ON_ONCE(!group_item))
 		return -EINVAL;
 
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 0/5] devlink: Preparations for trap policers support
  2020-03-22 18:48 [PATCH net-next 0/5] devlink: Preparations for trap policers support Ido Schimmel
                   ` (4 preceding siblings ...)
  2020-03-22 18:48 ` [PATCH net-next 5/5] devlink: Only pass packet trap group identifier in trap structure Ido Schimmel
@ 2020-03-23 18:46 ` Jakub Kicinski
  2020-03-24  4:41 ` David Miller
  6 siblings, 0 replies; 8+ messages in thread
From: Jakub Kicinski @ 2020-03-23 18:46 UTC (permalink / raw)
  To: Ido Schimmel; +Cc: netdev, davem, jiri, mlxsw, Ido Schimmel

On Sun, 22 Mar 2020 20:48:25 +0200 Ido Schimmel wrote:
> From: Ido Schimmel <idosch@mellanox.com>
> 
> This patch set prepares the code for devlink-trap policer support in a
> follow-up patch set [1][2]. No functional changes intended.
> 
> Policers are going to be added as attributes of packet trap groups,
> which are entities used to aggregate logically related packet traps.
> This will allow users, for example, to limit all the packets that
> encountered an exception during routing to 10Kpps.
> 
> However, currently, device drivers register their packet trap groups
> implicitly when they register their packet traps via
> devlink_traps_register(). This makes it difficult to pass additional
> attributes for the groups. For example, the policer bound to the group.
> 
> Therefore, this patch set converts device drivers to explicitly register
> their packet trap groups. This will later allow these drivers to
> register the group with additional attributes, if any.

Acked-by: Jakub Kicinski <kuba@kernel.org>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 0/5] devlink: Preparations for trap policers support
  2020-03-22 18:48 [PATCH net-next 0/5] devlink: Preparations for trap policers support Ido Schimmel
                   ` (5 preceding siblings ...)
  2020-03-23 18:46 ` [PATCH net-next 0/5] devlink: Preparations for trap policers support Jakub Kicinski
@ 2020-03-24  4:41 ` David Miller
  6 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2020-03-24  4:41 UTC (permalink / raw)
  To: idosch; +Cc: netdev, jiri, kuba, mlxsw, idosch

From: Ido Schimmel <idosch@idosch.org>
Date: Sun, 22 Mar 2020 20:48:25 +0200

> This patch set prepares the code for devlink-trap policer support in a
> follow-up patch set [1][2]. No functional changes intended.
 ...

Series applied, thanks.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-03-24  4:41 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-22 18:48 [PATCH net-next 0/5] devlink: Preparations for trap policers support Ido Schimmel
2020-03-22 18:48 ` [PATCH net-next 1/5] devlink: Add API to register packet trap groups Ido Schimmel
2020-03-22 18:48 ` [PATCH net-next 2/5] mlxsw: spectrum_trap: Explicitly " Ido Schimmel
2020-03-22 18:48 ` [PATCH net-next 3/5] netdevsim: " Ido Schimmel
2020-03-22 18:48 ` [PATCH net-next 4/5] devlink: Stop reference counting " Ido Schimmel
2020-03-22 18:48 ` [PATCH net-next 5/5] devlink: Only pass packet trap group identifier in trap structure Ido Schimmel
2020-03-23 18:46 ` [PATCH net-next 0/5] devlink: Preparations for trap policers support Jakub Kicinski
2020-03-24  4:41 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).