Re: [PATCH net-next 0/2] net: netconsole: convert to NBCON console infrastructure

[Petr Mladek <pmladek@suse.com>]

On Fri 2026-01-16 10:07:08, Breno Leitao wrote:
> Hello Petr,
> 
> On Fri, Jan 16, 2026 at 04:53:48PM +0100, Petr Mladek wrote:
> > > Otherwise, it looks good to me.
> > > 
> > > I tried to update your patch with the above proposal to see how
> > > it looks and I got:
> > 
> > The change seems to work. I have tested it with the following patch:
> 
> First of all, *thank you* so much for spending your time on it, this is
> helpful.

You are welcome.

> > Then the extended console format should show also:
> > 
> >      ,cpu=XXX,pid=YYY,comm=ZZZ
> 
> Are you using this just for testing, or do you plan to get this output?

I used this just for testing. But it looks like a good variant for me.

Note that the above is /dev/kmsg output. It would show dev_printk()
messages the following way:

6,295,1164587,-,caller=T1,cpu=10,pid=1,comm=swapper/0;pci 0000:00:02.1: enabling Extended Tags
 SUBSYSTEM=pci
 DEVICE=+pci:0000:00:02.1
6,296,1167287,-,caller=T1,cpu=10,pid=1,comm=swapper/0;pci 0000:00:02.2: [1b36:000c] type 01 class 0x060400 PCIe Root Port
 SUBSYSTEM=pci
 DEVICE=+pci:0000:00:02.2


We would need to use another format for (slow) consoles, for example:

     <level>[timestamp][Cxxx][context]comm[pid] message

, where:

   + [Cxxx] would show cpu number like the current context field, e.g [  C123]

   + [context] would show context, ideal in some existing format. But
     I know only about lockdep format which looks a bit hard to
     understand for me.

     For example, it might be, for example [TC,HD] for a task context
     with hard irqs disabled.


   + comm[pid] is inspired by the existing messages passed from systemd, e.g.
     <30>[    5.317305][    T1] systemd[1]: Starting Journal Service...
     <30>[    5.349000][    T1] systemd[1]: Starting Load Kernel Modules...

But I guess that this might be long bikesheding about this.

> Context: netconsole outputs the message in a different way, similarly to the
> printk dictionary. I.e, taskname and cpu come after, one entry per line:
> 
>   <message>
>    SUBSYSTEM=net
>    DEVICE=+pci:0000:00:1f.6
>    cpu=42
>    taskname=NetworkManager
>    ...

I see. Honestly, I never liked the way how dictionary was printed.
It used another syntax than loglevel, sequence id, and timestamp.
It might be confusing and complicate parsing.

But it is a personal opinion. Others might like it because
it might be easier for human eyes parsing.

IMHO, the information needs some parsing anyway to make it human readable:

  + people do not know the log level meaning out of head
  + message id is there primary to detect lost messages
  + timestamp needs conversion, definitely

Sigh, the use of lower case letters for "cpu" and "taskname" made it
even more inconsistent. I would voted against it if I did the review ;-)

> I would like to keep the same format, given users might be used to this format
> already, where netconsole grabs teh cpu,pid,comm data and massage it before
> outputing. Something as:
> 
>  static int sysdata_append_taskname(struct netconsole_target *nt, int offset,
> 				    struct nbcon_write_context *wctxt)
>  {
>      return scnprintf(&nt->sysdata[offset],
>               MAX_EXTRADATA_ENTRY_LEN, " taskname=%s\n",
> -             current->comm);
> +             wctxt->msg_comm);
>  }

I see. I think that you could keep it for now. It might take quite
some time until we integrate this for /dev/kmsg, ...

> Here is the full patch I was using to test the integration of netconsole and
> the previous printk patch:
> 
> https://github.com/leitao/linux/commit/4175dc10719a15844b3a0bd7aa38158a913181a3

BTW.1: I see that netconsole actually does not show pid. So that we do not
       need the trick with caller_id2. But people might want to add it in
       the future.

       I do not have strong opinion about it. We could change it
       anytime later if we did not export the new fields for crash
       dump, via the VMCOREINFO_OFFSET() macro.


BTW.2: I also noticed that sysdata_append_msgid() uses netconsole-specific
       message counter.

       Note that each message has its own sequence number. It is the
       .seq member in struct printk_info. It is printed in the extended
       console output, see info_print_ext_header(). So it is printed
       even on netconsole when this extended format is used.

       I wonder if the netconsole-specific counter was added
       intentionally.


Best Regards,
Petr