[PATCH net-next v6 0/2] netconsole: Enable compile time configuration

[PATCH net-next v6 0/2] netconsole: Enable compile time configuration

[Breno Leitao]

Enable netconsole features to be set at compilation time. Create two
Kconfig options that allow users to set extended logs and release
prepending features at compilation time.

The first patch de-duplicates the initialization code, and the second
patch adds the support in the de-duplicated code, avoiding touching two
different functions with the same change.

  v1 -> v2:
	* Improvements in the Kconfig help section.

  v2 -> v3:
	* Honour the Kconfig settings when creating sysfs targets
	* Add "by default" in a Kconfig help.

  v3 -> v4:
	* Create an additional patch, de-duplicating the initialization
	  code for netconsole_target, and just patching it.

  v4 -> v5:
	* Remove complex code in the variable initialization.

  v5 -> v6:
	* Return -EINVAL for invalid configuration during
	  initialization.

Breno Leitao (2):
  netconsole: Create a allocation helper
  netconsole: Enable compile time configuration

 drivers/net/Kconfig      | 22 ++++++++++++++++++
 drivers/net/netconsole.c | 48 +++++++++++++++++++++++-----------------
 2 files changed, 50 insertions(+), 20 deletions(-)

-- 
2.34.1

[PATCH net-next v6 1/2] netconsole: Create a allocation helper

[Breno Leitao]

De-duplicate the initialization and allocation code for struct
netconsole_target.

The same allocation and initialization code is duplicated in two
different places in the netconsole subsystem, when the netconsole target
is initialized by command line parameters (alloc_param_target()), and
dynamically by sysfs (make_netconsole_target()).

Create a helper function, and call it from the two different functions.

Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/net/netconsole.c | 43 +++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 87f18aedd3bd..670b6f0a054c 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -167,19 +167,16 @@ static void netconsole_target_put(struct netconsole_target *nt)
 
 #endif	/* CONFIG_NETCONSOLE_DYNAMIC */
 
