diff mbox series

[iproute2-net,3/3] devlink: Add reload stats to dev show

Message ID 1606389296-3906-4-git-send-email-moshe@mellanox.com (mailing list archive)
State Changes Requested
Delegated to: David Ahern
Headers show
Series devlink: Add devlink reload action limit and stats | expand

Checks

Context Check Description
netdev/apply fail Patch does not apply to net
netdev/tree_selection success Clearly marked for net

Commit Message

Moshe Shemesh Nov. 26, 2020, 11:14 a.m. UTC 
Show reload statistics through devlink dev show using devlink stats
flag. The reload statistics show the history per reload action type and
limit. Add remote reload statistics to show the history of actions
performed due devlink reload commands initiated by remote host.

Output examples:
$ devlink dev show -s
pci/0000:82:00.0:
  stats:
      reload:
          driver_reinit:
            unspecified 2
          fw_activate:
            unspecified 1 no_reset 0
      remote_reload:
          driver_reinit:
            unspecified 0
          fw_activate:
            unspecified 0 no_reset 0
pci/0000:82:00.1:
  stats:
      reload:
          driver_reinit:
            unspecified 0
          fw_activate:
            unspecified 0 no_reset 0
      remote_reload:
          driver_reinit:
            unspecified 1
          fw_activate:
            unspecified 1 no_reset 0

$ devlink dev show -s -jp
{
    "dev": {
        "pci/0000:82:00.0": {
            "stats": {
                "reload": {
                    "driver_reinit": {
                        "unspecified": 2
                    },
                    "fw_activate": {
                        "unspecified": 1,
                        "no_reset": 0
                    }
                },
                "remote_reload": {
                    "driver_reinit": {
                        "unspecified": 0
                    },
                    "fw_activate": {
                        "unspecified": 0,
                        "no_reset": 0
                    }
                }
            }
        },
        "pci/0000:82:00.1": {
            "stats": {
                "reload": {
                    "driver_reinit": {
                        "unspecified": 0
                    },
                    "fw_activate": {
                        "unspecified": 0,
                        "no_reset": 0
                    }
                },
                "remote_reload": {
                    "driver_reinit": {
                        "unspecified": 1
                    },
                    "fw_activate": {
                        "unspecified": 1,
                        "no_reset": 0
                    }
                }
            }
        }
    }
}

Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
---
 devlink/devlink.c            | 102 ++++++++++++++++++++++++++++++++++-
 include/uapi/linux/devlink.h |   2 +
 2 files changed, 102 insertions(+), 2 deletions(-)

Comments

David Ahern Nov. 29, 2020, 9:22 p.m. UTC | #1 
On 11/26/20 4:14 AM, Moshe Shemesh wrote:
> @@ -2975,17 +2996,93 @@ static int cmd_dev_param(struct dl *dl)
>  	return -ENOENT;
>  }
>  
> -static void pr_out_dev(struct dl *dl, struct nlattr **tb)
> +static void pr_out_action_stats(struct dl *dl, struct nlattr *action_stats)
> +{
> +	struct nlattr *tb_stats_entry[DEVLINK_ATTR_MAX + 1] = {};
> +	struct nlattr *reload_stats_entry;
> +	enum devlink_reload_limit limit;
> +	uint32_t value;
> +	int err;
> +
> +	mnl_attr_for_each_nested(reload_stats_entry, action_stats) {
> +		err = mnl_attr_parse_nested(reload_stats_entry, attr_cb, tb_stats_entry);

wrap lines at 80 columns unless it is a print statement.

> +		if (err != MNL_CB_OK)
> +			return;
> +		if (!tb_stats_entry[DEVLINK_ATTR_RELOAD_STATS_LIMIT] ||
> +		    !tb_stats_entry[DEVLINK_ATTR_RELOAD_STATS_VALUE])
> +			return;
> +
> +		check_indent_newline(dl);
> +		limit = mnl_attr_get_u8(tb_stats_entry[DEVLINK_ATTR_RELOAD_STATS_LIMIT]);
> +		value = mnl_attr_get_u32(tb_stats_entry[DEVLINK_ATTR_RELOAD_STATS_VALUE]);

Use temp variables for the attributes to make the code readable.

> +		print_uint_name_value(reload_limit_name(limit), value);
> +	}
> +}
> +

