On 2/28/24 09:29, 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")
> Signed-off-by: Matthew Wood <[email protected]>
> ---
> Documentation/networking/netconsole.rst | 6 +++---
> drivers/net/netconsole.c | 4 ++--
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/networking/netconsole.rst b/Documentation/networking/netconsole.rst
> index b28c525e5d1e..c2dec12f6060 100644
> --- a/Documentation/networking/netconsole.rst
> +++ b/Documentation/networking/netconsole.rst
> @@ -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..ffd3e19406b5 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -43,7 +43,7 @@ MODULE_DESCRIPTION("Console driver for network interfaces");
> MODULE_LICENSE("GPL");
>
> #define MAX_PARAM_LENGTH 256
> -#define MAX_USERDATA_NAME_LENGTH 54
> +#define MAX_USERDATA_NAME_LENGTH 53 /* 256 - 200 - 3 (for ' =\n' chars)*/
Could we take this opportunity to define MAX_USERDATA_NAME_LENGTH based
upon MAX_PARAM_LENGTH - MAX_USERDATA_VALUE_LENGTH - 3 then?
> #define MAX_USERDATA_VALUE_LENGTH 200
> #define MAX_USERDATA_ENTRY_LENGTH 256
> #define MAX_USERDATA_ITEMS 16
> @@ -671,7 +671,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,
--
Florian
On Wed, Feb 28, 2024 at 9:32 AM Florian Fainelli <[email protected]> wrote:
>
> On 2/28/24 09:29, 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")
> > Signed-off-by: Matthew Wood <[email protected]>
> > ---
> > Documentation/networking/netconsole.rst | 6 +++---
> > drivers/net/netconsole.c | 4 ++--
> > 2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/networking/netconsole.rst b/Documentation/networking/netconsole.rst
> > index b28c525e5d1e..c2dec12f6060 100644
> > --- a/Documentation/networking/netconsole.rst
> > +++ b/Documentation/networking/netconsole.rst
> > @@ -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..ffd3e19406b5 100644
> > --- a/drivers/net/netconsole.c
> > +++ b/drivers/net/netconsole.c
> > @@ -43,7 +43,7 @@ MODULE_DESCRIPTION("Console driver for network interfaces");
> > MODULE_LICENSE("GPL");
> >
> > #define MAX_PARAM_LENGTH 256
> > -#define MAX_USERDATA_NAME_LENGTH 54
> > +#define MAX_USERDATA_NAME_LENGTH 53 /* 256 - 200 - 3 (for ' =\n' chars)*/
>
> Could we take this opportunity to define MAX_USERDATA_NAME_LENGTH based
> upon MAX_PARAM_LENGTH - MAX_USERDATA_VALUE_LENGTH - 3 then?
>
> > #define MAX_USERDATA_VALUE_LENGTH 200
> > #define MAX_USERDATA_ENTRY_LENGTH 256
> > #define MAX_USERDATA_ITEMS 16
> > @@ -671,7 +671,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,
>
> --
> Florian
>
Yes, great suggestion. I'll send an update with this change tomorrow. thank you!