linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next RFC v2 0/3] Add devlink traps in devlink port context
@ 2020-09-17 10:36 Aya Levin
  2020-09-17 10:36 ` [PATCH net-next RFC v2 1/3] devlink: Wrap trap related lists a trap_lists struct Aya Levin
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Aya Levin @ 2020-09-17 10:36 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, Jiri Pirko, Ido Schimmel, netdev
  Cc: Moshe Shemesh, Eran Ben Elisha, linux-kernel, Aya Levin

Implement support for devlink traps on per-port basis. Dropped
packets in the RX flow are related to the Ethernet port and thus
should be in port context. Traps per device should trap global
configuration which may cause drops. Devlink traps is regard as a
debug mode. Using traps per port enable debug which doesn't effect
other ports on a device.

Patchset:
Patch 1: Refactors devlink trap for easier code re-use in the coming
patches
Patch 2: Adds devlink traps under devlink port context
ports context. In a nutshell it allows enable/disable of a trap on
all related ports which registered this trap.
Patch 3: Display a use in devlink traps in port context in mlx5
ethernet driver.

Changelog:
Minor changes in cover letter
v1->v2:
Patch 1: 
-Gather only the traps lists for future code reuse. Don't
 try to reuse the traps ops.
Ptach 2: 
-Add traps lock in devlink_port
-Add devlink_port ops and in it, add the trap ops
-Add support onlty for traps and exclude groups and policy
-Add separate netlink commands for port trap get and set 
-Allow trap registration without a corresponding group
Patch 3: removed
Ptach 4: 
-Is now patch 3
-Minor changes in trap's definition
-Adjustments to trap API and ops

Aya Levin (3):
  devlink: Wrap trap related lists a trap_lists struct
  devlink: Add devlink traps under devlink_ports context
  net/mlx5e: Add devlink trap to catch oversize packets

 drivers/net/ethernet/mellanox/mlx5/core/Makefile   |   2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en.h       |   2 +
 drivers/net/ethernet/mellanox/mlx5/core/en/traps.c |  38 ++
 drivers/net/ethernet/mellanox/mlx5/core/en/traps.h |  14 +
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c  |  48 +++
 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c    |  11 +-
 include/net/devlink.h                              |  54 ++-
 include/uapi/linux/devlink.h                       |   5 +
 net/core/devlink.c                                 | 453 ++++++++++++++++++---
 9 files changed, 556 insertions(+), 71 deletions(-)
 create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en/traps.c
 create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en/traps.h

-- 
2.14.1


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

* [PATCH net-next RFC v2 1/3] devlink: Wrap trap related lists a trap_lists struct
  2020-09-17 10:36 [PATCH net-next RFC v2 0/3] Add devlink traps in devlink port context Aya Levin
@ 2020-09-17 10:36 ` Aya Levin
  2020-09-17 10:36 ` [PATCH net-next RFC v2 2/3] devlink: Add devlink traps under devlink_ports context Aya Levin
  2020-09-17 10:36 ` [PATCH net-next RFC v2 3/3] net/mlx5e: Add devlink trap to catch oversize packets Aya Levin
  2 siblings, 0 replies; 4+ messages in thread
From: Aya Levin @ 2020-09-17 10:36 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, Jiri Pirko, Ido Schimmel, netdev
  Cc: Moshe Shemesh, Eran Ben Elisha, linux-kernel, Aya Levin

Bundle the trap related lists: trap_list, trap_group_list and
trap_policer_list in a dedicated struct. This will be handy in the
coming patches in the set introducing traps in devlink port context.
With trap_lists, code reuse is much simpler.

Signed-off-by: Aya Levin <ayal@nvidia.com>
---
Changelog:
v1->v2:
Patch 1: Encapsulate only the traps lists for future code reuse. Don't
try to reuse the traps ops.

 include/net/devlink.h |  10 +++--
 net/core/devlink.c    | 109 ++++++++++++++++++++++++++------------------------
 2 files changed, 63 insertions(+), 56 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index eaec0a8cc5ef..f11e09097e44 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -22,6 +22,12 @@
 
 struct devlink_ops;
 
