Received-SPF: pass (google.com: domain of linux-kernel+bounces-85643-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240228172948.221910-1-thepacketgeek@gmail.com> <77497701-3bd2-4f5f-9404-b32d9e91683b@gmail.com>
In-Reply-To: <77497701-3bd2-4f5f-9404-b32d9e91683b@gmail.com>
From: Matthew Wood <thepacketgeek@gmail.com>
Date: Wed, 28 Feb 2024 11:51:24 -0800
Message-ID: <CADvopva5LAe2WQa8N83mhi68++GRvh1Uw8SqatYY79uEOEZTEQ@mail.gmail.com>
Subject: Re: [PATCH net-next] net: netconsole: Add continuation line prefix to
 userdata messages
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, 
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Jonathan Corbet <corbet@lwn.net>, 
	Breno Leitao <leitao@debian.org>, netdev@vger.kernel.org, linux-doc@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Feb 28, 2024 at 9:32=E2=80=AFAM Florian Fainelli <f.fainelli@gmail.=
com> 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=3Dhello
> >    test2=3Dhello2
> >
> > And I confirmed all testing works as expected from the original patchse=
t
> >
> > Fixes: df03f830d099 ("net: netconsole: cache userdata formatted string =
in netconsole_target")
> > Signed-off-by: Matthew Wood <thepacketgeek@gmail.com>
> > ---
> >   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/ne=
tworking/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=3Dbar
> > - qux=3Dbaz
> > +  foo=3Dbar
> > +  qux=3Dbaz
> >
> >   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=3Dbar
> > +  foo=3Dbar
> >
> >   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 interf=
aces");
> >   MODULE_LICENSE("GPL");
> >
> >   #define MAX_PARAM_LENGTH    256
> > -#define MAX_USERDATA_NAME_LENGTH     54
> > +#define MAX_USERDATA_NAME_LENGTH     53 /* 256 - 200 - 3 (for ' =3D\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_targe=
t *nt)
> >                * checked to not exceed MAX items with child_count above
> >                */
> >               complete_idx +=3D scnprintf(&nt->userdata_complete[comple=
te_idx],
> > -                                       MAX_USERDATA_ENTRY_LENGTH, "%s=
=3D%s\n",
> > +                                       MAX_USERDATA_ENTRY_LENGTH, " %s=
=3D%s\n",
> >                                         item->ci_name, udm_item->value)=
;
> >       }
> >       nt->userdata_length =3D strnlen(nt->userdata_complete,
>
> --
> Florian
>

Yes, great suggestion. I'll send an update with this change tomorrow. thank=
 you!