Add a space (' ') prefix to every userdata line to match docs for
dev-kmsg. To account for this extra character in each userdata entry,
reduce userdata entry names (directory name) from 54 characters to 53.
According to the dev-kmsg docs, a space is used for subsequent lines to
mark them as continuation lines.
> A line starting with ' ', is a continuation line, adding
> key/value pairs to the log message, which provide the machine
> readable context of the message, for reliable processing in
> userspace.
Testing for this patch::
cd /sys/kernel/config/netconsole && mkdir cmdline0
cd cmdline0
mkdir userdata/test && echo "hello" > userdata/test/value
mkdir userdata/test2 && echo "hello2" > userdata/test2/value
echo "message" > /dev/kmsg
Outputs::
6.8.0-rc5-virtme,12,493,231373579,-;message
test=hello
test2=hello2
And I confirmed all testing works as expected from the original patchset
Fixes: df03f830d099 ("net: netconsole: cache userdata formatted string in netconsole_target")
Reviewed-by: Breno Leitao <[email protected]>
Signed-off-by: Matthew Wood <[email protected]>
---
v1 -> v2:
- Calculate 53 byte user data name from: entry length - formatting chars - value length
- Update docs to reflect 53 byte limit for user data name (director)
v2 -> v3:
- Added #define for userdata formatting character length (3)
- Matched all #defines indent level
Documentation/networking/netconsole.rst | 8 ++++----
drivers/net/netconsole.c | 14 +++++++++-----
2 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/Documentation/networking/netconsole.rst b/Documentation/networking/netconsole.rst
index b28c525e5d1e..d55c2a22ec7a 100644
--- a/Documentation/networking/netconsole.rst
+++ b/Documentation/networking/netconsole.rst
@@ -180,7 +180,7 @@ Custom user data can be appended to the end of messages with netconsole
dynamic configuration enabled. User data entries can be modified without
changing the "enabled" attribute of a target.
-Directories (keys) under `userdata` are limited to 54 character length, and
+Directories (keys) under `userdata` are limited to 53 character length, and
data in `userdata/<key>/value` are limited to 200 bytes::
cd /sys/kernel/config/netconsole && mkdir cmdline0
@@ -197,8 +197,8 @@ Messages will now include this additional user data::
Sends::
12,607,22085407756,-;This is a message
- foo=bar
- qux=baz
+ foo=bar
+ qux=baz
Preview the userdata that will be appended with::
@@ -218,7 +218,7 @@ The `qux` key is omitted since it has no value::
echo "This is a message" > /dev/kmsg
12,607,22085407756,-;This is a message
- foo=bar
+ foo=bar
Delete `userdata` entries with `rmdir`::
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 0de108a1c0c8..8cc28aec59c8 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -42,12 +42,16 @@ MODULE_AUTHOR("Maintainer: Matt Mackall <[email protected]>");
MODULE_DESCRIPTION("Console driver for network interfaces");
MODULE_LICENSE("GPL");
-#define MAX_PARAM_LENGTH 256
-#define MAX_USERDATA_NAME_LENGTH 54
-#define MAX_USERDATA_VALUE_LENGTH 200
+#define MAX_PARAM_LENGTH 256
+/* characters used for formatting each userdata entry line (' ', '=', '\n') */
+#define USERDATA_FORMAT_CHARS 3
#define MAX_USERDATA_ENTRY_LENGTH 256
+#define MAX_USERDATA_VALUE_LENGTH 200
+#define MAX_USERDATA_NAME_LENGTH MAX_USERDATA_ENTRY_LENGTH - \
+ MAX_USERDATA_VALUE_LENGTH - \
+ USERDATA_FORMAT_CHARS
#define MAX_USERDATA_ITEMS 16
-#define MAX_PRINT_CHUNK 1000
+#define MAX_PRINT_CHUNK 1000
static char config[MAX_PARAM_LENGTH];
module_param_string(netconsole, config, MAX_PARAM_LENGTH, 0);
@@ -671,7 +675,7 @@ static void update_userdata(struct netconsole_target *nt)
* checked to not exceed MAX items with child_count above
*/
complete_idx += scnprintf(&nt->userdata_complete[complete_idx],
- MAX_USERDATA_ENTRY_LENGTH, "%s=%s\n",
+ MAX_USERDATA_ENTRY_LENGTH, " %s=%s\n",
item->ci_name, udm_item->value);
}
nt->userdata_length = strnlen(nt->userdata_complete,
--
2.44.0
On Fri, 2024-03-01 at 10:58 -0800, Matthew Wood wrote:
> Add a space (' ') prefix to every userdata line to match docs for
> dev-kmsg. To account for this extra character in each userdata entry,
> reduce userdata entry names (directory name) from 54 characters to 53.
>
> According to the dev-kmsg docs, a space is used for subsequent lines to
> mark them as continuation lines.
>
> > A line starting with ' ', is a continuation line, adding
> > key/value pairs to the log message, which provide the machine
> > readable context of the message, for reliable processing in
> > userspace.
>
> Testing for this patch::
>
> cd /sys/kernel/config/netconsole && mkdir cmdline0
> cd cmdline0
> mkdir userdata/test && echo "hello" > userdata/test/value
> mkdir userdata/test2 && echo "hello2" > userdata/test2/value
> echo "message" > /dev/kmsg
>
> Outputs::
>
> 6.8.0-rc5-virtme,12,493,231373579,-;message
> test=hello
> test2=hello2
>
> And I confirmed all testing works as expected from the original patchset
>
> Fixes: df03f830d099 ("net: netconsole: cache userdata formatted string in netconsole_target")
> Reviewed-by: Breno Leitao <[email protected]>
I guess the tag arrived off-list, because I can't see any sign of it on
the ML?!?
> Signed-off-by: Matthew Wood <[email protected]>
> ---
>
> v1 -> v2:
> - Calculate 53 byte user data name from: entry length - formatting chars - value length
> - Update docs to reflect 53 byte limit for user data name (director)
> v2 -> v3:
> - Added #define for userdata formatting character length (3)
> - Matched all #defines indent level
>
> Documentation/networking/netconsole.rst | 8 ++++----
> drivers/net/netconsole.c | 14 +++++++++-----
> 2 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/networking/netconsole.rst b/Documentation/networking/netconsole.rst
> index b28c525e5d1e..d55c2a22ec7a 100644
> --- a/Documentation/networking/netconsole.rst
> +++ b/Documentation/networking/netconsole.rst
> @@ -180,7 +180,7 @@ Custom user data can be appended to the end of messages with netconsole
> dynamic configuration enabled. User data entries can be modified without
> changing the "enabled" attribute of a target.
>
> -Directories (keys) under `userdata` are limited to 54 character length, and
> +Directories (keys) under `userdata` are limited to 53 character length, and
> data in `userdata/<key>/value` are limited to 200 bytes::
>
> cd /sys/kernel/config/netconsole && mkdir cmdline0
> @@ -197,8 +197,8 @@ Messages will now include this additional user data::
> Sends::
>
> 12,607,22085407756,-;This is a message
> - foo=bar
> - qux=baz
> + foo=bar
> + qux=baz
>
> Preview the userdata that will be appended with::
>
> @@ -218,7 +218,7 @@ The `qux` key is omitted since it has no value::
>
> echo "This is a message" > /dev/kmsg
> 12,607,22085407756,-;This is a message
> - foo=bar
> + foo=bar
>
> Delete `userdata` entries with `rmdir`::
>
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 0de108a1c0c8..8cc28aec59c8 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -42,12 +42,16 @@ MODULE_AUTHOR("Maintainer: Matt Mackall <[email protected]>");
> MODULE_DESCRIPTION("Console driver for network interfaces");
> MODULE_LICENSE("GPL");
>
> -#define MAX_PARAM_LENGTH 256
> -#define MAX_USERDATA_NAME_LENGTH 54
> -#define MAX_USERDATA_VALUE_LENGTH 200
> +#define MAX_PARAM_LENGTH 256
> +/* characters used for formatting each userdata entry line (' ', '=', '\n') */
> +#define USERDATA_FORMAT_CHARS 3
> #define MAX_USERDATA_ENTRY_LENGTH 256
> +#define MAX_USERDATA_VALUE_LENGTH 200
> +#define MAX_USERDATA_NAME_LENGTH MAX_USERDATA_ENTRY_LENGTH - \
> + MAX_USERDATA_VALUE_LENGTH - \
> + USERDATA_FORMAT_CHARS
AFAICS this is not what Breno asked, and checkpatch complains
rightfully. More importantly it's fragile: what will be the result of
MAX_USERDATA_NAME_LENGTH * 2
?
At least some brackets are required:
#define MAX_USERDATA_NAME_LENGTH (MAX_USERDATA_ENTRY_LENGTH -
MAX_USERDATA_VALUE_LENGTH - \
USERDATA_FORMAT_CHARS)
Thanks,
Paolo
On Tue, Mar 5, 2024 at 4:14 AM Paolo Abeni <[email protected]> wrote:
>
> On Fri, 2024-03-01 at 10:58 -0800, Matthew Wood wrote:
> > Add a space (' ') prefix to every userdata line to match docs for
> > dev-kmsg. To account for this extra character in each userdata entry,
> > reduce userdata entry names (directory name) from 54 characters to 53.
> >
> > According to the dev-kmsg docs, a space is used for subsequent lines to
> > mark them as continuation lines.
> >
> > > A line starting with ' ', is a continuation line, adding
> > > key/value pairs to the log message, which provide the machine
> > > readable context of the message, for reliable processing in
> > > userspace.
> >
> > Testing for this patch::
> >
> > cd /sys/kernel/config/netconsole && mkdir cmdline0
> > cd cmdline0
> > mkdir userdata/test && echo "hello" > userdata/test/value
> > mkdir userdata/test2 && echo "hello2" > userdata/test2/value
> > echo "message" > /dev/kmsg
> >
> > Outputs::
> >
> > 6.8.0-rc5-virtme,12,493,231373579,-;message
> > test=hello
> > test2=hello2
> >
> > And I confirmed all testing works as expected from the original patchset
> >
> > Fixes: df03f830d099 ("net: netconsole: cache userdata formatted string in netconsole_target")
> > Reviewed-by: Breno Leitao <[email protected]>
>
> I guess the tag arrived off-list, because I can't see any sign of it on
> the ML?!?
>
> > Signed-off-by: Matthew Wood <[email protected]>
> > ---
> >
> > v1 -> v2:
> > - Calculate 53 byte user data name from: entry length - formatting chars - value length
> > - Update docs to reflect 53 byte limit for user data name (director)
> > v2 -> v3:
> > - Added #define for userdata formatting character length (3)
> > - Matched all #defines indent level
> >
> > Documentation/networking/netconsole.rst | 8 ++++----
> > drivers/net/netconsole.c | 14 +++++++++-----
> > 2 files changed, 13 insertions(+), 9 deletions(-)
> >
> > diff --git a/Documentation/networking/netconsole.rst b/Documentation/networking/netconsole.rst
> > index b28c525e5d1e..d55c2a22ec7a 100644
> > --- a/Documentation/networking/netconsole.rst
> > +++ b/Documentation/networking/netconsole.rst
> > @@ -180,7 +180,7 @@ Custom user data can be appended to the end of messages with netconsole
> > dynamic configuration enabled. User data entries can be modified without
> > changing the "enabled" attribute of a target.
> >
> > -Directories (keys) under `userdata` are limited to 54 character length, and
> > +Directories (keys) under `userdata` are limited to 53 character length, and
> > data in `userdata/<key>/value` are limited to 200 bytes::
> >
> > cd /sys/kernel/config/netconsole && mkdir cmdline0
> > @@ -197,8 +197,8 @@ Messages will now include this additional user data::
> > Sends::
> >
> > 12,607,22085407756,-;This is a message
> > - foo=bar
> > - qux=baz
> > + foo=bar
> > + qux=baz
> >
> > Preview the userdata that will be appended with::
> >
> > @@ -218,7 +218,7 @@ The `qux` key is omitted since it has no value::
> >
> > echo "This is a message" > /dev/kmsg
> > 12,607,22085407756,-;This is a message
> > - foo=bar
> > + foo=bar
> >
> > Delete `userdata` entries with `rmdir`::
> >
> > diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> > index 0de108a1c0c8..8cc28aec59c8 100644
> > --- a/drivers/net/netconsole.c
> > +++ b/drivers/net/netconsole.c
> > @@ -42,12 +42,16 @@ MODULE_AUTHOR("Maintainer: Matt Mackall <[email protected]>");
> > MODULE_DESCRIPTION("Console driver for network interfaces");
> > MODULE_LICENSE("GPL");
> >
> > -#define MAX_PARAM_LENGTH 256
> > -#define MAX_USERDATA_NAME_LENGTH 54
> > -#define MAX_USERDATA_VALUE_LENGTH 200
> > +#define MAX_PARAM_LENGTH 256
> > +/* characters used for formatting each userdata entry line (' ', '=', '\n') */
> > +#define USERDATA_FORMAT_CHARS 3
> > #define MAX_USERDATA_ENTRY_LENGTH 256
> > +#define MAX_USERDATA_VALUE_LENGTH 200
> > +#define MAX_USERDATA_NAME_LENGTH MAX_USERDATA_ENTRY_LENGTH - \
> > + MAX_USERDATA_VALUE_LENGTH - \
> > + USERDATA_FORMAT_CHARS
>
> AFAICS this is not what Breno asked, and checkpatch complains
> rightfully. More importantly it's fragile: what will be the result of
>
> MAX_USERDATA_NAME_LENGTH * 2
>
> ?
>
> At least some brackets are required:
>
> #define MAX_USERDATA_NAME_LENGTH (MAX_USERDATA_ENTRY_LENGTH -
> MAX_USERDATA_VALUE_LENGTH - \
> USERDATA_FORMAT_CHARS)
>
> Thanks,
>
> Paolo
>
Thank you Paolo, great points. I will fix the macro and remove the
Reviewed-by tag so Breno can reply on the ML