netdev - Re: [PATCH v2 iproute2-next 05/11] devlink: Add devlink health show command

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 21 Feb 2019 15:25:30 +0100
From:   Jiri Pirko <jiri@...nulli.us>
To:     Aya Levin <ayal@...lanox.com>
Cc:     David Ahern <dsahern@...il.com>, netdev@...r.kernel.org,
        Jiri Pirko <jiri@...lanox.com>,
        Moshe Shemesh <moshe@...lanox.com>,
        Eran Ben Elisha <eranbe@...lanox.com>
Subject: Re: [PATCH v2 iproute2-next 05/11] devlink: Add devlink health show
 command

Thu, Feb 21, 2019 at 02:42:41PM CET, ayal@...lanox.com wrote:
>Add devlink health show command which displays status and configuration
>info on a specific reporter on a device or dump the info on all
>reporters on all devices. The patch also contains helper functions to

What "patch"? Please format the description as you would command the
codebase what to do.


>display status and dump's time stamp.

[...]


>+static void pr_out_health(struct dl *dl, struct nlattr **tb_health)
>+{
>+	struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
>+	enum devlink_health_reporter_state state;
>+	const struct nlattr *attr;
>+	uint64_t time_ms;
>+	int err;
>+
>+	state = DEVLINK_HEALTH_REPORTER_STATE_HEALTHY;

Pointless assignment.


>+
>+	err = mnl_attr_parse_nested(tb_health[DEVLINK_ATTR_HEALTH_REPORTER],
>+				    attr_cb, tb);
>+	if (err != MNL_CB_OK)
>+		return;
>+
>+	if (!tb[DEVLINK_ATTR_HEALTH_REPORTER_NAME] ||
>+	    !tb[DEVLINK_ATTR_HEALTH_REPORTER_ERR_COUNT] ||
>+	    !tb[DEVLINK_ATTR_HEALTH_REPORTER_RECOVER_COUNT] ||
>+	    !tb[DEVLINK_ATTR_HEALTH_REPORTER_STATE])
>+		return;
>+
>+	pr_out_handle_start_arr(dl, tb_health);
>+
>+	pr_out_str(dl, "name",
>+		   mnl_attr_get_str(tb[DEVLINK_ATTR_HEALTH_REPORTER_NAME]));
>+	if (!dl->json_output) {
>+		__pr_out_newline();
>+		__pr_out_indent_inc();
>+	}
>+	state = mnl_attr_get_u8(tb[DEVLINK_ATTR_HEALTH_REPORTER_STATE]);
>+	pr_out_str(dl, "state", health_state_name(state));
>+	pr_out_u64(dl, "error",
>+		   mnl_attr_get_u64(tb[DEVLINK_ATTR_HEALTH_REPORTER_ERR_COUNT]));
>+	pr_out_u64(dl, "recover",
>+		   mnl_attr_get_u64(tb[DEVLINK_ATTR_HEALTH_REPORTER_RECOVER_COUNT]));
>+	if (tb[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS]) {
>+		char dump_date[HEALTH_REPORTER_TIMESTAMP_FMT_LEN];
>+		char dump_time[HEALTH_REPORTER_TIMESTAMP_FMT_LEN];

Newline here please.


>+		attr = tb[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS];
>+		time_ms = mnl_attr_get_u64(attr);
>+		format_logtime(time_ms, dump_date, dump_time);
>+
>+		pr_out_str(dl, "last_dump_date", dump_date);
>+		pr_out_str(dl, "last_dump_time", dump_time);
>+	}

[...]


>@@ -5776,7 +5958,11 @@ static int dl_cmd(struct dl *dl, int argc, char **argv)
> 	} else if (dl_argv_match(dl, "region")) {
> 		dl_arg_inc(dl);
> 		return cmd_region(dl);
>+	} else if (dl_argv_match(dl, "health")) {
>+		dl_arg_inc(dl);
>+		return cmd_health(dl);
> 	}
>+

Please avoid newlines like this. Unrelated to the patch.


> 	pr_err("Object \"%s\" not found\n", dl_argv(dl));
> 	return -ENOENT;
> }
>-- 
>2.14.1
>