linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v3 0/7] Add devlink-health support for devlink ports
@ 2020-07-10 12:25 Moshe Shemesh
  2020-07-10 12:25 ` [PATCH net-next v3 1/7] devlink: Refactor devlink health reporter constructor Moshe Shemesh
                   ` (8 more replies)
  0 siblings, 9 replies; 12+ messages in thread
From: Moshe Shemesh @ 2020-07-10 12:25 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: Jiri Pirko, netdev, linux-kernel, Moshe Shemesh

Implement support for devlink health reporters on per-port basis.

This patchset comes to fix a design issue as some health reporters report
on errors and run recovery on device level while the actual functionality
is on port level. As for the current implemented devlink health reporters
it is relevant only to Tx and Rx reporters of mlx5, which has only one
port, so no real effect on functionality, but this should be fixed before
more drivers will use devlink health reporters.

First part in the series prepares common functions parts for health
reporter implementation. Second introduces required API to devlink-health
and mlx5e ones demonstrate its usage and implement the feature for mlx5
driver.

The per-port reporter functionality is achieved by adding a list of
devlink_health_reporters to devlink_port struct in a manner similar to
existing device infrastructure. This is the only major difference and
it makes possible to fully reuse device reporters operations.
The effect will be seen in conjunction with iproute2 additions and
will affect all devlink health commands. User can distinguish between
device and port reporters by looking at a devlink handle. Port reporters
have a port index at the end of the address and such addresses can be
provided as a parameter in every place where devlink-health accepted it.
These can be obtained from devlink port show command.
For example:
$ devlink health show
pci/0000:00:0a.0:
  reporter fw
    state healthy error 0 recover 0 auto_dump true
pci/0000:00:0a.0/1:
  reporter tx
    state healthy error 0 recover 0 grace_period 500 auto_recover true auto_dump true
$ devlink health set pci/0000:00:0a.0/1 reporter tx grace_period 1000 \
auto_recover false auto_dump false
$ devlink health show pci/0000:00:0a.0/1 reporter tx
pci/0000:00:0a.0/1:
  reporter tx
    state healthy error 0 recover 0 grace_period 1000 auto_recover flase auto_dump false

Note: User can use the same devlink health uAPI commands can get now either
port health reporter or device health reporter.
For example, the recover command:
Before this patchset: devlink health recover DEV reporter REPORTER_NAME
After this patchset: devlink health recover { DEV | DEV/PORT_INDEX } reporter REPORTER_NAME

Changes v1 -> v2:
Fixed functions comment to match parameters list.

Changes v2 -> v3:
Added motivation to cover letter and note on uAPI.


Vladyslav Tarasiuk (7):
  devlink: Refactor devlink health reporter constructor
  devlink: Rework devlink health reporter destructor
  devlink: Create generic devlink health reporter search function
  devlink: Implement devlink health reporters on per-port basis
  devlink: Add devlink health port reporters API
  net/mlx5e: Move devlink port register and unregister calls
  net/mlx5e: Move devlink-health rx and tx reporters to devlink port

 .../ethernet/mellanox/mlx5/core/en/reporter_rx.c   |   9 +-
 .../ethernet/mellanox/mlx5/core/en/reporter_tx.c   |  13 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c  |  15 +-
 include/net/devlink.h                              |  11 +
 net/core/devlink.c                                 | 244 ++++++++++++++++-----
 5 files changed, 216 insertions(+), 76 deletions(-)

-- 
1.8.3.1


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

* [PATCH net-next v3 1/7] devlink: Refactor devlink health reporter constructor
  2020-07-10 12:25 [PATCH net-next v3 0/7] Add devlink-health support for devlink ports Moshe Shemesh
@ 2020-07-10 12:25 ` Moshe Shemesh
  2020-07-10 12:25 ` [PATCH net-next v3 2/7] devlink: Rework devlink health reporter destructor Moshe Shemesh
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Moshe Shemesh @ 2020-07-10 12:25 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: Jiri Pirko, netdev, linux-kernel, Vladyslav Tarasiuk

From: Vladyslav Tarasiuk <vladyslavt@mellanox.com>

Prepare a common routine in devlink_health_reporter_create() for usage
in similar functions for devlink port health reporters.

Signed-off-by: Vladyslav Tarasiuk <vladyslavt@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 net/core/devlink.c | 45 ++++++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 346d385..a203d35 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -5321,6 +5321,31 @@ struct devlink_health_reporter {
 	return NULL;
 }
 
