diff mbox series

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

Message ID 20230801100533.3350037-1-leitao@debian.org (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next,v3] netconsole: Enable compile time configuration | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1328 this patch: 1328
netdev/cc_maintainers success CCed 5 of 5 maintainers
netdev/build_clang success Errors and warnings before: 1351 this patch: 1351
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1351 this patch: 1351
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 50 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Breno Leitao Aug. 1, 2023, 10:05 a.m. UTC
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>
---
  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(+)

Comments

Eric Dumazet Aug. 3, 2023, 7:47 a.m. UTC | #1
On Tue, Aug 1, 2023 at 12:06 PM Breno Leitao <leitao@debian.org> 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@debian.org>
> ---
>   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);
Breno Leitao Aug. 3, 2023, 11:29 a.m. UTC | #2
On Thu, Aug 03, 2023 at 09:47:43AM +0200, Eric Dumazet wrote:
> On Tue, Aug 1, 2023 at 12:06 PM Breno Leitao <leitao@debian.org> 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@debian.org>
> > ---
> >   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 ?

That is a good idea, I will update.

Thanks!
diff mbox series

Patch

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;
+
 	/* Initialize the config_item member */
 	config_item_init_type_name(&nt->item, name, &netconsole_target_type);