that applies to all of the patches.
Moshe Shemesh Dec. 2, 2020, 7:01 a.m. UTC | #2 
On 11/29/2020 11:22 PM, David Ahern wrote:
> On 11/26/20 4:14 AM, Moshe Shemesh wrote:
>> @@ -2975,17 +2996,93 @@ static int cmd_dev_param(struct dl *dl)
>>   	return -ENOENT;
>>   }
>>   
>> -static void pr_out_dev(struct dl *dl, struct nlattr **tb)
>> +static void pr_out_action_stats(struct dl *dl, struct nlattr *action_stats)
>> +{
>> +	struct nlattr *tb_stats_entry[DEVLINK_ATTR_MAX + 1] = {};
>> +	struct nlattr *reload_stats_entry;
>> +	enum devlink_reload_limit limit;
>> +	uint32_t value;
>> +	int err;
>> +
>> +	mnl_attr_for_each_nested(reload_stats_entry, action_stats) {
>> +		err = mnl_attr_parse_nested(reload_stats_entry, attr_cb, tb_stats_entry);
> wrap lines at 80 columns unless it is a print statement.
Ack.
>
>> +		if (err != MNL_CB_OK)
>> +			return;
>> +		if (!tb_stats_entry[DEVLINK_ATTR_RELOAD_STATS_LIMIT] ||
>> +		    !tb_stats_entry[DEVLINK_ATTR_RELOAD_STATS_VALUE])
>> +			return;
>> +
>> +		check_indent_newline(dl);
>> +		limit = mnl_attr_get_u8(tb_stats_entry[DEVLINK_ATTR_RELOAD_STATS_LIMIT]);
>> +		value = mnl_attr_get_u32(tb_stats_entry[DEVLINK_ATTR_RELOAD_STATS_VALUE]);
> Use temp variables for the attributes to make the code readable.
Ack.
>
>> +		print_uint_name_value(reload_limit_name(limit), value);
>> +	}
>> +}
>> +
> that applies to all of the patches.
>
diff mbox series

Patch

diff --git a/devlink/devlink.c b/devlink/devlink.c
index bd588869..b588ba98 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -682,6 +682,15 @@  static const enum mnl_attr_data_type devlink_policy[DEVLINK_ATTR_MAX + 1] = {
 	[DEVLINK_ATTR_TRAP_METADATA] = MNL_TYPE_NESTED,
 	[DEVLINK_ATTR_TRAP_GROUP_NAME] = MNL_TYPE_STRING,
 	[DEVLINK_ATTR_RELOAD_FAILED] = MNL_TYPE_U8,
+	[DEVLINK_ATTR_DEV_STATS] = MNL_TYPE_NESTED,
+	[DEVLINK_ATTR_RELOAD_STATS] = MNL_TYPE_NESTED,
+	[DEVLINK_ATTR_RELOAD_STATS_ENTRY] = MNL_TYPE_NESTED,
+	[DEVLINK_ATTR_RELOAD_ACTION] = MNL_TYPE_U8,
+	[DEVLINK_ATTR_RELOAD_STATS_LIMIT] = MNL_TYPE_U8,
+	[DEVLINK_ATTR_RELOAD_STATS_VALUE] = MNL_TYPE_U32,
+	[DEVLINK_ATTR_REMOTE_RELOAD_STATS] = MNL_TYPE_NESTED,
+	[DEVLINK_ATTR_RELOAD_ACTION_INFO] = MNL_TYPE_NESTED,
+	[DEVLINK_ATTR_RELOAD_ACTION_STATS] = MNL_TYPE_NESTED,
 	[DEVLINK_ATTR_TRAP_POLICER_ID] = MNL_TYPE_U32,
 	[DEVLINK_ATTR_TRAP_POLICER_RATE] = MNL_TYPE_U64,
 	[DEVLINK_ATTR_TRAP_POLICER_BURST] = MNL_TYPE_U64,
@@ -2370,6 +2379,18 @@  static const char *reload_action_name(uint8_t reload_action)
 	}
 }
 
