netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC net-next 0/2] netconsole: NBCON Infrastructure Support
@ 2025-11-21 11:26 Breno Leitao
  2025-11-21 11:26 ` [PATCH RFC net-next 1/2] netconsole: extract message fragmentation into write_msg_target() Breno Leitao
  2025-11-21 11:26 ` [PATCH RFC net-next 2/2] netconsole: add CONFIG_NETCONSOLE_NBCON for nbcon support Breno Leitao
  0 siblings, 2 replies; 5+ messages in thread
From: Breno Leitao @ 2025-11-21 11:26 UTC (permalink / raw)
  To: Breno Leitao, Jakub Kicinski, horms, efault, john.ogness, pmladek
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni, netdev,
	linux-kernel, calvin, asml.silence, kernel-team, gustavold,
	asantostc

This RFC proposes enabling netconsole on the NBCON infrastructure.

Context:
=======

Mike[1] reported a netconsole HARDIRQ-safe → HARDIRQ-unsafe lock
warning a while ago. The root cause involved IRQ-unsafe locks
being called within the console lock context. These IRQ-unsafe locks are
on some very specific network drivers TX path (ieee80211 as in Mike's
report).

A possible solution is to mark these devices as NOT supported by
netpoll (aka IFF_DISABLE_NETPOLL). Another solution is to send "most" of
the netconsole messages from non-atomic contexts (aka thread in nbcon
parlance), and only rely on atomic context when the host is crashing.

On top of that, nbcon is a much modern console implementation, which
brings others benefits to netconsole, so, this patches move netconsole
to NBCON.

Until recently, NBCON lacked support for non-atomic consoles
(CON_NBCON_ATOMIC_UNSAFE), thus, this port was not possible so far.

John recently implemented CON_NBCON_ATOMIC_UNSAFE in commit 187de7c212e5
("printk: nbcon: Allow unsafe write_atomic() for panic"), enabling
netconsole to use nbcon framework.

The patchset implements NBCON support in 2 phases:

1. Refactoring: Extract message fragmentation logic into a reusable helper
function.

2. Extended console support: Introduce CONFIG_NETCONSOLE_NBCON for consoles
implementing device lock/unlock callbacks

Backward Compatibility
======================

When CONFIG_NETCONSOLE_NBCON is disabled (the default), both extended
and basic consoles continue using the legacy console infrastructure,
ensuring full backward compatibility.

Current Limitations
===================

Netconsole continues to call netpoll and network TX helpers with interrupts
disabled. The network xmit callbacks are called with IRQ disabled
(target_list_lock is an IRQ safe spinlock)

spin_lock_irqsave(&target_list_lock, *flags)
	list_for_each_entry(nt, &target_list, list)
		netpoll_send_udp();
			__netpoll_send_skb()
				lockdep_assert_irqs_disabled()

While this patchset doesn't fully resolve the issue in [1], it removes
one layer of the problem and, moves the problem into the network domain,
which is a huge win.

Also, the commit 187de7c212e5 ("printk: nbcon: Allow unsafe
write_atomic() for panic") is still not on net-next, thus, NIPA will
fail for this RFC. Also, this patch is based on linux-next as of
20251121 instead of net-next.

Next steps
==========

1) Move the target_list_lock to RCU
2) Assess if __netpoll_send_skb() can be called with IRQ enabled
3) Mark devices that rely on IRQ unsafe  contexts with IFF_DISABLE_NETPOLL
4) Use CON_NBCON_ATOMIC_UNSAFE only if the netpoll device has
   IFF_DISABLE_NETPOLL, otherwise, unset CON_NBCON_ATOMIC_UNSAFE and be
   a more normal NBCON user.

[1] https://lore.kernel.org/all/b2qps3uywhmjaym4mht2wpxul4yqtuuayeoq4iv4k3zf5wdgh3@tocu6c7mj4lt/

---
Breno Leitao (2):
      netconsole: extract message fragmentation into write_msg_target()
      netconsole: add CONFIG_NETCONSOLE_NBCON for nbcon support

 drivers/net/Kconfig      | 14 ++++++++
 drivers/net/netconsole.c | 94 ++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 98 insertions(+), 10 deletions(-)
---
base-commit: d724c6f85e80a23ed46b7ebc6e38b527c09d64f5
change-id: 20251117-nbcon-f24477ca9f3e

Best regards,
--  
Breno Leitao <leitao@debian.org>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH RFC net-next 1/2] netconsole: extract message fragmentation into write_msg_target()
  2025-11-21 11:26 [PATCH RFC net-next 0/2] netconsole: NBCON Infrastructure Support Breno Leitao
