netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] net: veth: Disable netpoll support
@ 2024-08-05  9:40 Breno Leitao
  2024-08-06 20:27 ` patchwork-bot+netdevbpf
  2025-12-04  9:20 ` Fabian Grünbichler
  0 siblings, 2 replies; 6+ messages in thread
From: Breno Leitao @ 2024-08-05  9:40 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: leit, open list:NETWORKING DRIVERS, open list

The current implementation of netpoll in veth devices leads to
suboptimal behavior, as it triggers warnings due to the invocation of
__netif_rx() within a softirq context. This is not compliant with
expected practices, as __netif_rx() has the following statement:

	lockdep_assert_once(hardirq_count() | softirq_count());

Given that veth devices typically do not benefit from the
functionalities provided by netpoll, Disable netpoll for veth
interfaces.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/net/veth.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 426e68a95067..34499b91a8bd 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -1696,6 +1696,7 @@ static void veth_setup(struct net_device *dev)
 	dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
 	dev->priv_flags |= IFF_NO_QUEUE;
 	dev->priv_flags |= IFF_PHONY_HEADROOM;
+	dev->priv_flags |= IFF_DISABLE_NETPOLL;
 
 	dev->netdev_ops = &veth_netdev_ops;
 	dev->xdp_metadata_ops = &veth_xdp_metadata_ops;
-- 
2.43.0


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

* Re: [PATCH net-next] net: veth: Disable netpoll support
  2024-08-05  9:40 [PATCH net-next] net: veth: Disable netpoll support Breno Leitao
@ 2024-08-06 20:27 ` patchwork-bot+netdevbpf
  2025-12-04  9:20 ` Fabian Grünbichler
  1 sibling, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-08-06 20:27 UTC (permalink / raw)
  To: Breno Leitao; +Cc: davem, edumazet, kuba, pabeni, leit, netdev, linux-kernel

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon,  5 Aug 2024 02:40:11 -0700 you wrote:
> The current implementation of netpoll in veth devices leads to
> suboptimal behavior, as it triggers warnings due to the invocation of
> __netif_rx() within a softirq context. This is not compliant with
> expected practices, as __netif_rx() has the following statement:
> 
> 	lockdep_assert_once(hardirq_count() | softirq_count());
> 
> [...]

Here is the summary with links:
  - [net-next] net: veth: Disable netpoll support
    https://git.kernel.org/netdev/net-next/c/45160cebd6ac

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

* Re: [PATCH net-next] net: veth: Disable netpoll support
  2024-08-05  9:40 [PATCH net-next] net: veth: Disable netpoll support Breno Leitao
  2024-08-06 20:27 ` patchwork-bot+netdevbpf
@ 2025-12-04  9:20 ` Fabian Grünbichler
  2025-12-04 13:44   ` Breno Leitao
  2025-12-05  1:34   ` Jakub Kicinski
  1 sibling, 2 replies; 6+ messages in thread
From: Fabian Grünbichler @ 2025-12-04  9:20 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Breno Leitao,
	Paolo Abeni
  Cc: leit, open list, open list:NETWORKING DRIVERS

On August 5, 2024 11:40 am, Breno Leitao wrote:
> The current implementation of netpoll in veth devices leads to
> suboptimal behavior, as it triggers warnings due to the invocation of
> __netif_rx() within a softirq context. This is not compliant with
> expected practices, as __netif_rx() has the following statement:
> 
> 	lockdep_assert_once(hardirq_count() | softirq_count());
> 
> Given that veth devices typically do not benefit from the
> functionalities provided by netpoll, Disable netpoll for veth
> interfaces.

this patch seems to have broken combining netconsole and bridges with
veth ports:

https://bugzilla.proxmox.com/show_bug.cgi?id=6873

any chance this is solvable?

> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  drivers/net/veth.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 426e68a95067..34499b91a8bd 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -1696,6 +1696,7 @@ static void veth_setup(struct net_device *dev)
>  	dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
>  	dev->priv_flags |= IFF_NO_QUEUE;
>  	dev->priv_flags |= IFF_PHONY_HEADROOM;
> +	dev->priv_flags |= IFF_DISABLE_NETPOLL;
>  
>  	dev->netdev_ops = &veth_netdev_ops;
>  	dev->xdp_metadata_ops = &veth_xdp_metadata_ops;
> -- 
> 2.43.0
> 
> 
> 
> 


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

