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

From: David Ahern <redacted>
Sent: Monday, June 7, 2021 9:44 PM

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

quoted

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

quoted

quoted

quoted

quoted

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

quoted

quoted

quoted

quoted

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.

quoted

quoted

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#bre
ak
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.

Thanks David for the detailed explanation.
It totally make sense. I am fixing it.