+static struct devlink_health_reporter *
+__devlink_health_reporter_create(struct devlink *devlink,
+				 const struct devlink_health_reporter_ops *ops,
+				 u64 graceful_period, void *priv)
+{
+	struct devlink_health_reporter *reporter;
+
+	if (WARN_ON(graceful_period && !ops->recover))
+		return ERR_PTR(-EINVAL);
+
+	reporter = kzalloc(sizeof(*reporter), GFP_KERNEL);
+	if (!reporter)
+		return ERR_PTR(-ENOMEM);
+
+	reporter->priv = priv;
+	reporter->ops = ops;
+	reporter->devlink = devlink;
+	reporter->graceful_period = graceful_period;
+	reporter->auto_recover = !!ops->recover;
+	reporter->auto_dump = !!ops->dump;
+	mutex_init(&reporter->dump_lock);
+	refcount_set(&reporter->refcount, 1);
+	return reporter;
+}
+
 /**
  *	devlink_health_reporter_create - create devlink health reporter
  *
@@ -5342,25 +5367,11 @@ struct devlink_health_reporter *
 		goto unlock;
 	}
 
-	if (WARN_ON(graceful_period && !ops->recover)) {
-		reporter = ERR_PTR(-EINVAL);
+	reporter = __devlink_health_reporter_create(devlink, ops,
+						    graceful_period, priv);
+	if (IS_ERR(reporter))
 		goto unlock;
-	}
-
-	reporter = kzalloc(sizeof(*reporter), GFP_KERNEL);
-	if (!reporter) {
-		reporter = ERR_PTR(-ENOMEM);
-		goto unlock;
-	}
 
-	reporter->priv = priv;
-	reporter->ops = ops;
-	reporter->devlink = devlink;
-	reporter->graceful_period = graceful_period;
-	reporter->auto_recover = !!ops->recover;
-	reporter->auto_dump = !!ops->dump;
-	mutex_init(&reporter->dump_lock);
-	refcount_set(&reporter->refcount, 1);
 	list_add_tail(&reporter->list, &devlink->reporter_list);
 unlock:
 	mutex_unlock(&devlink->reporters_lock);
-- 
1.8.3.1


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

* [PATCH net-next v3 2/7] devlink: Rework devlink health reporter destructor
  2020-07-10 12:25 [PATCH net-next v3 0/7] Add devlink-health support for devlink ports Moshe Shemesh
  2020-07-10 12:25 ` [PATCH net-next v3 1/7] devlink: Refactor devlink health reporter constructor Moshe Shemesh
@ 2020-07-10 12:25 ` Moshe Shemesh
  2020-07-10 12:25 ` [PATCH net-next v3 3/7] devlink: Create generic devlink health reporter search function Moshe Shemesh
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Moshe Shemesh @ 2020-07-10 12:25 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: Jiri Pirko, netdev, linux-kernel, Vladyslav Tarasiuk

From: Vladyslav Tarasiuk <vladyslavt@mellanox.com>

Devlink keeps its own reference to every reporter in a list and inits
refcount to 1 upon reporter's creation. Existing destructor waits to
free the memory indefinitely using msleep() until all references except
devlink's own are put.

Rework this mechanism by moving memory free routine to a separate
function, which is called when the last reporter reference is put.

Besides, it allows to call __devlink_health_reporter_destroy() while
locked on a reporters list mutex in symmetry to
__devlink_health_reporter_create(), which is required in follow-up
patch.

Signed-off-by: Vladyslav Tarasiuk <vladyslavt@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 net/core/devlink.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index a203d35..b85f211 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -5379,6 +5379,29 @@ struct devlink_health_reporter *
 }
 EXPORT_SYMBOL_GPL(devlink_health_reporter_create);
 
+static void
+devlink_health_reporter_free(struct devlink_health_reporter *reporter)
+{
+	mutex_destroy(&reporter->dump_lock);
+	if (reporter->dump_fmsg)
+		devlink_fmsg_free(reporter->dump_fmsg);
+	kfree(reporter);
+}
+
+static void
+devlink_health_reporter_put(struct devlink_health_reporter *reporter)
+{
+	if (refcount_dec_and_test(&reporter->refcount))
+		devlink_health_reporter_free(reporter);
+}
+
+static void
+__devlink_health_reporter_destroy(struct devlink_health_reporter *reporter)
+{
+	list_del(&reporter->list);
+	devlink_health_reporter_put(reporter);
+}
+
 /**
  *	devlink_health_reporter_destroy - destroy devlink health reporter
  *
@@ -5388,14 +5411,8 @@ struct devlink_health_reporter *
 devlink_health_reporter_destroy(struct devlink_health_reporter *reporter)
 {
 	mutex_lock(&reporter->devlink->reporters_lock);
-	list_del(&reporter->list);
+	__devlink_health_reporter_destroy(reporter);
 	mutex_unlock(&reporter->devlink->reporters_lock);
-	while (refcount_read(&reporter->refcount) > 1)
-		msleep(100);
-	mutex_destroy(&reporter->dump_lock);
-	if (reporter->dump_fmsg)
-		devlink_fmsg_free(reporter->dump_fmsg);
-	kfree(reporter);
 }
 EXPORT_SYMBOL_GPL(devlink_health_reporter_destroy);
 
@@ -5665,12 +5682,6 @@ int devlink_health_report(struct devlink_health_reporter *reporter,
 	return NULL;
 }
 
-static void
-devlink_health_reporter_put(struct devlink_health_reporter *reporter)
-{
-	refcount_dec(&reporter->refcount);
-}
-
 void
 devlink_health_reporter_state_update(struct devlink_health_reporter *reporter,
 				     enum devlink_health_reporter_state state)
-- 
1.8.3.1


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

* [PATCH net-next v3 3/7] devlink: Create generic devlink health reporter search function
  2020-07-10 12:25 [PATCH net-next v3 0/7] Add devlink-health support for devlink ports Moshe Shemesh
  2020-07-10 12:25 ` [PATCH net-next v3 1/7] devlink: Refactor devlink health reporter constructor Moshe Shemesh
  2020-07-10 12:25 ` [PATCH net-next v3 2/7] devlink: Rework devlink health reporter destructor Moshe Shemesh
@ 2020-07-10 12:25 ` Moshe Shemesh
  2020-07-10 12:25 ` [PATCH net-next v3 4/7] devlink: Implement devlink health reporters on per-port basis Moshe Shemesh
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Moshe Shemesh @ 2020-07-10 12:25 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: Jiri Pirko, netdev, linux-kernel, Vladyslav Tarasiuk

From: Vladyslav Tarasiuk <vladyslavt@mellanox.com>

Add a generic __devlink_health_reporter_find_by_name() that can be used
with arbitrary devlink health reporter list.

Signed-off-by: Vladyslav Tarasiuk <vladyslavt@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 net/core/devlink.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index b85f211..4e995de 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -5309,19 +5309,29 @@ struct devlink_health_reporter {
 EXPORT_SYMBOL_GPL(devlink_health_reporter_priv);
 
 static struct devlink_health_reporter *
-devlink_health_reporter_find_by_name(struct devlink *devlink,
-				     const char *reporter_name)
+__devlink_health_reporter_find_by_name(struct list_head *reporter_list,
+				       struct mutex *list_lock,
+				       const char *reporter_name)
 {
 	struct devlink_health_reporter *reporter;
 
-	lockdep_assert_held(&devlink->reporters_lock);
-	list_for_each_entry(reporter, &devlink->reporter_list, list)
+	lockdep_assert_held(list_lock);
+	list_for_each_entry(reporter, reporter_list, list)
 		if (!strcmp(reporter->ops->name, reporter_name))
 			return reporter;
 	return NULL;
 }
 
 static struct devlink_health_reporter *
+devlink_health_reporter_find_by_name(struct devlink *devlink,
+				     const char *reporter_name)
+{
+	return __devlink_health_reporter_find_by_name(&devlink->reporter_list,
+						      &devlink->reporters_lock,
+						      reporter_name);
+}
+
+static struct devlink_health_reporter *
 __devlink_health_reporter_create(struct devlink *devlink,
 				 const struct devlink_health_reporter_ops *ops,
 				 u64 graceful_period, void *priv)
-- 
1.8.3.1


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

* [PATCH net-next v3 4/7] devlink: Implement devlink health reporters on per-port basis
  2020-07-10 12:25 [PATCH net-next v3 0/7] Add devlink-health support for devlink ports Moshe Shemesh
                   ` (2 preceding siblings ...)
  2020-07-10 12:25 ` [PATCH net-next v3 3/7] devlink: Create generic devlink health reporter search function Moshe Shemesh
@ 2020-07-10 12:25 ` Moshe Shemesh
  2020-07-10 12:25 ` [PATCH net-next v3 5/7] devlink: Add devlink health port reporters API Moshe Shemesh
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Moshe Shemesh @ 2020-07-10 12:25 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: Jiri Pirko, netdev, linux-kernel, Vladyslav Tarasiuk

From: Vladyslav Tarasiuk <vladyslavt@mellanox.com>

Add devlink-health reporter support on per-port basis.
The main difference existing devlink-health is that port reporters are
stored in per-devlink_port lists. Upon creation of such health reporter the
reference to a port it belongs to is stored in reporter struct.

Fill the port index attribute in devlink-health response to
allow devlink userspace utility to distinguish between device and port
reporters.

Signed-off-by: Vladyslav Tarasiuk <vladyslavt@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/devlink.h |  2 ++
 net/core/devlink.c    | 94 +++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 79 insertions(+), 17 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 746bed5..bb11397 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -101,6 +101,8 @@ struct devlink_port {
 	u8 attrs_set:1,
 	   switch_port:1;
 	struct delayed_work type_warn_dw;
+	struct list_head reporter_list;
+	struct mutex reporters_lock; /* Protects reporter_list */
 };
 
 struct devlink_sb_pool_info {
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 4e995de..b4a231c 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -386,19 +386,21 @@ struct devlink_snapshot {
 	return NULL;
 }
 
-#define DEVLINK_NL_FLAG_NEED_DEVLINK	BIT(0)
-#define DEVLINK_NL_FLAG_NEED_PORT	BIT(1)
-#define DEVLINK_NL_FLAG_NEED_SB		BIT(2)
+#define DEVLINK_NL_FLAG_NEED_DEVLINK		BIT(0)
+#define DEVLINK_NL_FLAG_NEED_PORT		BIT(1)
+#define DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT	BIT(2)
+#define DEVLINK_NL_FLAG_NEED_SB			BIT(3)
 
 /* The per devlink instance lock is taken by default in the pre-doit
  * operation, yet several commands do not require this. The global
  * devlink lock is taken and protects from disruption by user-calls.
  */
-#define DEVLINK_NL_FLAG_NO_LOCK		BIT(3)
+#define DEVLINK_NL_FLAG_NO_LOCK			BIT(4)
 
 static int devlink_nl_pre_doit(const struct genl_ops *ops,
 			       struct sk_buff *skb, struct genl_info *info)
 {
+	struct devlink_port *devlink_port;
 	struct devlink *devlink;
 	int err;
 
@@ -413,14 +415,17 @@ static int devlink_nl_pre_doit(const struct genl_ops *ops,
 	if (ops->internal_flags & DEVLINK_NL_FLAG_NEED_DEVLINK) {
 		info->user_ptr[0] = devlink;
 	} else if (ops->internal_flags & DEVLINK_NL_FLAG_NEED_PORT) {
-		struct devlink_port *devlink_port;
-
 		devlink_port = devlink_port_get_from_info(devlink, info);
 		if (IS_ERR(devlink_port)) {
 			err = PTR_ERR(devlink_port);
 			goto unlock;
 		}
 		info->user_ptr[0] = devlink_port;
+	} else if (ops->internal_flags & DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT) {
+		info->user_ptr[0] = devlink;
+		devlink_port = devlink_port_get_from_info(devlink, info);
+		if (!IS_ERR(devlink_port))
+			info->user_ptr[1] = devlink_port;
 	}
 	if (ops->internal_flags & DEVLINK_NL_FLAG_NEED_SB) {
 		struct devlink_sb *devlink_sb;
@@ -5287,6 +5292,7 @@ struct devlink_health_reporter {
 	void *priv;
 	const struct devlink_health_reporter_ops *ops;
 	struct devlink *devlink;
+	struct devlink_port *devlink_port;
 	struct devlink_fmsg *dump_fmsg;
 	struct mutex dump_lock; /* lock parallel read/write from dump buffers */
 	u64 graceful_period;
@@ -5332,6 +5338,15 @@ struct devlink_health_reporter {
 }
 
 static struct devlink_health_reporter *
+devlink_port_health_reporter_find_by_name(struct devlink_port *devlink_port,
+					  const char *reporter_name)
+{
+	return __devlink_health_reporter_find_by_name(&devlink_port->reporter_list,
+						      &devlink_port->reporters_lock,
+						      reporter_name);
+}
+
+static struct devlink_health_reporter *
 __devlink_health_reporter_create(struct devlink *devlink,
 				 const struct devlink_health_reporter_ops *ops,
 				 u64 graceful_period, void *priv)
@@ -5443,6 +5458,10 @@ struct devlink_health_reporter *
 	if (devlink_nl_put_handle(msg, devlink))
 		goto genlmsg_cancel;
 
+	if (reporter->devlink_port) {
+		if (nla_put_u32(msg, DEVLINK_ATTR_PORT_INDEX, reporter->devlink_port->index))
+			goto genlmsg_cancel;
+	}
 	reporter_attr = nla_nest_start_noflag(msg,
 					      DEVLINK_ATTR_HEALTH_REPORTER);
 	if (!reporter_attr)
@@ -5650,17 +5669,28 @@ int devlink_health_report(struct devlink_health_reporter *reporter,
 				       struct nlattr **attrs)
 {
 	struct devlink_health_reporter *reporter;
+	struct devlink_port *devlink_port;
 	char *reporter_name;
 
 	if (!attrs[DEVLINK_ATTR_HEALTH_REPORTER_NAME])
 		return NULL;
 
 	reporter_name = nla_data(attrs[DEVLINK_ATTR_HEALTH_REPORTER_NAME]);
-	mutex_lock(&devlink->reporters_lock);
-	reporter = devlink_health_reporter_find_by_name(devlink, reporter_name);
-	if (reporter)
-		refcount_inc(&reporter->refcount);
-	mutex_unlock(&devlink->reporters_lock);
+	devlink_port = devlink_port_get_from_attrs(devlink, attrs);
+	if (IS_ERR(devlink_port)) {
+		mutex_lock(&devlink->reporters_lock);
+		reporter = devlink_health_reporter_find_by_name(devlink, reporter_name);
+		if (reporter)
+			refcount_inc(&reporter->refcount);
+		mutex_unlock(&devlink->reporters_lock);
+	} else {
+		mutex_lock(&devlink_port->reporters_lock);
+		reporter = devlink_port_health_reporter_find_by_name(devlink_port, reporter_name);
+		if (reporter)
+			refcount_inc(&reporter->refcount);
+		mutex_unlock(&devlink_port->reporters_lock);
+	}
+
 	return reporter;
 }
 
@@ -5748,6 +5778,7 @@ static int devlink_nl_cmd_health_reporter_get_doit(struct sk_buff *skb,
 					  struct netlink_callback *cb)
 {
 	struct devlink_health_reporter *reporter;
+	struct devlink_port *port;
 	struct devlink *devlink;
 	int start = cb->args[0];
 	int idx = 0;
@@ -5778,6 +5809,31 @@ static int devlink_nl_cmd_health_reporter_get_doit(struct sk_buff *skb,
 		}
 		mutex_unlock(&devlink->reporters_lock);
 	}
+
+	list_for_each_entry(devlink, &devlink_list, list) {
+		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
+			continue;
+		list_for_each_entry(port, &devlink->port_list, list) {
+			mutex_lock(&port->reporters_lock);
+			list_for_each_entry(reporter, &port->reporter_list, list) {
+				if (idx < start) {
+					idx++;
+					continue;
+				}
+				err = devlink_nl_health_reporter_fill(msg, devlink, reporter,
+								      DEVLINK_CMD_HEALTH_REPORTER_GET,
+								      NETLINK_CB(cb->skb).portid,
+								      cb->nlh->nlmsg_seq,
+								      NLM_F_MULTI);
+				if (err) {
+					mutex_unlock(&port->reporters_lock);
+					goto out;
+				}
+				idx++;
+			}
+			mutex_unlock(&port->reporters_lock);
+		}
+	}
 out:
 	mutex_unlock(&devlink_mutex);
 
@@ -7157,7 +7213,7 @@ static int devlink_nl_cmd_trap_policer_set_doit(struct sk_buff *skb,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 		.doit = devlink_nl_cmd_health_reporter_get_doit,
 		.dumpit = devlink_nl_cmd_health_reporter_get_dumpit,
-		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK |
+		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT |
 				  DEVLINK_NL_FLAG_NO_LOCK,
 		/* can be retrieved by unprivileged users */
 	},
@@ -7166,7 +7222,7 @@ static int devlink_nl_cmd_trap_policer_set_doit(struct sk_buff *skb,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 		.doit = devlink_nl_cmd_health_reporter_set_doit,
 		.flags = GENL_ADMIN_PERM,
-		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK |
+		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT |
 				  DEVLINK_NL_FLAG_NO_LOCK,
 	},
 	{
@@ -7174,7 +7230,7 @@ static int devlink_nl_cmd_trap_policer_set_doit(struct sk_buff *skb,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 		.doit = devlink_nl_cmd_health_reporter_recover_doit,
 		.flags = GENL_ADMIN_PERM,
-		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK |
+		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT |
 				  DEVLINK_NL_FLAG_NO_LOCK,
 	},
 	{
@@ -7182,7 +7238,7 @@ static int devlink_nl_cmd_trap_policer_set_doit(struct sk_buff *skb,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 		.doit = devlink_nl_cmd_health_reporter_diagnose_doit,
 		.flags = GENL_ADMIN_PERM,
-		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK |
+		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT |
 				  DEVLINK_NL_FLAG_NO_LOCK,
 	},
 	{
@@ -7191,7 +7247,7 @@ static int devlink_nl_cmd_trap_policer_set_doit(struct sk_buff *skb,
 			    GENL_DONT_VALIDATE_DUMP_STRICT,
 		.dumpit = devlink_nl_cmd_health_reporter_dump_get_dumpit,
 		.flags = GENL_ADMIN_PERM,
-		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK |
+		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT |
 				  DEVLINK_NL_FLAG_NO_LOCK,
 	},
 	{
@@ -7199,7 +7255,7 @@ static int devlink_nl_cmd_trap_policer_set_doit(struct sk_buff *skb,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 		.doit = devlink_nl_cmd_health_reporter_dump_clear_doit,
 		.flags = GENL_ADMIN_PERM,
-		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK |
+		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT |
 				  DEVLINK_NL_FLAG_NO_LOCK,
 	},
 	{
@@ -7459,6 +7515,8 @@ int devlink_port_register(struct devlink *devlink,
 	list_add_tail(&devlink_port->list, &devlink->port_list);
 	INIT_LIST_HEAD(&devlink_port->param_list);
 	mutex_unlock(&devlink->lock);
+	INIT_LIST_HEAD(&devlink_port->reporter_list);
+	mutex_init(&devlink_port->reporters_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);
@@ -7475,6 +7533,8 @@ void devlink_port_unregister(struct devlink_port *devlink_port)
 {
 	struct devlink *devlink = devlink_port->devlink;
 
+	WARN_ON(!list_empty(&devlink_port->reporter_list));
+	mutex_destroy(&devlink_port->reporters_lock);
 	devlink_port_type_warn_cancel(devlink_port);
 	devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_DEL);
 	mutex_lock(&devlink->lock);
-- 
1.8.3.1


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

* [PATCH net-next v3 5/7] devlink: Add devlink health port reporters API
  2020-07-10 12:25 [PATCH net-next v3 0/7] Add devlink-health support for devlink ports Moshe Shemesh
                   ` (3 preceding siblings ...)
  2020-07-10 12:25 ` [PATCH net-next v3 4/7] devlink: Implement devlink health reporters on per-port basis Moshe Shemesh
@ 2020-07-10 12:25 ` Moshe Shemesh
  2020-07-13 13:18   ` Qian Cai
  2020-07-10 12:25 ` [PATCH net-next v3 6/7] net/mlx5e: Move devlink port register and unregister calls Moshe Shemesh
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 12+ messages in thread
From: Moshe Shemesh @ 2020-07-10 12:25 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: Jiri Pirko, netdev, linux-kernel, Vladyslav Tarasiuk

From: Vladyslav Tarasiuk <vladyslavt@mellanox.com>

In order to use new devlink port health reporters infrastructure, add
corresponding constructor and destructor functions.

Signed-off-by: Vladyslav Tarasiuk <vladyslavt@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/devlink.h |  9 +++++++++
 net/core/devlink.c    | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index bb11397..913e867 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1338,9 +1338,18 @@ struct devlink_health_reporter *
 devlink_health_reporter_create(struct devlink *devlink,
 			       const struct devlink_health_reporter_ops *ops,
 			       u64 graceful_period, void *priv);
+
+struct devlink_health_reporter *
+devlink_port_health_reporter_create(struct devlink_port *port,
+				    const struct devlink_health_reporter_ops *ops,
+				    u64 graceful_period, void *priv);
+
 void
 devlink_health_reporter_destroy(struct devlink_health_reporter *reporter);
 
+void
+devlink_port_health_reporter_destroy(struct devlink_health_reporter *reporter);
+
 void *
 devlink_health_reporter_priv(struct devlink_health_reporter *reporter);
 int devlink_health_report(struct devlink_health_reporter *reporter,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index b4a231c..20a83aa 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -5372,6 +5372,42 @@ struct devlink_health_reporter {
 }
 
 /**
+ *	devlink_port_health_reporter_create - create devlink health reporter for
+ *	                                      specified port instance
+ *
+ *	@port: devlink_port which should contain the new reporter
+ *	@ops: ops
+ *	@graceful_period: to avoid recovery loops, in msecs
+ *	@priv: priv
+ */
+struct devlink_health_reporter *
+devlink_port_health_reporter_create(struct devlink_port *port,
+				    const struct devlink_health_reporter_ops *ops,
+				    u64 graceful_period, void *priv)
+{
+	struct devlink_health_reporter *reporter;
+
+	mutex_lock(&port->reporters_lock);
+	if (__devlink_health_reporter_find_by_name(&port->reporter_list,
+						   &port->reporters_lock, ops->name)) {
+		reporter = ERR_PTR(-EEXIST);
+		goto unlock;
+	}
+
+	reporter = __devlink_health_reporter_create(port->devlink, ops,
+						    graceful_period, priv);
+	if (IS_ERR(reporter))
+		goto unlock;
+
+	reporter->devlink_port = port;
+	list_add_tail(&reporter->list, &port->reporter_list);
+unlock:
+	mutex_unlock(&port->reporters_lock);
+	return reporter;
+}
+EXPORT_SYMBOL_GPL(devlink_port_health_reporter_create);
+
+/**
  *	devlink_health_reporter_create - create devlink health reporter
  *
  *	@devlink: devlink
@@ -5441,6 +5477,20 @@ struct devlink_health_reporter *
 }
 EXPORT_SYMBOL_GPL(devlink_health_reporter_destroy);
 
+/**
+ *	devlink_port_health_reporter_destroy - destroy devlink port health reporter
+ *
+ *	@reporter: devlink health reporter to destroy
+ */
+void
+devlink_port_health_reporter_destroy(struct devlink_health_reporter *reporter)
+{
+	mutex_lock(&reporter->devlink_port->reporters_lock);
+	__devlink_health_reporter_destroy(reporter);
+	mutex_unlock(&reporter->devlink_port->reporters_lock);
+}
+EXPORT_SYMBOL_GPL(devlink_port_health_reporter_destroy);
+
 static int
 devlink_nl_health_reporter_fill(struct sk_buff *msg,
 				struct devlink *devlink,
-- 
1.8.3.1


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

* [PATCH net-next v3 6/7] net/mlx5e: Move devlink port register and unregister calls
  2020-07-10 12:25 [PATCH net-next v3 0/7] Add devlink-health support for devlink ports Moshe Shemesh
                   ` (4 preceding siblings ...)
  2020-07-10 12:25 ` [PATCH net-next v3 5/7] devlink: Add devlink health port reporters API Moshe Shemesh
@ 2020-07-10 12:25 ` Moshe Shemesh
  2020-07-10 12:25 ` [PATCH net-next v3 7/7] net/mlx5e: Move devlink-health rx and tx reporters to devlink port Moshe Shemesh
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Moshe Shemesh @ 2020-07-10 12:25 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: Jiri Pirko, netdev, linux-kernel, Vladyslav Tarasiuk

From: Vladyslav Tarasiuk <vladyslavt@mellanox.com>

Register devlink ports upon NIC init. TX and RX health reporters handle
errors which may occur early on at driver initialization. And because
these reporters are to be moved to port context, they require devlink
ports to be already registered.

Signed-off-by: Vladyslav Tarasiuk <vladyslavt@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index b04c857..e054726 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -5086,6 +5086,9 @@ static int mlx5e_nic_init(struct mlx5_core_dev *mdev,
 	if (err)
 		mlx5_core_err(mdev, "TLS initialization failed, %d\n", err);
 	mlx5e_build_nic_netdev(netdev);
+	err = mlx5e_devlink_port_register(priv);
+	if (err)
+		mlx5_core_err(mdev, "mlx5e_devlink_port_register failed, %d\n", err);
 	mlx5e_health_create_reporters(priv);
 
 	return 0;
@@ -5094,6 +5097,7 @@ static int mlx5e_nic_init(struct mlx5_core_dev *mdev,
 static void mlx5e_nic_cleanup(struct mlx5e_priv *priv)
 {
 	mlx5e_health_destroy_reporters(priv);
+	mlx5e_devlink_port_unregister(priv);
 	mlx5e_tls_cleanup(priv);
 	mlx5e_ipsec_cleanup(priv);
 	mlx5e_netdev_cleanup(priv->netdev, priv);
@@ -5526,16 +5530,10 @@ static void *mlx5e_add(struct mlx5_core_dev *mdev)
 		goto err_destroy_netdev;
 	}
 
-	err = mlx5e_devlink_port_register(priv);
-	if (err) {
-		mlx5_core_err(mdev, "mlx5e_devlink_port_register failed, %d\n", err);
-		goto err_detach;
-	}
-
 	err = register_netdev(netdev);
 	if (err) {
 		mlx5_core_err(mdev, "register_netdev failed, %d\n", err);
-		goto err_devlink_port_unregister;
+		goto err_detach;
 	}
 
 	mlx5e_devlink_port_type_eth_set(priv);
@@ -5543,8 +5541,6 @@ static void *mlx5e_add(struct mlx5_core_dev *mdev)
 	mlx5e_dcbnl_init_app(priv);
 	return priv;
 
-err_devlink_port_unregister:
-	mlx5e_devlink_port_unregister(priv);
 err_detach:
 	mlx5e_detach(mdev, priv);
 err_destroy_netdev:
@@ -5565,7 +5561,6 @@ static void mlx5e_remove(struct mlx5_core_dev *mdev, void *vpriv)
 	priv = vpriv;
 	mlx5e_dcbnl_delete_app(priv);
 	unregister_netdev(priv->netdev);
-	mlx5e_devlink_port_unregister(priv);
 	mlx5e_detach(mdev, vpriv);
 	mlx5e_destroy_netdev(priv);
 }
-- 
1.8.3.1


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

* [PATCH net-next v3 7/7] net/mlx5e: Move devlink-health rx and tx reporters to devlink port
  2020-07-10 12:25 [PATCH net-next v3 0/7] Add devlink-health support for devlink ports Moshe Shemesh
                   ` (5 preceding siblings ...)
  2020-07-10 12:25 ` [PATCH net-next v3 6/7] net/mlx5e: Move devlink port register and unregister calls Moshe Shemesh
@ 2020-07-10 12:25 ` Moshe Shemesh
  2020-07-10 21:11 ` [PATCH net-next v3 0/7] Add devlink-health support for devlink ports Jakub Kicinski
  2020-07-10 21:32 ` David Miller
  8 siblings, 0 replies; 12+ messages in thread
From: Moshe Shemesh @ 2020-07-10 12:25 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: Jiri Pirko, netdev, linux-kernel, Vladyslav Tarasiuk

From: Vladyslav Tarasiuk <vladyslavt@mellanox.com>

Utilize new devlink-health port reporters API to move rx and tx
reporters from device to port.

Signed-off-by: Vladyslav Tarasiuk <vladyslavt@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c |  9 +++------
 drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c | 13 ++++---------
 2 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c
index 32ed106..9913647 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c
@@ -611,13 +611,10 @@ void mlx5e_reporter_icosq_cqe_err(struct mlx5e_icosq *icosq)
 
 void mlx5e_reporter_rx_create(struct mlx5e_priv *priv)
 {
-	struct devlink *devlink = priv_to_devlink(priv->mdev);
 	struct devlink_health_reporter *reporter;
 
-	reporter = devlink_health_reporter_create(devlink,
-						  &mlx5_rx_reporter_ops,
-						  MLX5E_REPORTER_RX_GRACEFUL_PERIOD,
-						  priv);
+	reporter = devlink_port_health_reporter_create(&priv->dl_port, &mlx5_rx_reporter_ops,
+						       MLX5E_REPORTER_RX_GRACEFUL_PERIOD, priv);
 	if (IS_ERR(reporter)) {
 		netdev_warn(priv->netdev, "Failed to create rx reporter, err = %ld\n",
 			    PTR_ERR(reporter));
@@ -631,5 +628,5 @@ void mlx5e_reporter_rx_destroy(struct mlx5e_priv *priv)
 	if (!priv->rx_reporter)
 		return;
 
-	devlink_health_reporter_destroy(priv->rx_reporter);
+	devlink_port_health_reporter_destroy(priv->rx_reporter);
 }
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
index 8265843..8be6eaa 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
@@ -440,14 +440,9 @@ int mlx5e_reporter_tx_timeout(struct mlx5e_txqsq *sq)
 void mlx5e_reporter_tx_create(struct mlx5e_priv *priv)
 {
 	struct devlink_health_reporter *reporter;
-	struct mlx5_core_dev *mdev = priv->mdev;
-	struct devlink *devlink;
-
-	devlink = priv_to_devlink(mdev);
-	reporter =
-		devlink_health_reporter_create(devlink, &mlx5_tx_reporter_ops,
-					       MLX5_REPORTER_TX_GRACEFUL_PERIOD,
-					       priv);
+
+	reporter = devlink_port_health_reporter_create(&priv->dl_port, &mlx5_tx_reporter_ops,
+						       MLX5_REPORTER_TX_GRACEFUL_PERIOD, priv);
 	if (IS_ERR(reporter)) {
 		netdev_warn(priv->netdev,
 			    "Failed to create tx reporter, err = %ld\n",
@@ -462,5 +457,5 @@ void mlx5e_reporter_tx_destroy(struct mlx5e_priv *priv)
 	if (!priv->tx_reporter)
 		return;
 
-	devlink_health_reporter_destroy(priv->tx_reporter);
+	devlink_port_health_reporter_destroy(priv->tx_reporter);
 }
-- 
1.8.3.1


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

* Re: [PATCH net-next v3 0/7] Add devlink-health support for devlink ports
  2020-07-10 12:25 [PATCH net-next v3 0/7] Add devlink-health support for devlink ports Moshe Shemesh
                   ` (6 preceding siblings ...)
  2020-07-10 12:25 ` [PATCH net-next v3 7/7] net/mlx5e: Move devlink-health rx and tx reporters to devlink port Moshe Shemesh
@ 2020-07-10 21:11 ` Jakub Kicinski
  2020-07-10 21:32 ` David Miller
  8 siblings, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2020-07-10 21:11 UTC (permalink / raw)
  To: Moshe Shemesh; +Cc: David S. Miller, Jiri Pirko, netdev, linux-kernel

On Fri, 10 Jul 2020 15:25:06 +0300 Moshe Shemesh wrote:
> Changes v2 -> v3:
> Added motivation to cover letter and note on uAPI.

I guess this will be a test of how many production users this API has...

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

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

* Re: [PATCH net-next v3 0/7] Add devlink-health support for devlink ports
  2020-07-10 12:25 [PATCH net-next v3 0/7] Add devlink-health support for devlink ports Moshe Shemesh
                   ` (7 preceding siblings ...)
  2020-07-10 21:11 ` [PATCH net-next v3 0/7] Add devlink-health support for devlink ports Jakub Kicinski
@ 2020-07-10 21:32 ` David Miller
  8 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2020-07-10 21:32 UTC (permalink / raw)
  To: moshe; +Cc: kuba, jiri, netdev, linux-kernel

From: Moshe Shemesh <moshe@mellanox.com>
Date: Fri, 10 Jul 2020 15:25:06 +0300

> Implement support for devlink health reporters on per-port basis.

Series applied, thank you.

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

* Re: [PATCH net-next v3 5/7] devlink: Add devlink health port reporters API
  2020-07-10 12:25 ` [PATCH net-next v3 5/7] devlink: Add devlink health port reporters API Moshe Shemesh
@ 2020-07-13 13:18   ` Qian Cai
  2020-07-13 14:07     ` Ido Schimmel
  0 siblings, 1 reply; 12+ messages in thread
From: Qian Cai @ 2020-07-13 13:18 UTC (permalink / raw)
  To: Moshe Shemesh
  Cc: David S. Miller, Jakub Kicinski, Jiri Pirko, netdev,
	linux-kernel, Vladyslav Tarasiuk

On Fri, Jul 10, 2020 at 03:25:11PM +0300, Moshe Shemesh wrote:
> From: Vladyslav Tarasiuk <vladyslavt@mellanox.com>
> 
> In order to use new devlink port health reporters infrastructure, add
> corresponding constructor and destructor functions.
> 
> Signed-off-by: Vladyslav Tarasiuk <vladyslavt@mellanox.com>
> Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>

This will trigger an use-after-free below while doing SR-IOV,

# echo 1 > /sys/class/net/enp11s0f1np1/device/sriov_numvfs
# git clone https://github.com/cailca/linux-mm
# cd linux-mm; make
#./random -x 0-100 -k0000:0b:01.2 (just use vfio-pci to passthrough via qemu-kvm.)

(arm64.config is also included in the repo.)

[ 1882.029101][ T4248] BUG: KASAN: use-after-free in devlink_port_health_reporter_destroy+0x150/0x198
devlink_port_health_reporter_destroy at net/core/devlink.c:5490
[ 1882.038060][ T4248] Read of size 8 at addr ffff0089b4115028 by task random/4248
[ 1882.045375][ T4248] CPU: 20 PID: 4248 Comm: random Not tainted 5.8.0-rc4-next-20200713 #1
[ 1882.053547][ T4248] Hardware name: HPE Apollo 70             /C01_APACHE_MB         , BIOS L50_5.13_1.11 06/18/2019
[ 1882.063977][ T4248] Call trace:
[ 1882.067119][ T4248]  dump_backtrace+0x0/0x398
[ 1882.071474][ T4248]  show_stack+0x14/0x20
[ 1882.075481][ T4248]  dump_stack+0x140/0x1c8
[ 1882.079663][ T4248]  print_address_description.constprop.10+0x54/0x550
[ 1882.086187][ T4248]  kasan_report+0x134/0x1b8
[ 1882.090541][ T4248]  __asan_report_load8_noabort+0x2c/0x50
[ 1882.096023][ T4248]  devlink_port_health_reporter_destroy+0x150/0x198
[ 1882.102521][ T4248]  mlx5e_reporter_rx_destroy+0x3c/0xc8 [mlx5_core]
[ 1882.108915][ T4248]  mlx5e_health_destroy_reporters+0x14/0x28 [mlx5_core]
[ 1882.115743][ T4248]  mlx5e_nic_cleanup+0x14/0x88 [mlx5_core]
[ 1882.121442][ T4248]  mlx5e_destroy_netdev+0x84/0xb8 [mlx5_core]
[ 1882.127401][ T4248]  mlx5e_remove+0x50/0x68 [mlx5_core]
[ 1882.132666][ T4248]  mlx5_remove_device+0x1f8/0x298 [mlx5_core]
[ 1882.138625][ T4248]  mlx5_unregister_device+0x5c/0x1b8 [mlx5_core]
[ 1882.144845][ T4248]  mlx5_unload_one+0x38/0x240 [mlx5_core]
[ 1882.150457][ T4248]  remove_one+0x58/0x98 [mlx5_core]
[ 1882.155510][ T4248]  pci_device_remove+0x8c/0x1e0
[ 1882.160213][ T4248]  device_release_driver_internal+0x1bc/0x3e0
[ 1882.166129][ T4248]  device_driver_detach+0x38/0x50
[ 1882.171003][ T4248]  unbind_store+0x174/0x1c8
[ 1882.175356][ T4248]  drv_attr_store+0x60/0xa0
[ 1882.179710][ T4248]  sysfs_kf_write+0xdc/0x128
[ 1882.184151][ T4248]  kernfs_fop_write+0x23c/0x448
[ 1882.188852][ T4248]  vfs_write+0x160/0x4a0
[ 1882.192944][ T4248]  ksys_write+0xe8/0x1b8
[ 1882.197037][ T4248]  __arm64_sys_write+0x68/0x98
[ 1882.201652][ T4248]  do_el0_svc+0x124/0x220
[ 1882.205833][ T4248]  el0_sync_handler+0x260/0x410
[ 1882.210533][ T4248]  el0_sync+0x140/0x180
[ 1882.214545][ T4248] Allocated by task 1341:
[ 1882.218726][ T4248]  kasan_save_stack+0x24/0x50
[ 1882.223253][ T4248]  __kasan_kmalloc.isra.10+0xc4/0xe0
[ 1882.228387][ T4248]  kasan_kmalloc+0xc/0x18
[ 1882.232567][ T4248]  kmem_cache_alloc_trace+0x1ec/0x318
[ 1882.237789][ T4248]  __devlink_health_reporter_create+0x78/0x290
[ 1882.243791][ T4248]  devlink_port_health_reporter_create+0xb8/0x1d8
[ 1882.250098][ T4248]  mlx5e_reporter_rx_create+0x34/0xb8 [mlx5_core]
[ 1882.256404][ T4248]  mlx5e_health_create_reporters+0x1c/0x28 [mlx5_core]
[ 1882.263144][ T4248]  mlx5e_nic_init+0x898/0xe18 [mlx5_core]
[ 1882.268756][ T4248]  mlx5e_create_netdev+0xb4/0x200 [mlx5_core]
[ 1882.274714][ T4248]  mlx5e_add+0x180/0x530 [mlx5_core]
[ 1882.279892][ T4248]  mlx5_add_device+0xb8/0x2d0 [mlx5_core]
[ 1882.285504][ T4248]  mlx5_register_device+0xe4/0x150 [mlx5_core]
[ 1882.291549][ T4248]  mlx5_load_one+0x3b4/0x10d8 [mlx5_core]
[ 1882.297162][ T4248]  init_one+0x6c4/0xca0 [mlx5_core]
[ 1882.302212][ T4248]  local_pci_probe+0xc0/0x168
[ 1882.306741][ T4248]  work_for_cpu_fn+0x4c/0x90
[ 1882.311182][ T4248]  process_one_work+0x7f0/0x1b28
[ 1882.315969][ T4248]  worker_thread+0x32c/0xac8
[ 1882.320410][ T4248]  kthread+0x398/0x440
[ 1882.324329][ T4248]  ret_from_fork+0x10/0x18
[ 1882.328600][ T4248] Freed by task 4248:
[ 1882.332433][ T4248]  kasan_save_stack+0x24/0x50
[ 1882.336959][ T4248]  kasan_set_track+0x24/0x38
[ 1882.341400][ T4248]  kasan_set_free_info+0x20/0x40
[ 1882.346187][ T4248]  __kasan_slab_free+0x118/0x188
[ 1882.350974][ T4248]  kasan_slab_free+0x10/0x18
[ 1882.355416][ T4248]  slab_free_freelist_hook+0x110/0x298
[ 1882.360723][ T4248]  kfree+0x128/0x568
[ 1882.364469][ T4248]  devlink_health_reporter_put+0x74/0xc8
[ 1882.369950][ T4248]  devlink_port_health_reporter_destroy+0x10c/0x198
[ 1882.376430][ T4248]  mlx5e_reporter_rx_destroy+0x3c/0xc8 [mlx5_core]
[ 1882.382823][ T4248]  mlx5e_health_destroy_reporters+0x14/0x28 [mlx5_core]
[ 1882.389650][ T4248]  mlx5e_nic_cleanup+0x14/0x88 [mlx5_core]
[ 1882.395348][ T4248]  mlx5e_destroy_netdev+0x84/0xb8 [mlx5_core]
[ 1882.401307][ T4248]  mlx5e_remove+0x50/0x68 [mlx5_core]
[ 1882.406572][ T4248]  mlx5_remove_device+0x1f8/0x298 [mlx5_core]
[ 1882.412530][ T4248]  mlx5_unregister_device+0x5c/0x1b8 [mlx5_core]
[ 1882.418749][ T4248]  mlx5_unload_one+0x38/0x240 [mlx5_core]
[ 1882.424360][ T4248]  remove_one+0x58/0x98 [mlx5_core]
[ 1882.429411][ T4248]  pci_device_remove+0x8c/0x1e0
[ 1882.434112][ T4248]  device_release_driver_internal+0x1bc/0x3e0
[ 1882.440027][ T4248]  device_driver_detach+0x38/0x50
[ 1882.444901][ T4248]  unbind_store+0x174/0x1c8
[ 1882.449253][ T4248]  drv_attr_store+0x60/0xa0
[ 1882.453607][ T4248]  sysfs_kf_write+0xdc/0x128
[ 1882.458047][ T4248]  kernfs_fop_write+0x23c/0x448
[ 1882.462747][ T4248]  vfs_write+0x160/0x4a0
[ 1882.466839][ T4248]  ksys_write+0xe8/0x1b8
[ 1882.470931][ T4248]  __arm64_sys_write+0x68/0x98
[ 1882.475546][ T4248]  do_el0_svc+0x124/0x220
[ 1882.479726][ T4248]  el0_sync_handler+0x260/0x410
[ 1882.484425][ T4248]  el0_sync+0x140/0x180
[ 1882.488436][ T4248] The buggy address belongs to the object at ffff0089b4115000
[ 1882.488436][ T4248]  which belongs to the cache kmalloc-512 of size 512
[ 1882.502339][ T4248] The buggy address is located 40 bytes inside of
[ 1882.502339][ T4248]  512-byte region [ffff0089b4115000, ffff0089b4115200)
[ 1882.515371][ T4248] The buggy address belongs to the page:
[ 1882.520855][ T4248] page:000000009f41bfb0 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x8a3411
[ 1882.530940][ T4248] flags: 0x7ffff800000200(slab)
[ 1882.535645][ T4248] raw: 007ffff800000200 ffffffe0225514c8 ffffffe022552788 ffff000000320680
[ 1882.544079][ T4248] raw: 0000000000000000 00000000002a002a 00000001ffffffff ffff0089b411e601
[ 1882.552511][ T4248] page dumped because: kasan: bad access detected
[ 1882.558772][ T4248] page->mem_cgroup:ffff0089b411e601
[ 1882.563823][ T4248] Memory state around the buggy address:
[ 1882.569305][ T4248]  ffff0089b4114f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 1882.577217][ T4248]  ffff0089b4114f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 1882.585128][ T4248] >ffff0089b4115000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 1882.593039][ T4248]                                   ^
[ 1882.598260][ T4248]  ffff0089b4115080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 1882.606172][ T4248]  f================int
[ 1888.192093][ T6454] vfio-pci 0000:0b:01.2: enabling device (0000 -> 0002)

> ---
>  include/net/devlink.h |  9 +++++++++
>  net/core/devlink.c    | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/include/net/devlink.h b/include/net/devlink.h
> index bb11397..913e867 100644
> --- a/include/net/devlink.h
> +++ b/include/net/devlink.h
> @@ -1338,9 +1338,18 @@ struct devlink_health_reporter *
>  devlink_health_reporter_create(struct devlink *devlink,
>  			       const struct devlink_health_reporter_ops *ops,
>  			       u64 graceful_period, void *priv);
> +
> +struct devlink_health_reporter *
> +devlink_port_health_reporter_create(struct devlink_port *port,
> +				    const struct devlink_health_reporter_ops *ops,
> +				    u64 graceful_period, void *priv);
> +
>  void
>  devlink_health_reporter_destroy(struct devlink_health_reporter *reporter);
>  
> +void
> +devlink_port_health_reporter_destroy(struct devlink_health_reporter *reporter);
> +
>  void *
>  devlink_health_reporter_priv(struct devlink_health_reporter *reporter);
>  int devlink_health_report(struct devlink_health_reporter *reporter,
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index b4a231c..20a83aa 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -5372,6 +5372,42 @@ struct devlink_health_reporter {
>  }
>  
>  /**
> + *	devlink_port_health_reporter_create - create devlink health reporter for
> + *	                                      specified port instance
> + *
> + *	@port: devlink_port which should contain the new reporter
> + *	@ops: ops
> + *	@graceful_period: to avoid recovery loops, in msecs
> + *	@priv: priv
> + */
> +struct devlink_health_reporter *
> +devlink_port_health_reporter_create(struct devlink_port *port,
> +				    const struct devlink_health_reporter_ops *ops,
> +				    u64 graceful_period, void *priv)
> +{
> +	struct devlink_health_reporter *reporter;
> +
> +	mutex_lock(&port->reporters_lock);
> +	if (__devlink_health_reporter_find_by_name(&port->reporter_list,
> +						   &port->reporters_lock, ops->name)) {
> +		reporter = ERR_PTR(-EEXIST);
> +		goto unlock;
> +	}
> +
> +	reporter = __devlink_health_reporter_create(port->devlink, ops,
> +						    graceful_period, priv);
> +	if (IS_ERR(reporter))
> +		goto unlock;
> +
> +	reporter->devlink_port = port;
> +	list_add_tail(&reporter->list, &port->reporter_list);
> +unlock:
> +	mutex_unlock(&port->reporters_lock);
> +	return reporter;
> +}
> +EXPORT_SYMBOL_GPL(devlink_port_health_reporter_create);
> +
> +/**
>   *	devlink_health_reporter_create - create devlink health reporter
>   *
>   *	@devlink: devlink
> @@ -5441,6 +5477,20 @@ struct devlink_health_reporter *
>  }
>  EXPORT_SYMBOL_GPL(devlink_health_reporter_destroy);
>  
> +/**
> + *	devlink_port_health_reporter_destroy - destroy devlink port health reporter
> + *
> + *	@reporter: devlink health reporter to destroy
> + */
> +void
> +devlink_port_health_reporter_destroy(struct devlink_health_reporter *reporter)
> +{
> +	mutex_lock(&reporter->devlink_port->reporters_lock);
> +	__devlink_health_reporter_destroy(reporter);
> +	mutex_unlock(&reporter->devlink_port->reporters_lock);
> +}
> +EXPORT_SYMBOL_GPL(devlink_port_health_reporter_destroy);
> +
>  static int
>  devlink_nl_health_reporter_fill(struct sk_buff *msg,
>  				struct devlink *devlink,
> -- 
> 1.8.3.1
> 

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

* Re: [PATCH net-next v3 5/7] devlink: Add devlink health port reporters API
  2020-07-13 13:18   ` Qian Cai
@ 2020-07-13 14:07     ` Ido Schimmel
  0 siblings, 0 replies; 12+ messages in thread
From: Ido Schimmel @ 2020-07-13 14:07 UTC (permalink / raw)
  To: Qian Cai
  Cc: Moshe Shemesh, David S. Miller, Jakub Kicinski, Jiri Pirko,
	netdev, linux-kernel, Vladyslav Tarasiuk

On Mon, Jul 13, 2020 at 09:18:25AM -0400, Qian Cai wrote:
> On Fri, Jul 10, 2020 at 03:25:11PM +0300, Moshe Shemesh wrote:
> > From: Vladyslav Tarasiuk <vladyslavt@mellanox.com>
> > 
> > In order to use new devlink port health reporters infrastructure, add
> > corresponding constructor and destructor functions.
> > 
> > Signed-off-by: Vladyslav Tarasiuk <vladyslavt@mellanox.com>
> > Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
> > Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> 
> This will trigger an use-after-free below while doing SR-IOV,

Yes. I sent a patch for internal review and I'm waiting for Vladyslav
and Moshe to review it. Will copy you once I post it.

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

end of thread, other threads:[~2020-07-13 14:08 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-10 12:25 [PATCH net-next v3 0/7] Add devlink-health support for devlink ports Moshe Shemesh
2020-07-10 12:25 ` [PATCH net-next v3 1/7] devlink: Refactor devlink health reporter constructor Moshe Shemesh
2020-07-10 12:25 ` [PATCH net-next v3 2/7] devlink: Rework devlink health reporter destructor Moshe Shemesh
2020-07-10 12:25 ` [PATCH net-next v3 3/7] devlink: Create generic devlink health reporter search function Moshe Shemesh
2020-07-10 12:25 ` [PATCH net-next v3 4/7] devlink: Implement devlink health reporters on per-port basis Moshe Shemesh
2020-07-10 12:25 ` [PATCH net-next v3 5/7] devlink: Add devlink health port reporters API Moshe Shemesh
2020-07-13 13:18   ` Qian Cai
2020-07-13 14:07     ` Ido Schimmel
2020-07-10 12:25 ` [PATCH net-next v3 6/7] net/mlx5e: Move devlink port register and unregister calls Moshe Shemesh
2020-07-10 12:25 ` [PATCH net-next v3 7/7] net/mlx5e: Move devlink-health rx and tx reporters to devlink port Moshe Shemesh
2020-07-10 21:11 ` [PATCH net-next v3 0/7] Add devlink-health support for devlink ports Jakub Kicinski
2020-07-10 21:32 ` 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).