* Re: [PATCH net-next] net: veth: Disable netpoll support
  2025-12-04  9:20 ` Fabian Grünbichler
@ 2025-12-04 13:44   ` Breno Leitao
  2025-12-05  1:34   ` Jakub Kicinski
  1 sibling, 0 replies; 6+ messages in thread
From: Breno Leitao @ 2025-12-04 13:44 UTC (permalink / raw)
  To: Fabian Grünbichler
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, leit,
	open list, open list:NETWORKING DRIVERS

hello Fabian,

On Thu, Dec 04, 2025 at 10:20:06AM +0100, Fabian Grünbichler wrote:
> On August 5, 2024 11:40 am, Breno Leitao wrote:
> > The current implementation of netpoll in veth devices leads to
> > suboptimal behavior, as it triggers warnings due to the invocation of
> > __netif_rx() within a softirq context. This is not compliant with
> > expected practices, as __netif_rx() has the following statement:
> > 
> > 	lockdep_assert_once(hardirq_count() | softirq_count());
> > 
> > Given that veth devices typically do not benefit from the
> > functionalities provided by netpoll, Disable netpoll for veth
> > interfaces.
> 
> this patch seems to have broken combining netconsole and bridges with
> veth ports:

Sorry about it, but, veth ends up calling __netif_rx() from a process
context, which kicks the lockdep above.

__netif_rx() should be only called from soft or hard IRQ, which is not
how netpoll operates. A printk message can be printed for any context.

> https://bugzilla.proxmox.com/show_bug.cgi?id=6873
> 
> any chance this is solvable?

I don't see a clear way to solve it from a netpoll point of view,
honestly.

From a veth perspective, I am wonderig if veth_forward_skb() can call
netif_rx(), which seems to be safe in any context. Something as:

	diff --git a/drivers/net/veth.c b/drivers/net/veth.c
	index cc502bf022d5..cf6443e5d7bc 100644
	--- a/drivers/net/veth.c
	+++ b/drivers/net/veth.c
	@@ -318,7 +318,7 @@ static int veth_forward_skb(struct net_device *dev, struct sk_buff *skb,
	{
		return __dev_forward_skb(dev, skb) ?: xdp ?
			veth_xdp_rx(rq, skb) :
	-               __netif_rx(skb);
	+               netif_rx(skb);
	}

	/* return true if the specified skb has chances of GRO aggregation
	@@ -1734,7 +1734,6 @@ static void veth_setup(struct net_device *dev)
		dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
		dev->priv_flags |= IFF_NO_QUEUE;
		dev->priv_flags |= IFF_PHONY_HEADROOM;
	-       dev->priv_flags |= IFF_DISABLE_NETPOLL;
		dev->lltx = true;

		dev->netdev_ops = &veth_netdev_ops;

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

* Re: [PATCH net-next] net: veth: Disable netpoll support
  2025-12-04  9:20 ` Fabian Grünbichler
  2025-12-04 13:44   ` Breno Leitao
@ 2025-12-05  1:34   ` Jakub Kicinski
  2025-12-05  7:35     ` Fabian Grünbichler
  1 sibling, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2025-12-05  1:34 UTC (permalink / raw)
  To: Fabian Grünbichler
  Cc: David S. Miller, Eric Dumazet, Breno Leitao, Paolo Abeni, leit,
	open list, open list:NETWORKING DRIVERS

On Thu, 04 Dec 2025 10:20:06 +0100 Fabian Grünbichler wrote:
> On August 5, 2024 11:40 am, Breno Leitao wrote:
> > The current implementation of netpoll in veth devices leads to
> > suboptimal behavior, as it triggers warnings due to the invocation of
> > __netif_rx() within a softirq context. This is not compliant with
> > expected practices, as __netif_rx() has the following statement:
> > 
> > 	lockdep_assert_once(hardirq_count() | softirq_count());
> > 
> > Given that veth devices typically do not benefit from the
> > functionalities provided by netpoll, Disable netpoll for veth
> > interfaces.  
> 
> this patch seems to have broken combining netconsole and bridges with
> veth ports:
> 
> https://bugzilla.proxmox.com/show_bug.cgi?id=6873
> 
> any chance this is solvable?

What's the reason to set up netcons over veth?
Note that unlike normal IP traffic netcons just blindly pipes out fully
baked skbs, it doesn't use the IP stack. So unlike normal IP traffic
I think you can still point it at the physical netdev, even if that
physical netdev is under a bridge.

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

* Re: [PATCH net-next] net: veth: Disable netpoll support
  2025-12-05  1:34   ` Jakub Kicinski
@ 2025-12-05  7:35     ` Fabian Grünbichler
  0 siblings, 0 replies; 6+ messages in thread
From: Fabian Grünbichler @ 2025-12-05  7:35 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: David S. Miller, Eric Dumazet, Breno Leitao, leit, open list,
	open list:NETWORKING DRIVERS, Paolo Abeni

On December 5, 2025 2:34 am, Jakub Kicinski wrote:
> On Thu, 04 Dec 2025 10:20:06 +0100 Fabian Grünbichler wrote:
>> On August 5, 2024 11:40 am, Breno Leitao wrote:
>> > The current implementation of netpoll in veth devices leads to
>> > suboptimal behavior, as it triggers warnings due to the invocation of
>> > __netif_rx() within a softirq context. This is not compliant with
>> > expected practices, as __netif_rx() has the following statement:
>> > 
>> > 	lockdep_assert_once(hardirq_count() | softirq_count());
>> > 
>> > Given that veth devices typically do not benefit from the
>> > functionalities provided by netpoll, Disable netpoll for veth
>> > interfaces.  
>> 
>> this patch seems to have broken combining netconsole and bridges with
>> veth ports:
>> 
>> https://bugzilla.proxmox.com/show_bug.cgi?id=6873
>> 
>> any chance this is solvable?
> 
> What's the reason to set up netcons over veth?

I don't think there is a particular reason to do so, the veth devices
just get "caught in the crossfire", so to speak - if the netconsole
setup includes the bridge that the veth device is plugged into.

> Note that unlike normal IP traffic netcons just blindly pipes out fully
> baked skbs, it doesn't use the IP stack. So unlike normal IP traffic
> I think you can still point it at the physical netdev, even if that
> physical netdev is under a bridge.

yes, pointing it at a physical bridge port works!

I mainly wanted to make you aware of this regression, since it seems it
was not on the radar when the original patch was written and applied. if
fixing it is too much of a hassle/has too many other unwanted side
effects, I do think (hope? ;)) people can live with this restriction. I
definitely agree that restricting netconsole to the actual links where
the traffic is supposed to go out is the sensible choice in any case.


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

end of thread, other threads:[~2025-12-05  7:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-05  9:40 [PATCH net-next] net: veth: Disable netpoll support Breno Leitao
2024-08-06 20:27 ` patchwork-bot+netdevbpf
2025-12-04  9:20 ` Fabian Grünbichler
2025-12-04 13:44   ` Breno Leitao
2025-12-05  1:34   ` Jakub Kicinski
2025-12-05  7:35     ` Fabian Grünbichler

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).