diff mbox series

[iproute2-next] devlink: introduce -h[ex] cmdline option to allow dumping numbers in hex format

Message ID 20220419171637.1147925-1-jiri@resnulli.us (mailing list archive)
State Superseded
Delegated to: David Ahern
Headers show
Series [iproute2-next] devlink: introduce -h[ex] cmdline option to allow dumping numbers in hex format | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Jiri Pirko April 19, 2022, 5:16 p.m. UTC 
From: Jiri Pirko <jiri@nvidia.com>

For health reporter dumps it is quite convenient to have the numbers in
hexadecimal format. Introduce a command line option to allow user to
achieve that output.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 devlink/devlink.c  | 19 +++++++++++++------
 man/man8/devlink.8 |  4 ++++
 2 files changed, 17 insertions(+), 6 deletions(-)

Comments

Shannon Nelson April 22, 2022, 9:36 p.m. UTC | #1 
On 4/19/22 10:16 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> For health reporter dumps it is quite convenient to have the numbers in
> hexadecimal format. Introduce a command line option to allow user to
> achieve that output.
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
>   devlink/devlink.c  | 19 +++++++++++++------
>   man/man8/devlink.8 |  4 ++++
>   2 files changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/devlink/devlink.c b/devlink/devlink.c
> index aab739f7f437..bc681737ec8a 100644
> --- a/devlink/devlink.c
> +++ b/devlink/devlink.c
> @@ -367,6 +367,7 @@ struct dl {
>   	bool pretty_output;
>   	bool verbose;
>   	bool stats;
> +	bool hex;
>   	struct {
>   		bool present;
>   		char *bus_name;
> @@ -8044,6 +8045,8 @@ static int cmd_health_dump_clear(struct dl *dl)
>   
>   static int fmsg_value_show(struct dl *dl, int type, struct nlattr *nl_data)
>   {
> +	const char *num_fmt = dl->hex ? "%x" : "%u";
> +	const char *num64_fmt = dl->hex ? "%"PRIx64 : "%"PRIu64;

Can we get a leading "0x" on these to help identify that they are hex 
digits?

>   	uint8_t *data;
>   	uint32_t len;
>   
> @@ -8053,16 +8056,16 @@ static int fmsg_value_show(struct dl *dl, int type, struct nlattr *nl_data)
>   		print_bool(PRINT_ANY, NULL, "%s", mnl_attr_get_u8(nl_data));
>   		break;
>   	case MNL_TYPE_U8:
> -		print_uint(PRINT_ANY, NULL, "%u", mnl_attr_get_u8(nl_data));
> +		print_uint(PRINT_ANY, NULL, num_fmt, mnl_attr_get_u8(nl_data));
>   		break;
>   	case MNL_TYPE_U16:
> -		print_uint(PRINT_ANY, NULL, "%u", mnl_attr_get_u16(nl_data));
> +		print_uint(PRINT_ANY, NULL, num_fmt, mnl_attr_get_u16(nl_data));
>   		break;
>   	case MNL_TYPE_U32:
> -		print_uint(PRINT_ANY, NULL, "%u", mnl_attr_get_u32(nl_data));
> +		print_uint(PRINT_ANY, NULL, num_fmt, mnl_attr_get_u32(nl_data));
>   		break;
>   	case MNL_TYPE_U64:
> -		print_u64(PRINT_ANY, NULL, "%"PRIu64, mnl_attr_get_u64(nl_data));
> +		print_u64(PRINT_ANY, NULL, num64_fmt, mnl_attr_get_u64(nl_data));
>   		break;
>   	case MNL_TYPE_NUL_STRING:
>   		print_string(PRINT_ANY, NULL, "%s", mnl_attr_get_str(nl_data));
> @@ -8939,7 +8942,7 @@ static void help(void)
>   	pr_err("Usage: devlink [ OPTIONS ] OBJECT { COMMAND | help }\n"
>   	       "       devlink [ -f[orce] ] -b[atch] filename -N[etns] netnsname\n"
>   	       "where  OBJECT := { dev | port | sb | monitor | dpipe | resource | region | health | trap }\n"
> -	       "       OPTIONS := { -V[ersion] | -n[o-nice-names] | -j[son] | -p[retty] | -v[erbose] -s[tatistics] }\n");
> +	       "       OPTIONS := { -V[ersion] | -n[o-nice-names] | -j[son] | -p[retty] | -v[erbose] -s[tatistics] -h[ex] }\n");
>   }
>   
>   static int dl_cmd(struct dl *dl, int argc, char **argv)
> @@ -9053,6 +9056,7 @@ int main(int argc, char **argv)
>   		{ "statistics",		no_argument,		NULL, 's' },
>   		{ "Netns",		required_argument,	NULL, 'N' },
>   		{ "iec",		no_argument,		NULL, 'i' },
> +		{ "hex",		no_argument,		NULL, 'h' },

Can we use 'x' instead of 'h' here?  Most times '-h' means 'help', and 
might surprise unsuspecting users when it isn't a help flag.

Thanks,
sln

>   		{ NULL, 0, NULL, 0 }
>   	};
>   	const char *batch_file = NULL;
> @@ -9068,7 +9072,7 @@ int main(int argc, char **argv)
>   		return EXIT_FAILURE;
>   	}
>   
> -	while ((opt = getopt_long(argc, argv, "Vfb:njpvsN:i",
> +	while ((opt = getopt_long(argc, argv, "Vfb:njpvsN:ih",
>   				  long_options, NULL)) >= 0) {
>   
>   		switch (opt) {
> @@ -9106,6 +9110,9 @@ int main(int argc, char **argv)
>   		case 'i':
>   			use_iec = true;
>   			break;
> +		case 'h':
> +			dl->hex = true;
> +			break;
>   		default:
>   			pr_err("Unknown option.\n");
>   			help();
> diff --git a/man/man8/devlink.8 b/man/man8/devlink.8
> index 840cf44cf97b..3797a27cefc5 100644
> --- a/man/man8/devlink.8
> +++ b/man/man8/devlink.8
> @@ -63,6 +63,10 @@ Switches to the specified network namespace.
>   .BR "\-i", " --iec"
>   Print human readable rates in IEC units (e.g. 1Ki = 1024).
>   
> +.TP
> +.BR "\-h", " --hex"
> +Print dump numbers in hexadecimal format.
> +
>   .SS
>   .I OBJECT
>
Stephen Hemminger April 22, 2022, 11:10 p.m. UTC | #2 
On Fri, 22 Apr 2022 14:36:21 -0700
Shannon Nelson <snelson@pensando.io> wrote:

> >   static int fmsg_value_show(struct dl *dl, int type, struct nlattr *nl_data)
> >   {
> > +	const char *num_fmt = dl->hex ? "%x" : "%u";
> > +	const char *num64_fmt = dl->hex ? "%"PRIx64 : "%"PRIu64;  
> 
> Can we get a leading "0x" on these to help identify that they are hex 
> digits?

Yes use %#x
David Ahern April 23, 2022, 12:20 a.m. UTC | #3 
On 4/22/22 3:36 PM, Shannon Nelson wrote:
>> @@ -9053,6 +9056,7 @@ int main(int argc, char **argv)
>>           { "statistics",        no_argument,        NULL, 's' },
>>           { "Netns",        required_argument,    NULL, 'N' },
>>           { "iec",        no_argument,        NULL, 'i' },
>> +        { "hex",        no_argument,        NULL, 'h' },
> 
> Can we use 'x' instead of 'h' here?  Most times '-h' means 'help', and
> might surprise unsuspecting users when it isn't a help flag.
> 

agreed. -h almost always means help
Jiri Pirko April 25, 2022, 9:31 a.m. UTC | #4 
Sat, Apr 23, 2022 at 01:10:54AM CEST, stephen@networkplumber.org wrote:
>On Fri, 22 Apr 2022 14:36:21 -0700
>Shannon Nelson <snelson@pensando.io> wrote:
>
>> >   static int fmsg_value_show(struct dl *dl, int type, struct nlattr *nl_data)
>> >   {
>> > +	const char *num_fmt = dl->hex ? "%x" : "%u";
>> > +	const char *num64_fmt = dl->hex ? "%"PRIx64 : "%"PRIu64;  
>> 
>> Can we get a leading "0x" on these to help identify that they are hex 
>> digits?
>
>Yes use %#x

Changed in v2.
Jiri Pirko April 25, 2022, 9:31 a.m. UTC | #5 
Sat, Apr 23, 2022 at 02:20:06AM CEST, dsahern@gmail.com wrote:
>On 4/22/22 3:36 PM, Shannon Nelson wrote:
>>> @@ -9053,6 +9056,7 @@ int main(int argc, char **argv)
>>>           { "statistics",        no_argument,        NULL, 's' },
>>>           { "Netns",        required_argument,    NULL, 'N' },
>>>           { "iec",        no_argument,        NULL, 'i' },
>>> +        { "hex",        no_argument,        NULL, 'h' },
>> 
>> Can we use 'x' instead of 'h' here?  Most times '-h' means 'help', and
>> might surprise unsuspecting users when it isn't a help flag.
>> 
>
>agreed. -h almost always means help

Changed in v2.
diff mbox series

Patch

diff --git a/devlink/devlink.c b/devlink/devlink.c
index aab739f7f437..bc681737ec8a 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -367,6 +367,7 @@  struct dl {
 	bool pretty_output;
 	bool verbose;
 	bool stats;
+	bool hex;
 	struct {
 		bool present;
 		char *bus_name;
@@ -8044,6 +8045,8 @@  static int cmd_health_dump_clear(struct dl *dl)
 
 static int fmsg_value_show(struct dl *dl, int type, struct nlattr *nl_data)
 {
+	const char *num_fmt = dl->hex ? "%x" : "%u";
+	const char *num64_fmt = dl->hex ? "%"PRIx64 : "%"PRIu64;
 	uint8_t *data;
 	uint32_t len;
 
@@ -8053,16 +8056,16 @@  static int fmsg_value_show(struct dl *dl, int type, struct nlattr *nl_data)
 		print_bool(PRINT_ANY, NULL, "%s", mnl_attr_get_u8(nl_data));
 		break;
 	case MNL_TYPE_U8:
-		print_uint(PRINT_ANY, NULL, "%u", mnl_attr_get_u8(nl_data));
+		print_uint(PRINT_ANY, NULL, num_fmt, mnl_attr_get_u8(nl_data));
 		break;
 	case MNL_TYPE_U16:
-		print_uint(PRINT_ANY, NULL, "%u", mnl_attr_get_u16(nl_data));
+		print_uint(PRINT_ANY, NULL, num_fmt, mnl_attr_get_u16(nl_data));
 		break;
 	case MNL_TYPE_U32:
-		print_uint(PRINT_ANY, NULL, "%u", mnl_attr_get_u32(nl_data));
+		print_uint(PRINT_ANY, NULL, num_fmt, mnl_attr_get_u32(nl_data));
 		break;
 	case MNL_TYPE_U64:
-		print_u64(PRINT_ANY, NULL, "%"PRIu64, mnl_attr_get_u64(nl_data));
+		print_u64(PRINT_ANY, NULL, num64_fmt, mnl_attr_get_u64(nl_data));
 		break;
 	case MNL_TYPE_NUL_STRING:
 		print_string(PRINT_ANY, NULL, "%s", mnl_attr_get_str(nl_data));
@@ -8939,7 +8942,7 @@  static void help(void)
 	pr_err("Usage: devlink [ OPTIONS ] OBJECT { COMMAND | help }\n"
 	       "       devlink [ -f[orce] ] -b[atch] filename -N[etns] netnsname\n"
 	       "where  OBJECT := { dev | port | sb | monitor | dpipe | resource | region | health | trap }\n"
-	       "       OPTIONS := { -V[ersion] | -n[o-nice-names] | -j[son] | -p[retty] | -v[erbose] -s[tatistics] }\n");
+	       "       OPTIONS := { -V[ersion] | -n[o-nice-names] | -j[son] | -p[retty] | -v[erbose] -s[tatistics] -h[ex] }\n");
 }
 
 static int dl_cmd(struct dl *dl, int argc, char **argv)
@@ -9053,6 +9056,7 @@  int main(int argc, char **argv)
 		{ "statistics",		no_argument,		NULL, 's' },
 		{ "Netns",		required_argument,	NULL, 'N' },
 		{ "iec",		no_argument,		NULL, 'i' },
+		{ "hex",		no_argument,		NULL, 'h' },
 		{ NULL, 0, NULL, 0 }
 	};
 	const char *batch_file = NULL;
@@ -9068,7 +9072,7 @@  int main(int argc, char **argv)
 		return EXIT_FAILURE;
 	}
 
-	while ((opt = getopt_long(argc, argv, "Vfb:njpvsN:i",
+	while ((opt = getopt_long(argc, argv, "Vfb:njpvsN:ih",
 				  long_options, NULL)) >= 0) {
 
 		switch (opt) {
@@ -9106,6 +9110,9 @@  int main(int argc, char **argv)
 		case 'i':
 			use_iec = true;
 			break;
+		case 'h':
+			dl->hex = true;
+			break;
 		default:
 			pr_err("Unknown option.\n");
 			help();
diff --git a/man/man8/devlink.8 b/man/man8/devlink.8
index 840cf44cf97b..3797a27cefc5 100644
--- a/man/man8/devlink.8
+++ b/man/man8/devlink.8
@@ -63,6 +63,10 @@  Switches to the specified network namespace.
 .BR "\-i", " --iec"
 Print human readable rates in IEC units (e.g. 1Ki = 1024).
 
+.TP
+.BR "\-h", " --hex"
+Print dump numbers in hexadecimal format.
+
 .SS
 .I OBJECT