Re: [PATCH net-next v3] netconsole: Enable compile time configuration

From: Eric Dumazet
Date: Thu Aug 03 2023 - 04:00:39 EST


On Tue, Aug 1, 2023 at 12:06 PM Breno Leitao <leitao@xxxxxxxxxx> 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.
>
> 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@xxxxxxxxxx>
> ---
> 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.
> ---
> drivers/net/Kconfig | 22 ++++++++++++++++++++++
> drivers/net/netconsole.c | 10 ++++++++++
> 2 files changed, 32 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 87f18aedd3bd..e3b6155f4529 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -181,6 +181,11 @@ static struct netconsole_target *alloc_param_target(char *target_config)
> if (!nt)
> goto fail;
>
> + 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;
> @@ -681,6 +686,11 @@ static struct config_item *make_netconsole_target(struct config_group *group,
> nt->np.remote_port = 6666;
> eth_broadcast_addr(nt->np.remote_mac);
>
> + if (IS_ENABLED(CONFIG_NETCONSOLE_EXTENDED_LOG))
> + nt->extended = true;
> + if (IS_ENABLED(CONFIG_NETCONSOLE_PREPEND_RELEASE))
> + nt->release = true;
> +

Instead of duplicating these, what about adding a preliminary helper
in a separate patch ?

Something like this:

drivers/net/netconsole.c | 45 ++++++++++++++++++++-------------------------
1 file changed, 20 insertions(+), 25 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 87f18aedd3bd7b3da9d2481dc9898e94dd75917b..a022ceaa2e3c4783ca0ea61c1c7b9f911d087abc
100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -167,26 +167,32 @@ static void netconsole_target_put(struct
netconsole_target *nt)

#endif /* CONFIG_NETCONSOLE_DYNAMIC */

+/*
+ * Allocate and initialize with defaults.
+ * Note that these targets get their config_item fields zeroed-out.
+ */
+static struct netconsole_target *alloc_and_init(void)
+{
+ struct netconsole_target *nt = kzalloc(sizeof(*nt), GFP_KERNEL);
+
+ if (nt) {
+ 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);
+ }
+ 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 = alloc_and_init();
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;
-
- 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);
-
if (*target_config == '+') {
nt->extended = true;
target_config++;
@@ -664,23 +670,12 @@ static const struct config_item_type
netconsole_target_type = {
static struct config_item *make_netconsole_target(struct config_group *group,
const char *name)
{
+ struct netconsole_target *nt = alloc_and_init();
unsigned long flags;
- struct netconsole_target *nt;

- /*
- * Allocate and initialize with defaults.
- * Target is disabled at creation (!enabled).
- */
- nt = kzalloc(sizeof(*nt), GFP_KERNEL);
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);