@ 2025-11-21 11:26 ` Breno Leitao
  2025-11-24 13:08   ` Petr Mladek
  2025-11-21 11:26 ` [PATCH RFC net-next 2/2] netconsole: add CONFIG_NETCONSOLE_NBCON for nbcon support Breno Leitao
  1 sibling, 1 reply; 5+ messages in thread
From: Breno Leitao @ 2025-11-21 11:26 UTC (permalink / raw)
  To: Breno Leitao, Jakub Kicinski, horms, efault, john.ogness, pmladek
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni, netdev,
	linux-kernel, calvin, asml.silence, kernel-team, gustavold,
	asantostc

Refactor the message fragmentation logic in write_msg() by extracting it
into a separate write_msg_target() helper function. This makes the code
more maintainable and prepares for future reuse in nbcon support for
non-extended consoles.

The helper function takes a target, message, and length, then handles
splitting the message into MAX_PRINT_CHUNK-sized fragments for sending
via send_udp().

No functional change intended.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/net/netconsole.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index bb6e03a92956..f4b1706fb081 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -1559,6 +1559,20 @@ static void append_release(char *buf)
 	scnprintf(buf, MAX_PRINT_CHUNK, "%s,", release);
 }
 
+static void write_msg_target(struct netconsole_target *nt, const char *msg,
+			     unsigned int len)
+{
+	const char *tmp = msg;
+	int frag, left = len;
+
+	while (left > 0) {
+		frag = min(left, MAX_PRINT_CHUNK);
+		send_udp(nt, tmp, frag);
+		tmp += frag;
+		left -= frag;
+	}
+}
+
 static void send_fragmented_body(struct netconsole_target *nt,
 				 const char *msgbody, int header_len,
 				 int msgbody_len, int extradata_len)
@@ -1728,10 +1742,8 @@ static void write_ext_msg(struct console *con, const char *msg,
 
 static void write_msg(struct console *con, const char *msg, unsigned int len)
 {
-	int frag, left;
-	unsigned long flags;
 	struct netconsole_target *nt;
-	const char *tmp;
+	unsigned long flags;
 
 	if (oops_only && !oops_in_progress)
 		return;
@@ -1748,13 +1760,7 @@ static void write_msg(struct console *con, const char *msg, unsigned int len)
 			 * at least one target if we die inside here, instead
 			 * of unnecessarily keeping all targets in lock-step.
 			 */
-			tmp = msg;
-			for (left = len; left;) {
-				frag = min(left, MAX_PRINT_CHUNK);
-				send_udp(nt, tmp, frag);
-				tmp += frag;
-				left -= frag;
-			}
+			write_msg_target(nt, msg, len);
 		}
 	}
 	spin_unlock_irqrestore(&target_list_lock, flags);

-- 
2.47.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH RFC net-next 2/2] netconsole: add CONFIG_NETCONSOLE_NBCON for nbcon support
  2025-11-21 11:26 [PATCH RFC net-next 0/2] netconsole: NBCON Infrastructure Support Breno Leitao
  2025-11-21 11:26 ` [PATCH RFC net-next 1/2] netconsole: extract message fragmentation into write_msg_target() Breno Leitao
@ 2025-11-21 11:26 ` Breno Leitao
  2025-11-24 13:55   ` Petr Mladek
  1 sibling, 1 reply; 5+ messages in thread
From: Breno Leitao @ 2025-11-21 11:26 UTC (permalink / raw)
  To: Breno Leitao, Jakub Kicinski, horms, efault, john.ogness, pmladek
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni, netdev,
	linux-kernel, calvin, asml.silence, kernel-team, gustavold,
	asantostc

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.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/net/Kconfig      | 14 ++++++++++
 drivers/net/netconsole.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index ac12eaf11755..aa8771b5b723 100644
--- 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
+	  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);
+
+		nbcon_exit_unsafe(wctxt);
+	}
+}
+
+static void netcon_write_nbcon_ext(struct console *con,
+				   struct nbcon_write_context *wctxt)
+{
+	netcon_write_nbcon(con, wctxt, true);
+}
+
+static void netcon_write_nbcon_basic(struct console *con,
+				     struct nbcon_write_context *wctxt)
+{
+	netcon_write_nbcon(con, wctxt, false);
+}
+
+static void netconsole_device_lock(struct console *con, unsigned long *flags)
+{
+	/* protects all the targets at the same time */
+	spin_lock_irqsave(&target_list_lock, *flags);
+}
+
+static void netconsole_device_unlock(struct console *con, unsigned long flags)
+{
+	spin_unlock_irqrestore(&target_list_lock, flags);
+}
+
+#else
+
 static void write_ext_msg(struct console *con, const char *msg,
 			  unsigned int len)
 {
@@ -1765,6 +1816,7 @@ static void write_msg(struct console *con, const char *msg, unsigned int len)
 	}
 	spin_unlock_irqrestore(&target_list_lock, flags);
 }