+static const char *reload_limit_name(uint8_t reload_limit)
+{
+	switch (reload_limit) {
+	case DEVLINK_RELOAD_LIMIT_UNSPEC:
+		return "unspecified";
+	case DEVLINK_RELOAD_LIMIT_NO_RESET:
+		return "no_reset";
+	default:
+		return "<unknown reload action>";
+	}
+}
+
 static const char *eswitch_mode_name(uint32_t mode)
 {
 	switch (mode) {
@@ -2975,17 +2996,93 @@  static int cmd_dev_param(struct dl *dl)
 	return -ENOENT;
 }
 
-static void pr_out_dev(struct dl *dl, struct nlattr **tb)
+static void pr_out_action_stats(struct dl *dl, struct nlattr *action_stats)
+{
+	struct nlattr *tb_stats_entry[DEVLINK_ATTR_MAX + 1] = {};
+	struct nlattr *reload_stats_entry;
+	enum devlink_reload_limit limit;
+	uint32_t value;
+	int err;
+
+	mnl_attr_for_each_nested(reload_stats_entry, action_stats) {
+		err = mnl_attr_parse_nested(reload_stats_entry, attr_cb, tb_stats_entry);
+		if (err != MNL_CB_OK)
+			return;
+		if (!tb_stats_entry[DEVLINK_ATTR_RELOAD_STATS_LIMIT] ||
+		    !tb_stats_entry[DEVLINK_ATTR_RELOAD_STATS_VALUE])
+			return;
+
+		check_indent_newline(dl);
+		limit = mnl_attr_get_u8(tb_stats_entry[DEVLINK_ATTR_RELOAD_STATS_LIMIT]);
+		value = mnl_attr_get_u32(tb_stats_entry[DEVLINK_ATTR_RELOAD_STATS_VALUE]);
+		print_uint_name_value(reload_limit_name(limit), value);
+	}
+}
+
+static void pr_out_reload_stats(struct dl *dl, struct nlattr *reload_stats)
+{
+	struct nlattr *tb_action_info[DEVLINK_ATTR_MAX + 1] = {};
+	enum devlink_reload_action action;
+	struct nlattr *action_info;
+	int err;
+
+	mnl_attr_for_each_nested(action_info, reload_stats) {
+		err = mnl_attr_parse_nested(action_info, attr_cb, tb_action_info);
+		if (err != MNL_CB_OK)
+			return;
+		if (!tb_action_info[DEVLINK_ATTR_RELOAD_ACTION] ||
+		    !tb_action_info[DEVLINK_ATTR_RELOAD_ACTION_STATS])
+			return;
+
+		action = mnl_attr_get_u8(tb_action_info[DEVLINK_ATTR_RELOAD_ACTION]);
+		pr_out_object_start(dl, reload_action_name(action));
+		pr_out_action_stats(dl, tb_action_info[DEVLINK_ATTR_RELOAD_ACTION_STATS]);
+		pr_out_object_end(dl);
+	}
+}
+
+static void pr_out_reload_data(struct dl *dl, struct nlattr **tb)
 {
+	struct nlattr *tb_stats[DEVLINK_ATTR_MAX + 1] = {};
 	uint8_t reload_failed = 0;
+	int err;
 
 	if (tb[DEVLINK_ATTR_RELOAD_FAILED])
 		reload_failed = mnl_attr_get_u8(tb[DEVLINK_ATTR_RELOAD_FAILED]);
 
 	if (reload_failed) {
-		__pr_out_handle_start(dl, tb, true, false);
 		check_indent_newline(dl);
 		print_bool(PRINT_ANY, "reload_failed", "reload_failed %s", true);
+	}
+	if (!tb[DEVLINK_ATTR_DEV_STATS] || !dl->stats)
+		return;
+	err = mnl_attr_parse_nested(tb[DEVLINK_ATTR_DEV_STATS], attr_cb, tb_stats);
+	if (err != MNL_CB_OK)
+		return;
+
+	pr_out_object_start(dl, "stats");
+
+	if (tb_stats[DEVLINK_ATTR_RELOAD_STATS]) {
+		pr_out_object_start(dl, "reload");
+		pr_out_reload_stats(dl, tb_stats[DEVLINK_ATTR_RELOAD_STATS]);
+		pr_out_object_end(dl);
+	}
+	if (tb_stats[DEVLINK_ATTR_REMOTE_RELOAD_STATS]) {
+		pr_out_object_start(dl, "remote_reload");
+		pr_out_reload_stats(dl, tb_stats[DEVLINK_ATTR_REMOTE_RELOAD_STATS]);
+		pr_out_object_end(dl);
+	}
+
+	pr_out_object_end(dl);
+}
+
+
+static void pr_out_dev(struct dl *dl, struct nlattr **tb)
+{
+	if ((tb[DEVLINK_ATTR_RELOAD_FAILED] && mnl_attr_get_u8(tb[DEVLINK_ATTR_RELOAD_FAILED])) ||
+	    (tb[DEVLINK_ATTR_DEV_STATS] && dl->stats)) {
+		__pr_out_handle_start(dl, tb, true, false);
+		pr_out_reload_data(dl, tb);
 		pr_out_handle_end(dl);
 	} else {
 		pr_out_handle(dl, tb);
@@ -4844,6 +4941,7 @@  static int cmd_mon_show_cb(const struct nlmsghdr *nlh, void *data)
 		if (!tb[DEVLINK_ATTR_BUS_NAME] || !tb[DEVLINK_ATTR_DEV_NAME])
 			return MNL_CB_ERROR;
 		pr_out_mon_header(genl->cmd);
+		dl->stats = true;
 		pr_out_dev(dl, tb);
 		pr_out_mon_footer();
 		break;
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index e639a4e5..6ffa0121 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -526,6 +526,8 @@  enum devlink_attr {
 	DEVLINK_ATTR_RELOAD_STATS_LIMIT,	/* u8 */
 	DEVLINK_ATTR_RELOAD_STATS_VALUE,	/* u32 */
 	DEVLINK_ATTR_REMOTE_RELOAD_STATS,	/* nested */
+	DEVLINK_ATTR_RELOAD_ACTION_INFO,	/* nested */
+	DEVLINK_ATTR_RELOAD_ACTION_STATS,	/* nested */
 
 	/* add new attributes above here, update the policy in devlink.c */