Received-SPF: pass (google.com: domain of linux-kernel+bounces-85443-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <77497701-3bd2-4f5f-9404-b32d9e91683b@gmail.com>
Date: Wed, 28 Feb 2024 09:32:19 -0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next] net: netconsole: Add continuation line prefix to
 userdata messages
Content-Language: en-US
To: Matthew Wood <thepacketgeek@gmail.com>,
 "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>
Cc: netdev@vger.kernel.org, linux-doc@vger.kernel.org,
 linux-kernel@vger.kernel.org
References: <20240228172948.221910-1-thepacketgeek@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
In-Reply-To: <20240228172948.221910-1-thepacketgeek@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

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 <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/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