+#endif
 
 static int netconsole_parser_cmdline(struct netpoll *np, char *opt)
 {
@@ -1923,14 +1975,30 @@ static void free_param_target(struct netconsole_target *nt)
 
 static struct console netconsole_ext = {
 	.name	= "netcon_ext",
+#ifdef CONFIG_NETCONSOLE_NBCON
+	.flags = CON_ENABLED | CON_EXTENDED | CON_NBCON | CON_NBCON_ATOMIC_UNSAFE,
+	.write_thread = netcon_write_nbcon_ext,
+	.write_atomic = netcon_write_nbcon_ext,
+	.device_lock = netconsole_device_lock,
+	.device_unlock = netconsole_device_unlock,
+#else
 	.flags	= CON_ENABLED | CON_EXTENDED,
 	.write	= write_ext_msg,
+#endif
 };
 
 static struct console netconsole = {
 	.name	= "netcon",
+#ifdef CONFIG_NETCONSOLE_NBCON
+	.flags = CON_ENABLED | CON_NBCON | CON_NBCON_ATOMIC_UNSAFE,
+	.write_thread = netcon_write_nbcon_basic,
+	.write_atomic = netcon_write_nbcon_basic,
+	.device_lock = netconsole_device_lock,
+	.device_unlock = netconsole_device_unlock,
+#else
 	.flags	= CON_ENABLED,
 	.write	= write_msg,
+#endif
 };
 
 static int __init init_netconsole(void)

-- 
2.47.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH RFC net-next 1/2] netconsole: extract message fragmentation into write_msg_target()
  2025-11-21 11:26 ` [PATCH RFC net-next 1/2] netconsole: extract message fragmentation into write_msg_target() Breno Leitao
@ 2025-11-24 13:08   ` Petr Mladek
  0 siblings, 0 replies; 5+ messages in thread
From: Petr Mladek @ 2025-11-24 13:08 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Jakub Kicinski, horms, efault, john.ogness, Andrew Lunn,
	David S. Miller, Eric Dumazet, Paolo Abeni, netdev, linux-kernel,
	calvin, asml.silence, kernel-team, gustavold, asantostc

On Fri 2025-11-21 03:26:07, Breno Leitao wrote:
> Refactor the message fragmentation logic in write_msg() by extracting it
> into a separate write_msg_target() helper function. This makes the code
> more maintainable and prepares for future reuse in nbcon support for
> non-extended consoles.
> 
> The helper function takes a target, message, and length, then handles
> splitting the message into MAX_PRINT_CHUNK-sized fragments for sending
> via send_udp().
> 
> No functional change intended.

> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -1559,6 +1559,20 @@ static void append_release(char *buf)
>  	scnprintf(buf, MAX_PRINT_CHUNK, "%s,", release);
>  }
>  
> +static void write_msg_target(struct netconsole_target *nt, const char *msg,
> +			     unsigned int len)
> +{
> +	const char *tmp = msg;
> +	int frag, left = len;
> +
> +	while (left > 0) {
> +		frag = min(left, MAX_PRINT_CHUNK);
> +		send_udp(nt, tmp, frag);
> +		tmp += frag;
> +		left -= frag;
> +	}
> +}
> +
>  static void send_fragmented_body(struct netconsole_target *nt,
>  				 const char *msgbody, int header_len,
>  				 int msgbody_len, int extradata_len)
> @@ -1748,13 +1760,7 @@ static void write_msg(struct console *con, const char *msg, unsigned int len)
>  			 * at least one target if we die inside here, instead
>  			 * of unnecessarily keeping all targets in lock-step.
>  			 */
> -			tmp = msg;
> -			for (left = len; left;) {
> -				frag = min(left, MAX_PRINT_CHUNK);
> -				send_udp(nt, tmp, frag);
> -				tmp += frag;
> -				left -= frag;
> -			}
> +			write_msg_target(nt, msg, len);

I would call the new function send_msg_udp() to make it symetric with:

static void write_ext_msg(struct console *con, const char *msg,
			  unsigned int len)
{
[...]
			send_ext_msg_udp(nt, msg, len);
[...]
}

By other words, use "write_*()" when struct console * is passed
and send_*_udp() when struct netconsole_target * is passed.

The inconsistence confused me... ;-)

Note that write_msg()/write_ext_msg() are cut&pasted.
The only difference would be send_msg_udp()/send_ext_msg_udp().

>  		}
>  	}
>  	spin_unlock_irqrestore(&target_list_lock, flags);

Otherwise, I confirm that it is just a refactoring with no
functional change.

Feel free to use, ideally after the renaming function:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH RFC net-next 2/2] netconsole: add CONFIG_NETCONSOLE_NBCON for nbcon support
  2025-11-21 11:26 ` [PATCH RFC net-next 2/2] netconsole: add CONFIG_NETCONSOLE_NBCON for nbcon support Breno Leitao
@ 2025-11-24 13:55   ` Petr Mladek
  0 siblings, 0 replies; 5+ messages in thread
From: Petr Mladek @ 2025-11-24 13:55 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Jakub Kicinski, horms, efault, john.ogness, Andrew Lunn,
	David S. Miller, Eric Dumazet, Paolo Abeni, netdev, linux-kernel,
	calvin, asml.silence, kernel-team, gustavold, asantostc

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-11-24 13:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-21 11:26 [PATCH RFC net-next 0/2] netconsole: NBCON Infrastructure Support Breno Leitao
2025-11-21 11:26 ` [PATCH RFC net-next 1/2] netconsole: extract message fragmentation into write_msg_target() Breno Leitao
2025-11-24 13:08   ` Petr Mladek
2025-11-21 11:26 ` [PATCH RFC net-next 2/2] netconsole: add CONFIG_NETCONSOLE_NBCON for nbcon support Breno Leitao
2025-11-24 13:55   ` Petr Mladek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).