linux-kernel - Re: [PATCH RFC net-next 2/2] netconsole: add CONFIG_NETCONSOLE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aSRjtgmr9xKOX1Ek@pathway.suse.cz>
Date: Mon, 24 Nov 2025 14:55:02 +0100
From: Petr Mladek <pmladek@...e.com>
To: Breno Leitao <leitao@...ian.org>
Cc: Jakub Kicinski <kuba@...nel.org>, horms@...nel.org, efault@....de,
	john.ogness@...utronix.de, Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	calvin@...nvd.org, asml.silence@...il.com, kernel-team@...a.com,
	gustavold@...il.com, asantostc@...il.com
Subject: Re: [PATCH RFC net-next 2/2] netconsole: add CONFIG_NETCONSOLE_NBCON
 for nbcon support

On Fri 2025-11-21 03:26:08, Breno Leitao wrote:
> Add optional support for the nbcon infrastructure to netconsole via a new
> CONFIG_NETCONSOLE_NBCON compile-time option.
> 
> The nbcon infrastructure provides a lock-free, priority-based console
> system that supports atomic printing from any context including NMI,
> with safe handover mechanisms between different priority levels. This
> makes it particularly suitable for crash-safe kernel logging.
> 
> When disabled (default), netconsole uses the legacy console callbacks,
> maintaining full backward compatibility.
> 
> PS: .write_atomic and .write_thread uses the same callback, given that
> there is no safe .write_atomic, so .write_atomic is called as the last
> resource. This is what CON_NBCON_ATOMIC_UNSAFE is telling nbcon.

Makes sense. CON_NBCON_ATOMIC_UNSAFE also explains why target_list_lock
need not be synchronized with nbcon context locking [*]. The _unsafe_
.write_atomic() callback might be called only by the final
nbcon_atomic_flush_unsafe() when even the nbcon context
synchronization can be ignored.

[*] For example, see how port->lock is synchronized with the nbcon
    context by uart_port_lock() wrapper.

> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -369,6 +369,20 @@ config NETCONSOLE_PREPEND_RELEASE
>  	  message.  See <file:Documentation/networking/netconsole.rst> for
>  	  details.
>  
> +config NETCONSOLE_NBCON
> +	bool "Use nbcon infrastructure (EXPERIMENTAL)"
> +	depends on NETCONSOLE
> +	default n
> +	help
> +	  Enable nbcon support for netconsole. This uses the new lock-free

Strictly speaking, it is not lock-free. The main feature is that it is
threaded so that it does not block the printk() caller.

Nbcon consoles also support synchronous flushing in emergecy situations.
But it does not work with netconsoles because they do not support
atomic operations. They are flushed only by the final desperate flush
in panic() when all locks are ignored.

> +	  console infrastructure which supports threaded and atomic printing.
> +	  Given that netconsole does not support atomic operations, the current
> +	  implementation focuses on threaded callbacks, unless the host is
> +	  crashing, then it uses an unsafe atomic callbacks. This feature is
> +	  available for both extended and non-extended consoles.
> +
> +	  If unsure, say N to use the legacy console infrastructure.
> +
>  config NETPOLL
>  	def_bool NETCONSOLE
>  
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index f4b1706fb081..2943f00b83f6 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -1724,6 +1724,57 @@ static void send_ext_msg_udp(struct netconsole_target *nt, const char *msg,
>  				   extradata_len);
>  }
>  
> +#ifdef CONFIG_NETCONSOLE_NBCON
> +static void netcon_write_nbcon(struct console *con,
> +			       struct nbcon_write_context *wctxt,
> +			       bool extended)
> +{
> +	struct netconsole_target *nt;
> +
> +	lockdep_assert_held(&target_list_lock);
> +
> +	list_for_each_entry(nt, &target_list, list) {
> +		if (nt->extended != extended || !nt->enabled ||
> +		    !netif_running(nt->np.dev))
> +			continue;
> +
> +		if (!nbcon_enter_unsafe(wctxt))
> +			continue;
> +
> +		if (extended)
> +			send_ext_msg_udp(nt, wctxt->outbuf, wctxt->len);
> +		else
> +			write_msg_target(nt, wctxt->outbuf, wctxt->len);

If you accepted the rename in the 1st patch then this would be ;-)

		if (extended)
			send_ext_msg_udp(nt, wctxt->outbuf, wctxt->len);
		else
			send_msg_udp(nt, wctxt->outbuf, wctxt->len);

> +
> +		nbcon_exit_unsafe(wctxt);
> +	}
> +}

Otherwise, it looks good from my POV.

Best Regards,
Petr