Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 3 Aug 2023 04:29:20 -0700
From:   Breno Leitao <leitao@debian.org>
To:     Eric Dumazet <edumazet@google.com>
Cc:     rdunlap@infradead.org, benjamin.poirier@gmail.com,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, leit@meta.com,
        "open list:NETWORKING DRIVERS" <netdev@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next v3] netconsole: Enable compile time configuration
Message-ID: <ZMuPkMlg1/kfRyXk@gmail.com>
References: <20230801100533.3350037-1-leitao@debian.org>
 <CANn89iKuHxUGphhDkKz2ZWS3YR3-BkieTb4b4gKMR9B7jxKpWQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CANn89iKuHxUGphhDkKz2ZWS3YR3-BkieTb4b4gKMR9B7jxKpWQ@mail.gmail.com>
Precedence: bulk

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!