+struct devlink_trap_lists {
+	struct list_head trap_list;
+	struct list_head trap_group_list;
+	struct list_head trap_policer_list;
+};
+
 struct devlink {
 	struct list_head list;
 	struct list_head port_list;
@@ -33,9 +39,7 @@ struct devlink {
 	struct list_head reporter_list;
 	struct mutex reporters_lock; /* protects reporter_list */
 	struct devlink_dpipe_headers *dpipe_headers;
-	struct list_head trap_list;
-	struct list_head trap_group_list;
-	struct list_head trap_policer_list;
+	struct devlink_trap_lists trap_lists;
 	const struct devlink_ops *ops;
 	struct xarray snapshot_ids;
 	struct device *dev;
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 19037f114307..fde6f2c5c409 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6158,11 +6158,11 @@ struct devlink_trap_item {
 };
 
 static struct devlink_trap_policer_item *
-devlink_trap_policer_item_lookup(struct devlink *devlink, u32 id)
+devlink_trap_policer_item_lookup(struct devlink_trap_lists *trap_lists, u32 id)
 {
 	struct devlink_trap_policer_item *policer_item;
 
-	list_for_each_entry(policer_item, &devlink->trap_policer_list, list) {
+	list_for_each_entry(policer_item, &trap_lists->trap_policer_list, list) {
 		if (policer_item->policer->id == id)
 			return policer_item;
 	}
@@ -6171,11 +6171,11 @@ devlink_trap_policer_item_lookup(struct devlink *devlink, u32 id)
 }
 
 static struct devlink_trap_item *
-devlink_trap_item_lookup(struct devlink *devlink, const char *name)
+devlink_trap_item_lookup(struct devlink_trap_lists *trap_lists, const char *name)
 {
 	struct devlink_trap_item *trap_item;
 
-	list_for_each_entry(trap_item, &devlink->trap_list, list) {
+	list_for_each_entry(trap_item, &trap_lists->trap_list, list) {
 		if (!strcmp(trap_item->trap->name, name))
 			return trap_item;
 	}
@@ -6184,7 +6184,7 @@ devlink_trap_item_lookup(struct devlink *devlink, const char *name)
 }
 
 static struct devlink_trap_item *
-devlink_trap_item_get_from_info(struct devlink *devlink,
+devlink_trap_item_get_from_info(struct devlink_trap_lists *trap_lists,
 				struct genl_info *info)
 {
 	struct nlattr *attr;
@@ -6193,7 +6193,7 @@ devlink_trap_item_get_from_info(struct devlink *devlink,
 		return NULL;
 	attr = info->attrs[DEVLINK_ATTR_TRAP_NAME];
 
-	return devlink_trap_item_lookup(devlink, nla_data(attr));
+	return devlink_trap_item_lookup(trap_lists, nla_data(attr));
 }
 
 static int
@@ -6352,10 +6352,10 @@ static int devlink_nl_cmd_trap_get_doit(struct sk_buff *skb,
 	struct sk_buff *msg;
 	int err;
 
-	if (list_empty(&devlink->trap_list))
+	if (list_empty(&devlink->trap_lists.trap_list))
 		return -EOPNOTSUPP;
 
-	trap_item = devlink_trap_item_get_from_info(devlink, info);
+	trap_item = devlink_trap_item_get_from_info(&devlink->trap_lists, info);
 	if (!trap_item) {
 		NL_SET_ERR_MSG_MOD(extack, "Device did not register this trap");
 		return -ENOENT;
@@ -6392,7 +6392,7 @@ static int devlink_nl_cmd_trap_get_dumpit(struct sk_buff *msg,
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
 			continue;
 		mutex_lock(&devlink->lock);
-		list_for_each_entry(trap_item, &devlink->trap_list, list) {
+		list_for_each_entry(trap_item, &devlink->trap_lists.trap_list, list) {
 			if (idx < start) {
 				idx++;
 				continue;
@@ -6468,10 +6468,10 @@ static int devlink_nl_cmd_trap_set_doit(struct sk_buff *skb,
 	struct devlink_trap_item *trap_item;
 	int err;
 
-	if (list_empty(&devlink->trap_list))
+	if (list_empty(&devlink->trap_lists.trap_list))
 		return -EOPNOTSUPP;
 
-	trap_item = devlink_trap_item_get_from_info(devlink, info);
+	trap_item = devlink_trap_item_get_from_info(&devlink->trap_lists, info);
 	if (!trap_item) {
 		NL_SET_ERR_MSG_MOD(extack, "Device did not register this trap");
 		return -ENOENT;
@@ -6485,11 +6485,11 @@ static int devlink_nl_cmd_trap_set_doit(struct sk_buff *skb,
 }
 
 static struct devlink_trap_group_item *
-devlink_trap_group_item_lookup(struct devlink *devlink, const char *name)
+devlink_trap_group_item_lookup(struct devlink_trap_lists *trap_lists, const char *name)
 {
 	struct devlink_trap_group_item *group_item;
 
-	list_for_each_entry(group_item, &devlink->trap_group_list, list) {
+	list_for_each_entry(group_item, &trap_lists->trap_group_list, list) {
 		if (!strcmp(group_item->group->name, name))
 			return group_item;
 	}
@@ -6498,11 +6498,11 @@ devlink_trap_group_item_lookup(struct devlink *devlink, const char *name)
 }
 
 static struct devlink_trap_group_item *
-devlink_trap_group_item_lookup_by_id(struct devlink *devlink, u16 id)
+devlink_trap_group_item_lookup_by_id(struct devlink_trap_lists *trap_lists, u16 id)
 {
 	struct devlink_trap_group_item *group_item;
 
-	list_for_each_entry(group_item, &devlink->trap_group_list, list) {
+	list_for_each_entry(group_item, &trap_lists->trap_group_list, list) {
 		if (group_item->group->id == id)
 			return group_item;
 	}
@@ -6511,7 +6511,7 @@ devlink_trap_group_item_lookup_by_id(struct devlink *devlink, u16 id)
 }
 
 static struct devlink_trap_group_item *
-devlink_trap_group_item_get_from_info(struct devlink *devlink,
+devlink_trap_group_item_get_from_info(struct devlink_trap_lists *trap_lists,
 				      struct genl_info *info)
 {
 	char *name;
@@ -6520,7 +6520,7 @@ devlink_trap_group_item_get_from_info(struct devlink *devlink,
 		return NULL;
 	name = nla_data(info->attrs[DEVLINK_ATTR_TRAP_GROUP_NAME]);
 
-	return devlink_trap_group_item_lookup(devlink, name);
+	return devlink_trap_group_item_lookup(trap_lists, name);
 }
 
 static int
@@ -6574,10 +6574,10 @@ static int devlink_nl_cmd_trap_group_get_doit(struct sk_buff *skb,
 	struct sk_buff *msg;
 	int err;
 
-	if (list_empty(&devlink->trap_group_list))
+	if (list_empty(&devlink->trap_lists.trap_group_list))
 		return -EOPNOTSUPP;
 
-	group_item = devlink_trap_group_item_get_from_info(devlink, info);
+	group_item = devlink_trap_group_item_get_from_info(&devlink->trap_lists, info);
 	if (!group_item) {
 		NL_SET_ERR_MSG_MOD(extack, "Device did not register this trap group");
 		return -ENOENT;
@@ -6616,7 +6616,7 @@ static int devlink_nl_cmd_trap_group_get_dumpit(struct sk_buff *msg,
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
 			continue;
 		mutex_lock(&devlink->lock);
-		list_for_each_entry(group_item, &devlink->trap_group_list,
+		list_for_each_entry(group_item, &devlink->trap_lists.trap_group_list,
 				    list) {
 			if (idx < start) {
 				idx++;
@@ -6652,7 +6652,7 @@ __devlink_trap_group_action_set(struct devlink *devlink,
 	struct devlink_trap_item *trap_item;
 	int err;
 
-	list_for_each_entry(trap_item, &devlink->trap_list, list) {
+	list_for_each_entry(trap_item, &devlink->trap_lists.trap_list, list) {
 		if (strcmp(trap_item->group_item->group->name, group_name))
 			continue;
 		err = __devlink_trap_action_set(devlink, trap_item,
@@ -6712,7 +6712,7 @@ static int devlink_trap_group_set(struct devlink *devlink,
 		u32 policer_id;
 
 		policer_id = nla_get_u32(attrs[DEVLINK_ATTR_TRAP_POLICER_ID]);
-		policer_item = devlink_trap_policer_item_lookup(devlink,
+		policer_item = devlink_trap_policer_item_lookup(&devlink->trap_lists,
 								policer_id);
 		if (policer_id && !policer_item) {
 			NL_SET_ERR_MSG_MOD(extack, "Device did not register this trap policer");
@@ -6740,10 +6740,10 @@ static int devlink_nl_cmd_trap_group_set_doit(struct sk_buff *skb,
 	bool modified = false;
 	int err;
 
-	if (list_empty(&devlink->trap_group_list))
+	if (list_empty(&devlink->trap_lists.trap_group_list))
 		return -EOPNOTSUPP;
 
-	group_item = devlink_trap_group_item_get_from_info(devlink, info);
+	group_item = devlink_trap_group_item_get_from_info(&devlink->trap_lists, info);
 	if (!group_item) {
 		NL_SET_ERR_MSG_MOD(extack, "Device did not register this trap group");
 		return -ENOENT;
@@ -6767,7 +6767,7 @@ static int devlink_nl_cmd_trap_group_set_doit(struct sk_buff *skb,
 }
 
 static struct devlink_trap_policer_item *
-devlink_trap_policer_item_get_from_info(struct devlink *devlink,
+devlink_trap_policer_item_get_from_info(struct devlink_trap_lists *trap_lists,
 					struct genl_info *info)
 {
 	u32 id;
@@ -6776,7 +6776,7 @@ devlink_trap_policer_item_get_from_info(struct devlink *devlink,
 		return NULL;
 	id = nla_get_u32(info->attrs[DEVLINK_ATTR_TRAP_POLICER_ID]);
 
-	return devlink_trap_policer_item_lookup(devlink, id);
+	return devlink_trap_policer_item_lookup(trap_lists, id);
 }
 
 static int
@@ -6862,10 +6862,10 @@ static int devlink_nl_cmd_trap_policer_get_doit(struct sk_buff *skb,
 	struct sk_buff *msg;
 	int err;
 
-	if (list_empty(&devlink->trap_policer_list))
+	if (list_empty(&devlink->trap_lists.trap_policer_list))
 		return -EOPNOTSUPP;
 
-	policer_item = devlink_trap_policer_item_get_from_info(devlink, info);
+	policer_item = devlink_trap_policer_item_get_from_info(&devlink->trap_lists, info);
 	if (!policer_item) {
 		NL_SET_ERR_MSG_MOD(extack, "Device did not register this trap policer");
 		return -ENOENT;
@@ -6904,7 +6904,7 @@ static int devlink_nl_cmd_trap_policer_get_dumpit(struct sk_buff *msg,
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
 			continue;
 		mutex_lock(&devlink->lock);
-		list_for_each_entry(policer_item, &devlink->trap_policer_list,
+		list_for_each_entry(policer_item, &devlink->trap_lists.trap_policer_list,
 				    list) {
 			if (idx < start) {
 				idx++;
@@ -6987,13 +6987,13 @@ static int devlink_nl_cmd_trap_policer_set_doit(struct sk_buff *skb,
 	struct netlink_ext_ack *extack = info->extack;
 	struct devlink *devlink = info->user_ptr[0];
 
-	if (list_empty(&devlink->trap_policer_list))
+	if (list_empty(&devlink->trap_lists.trap_policer_list))
 		return -EOPNOTSUPP;
 
 	if (!devlink->ops->trap_policer_set)
 		return -EOPNOTSUPP;
 
-	policer_item = devlink_trap_policer_item_get_from_info(devlink, info);
+	policer_item = devlink_trap_policer_item_get_from_info(&devlink->trap_lists, info);
 	if (!policer_item) {
 		NL_SET_ERR_MSG_MOD(extack, "Device did not register this trap policer");
 		return -ENOENT;
@@ -7401,9 +7401,9 @@ struct devlink *devlink_alloc(const struct devlink_ops *ops, size_t priv_size)
 	INIT_LIST_HEAD(&devlink->param_list);
 	INIT_LIST_HEAD(&devlink->region_list);
 	INIT_LIST_HEAD(&devlink->reporter_list);
-	INIT_LIST_HEAD(&devlink->trap_list);
-	INIT_LIST_HEAD(&devlink->trap_group_list);
-	INIT_LIST_HEAD(&devlink->trap_policer_list);
+	INIT_LIST_HEAD(&devlink->trap_lists.trap_list);
+	INIT_LIST_HEAD(&devlink->trap_lists.trap_group_list);
+	INIT_LIST_HEAD(&devlink->trap_lists.trap_policer_list);
 	mutex_init(&devlink->lock);
 	mutex_init(&devlink->reporters_lock);
 	return devlink;
@@ -7488,9 +7488,9 @@ void devlink_free(struct devlink *devlink)
 {
 	mutex_destroy(&devlink->reporters_lock);
 	mutex_destroy(&devlink->lock);
-	WARN_ON(!list_empty(&devlink->trap_policer_list));
-	WARN_ON(!list_empty(&devlink->trap_group_list));
-	WARN_ON(!list_empty(&devlink->trap_list));
+	WARN_ON(!list_empty(&devlink->trap_lists.trap_policer_list));
+	WARN_ON(!list_empty(&devlink->trap_lists.trap_group_list));
+	WARN_ON(!list_empty(&devlink->trap_lists.trap_list));
 	WARN_ON(!list_empty(&devlink->reporter_list));
 	WARN_ON(!list_empty(&devlink->region_list));
 	WARN_ON(!list_empty(&devlink->param_list));
@@ -8984,13 +8984,13 @@ devlink_trap_group_notify(struct devlink *devlink,
 }
 
 static int
-devlink_trap_item_group_link(struct devlink *devlink,
+devlink_trap_item_group_link(struct devlink_trap_lists *trap_lists,
 			     struct devlink_trap_item *trap_item)
 {
 	u16 group_id = trap_item->trap->init_group_id;
 	struct devlink_trap_group_item *group_item;
 
-	group_item = devlink_trap_group_item_lookup_by_id(devlink, group_id);
+	group_item = devlink_trap_group_item_lookup_by_id(trap_lists, group_id);
 	if (WARN_ON_ONCE(!group_item))
 		return -EINVAL;
 
@@ -9027,10 +9027,11 @@ static int
 devlink_trap_register(struct devlink *devlink,
 		      const struct devlink_trap *trap, void *priv)
 {
+	struct devlink_trap_lists *trap_lists = &devlink->trap_lists;
 	struct devlink_trap_item *trap_item;
 	int err;
 
-	if (devlink_trap_item_lookup(devlink, trap->name))
+	if (devlink_trap_item_lookup(trap_lists, trap->name))
 		return -EEXIST;
 
 	trap_item = kzalloc(sizeof(*trap_item), GFP_KERNEL);
@@ -9047,7 +9048,7 @@ devlink_trap_register(struct devlink *devlink,
 	trap_item->action = trap->init_action;
 	trap_item->priv = priv;
 
-	err = devlink_trap_item_group_link(devlink, trap_item);
+	err = devlink_trap_item_group_link(trap_lists, trap_item);
 	if (err)
 		goto err_group_link;
 
@@ -9055,7 +9056,7 @@ devlink_trap_register(struct devlink *devlink,
 	if (err)
 		goto err_trap_init;
 
-	list_add_tail(&trap_item->list, &devlink->trap_list);
+	list_add_tail(&trap_item->list, &trap_lists->trap_list);
 	devlink_trap_notify(devlink, trap_item, DEVLINK_CMD_TRAP_NEW);
 
 	return 0;
@@ -9073,7 +9074,7 @@ static void devlink_trap_unregister(struct devlink *devlink,
 {
 	struct devlink_trap_item *trap_item;
 
-	trap_item = devlink_trap_item_lookup(devlink, trap->name);
+	trap_item = devlink_trap_item_lookup(&devlink->trap_lists, trap->name);
 	if (WARN_ON_ONCE(!trap_item))
 		return;
 
@@ -9090,7 +9091,7 @@ static void devlink_trap_disable(struct devlink *devlink,
 {
 	struct devlink_trap_item *trap_item;
 
-	trap_item = devlink_trap_item_lookup(devlink, trap->name);
+	trap_item = devlink_trap_item_lookup(&devlink->trap_lists, trap->name);
 	if (WARN_ON_ONCE(!trap_item))
 		return;
 
@@ -9245,7 +9246,7 @@ void *devlink_trap_ctx_priv(void *trap_ctx)
 EXPORT_SYMBOL_GPL(devlink_trap_ctx_priv);
 
 static int
-devlink_trap_group_item_policer_link(struct devlink *devlink,
+devlink_trap_group_item_policer_link(struct devlink_trap_lists *trap_lists,
 				     struct devlink_trap_group_item *group_item)
 {
 	u32 policer_id = group_item->group->init_policer_id;
@@ -9254,7 +9255,7 @@ devlink_trap_group_item_policer_link(struct devlink *devlink,
 	if (policer_id == 0)
 		return 0;
 
-	policer_item = devlink_trap_policer_item_lookup(devlink, policer_id);
+	policer_item = devlink_trap_policer_item_lookup(trap_lists, policer_id);
 	if (WARN_ON_ONCE(!policer_item))
 		return -EINVAL;
 
@@ -9267,10 +9268,11 @@ static int
 devlink_trap_group_register(struct devlink *devlink,
 			    const struct devlink_trap_group *group)
 {
+	struct devlink_trap_lists *trap_lists = &devlink->trap_lists;
 	struct devlink_trap_group_item *group_item;
 	int err;
 
-	if (devlink_trap_group_item_lookup(devlink, group->name))
+	if (devlink_trap_group_item_lookup(trap_lists, group->name))
 		return -EEXIST;
 
 	group_item = kzalloc(sizeof(*group_item), GFP_KERNEL);
@@ -9285,7 +9287,7 @@ devlink_trap_group_register(struct devlink *devlink,
 
 	group_item->group = group;
 
-	err = devlink_trap_group_item_policer_link(devlink, group_item);
+	err = devlink_trap_group_item_policer_link(trap_lists, group_item);
 	if (err)
 		goto err_policer_link;
 
@@ -9295,7 +9297,7 @@ devlink_trap_group_register(struct devlink *devlink,
 			goto err_group_init;
 	}
 
-	list_add_tail(&group_item->list, &devlink->trap_group_list);
+	list_add_tail(&group_item->list, &trap_lists->trap_group_list);
 	devlink_trap_group_notify(devlink, group_item,
 				  DEVLINK_CMD_TRAP_GROUP_NEW);
 
@@ -9315,7 +9317,7 @@ devlink_trap_group_unregister(struct devlink *devlink,
 {
 	struct devlink_trap_group_item *group_item;
 
-	group_item = devlink_trap_group_item_lookup(devlink, group->name);
+	group_item = devlink_trap_group_item_lookup(&devlink->trap_lists, group->name);
 	if (WARN_ON_ONCE(!group_item))
 		return;
 
@@ -9414,10 +9416,11 @@ static int
 devlink_trap_policer_register(struct devlink *devlink,
 			      const struct devlink_trap_policer *policer)
 {
+	struct devlink_trap_lists *trap_lists = &devlink->trap_lists;
 	struct devlink_trap_policer_item *policer_item;
 	int err;
 
-	if (devlink_trap_policer_item_lookup(devlink, policer->id))
+	if (devlink_trap_policer_item_lookup(trap_lists, policer->id))
 		return -EEXIST;
 
 	policer_item = kzalloc(sizeof(*policer_item), GFP_KERNEL);
@@ -9434,7 +9437,7 @@ devlink_trap_policer_register(struct devlink *devlink,
 			goto err_policer_init;
 	}
 
-	list_add_tail(&policer_item->list, &devlink->trap_policer_list);
+	list_add_tail(&policer_item->list, &trap_lists->trap_policer_list);
 	devlink_trap_policer_notify(devlink, policer_item,
 				    DEVLINK_CMD_TRAP_POLICER_NEW);
 
@@ -9451,7 +9454,7 @@ devlink_trap_policer_unregister(struct devlink *devlink,
 {
 	struct devlink_trap_policer_item *policer_item;
 
-	policer_item = devlink_trap_policer_item_lookup(devlink, policer->id);
+	policer_item = devlink_trap_policer_item_lookup(&devlink->trap_lists, policer->id);
 	if (WARN_ON_ONCE(!policer_item))
 		return;
 
-- 
2.14.1


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

* [PATCH net-next RFC v2 2/3] devlink: Add devlink traps under devlink_ports context
  2020-09-17 10:36 [PATCH net-next RFC v2 0/3] Add devlink traps in devlink port context Aya Levin
  2020-09-17 10:36 ` [PATCH net-next RFC v2 1/3] devlink: Wrap trap related lists a trap_lists struct Aya Levin
@ 2020-09-17 10:36 ` Aya Levin
  2020-09-17 10:36 ` [PATCH net-next RFC v2 3/3] net/mlx5e: Add devlink trap to catch oversize packets Aya Levin
  2 siblings, 0 replies; 4+ messages in thread
From: Aya Levin @ 2020-09-17 10:36 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, Jiri Pirko, Ido Schimmel, netdev
  Cc: Moshe Shemesh, Eran Ben Elisha, linux-kernel, Aya Levin

There are some cases where we would like to trap dropped packets only
for a single port on a device without affecting the others. For that
purpose:
- Add trap lists and trap ops to devlink_port
- Add corresponding trap API to manage traps
- Add matching netlink commands

Signed-off-by: Aya Levin <ayal@nvidia.com>
---
Changelog:
v1->v2:
Add traps lock in devlink_port
Add devlink_port ops and in it, add the trap ops
Add support onlty for traps and exclude groups and policer
Add seperate netlink commands foor port trap get and set 
Allow trap registration without a corresponding group

 include/net/devlink.h        |  44 ++++++
 include/uapi/linux/devlink.h |   5 +
 net/core/devlink.c           | 346 +++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 382 insertions(+), 13 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index f11e09097e44..93eb7033ce00 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -21,6 +21,8 @@
 #include <linux/xarray.h>
 
 struct devlink_ops;
+struct devlink_port_ops;
+
 
 struct devlink_trap_lists {
 	struct list_head trap_list;
@@ -129,6 +131,9 @@ struct devlink_port {
 	struct delayed_work type_warn_dw;
 	struct list_head reporter_list;
 	struct mutex reporters_lock; /* Protects reporter_list */
+	struct mutex trap_lists_lock;
+	struct devlink_trap_lists trap_lists;
+	const struct devlink_port_ops *ops;
 };
 
 struct devlink_sb_pool_info {
@@ -1177,6 +1182,35 @@ struct devlink_ops {
 					 struct netlink_ext_ack *extack);
 };
 
+struct devlink_port_ops {
+	/**
+	 * @trap_init: Trap initialization function.
+	 *
+	 * Should be used by device drivers to initialize the trap in the
+	 * underlying device. Drivers should also store the provided trap
+	 * context, so that they could efficiently pass it to
+	 * devlink_trap_report() when the trap is triggered.
+	 */
+	int (*trap_init)(struct devlink_port *devlink,
+			 const struct devlink_trap *trap, void *trap_ctx);
+	/**
+	 * @trap_fini: Trap de-initialization function.
+	 *
+	 * Should be used by device drivers to de-initialize the trap in the
+	 * underlying device.
+	 */
+
+	void (*trap_fini)(struct devlink_port *devlink_port,
+			  const struct devlink_trap *trap, void *trap_ctx);
+	/**
+	 * @trap_action_set: Trap action set function.
+	 */
+	int (*trap_action_set)(struct devlink_port *devlink_port,
+			       const struct devlink_trap *trap,
+			       enum devlink_trap_action action,
+			       struct netlink_ext_ack *extack);
+};
+
 static inline void *devlink_priv(struct devlink *devlink)
 {
 	BUG_ON(!devlink);
@@ -1220,6 +1254,8 @@ int devlink_port_register(struct devlink *devlink,
 			  struct devlink_port *devlink_port,
 			  unsigned int port_index);
 void devlink_port_unregister(struct devlink_port *devlink_port);
+void devlink_port_set_ops(struct devlink_port *devlink_port,
+			  const struct devlink_port_ops *ops);
 void devlink_port_type_eth_set(struct devlink_port *devlink_port,
 			       struct net_device *netdev);
 void devlink_port_type_ib_set(struct devlink_port *devlink_port,
@@ -1429,6 +1465,14 @@ void
 devlink_trap_policers_unregister(struct devlink *devlink,
 				 const struct devlink_trap_policer *policers,
 				 size_t policers_count);
+int devlink_port_traps_register(struct devlink_port *devlink_port,
+				const struct devlink_trap *traps,
+				size_t traps_count, void *priv);
+void devlink_port_traps_unregister(struct devlink_port *devlink_port,
+				   const struct devlink_trap *traps,
+				   size_t traps_count);
+void devlink_port_trap_report(struct devlink_port *devlink_port, struct sk_buff *skb,
+			      void *trap_ctx, const struct flow_action_cookie *fa_cookie);
 
 #if IS_ENABLED(CONFIG_NET_DEVLINK)
 
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 40d35145c879..401ad93dab67 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -122,6 +122,11 @@ enum devlink_command {
 	DEVLINK_CMD_TRAP_POLICER_NEW,
 	DEVLINK_CMD_TRAP_POLICER_DEL,
 
+	DEVLINK_CMD_PORT_TRAP_GET,		/* can dump */
+	DEVLINK_CMD_PORT_TRAP_SET,
+	DEVLINK_CMD_PORT_TRAP_NEW,
+	DEVLINK_CMD_PORT_TRAP_DEL,
+
 	/* add new commands above here */
 	__DEVLINK_CMD_MAX,
 	DEVLINK_CMD_MAX = __DEVLINK_CMD_MAX - 1
diff --git a/net/core/devlink.c b/net/core/devlink.c
index fde6f2c5c409..438bd88c2c1b 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6309,8 +6309,8 @@ static int devlink_nl_trap_fill(struct sk_buff *msg, struct devlink *devlink,
 	if (devlink_nl_put_handle(msg, devlink))
 		goto nla_put_failure;
 
-	if (nla_put_string(msg, DEVLINK_ATTR_TRAP_GROUP_NAME,
-			   group_item->group->name))
+	if (group_item &&
+	    nla_put_string(msg, DEVLINK_ATTR_TRAP_GROUP_NAME, group_item->group->name))
 		goto nla_put_failure;
 
 	if (nla_put_string(msg, DEVLINK_ATTR_TRAP_NAME, trap_item->trap->name))
@@ -7002,6 +7002,145 @@ static int devlink_nl_cmd_trap_policer_set_doit(struct sk_buff *skb,
 	return devlink_trap_policer_set(devlink, policer_item, info);
 }
 
+static int devlink_nl_cmd_port_trap_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	struct devlink_port *devlink_port = info->user_ptr[0];
+	struct netlink_ext_ack *extack = info->extack;
+	struct devlink_trap_item *trap_item;
+	struct sk_buff *msg;
+	int err;
+
+	if (list_empty(&devlink_port->trap_lists.trap_list))
+		return -EOPNOTSUPP;
+
+	trap_item = devlink_trap_item_get_from_info(&devlink_port->trap_lists, info);
+	if (!trap_item) {
+		NL_SET_ERR_MSG_MOD(extack, "Port did not register this trap");
+		return -ENOENT;
+	}
+
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	err = devlink_nl_trap_fill(msg, devlink_port->devlink, trap_item,
+				   DEVLINK_CMD_TRAP_NEW, devlink_port->index,
+				   info->snd_seq, 0);
+	if (err)
+		goto err_trap_fill;
+
+	return genlmsg_reply(msg, info);
+
+err_trap_fill:
+	nlmsg_free(msg);
+	return err;
+}
+
+static int devlink_nl_cmd_port_trap_get_dumpit(struct sk_buff *msg, struct netlink_callback *cb)
+{
+	struct devlink_trap_item *trap_item;
+	struct devlink_port *devlink_port;
+	struct devlink *devlink;
+	int start = cb->args[0];
+	int idx = 0;
+	int err;
+
+	mutex_lock(&devlink_mutex);
+	list_for_each_entry(devlink, &devlink_list, list) {
+		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
+			continue;
+		mutex_lock(&devlink->lock);
+		list_for_each_entry(devlink_port, &devlink->port_list, list) {
+			mutex_lock(&devlink_port->trap_lists_lock);
+			list_for_each_entry(trap_item, &devlink->trap_lists.trap_list, list) {
+				if (idx < start) {
+					idx++;
+					continue;
+				}
+				err = devlink_nl_trap_fill(msg, devlink, trap_item,
+							   DEVLINK_CMD_TRAP_NEW,
+							   devlink_port->index,
+							   cb->nlh->nlmsg_seq,
+							   NLM_F_MULTI);
+				if (err) {
+					mutex_unlock(&devlink_port->trap_lists_lock);
+					mutex_unlock(&devlink->lock);
+					goto out;
+				}
+				idx++;
+			}
+			mutex_unlock(&devlink_port->trap_lists_lock);
+		}
+		mutex_unlock(&devlink->lock);
+	}
+out:
+	mutex_unlock(&devlink_mutex);
+
+	cb->args[0] = idx;
+	return msg->len;
+}
+
+static int __devlink_port_trap_action_set(struct devlink_port *devlink_port,
+					  struct devlink_trap_item *trap_item,
+					  enum devlink_trap_action trap_action,
+					  struct netlink_ext_ack *extack)
+{
+	int err;
+
+	if (trap_item->action != trap_action &&
+	    trap_item->trap->type != DEVLINK_TRAP_TYPE_DROP) {
+		NL_SET_ERR_MSG_MOD(extack, "Cannot change action of non-drop traps. Skipping");
+		return 0;
+	}
+
+	err = devlink_port->ops->trap_action_set(devlink_port, trap_item->trap,
+						 trap_action, extack);
+	if (err)
+		return err;
+
+	trap_item->action = trap_action;
+
+	return 0;
+}
+
+static int devlink_port_trap_action_set(struct devlink_port *devlink_port,
+					struct devlink_trap_item *trap_item,
+					struct genl_info *info)
+{
+	enum devlink_trap_action trap_action;
+	int err;
+
+	if (!info->attrs[DEVLINK_ATTR_TRAP_ACTION])
+		return 0;
+
+	err = devlink_trap_action_get_from_info(info, &trap_action);
+	if (err) {
+		NL_SET_ERR_MSG_MOD(info->extack, "Invalid trap action");
+		return -EINVAL;
+	}
+
+	return __devlink_port_trap_action_set(devlink_port, trap_item, trap_action,
+					      info->extack);
+}
+
+static int devlink_nl_cmd_port_trap_set_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	struct devlink_port *devlink_port = info->user_ptr[0];
+	struct netlink_ext_ack *extack = info->extack;
+	struct devlink_trap_item *trap_item;
+
+	if (list_empty(&devlink_port->trap_lists.trap_list))
+		return -EOPNOTSUPP;
+
+	trap_item = devlink_trap_item_get_from_info(&devlink_port->trap_lists, info);
+	if (!trap_item) {
+		NL_SET_ERR_MSG_MOD(extack, "Port did not register this trap");
+		return -ENOENT;
+	}
+
+	return devlink_port_trap_action_set(devlink_port, trap_item, info);
+}
+
 static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
 	[DEVLINK_ATTR_UNSPEC] = { .strict_start_type =
 		DEVLINK_ATTR_TRAP_POLICER_ID },
@@ -7355,6 +7494,20 @@ static const struct genl_ops devlink_nl_ops[] = {
 		.doit = devlink_nl_cmd_trap_policer_set_doit,
 		.flags = GENL_ADMIN_PERM,
 	},
+	{
+		.cmd = DEVLINK_CMD_PORT_TRAP_GET,
+		.doit = devlink_nl_cmd_port_trap_get_doit,
+		.dumpit = devlink_nl_cmd_port_trap_get_dumpit,
+		.internal_flags = DEVLINK_NL_FLAG_NEED_PORT,
+		/* can be retrieved by unprivileged users */
+	},
+	{
+		.cmd = DEVLINK_CMD_PORT_TRAP_SET,
+		.doit = devlink_nl_cmd_port_trap_set_doit,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = DEVLINK_NL_FLAG_NEED_PORT,
+	},
+
 };
 
 static struct genl_family devlink_nl_family __ro_after_init = {
@@ -7567,10 +7720,15 @@ int devlink_port_register(struct devlink *devlink,
 	mutex_init(&devlink_port->reporters_lock);
 	list_add_tail(&devlink_port->list, &devlink->port_list);
 	INIT_LIST_HEAD(&devlink_port->param_list);
+	INIT_LIST_HEAD(&devlink_port->trap_lists.trap_list);
+	INIT_LIST_HEAD(&devlink_port->trap_lists.trap_group_list);
+	INIT_LIST_HEAD(&devlink_port->trap_lists.trap_policer_list);
+	mutex_init(&devlink_port->trap_lists_lock);
 	mutex_unlock(&devlink->lock);
 	INIT_DELAYED_WORK(&devlink_port->type_warn_dw, &devlink_port_type_warn);
 	devlink_port_type_warn_schedule(devlink_port);
 	devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(devlink_port_register);
@@ -7590,10 +7748,21 @@ void devlink_port_unregister(struct devlink_port *devlink_port)
 	list_del(&devlink_port->list);
 	mutex_unlock(&devlink->lock);
 	WARN_ON(!list_empty(&devlink_port->reporter_list));
+	WARN_ON(!list_empty(&devlink_port->trap_lists.trap_list));
+	WARN_ON(!list_empty(&devlink_port->trap_lists.trap_group_list));
+	WARN_ON(!list_empty(&devlink_port->trap_lists.trap_group_list));
 	mutex_destroy(&devlink_port->reporters_lock);
+	mutex_destroy(&devlink_port->trap_lists_lock);
 }
 EXPORT_SYMBOL_GPL(devlink_port_unregister);
 
+void devlink_port_set_ops(struct devlink_port *devlink_port,
+			  const struct devlink_port_ops *ops)
+{
+	devlink_port->ops = ops;
+}
+EXPORT_SYMBOL_GPL(devlink_port_set_ops);
+
 static void __devlink_port_type_set(struct devlink_port *devlink_port,
 				    enum devlink_port_type type,
 				    void *type_dev)
@@ -9023,30 +9192,42 @@ static void devlink_trap_notify(struct devlink *devlink,
 				msg, 0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
 }
 
-static int
-devlink_trap_register(struct devlink *devlink,
-		      const struct devlink_trap *trap, void *priv)
+static struct devlink_trap_item *devlink_create_trap_item(struct devlink_trap_lists *trap_lists,
+							  const struct devlink_trap *trap,
+							  void *priv)
 {
-	struct devlink_trap_lists *trap_lists = &devlink->trap_lists;
 	struct devlink_trap_item *trap_item;
-	int err;
 
 	if (devlink_trap_item_lookup(trap_lists, trap->name))
-		return -EEXIST;
+		return ERR_PTR(-EEXIST);
 
 	trap_item = kzalloc(sizeof(*trap_item), GFP_KERNEL);
 	if (!trap_item)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	trap_item->stats = netdev_alloc_pcpu_stats(struct devlink_stats);
 	if (!trap_item->stats) {
-		err = -ENOMEM;
-		goto err_stats_alloc;
+		kfree(trap_item);
+		return ERR_PTR(-ENOMEM);
 	}
 
 	trap_item->trap = trap;
 	trap_item->action = trap->init_action;
 	trap_item->priv = priv;
+	return trap_item;
+}
+
+static int
+devlink_trap_register(struct devlink *devlink,
+		      const struct devlink_trap *trap, void *priv)
+{
+	struct devlink_trap_lists *trap_lists = &devlink->trap_lists;
+	struct devlink_trap_item *trap_item;
+	int err;
+
+	trap_item = devlink_create_trap_item(trap_lists, trap, priv);
+	if (IS_ERR(trap_item))
+		return PTR_ERR(trap_item);
 
 	err = devlink_trap_item_group_link(trap_lists, trap_item);
 	if (err)
@@ -9064,7 +9245,6 @@ devlink_trap_register(struct devlink *devlink,
 err_trap_init:
 err_group_link:
 	free_percpu(trap_item->stats);
-err_stats_alloc:
 	kfree(trap_item);
 	return err;
 }
@@ -9086,6 +9266,23 @@ static void devlink_trap_unregister(struct devlink *devlink,
 	kfree(trap_item);
 }
 
+static void devlink_port_trap_unregister(struct devlink_port *devlink_port,
+					 const struct devlink_trap *trap)
+{
+	struct devlink_trap_item *trap_item;
+
+	trap_item = devlink_trap_item_lookup(&devlink_port->trap_lists, trap->name);
+	if (WARN_ON_ONCE(!trap_item))
+		return;
+
+	devlink_trap_notify(devlink_port->devlink, trap_item, DEVLINK_CMD_TRAP_DEL);
+	list_del(&trap_item->list);
+	if (devlink_port->ops->trap_fini)
+		devlink_port->ops->trap_fini(devlink_port, trap, trap_item);
+	free_percpu(trap_item->stats);
+	kfree(trap_item);
+}
+
 static void devlink_trap_disable(struct devlink *devlink,
 				 const struct devlink_trap *trap)
 {
@@ -9100,6 +9297,19 @@ static void devlink_trap_disable(struct devlink *devlink,
 	trap_item->action = DEVLINK_TRAP_ACTION_DROP;
 }
 
+static void devlink_port_trap_disable(struct devlink_port *devlink_port,
+				      const struct devlink_trap *trap)
+{
+	struct devlink_trap_item *trap_item;
+
+	trap_item = devlink_trap_item_lookup(&devlink_port->trap_lists, trap->name);
+	if (WARN_ON_ONCE(!trap_item))
+		return;
+
+	devlink_port->ops->trap_action_set(devlink_port, trap, DEVLINK_TRAP_ACTION_DROP, NULL);
+	trap_item->action = DEVLINK_TRAP_ACTION_DROP;
+}
+
 /**
  * devlink_traps_register - Register packet traps with devlink.
  * @devlink: devlink.
@@ -9189,7 +9399,8 @@ devlink_trap_report_metadata_fill(struct net_dm_hw_metadata *hw_metadata,
 {
 	struct devlink_trap_group_item *group_item = trap_item->group_item;
 
-	hw_metadata->trap_group_name = group_item->group->name;
+	if (group_item)
+		hw_metadata->trap_group_name = group_item->group->name;
 	hw_metadata->trap_name = trap_item->trap->name;
 	hw_metadata->fa_cookie = fa_cookie;
 
@@ -9529,6 +9740,115 @@ devlink_trap_policers_unregister(struct devlink *devlink,
 }
 EXPORT_SYMBOL_GPL(devlink_trap_policers_unregister);
 
+static int
+devlink_port_trap_register(struct devlink_port *devlink_port,
+			   const struct devlink_trap *trap, void *priv)
+{
+	struct devlink_trap_lists *trap_lists = &devlink_port->trap_lists;
+	struct devlink_trap_item *trap_item;
+	int err;
+
+	trap_item = devlink_create_trap_item(trap_lists, trap, priv);
+	if (IS_ERR(trap_item))
+		return PTR_ERR(trap_item);
+
+	if (!(list_empty(&trap_lists->trap_group_list))) {
+		err = devlink_trap_item_group_link(trap_lists, trap_item);
+		if (err)
+			goto err_group_link;
+	}
+
+	err = devlink_port->ops->trap_init(devlink_port, trap, trap_item);
+	if (err)
+		goto err_trap_init;
+
+	list_add_tail(&trap_item->list, &trap_lists->trap_list);
+	devlink_trap_notify(devlink_port->devlink, trap_item, DEVLINK_CMD_TRAP_NEW);
+
+	return 0;
+
+err_trap_init:
+err_group_link:
+	free_percpu(trap_item->stats);
+	kfree(trap_item);
+	return err;
+}
+
+int devlink_port_traps_register(struct devlink_port *devlink_port,
+				const struct devlink_trap *traps,
+				size_t traps_count, void *priv)
+{
+	int i, err;
+
+	if (!devlink_port->ops->trap_init || !devlink_port->ops->trap_action_set)
+		return -EINVAL;
+
+	mutex_lock(&devlink_port->trap_lists_lock);
+	for (i = 0; i < traps_count; i++) {
+		const struct devlink_trap *trap = &traps[i];
+
+		err = devlink_trap_verify(trap);
+		if (err)
+			goto err_trap_verify;
+
+		err = devlink_port_trap_register(devlink_port, trap, priv);
+		if (err)
+			goto err_trap_register;
+	}
+	mutex_unlock(&devlink_port->trap_lists_lock);
+
+	return 0;
+
+err_trap_register:
+err_trap_verify:
+	for (i--; i >= 0; i--)
+		devlink_port_trap_unregister(devlink_port, &traps[i]);
+	mutex_unlock(&devlink_port->trap_lists_lock);
+	return err;
+}
+EXPORT_SYMBOL_GPL(devlink_port_traps_register);
+
+void devlink_port_traps_unregister(struct devlink_port *devlink_port,
+				   const struct devlink_trap *traps,
+				   size_t traps_count)
+{
+	int i;
+
+	mutex_lock(&devlink_port->trap_lists_lock);
+	/* Make sure we do not have any packets in-flight while unregistering
+	 * traps by disabling all of them and waiting for a grace period.
+	 */
+	for (i = traps_count - 1; i >= 0; i--)
+		devlink_port_trap_disable(devlink_port, &traps[i]);
+	synchronize_rcu();
+	for (i = traps_count - 1; i >= 0; i--)
+		devlink_port_trap_unregister(devlink_port, &traps[i]);
+	mutex_unlock(&devlink_port->trap_lists_lock);
+}
+EXPORT_SYMBOL_GPL(devlink_port_traps_unregister);
+
+void devlink_port_trap_report(struct devlink_port *devlink_port, struct sk_buff *skb,
+			      void *trap_ctx, const struct flow_action_cookie *fa_cookie)
+{
+	struct devlink_trap_item *trap_item = trap_ctx;
+	struct net_dm_hw_metadata hw_metadata = {};
+
+	devlink_trap_stats_update(trap_item->stats, skb->len);
+	devlink_trap_stats_update(trap_item->group_item->stats, skb->len);
+
+	/* Control packets were not dropped by the device or encountered an
+	 * exception during forwarding and therefore should not be reported to
+	 * the kernel's drop monitor.
+	 */
+	if (trap_item->trap->type == DEVLINK_TRAP_TYPE_CONTROL)
+		return;
+
+	devlink_trap_report_metadata_fill(&hw_metadata, trap_item,
+					  devlink_port, fa_cookie);
+	net_dm_hw_report(skb, &hw_metadata);
+}
+EXPORT_SYMBOL_GPL(devlink_port_trap_report);
+
 static void __devlink_compat_running_version(struct devlink *devlink,
 					     char *buf, size_t len)
 {
-- 
2.14.1


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

* [PATCH net-next RFC v2 3/3] net/mlx5e: Add devlink trap to catch oversize packets
  2020-09-17 10:36 [PATCH net-next RFC v2 0/3] Add devlink traps in devlink port context Aya Levin
  2020-09-17 10:36 ` [PATCH net-next RFC v2 1/3] devlink: Wrap trap related lists a trap_lists struct Aya Levin
  2020-09-17 10:36 ` [PATCH net-next RFC v2 2/3] devlink: Add devlink traps under devlink_ports context Aya Levin
@ 2020-09-17 10:36 ` Aya Levin
  2 siblings, 0 replies; 4+ messages in thread
From: Aya Levin @ 2020-09-17 10:36 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, Jiri Pirko, Ido Schimmel, netdev
  Cc: Moshe Shemesh, Eran Ben Elisha, linux-kernel, Aya Levin

From: Aya Levin <ayal@mellanox.com>

Register MTU error trap to allow visibility of oversize packets. Display
a naive use of devlink trap in devlink port context.

Signed-off-by: Aya Levin <ayal@mellanox.com>
---
Changelog:
v1->v2:
-Minor changes in trap's definition
-Adjustments to trap API and ops

 drivers/net/ethernet/mellanox/mlx5/core/Makefile   |  2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en.h       |  2 +
 drivers/net/ethernet/mellanox/mlx5/core/en/traps.c | 38 +++++++++++++++++
 drivers/net/ethernet/mellanox/mlx5/core/en/traps.h | 14 +++++++
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c  | 48 ++++++++++++++++++++++
 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c    | 11 ++++-
 6 files changed, 112 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en/traps.c
 create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en/traps.h

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Makefile b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
index 9826a041e407..32436325725c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Makefile
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
@@ -25,7 +25,7 @@ mlx5_core-$(CONFIG_MLX5_CORE_EN) += en_main.o en_common.o en_fs.o en_ethtool.o \
 		en_tx.o en_rx.o en_dim.o en_txrx.o en/xdp.o en_stats.o \
 		en_selftest.o en/port.o en/monitor_stats.o en/health.o \
 		en/reporter_tx.o en/reporter_rx.o en/params.o en/xsk/pool.o \
-		en/xsk/setup.o en/xsk/rx.o en/xsk/tx.o en/devlink.o
+		en/xsk/setup.o en/xsk/rx.o en/xsk/tx.o en/devlink.o en/traps.o
 
 #
 # Netdev extra
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 0df40d24acb0..6e652a513a84 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -824,6 +824,8 @@ struct mlx5e_priv {
 	struct mlx5e_hv_vhca_stats_agent stats_agent;
 #endif
 	struct mlx5e_scratchpad    scratchpad;
+	bool trap_oversize;
+	void *trap_mtu;
 };
 
 struct mlx5e_rx_handlers {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/traps.c b/drivers/net/ethernet/mellanox/mlx5/core/en/traps.c
new file mode 100644
index 000000000000..211407666c3a
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/traps.c
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2020 Mellanox Technologies.
+#include "traps.h"
+
+#define MLX5E_TRAP(_id, _type, _group_id)                               \
+	DEVLINK_TRAP_GENERIC(_type, DROP, _id,                          \
+			     DEVLINK_TRAP_GROUP_GENERIC_ID_##_group_id, \
+			     DEVLINK_TRAP_METADATA_TYPE_F_IN_PORT)
+static struct devlink_trap mlx5e_traps_arr[] = {
+	MLX5E_TRAP(MTU_ERROR, EXCEPTION, L2_DROPS),
+};
+
+int mlx5e_devlink_traps_create(struct mlx5e_priv *priv)
+{
+	struct devlink_port *dl_port = &priv->dl_port;
+
+	return  devlink_port_traps_register(dl_port, mlx5e_traps_arr,
+					   ARRAY_SIZE(mlx5e_traps_arr),
+					   priv);
+}
+
+void mlx5e_devlink_traps_destroy(struct mlx5e_priv *priv)
+{
+	struct devlink_port *dl_port = &priv->dl_port;
+
+	devlink_port_traps_unregister(dl_port, mlx5e_traps_arr,
+				      ARRAY_SIZE(mlx5e_traps_arr));
+}
+
+struct devlink_trap *mlx5e_trap_lookup(u16 id)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(mlx5e_traps_arr); i++)
+		if (mlx5e_traps_arr[i].id == id)
+			return &mlx5e_traps_arr[i];
+	return NULL;
+}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/traps.h b/drivers/net/ethernet/mellanox/mlx5/core/en/traps.h
new file mode 100644
index 000000000000..7d95cd4b571c
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/traps.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2020 Mellanox Technologies.*/
+
+#ifndef __MLX5E_EN_TRAPS_H
+#define __MLX5E_EN_TRAPS_H
+
+#include "en.h"
+
+int mlx5e_devlink_traps_create(struct mlx5e_priv *priv);
+void mlx5e_devlink_traps_destroy(struct mlx5e_priv *priv);
+struct devlink_trap *mlx5e_trap_lookup(u16 id);
+
+#endif
+
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 472252ea67a1..81d1e6186bb8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -64,6 +64,7 @@
 #include "en/hv_vhca_stats.h"
 #include "en/devlink.h"
 #include "lib/mlx5.h"
+#include "en/traps.h"
 
 bool mlx5e_check_fragmented_striding_rq_cap(struct mlx5_core_dev *mdev)
 {
@@ -5003,6 +5004,50 @@ void mlx5e_destroy_q_counters(struct mlx5e_priv *priv)
 	}
 }
 
+static int mlx5e_devlink_trap_init(struct devlink_port *devlink_port,
+				   const struct devlink_trap *trap,
+				   void *trap_ctx)
+{
+	struct mlx5e_priv *priv = (struct mlx5e_priv *)devlink_trap_ctx_priv(trap_ctx);
+
+	if (!mlx5e_trap_lookup(trap->id))
+		return -EINVAL;
+	priv->trap_oversize = false;
+	priv->trap_mtu = trap_ctx;
+	return 0;
+}
+
+static void mlx5e_devlink_trap_fini(struct devlink_port *devlink_port,
+				    const struct devlink_trap *trap,
+				    void *trap_ctx)
+{
+	struct mlx5e_priv *priv = (struct mlx5e_priv *)devlink_trap_ctx_priv(trap_ctx);
+
+	if (!mlx5e_trap_lookup(trap->id))
+		return;
+	priv->trap_oversize = false;
+	priv->trap_mtu = NULL;
+}
+
+static int mlx5e_devlink_trap_action_set(struct devlink_port *devlink_port,
+					 const struct devlink_trap *trap,
+					 enum devlink_trap_action action,
+					 struct netlink_ext_ack *extack)
+{
+	struct mlx5e_priv *priv = container_of(devlink_port, struct mlx5e_priv, dl_port);
+
+	if (!mlx5e_trap_lookup(trap->id))
+		return -EINVAL;
+	priv->trap_oversize = !!action;
+	return 0;
+}
+
+static const struct devlink_port_ops mlx5e_devlink_port_ops = {
+	.trap_init		= mlx5e_devlink_trap_init,
+	.trap_fini		= mlx5e_devlink_trap_fini,
+	.trap_action_set	= mlx5e_devlink_trap_action_set,
+};
+
 static int mlx5e_nic_init(struct mlx5_core_dev *mdev,
 			  struct net_device *netdev,
 			  const struct mlx5e_profile *profile,
@@ -5032,12 +5077,15 @@ static int mlx5e_nic_init(struct mlx5_core_dev *mdev,
 	if (err)
 		mlx5_core_err(mdev, "mlx5e_devlink_port_register failed, %d\n", err);
 	mlx5e_health_create_reporters(priv);
+	devlink_port_set_ops(&priv->dl_port, &mlx5e_devlink_port_ops);
+	mlx5e_devlink_traps_create(priv);
 
 	return 0;
 }
 
 static void mlx5e_nic_cleanup(struct mlx5e_priv *priv)
 {
+	mlx5e_devlink_traps_destroy(priv);
 	mlx5e_health_destroy_reporters(priv);
 	mlx5e_devlink_port_unregister(priv);
 	mlx5e_tls_cleanup(priv);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index 85d545bb47be..ec2fd39a57d5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -1445,6 +1445,7 @@ mlx5e_skb_from_cqe_mpwrq_linear(struct mlx5e_rq *rq, struct mlx5e_mpw_info *wi,
 				u16 cqe_bcnt, u32 head_offset, u32 page_idx)
 {
 	struct mlx5e_dma_info *di = &wi->umr.dma_info[page_idx];
+	struct mlx5e_priv *priv = rq->channel->priv;
 	u16 rx_headroom = rq->buff.headroom;
 	u32 cqe_bcnt32 = cqe_bcnt;
 	struct xdp_buff xdp;
@@ -1452,11 +1453,14 @@ mlx5e_skb_from_cqe_mpwrq_linear(struct mlx5e_rq *rq, struct mlx5e_mpw_info *wi,
 	void *va, *data;
 	u32 frag_size;
 	bool consumed;
+	bool trap;
 
 	/* Check packet size. Note LRO doesn't use linear SKB */
 	if (unlikely(cqe_bcnt > rq->hw_mtu)) {
 		rq->stats->oversize_pkts_sw_drop++;
-		return NULL;
+		if (!priv->trap_oversize)
+			return NULL;
+		trap = true;
 	}
 
 	va             = page_address(di->page) + head_offset;
@@ -1483,7 +1487,10 @@ mlx5e_skb_from_cqe_mpwrq_linear(struct mlx5e_rq *rq, struct mlx5e_mpw_info *wi,
 	skb = mlx5e_build_linear_skb(rq, va, frag_size, rx_headroom, cqe_bcnt32);
 	if (unlikely(!skb))
 		return NULL;
-
+	if (trap) {
+		devlink_port_trap_report(&priv->dl_port, skb, priv->trap_mtu, NULL);
+		return NULL;
+	}
 	/* queue up for recycling/reuse */
 	page_ref_inc(di->page);
 
-- 
2.14.1


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

end of thread, other threads:[~2020-09-17 10:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-17 10:36 [PATCH net-next RFC v2 0/3] Add devlink traps in devlink port context Aya Levin
2020-09-17 10:36 ` [PATCH net-next RFC v2 1/3] devlink: Wrap trap related lists a trap_lists struct Aya Levin
2020-09-17 10:36 ` [PATCH net-next RFC v2 2/3] devlink: Add devlink traps under devlink_ports context Aya Levin
2020-09-17 10:36 ` [PATCH net-next RFC v2 3/3] net/mlx5e: Add devlink trap to catch oversize packets Aya Levin

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).