Re: [PATCH RESEND iproute2-next] devlink: Add optional controller user input

On 6/7/21 9:12 AM, Parav Pandit wrote:

quoted

From: David Ahern <redacted>
Sent: Monday, June 7, 2021 8:11 PM

On 6/7/21 5:43 AM, Parav Pandit wrote:

quoted

Hi David,

quoted

From: David Ahern <redacted>
Sent: Monday, June 7, 2021 8:31 AM

On 6/3/21 5:19 AM, Parav Pandit wrote:

quoted

@@ -3795,7 +3806,7 @@ static void cmd_port_help(void)
 	pr_err("       devlink port param set DEV/PORT_INDEX name

PARAMETER value VALUE cmode { permanent | driverinit | runtime }\n");

quoted

 	pr_err("       devlink port param show [DEV/PORT_INDEX name

PARAMETER]\n");

quoted

 	pr_err("       devlink port health show [ DEV/PORT_INDEX reporter

REPORTER_NAME ]\n");

quoted

-	pr_err("       devlink port add DEV/PORT_INDEX flavour FLAVOUR

pfnum PFNUM [ sfnum SFNUM ]\n");

quoted

+	pr_err("       devlink port add DEV/PORT_INDEX flavour FLAVOUR

pfnum PFNUM [ sfnum SFNUM ] [ controller CNUM ]\n");

quoted

 	pr_err("       devlink port del DEV/PORT_INDEX\n");
 }

@@ -4324,7 +4335,7 @@ static int __cmd_health_show(struct dl *dl,

bool show_device, bool show_port);

 static void cmd_port_add_help(void)  {
-	pr_err("       devlink port add { DEV | DEV/PORT_INDEX } flavour

FLAVOUR pfnum PFNUM [ sfnum SFNUM ]\n");

quoted

+	pr_err("       devlink port add { DEV | DEV/PORT_INDEX } flavour

FLAVOUR pfnum PFNUM [ sfnum SFNUM ] [ controller CNUM ]\n");

This line and the one above need to be wrapped. This addition puts it
well into the 90s.

It’s a print message.
I was following coding style of [1] that says "However, never break user-

visible strings such as printk messages because that breaks the ability to grep
for them.".

quoted

Recent code of dcb_ets.c has similar long string in print. So I didn't wrap it.

I missed that when reviewing the dcb command then.

quoted

Should we warp it?

[1]
https://www.kernel.org/doc/html/latest/process/coding-style.html#break
ing-long-lines-and-strings

[1] is referring to messages from kernel code, and I agree with that style. This
is help message from iproute2. I tend to keep my terminal widths between
80 and 90 columns, so the long help lines from commands are not very
friendly causing me to resize the terminal.

I see. So do you recommend splitting the print message?
I personally feel easier to follow kernel coding standard as much possible in spirit of "grep them". 😊
But its really up to you. Please let me know.

There are different type of strings:
1. help,
2. error messages,
3. informational messages,
4. displaying a configuration

1. is "how do I use this command". There is no reason to make that 1
gigantic line. All of the iproute2 commands wrap the help. My comment
above is on this category.

2. and 3. should not be wrapped to allow someone to attempt to go from
"why did I get this output" to a line of code (or many lines). This is
the kernel reference above.

4. Displaying attributes and settings for some object getting dumped.
The lines can get really long and unreadable to humans; these should be
split across multiple lines - like iproute2 commands do. There is no
reason for this to be on online unless the user asks for it via -oneline
option.