-/* Allocate new target (from boot/module param) and setup netpoll for it */
-static struct netconsole_target *alloc_param_target(char *target_config)
+/* Allocate and initialize with defaults.
+ * Note that these targets get their config_item fields zeroed-out.
+ */
+static struct netconsole_target *alloc_and_init(void)
 {
-	int err = -ENOMEM;
 	struct netconsole_target *nt;
 
-	/*
-	 * Allocate and initialize with defaults.
-	 * Note that these targets get their config_item fields zeroed-out.
-	 */
 	nt = kzalloc(sizeof(*nt), GFP_KERNEL);
 	if (!nt)
-		goto fail;
+		return nt;
 
 	nt->np.name = "netconsole";
 	strscpy(nt->np.dev_name, "eth0", IFNAMSIZ);
@@ -187,6 +184,21 @@ static struct netconsole_target *alloc_param_target(char *target_config)
 	nt->np.remote_port = 6666;
 	eth_broadcast_addr(nt->np.remote_mac);
 
+	return nt;
+}
+
+/* Allocate new target (from boot/module param) and setup netpoll for it */
+static struct netconsole_target *alloc_param_target(char *target_config)
+{
+	struct netconsole_target *nt;
+	int err;
+
+	nt = alloc_and_init();
+	if (!nt) {
+		err = -ENOMEM;
+		goto fail;
+	}
+
 	if (*target_config == '+') {
 		nt->extended = true;
 		target_config++;
@@ -195,6 +207,7 @@ static struct netconsole_target *alloc_param_target(char *target_config)
 	if (*target_config == 'r') {
 		if (!nt->extended) {
 			pr_err("Netconsole configuration error. Release feature requires extended log message");
+			err = -EINVAL;
 			goto fail;
 		}
 		nt->release = true;
@@ -664,23 +677,13 @@ static const struct config_item_type netconsole_target_type = {
 static struct config_item *make_netconsole_target(struct config_group *group,
 						  const char *name)
 {
-	unsigned long flags;
 	struct netconsole_target *nt;
+	unsigned long flags;
 
-	/*
-	 * Allocate and initialize with defaults.
-	 * Target is disabled at creation (!enabled).
-	 */
-	nt = kzalloc(sizeof(*nt), GFP_KERNEL);
+	nt = alloc_and_init();
 	if (!nt)
 		return ERR_PTR(-ENOMEM);
 
-	nt->np.name = "netconsole";
-	strscpy(nt->np.dev_name, "eth0", IFNAMSIZ);
-	nt->np.local_port = 6665;
-	nt->np.remote_port = 6666;
-	eth_broadcast_addr(nt->np.remote_mac);
-
 	/* Initialize the config_item member */
 	config_item_init_type_name(&nt->item, name, &netconsole_target_type);
 
-- 
2.34.1

[PATCH net-next v6 2/2] netconsole: Enable compile time configuration

[Breno Leitao]

Enable netconsole features to be set at compilation time. Create two
Kconfig options that allow users to set extended logs and release
prepending features at compilation time.

Right now, the user needs to pass command line parameters to netconsole,
such as "+"/"r" to enable extended logs and version prepending features.

With these two options, the user could set the default values for the
features at compile time, and don't need to pass it in the command line
to get them enabled, simplifying the command line.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/net/Kconfig      | 22 ++++++++++++++++++++++
 drivers/net/netconsole.c |  5 +++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 368c6f5b327e..55fb9509bcae 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -332,6 +332,28 @@ config NETCONSOLE_DYNAMIC
 	  at runtime through a userspace interface exported using configfs.
 	  See <file:Documentation/networking/netconsole.rst> for details.
 
+config NETCONSOLE_EXTENDED_LOG
+	bool "Set kernel extended message by default"
+	depends on NETCONSOLE
+	default n
+	help
+	  Set extended log support for netconsole message. If this option is
+	  set, log messages are transmitted with extended metadata header in a
+	  format similar to /dev/kmsg.  See
+	  <file:Documentation/networking/netconsole.rst> for details.
+
+config NETCONSOLE_PREPEND_RELEASE
+	bool "Prepend kernel release version in the message by default"
+	depends on NETCONSOLE_EXTENDED_LOG
+	default n
+	help
+	  Set kernel release to be prepended to each netconsole message by
+	  default. If this option is set, the kernel release is prepended into
+	  the first field of every netconsole message, so, the netconsole
+	  server/peer can easily identify what kernel release is logging each
+	  message.  See <file:Documentation/networking/netconsole.rst> for
+	  details.
+
 config NETPOLL
 	def_bool NETCONSOLE
 
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 670b6f0a054c..3111e1648592 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -178,6 +178,11 @@ static struct netconsole_target *alloc_and_init(void)
 	if (!nt)
 		return nt;
 
+	if (IS_ENABLED(CONFIG_NETCONSOLE_EXTENDED_LOG))
+		nt->extended = true;
+	if (IS_ENABLED(CONFIG_NETCONSOLE_PREPEND_RELEASE))
+		nt->release = true;
+
 	nt->np.name = "netconsole";
 	strscpy(nt->np.dev_name, "eth0", IFNAMSIZ);
 	nt->np.local_port = 6665;
-- 
2.34.1

Re: [PATCH net-next v6 0/2] netconsole: Enable compile time configuration

[Simon Horman]

On Fri, Aug 11, 2023 at 02:31:56AM -0700, Breno Leitao wrote:
> Enable netconsole features to be set at compilation time. Create two
> Kconfig options that allow users to set extended logs and release
> prepending features at compilation time.
> 
> The first patch de-duplicates the initialization code, and the second
> patch adds the support in the de-duplicated code, avoiding touching two
> different functions with the same change.
> 
>   v1 -> v2:
> 	* Improvements in the Kconfig help section.
> 
>   v2 -> v3:
> 	* Honour the Kconfig settings when creating sysfs targets
> 	* Add "by default" in a Kconfig help.
> 
>   v3 -> v4:
> 	* Create an additional patch, de-duplicating the initialization
> 	  code for netconsole_target, and just patching it.
> 
>   v4 -> v5:
> 	* Remove complex code in the variable initialization.
> 
>   v5 -> v6:
> 	* Return -EINVAL for invalid configuration during
> 	  initialization.
> 
> Breno Leitao (2):
>   netconsole: Create a allocation helper
>   netconsole: Enable compile time configuration
> 
>  drivers/net/Kconfig      | 22 ++++++++++++++++++
>  drivers/net/netconsole.c | 48 +++++++++++++++++++++++-----------------
>  2 files changed, 50 insertions(+), 20 deletions(-)

Thanks Breno,

thanks for addressing the feedback given for previous revisions.

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next v6 0/2] netconsole: Enable compile time configuration

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 11 Aug 2023 02:31:56 -0700 you wrote:
> Enable netconsole features to be set at compilation time. Create two
> Kconfig options that allow users to set extended logs and release
> prepending features at compilation time.
> 
> The first patch de-duplicates the initialization code, and the second
> patch adds the support in the de-duplicated code, avoiding touching two
> different functions with the same change.
> 
> [...]

Here is the summary with links:
  - [net-next,v6,1/2] netconsole: Create a allocation helper
    https://git.kernel.org/netdev/net-next/c/b0a9e2c9a99f
  - [net-next,v6,2/2] netconsole: Enable compile time configuration
    https://git.kernel.org/netdev/net-next/c/fad361a2ee90

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html