netdev - Re: [PATCH net-next] virtio_net: Fix napi_skb_cache

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Ztc5QllkqaKZsaoN@LeoBras>
Date: Tue,  3 Sep 2024 13:28:50 -0300
From: Leonardo Bras <leobras@...hat.com>
To: Breno Leitao <leitao@...ian.org>
Cc: Leonardo Bras <leobras@...hat.com>,
	"Michael S. Tsirkin" <mst@...hat.com>,
	Jason Wang <jasowang@...hat.com>,
	Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
	Eugenio Pérez <eperezma@...hat.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Paolo Abeni <pabeni@...hat.com>,
	rbc@...a.com,
	horms@...nel.org,
	"open list:VIRTIO CORE AND NET DRIVERS" <virtualization@...ts.linux.dev>,
	"open list:NETWORKING DRIVERS" <netdev@...r.kernel.org>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next] virtio_net: Fix napi_skb_cache_put warning

On Mon, Jul 15, 2024 at 04:25:06AM -0700, Breno Leitao wrote:
> Hello Michael,
> 
> On Sun, Jul 14, 2024 at 03:38:42AM -0400, Michael S. Tsirkin wrote:
> > On Fri, Jul 12, 2024 at 04:53:25AM -0700, Breno Leitao wrote:
> > > After the commit bdacf3e34945 ("net: Use nested-BH locking for
> > > napi_alloc_cache.") was merged, the following warning began to appear:
> > > 
> > > 	 WARNING: CPU: 5 PID: 1 at net/core/skbuff.c:1451 napi_skb_cache_put+0x82/0x4b0
> > > 
> > > 	  __warn+0x12f/0x340
> > > 	  napi_skb_cache_put+0x82/0x4b0
> > > 	  napi_skb_cache_put+0x82/0x4b0
> > > 	  report_bug+0x165/0x370
> > > 	  handle_bug+0x3d/0x80
> > > 	  exc_invalid_op+0x1a/0x50
> > > 	  asm_exc_invalid_op+0x1a/0x20
> > > 	  __free_old_xmit+0x1c8/0x510
> > > 	  napi_skb_cache_put+0x82/0x4b0
> > > 	  __free_old_xmit+0x1c8/0x510
> > > 	  __free_old_xmit+0x1c8/0x510
> > > 	  __pfx___free_old_xmit+0x10/0x10
> > > 
> > > The issue arises because virtio is assuming it's running in NAPI context
> > > even when it's not, such as in the netpoll case.
> > > 
> > > To resolve this, modify virtnet_poll_tx() to only set NAPI when budget
> > > is available. Same for virtnet_poll_cleantx(), which always assumed that
> > > it was in a NAPI context.
> > > 
> > > Fixes: df133f3f9625 ("virtio_net: bulk free tx skbs")
> > > Suggested-by: Jakub Kicinski <kuba@...nel.org>
> > > Signed-off-by: Breno Leitao <leitao@...ian.org>
> > 
> > Acked-by: Michael S. Tsirkin <mst@...hat.com>
> > 
> > though I'm not sure I understand the connection with bdacf3e34945.
> 
> The warning above appeared after bdacf3e34945 landed.

Hi Breno,
Thanks for fixing this!

I think the confusion is around the fact that the commit on Fixes 
(df133f3f9625) tag is different from the commit in the commit message
(bdacf3e34945).

Please help me check if the following is correct:
###
Any tree which includes df133f3f9625 ("virtio_net: bulk free tx skbs") 
should also include your patch, since it fixes stuff in there.

The fact that the warning was only made visible in 
bdacf3e34945 ("net: Use nested-BH locking for napi_alloc_cache.")
does not change the fact that it was already present before.

Also, having bdacf3e34945 is not necessary for the backport, since
it only made the bug visible.
###

Are above statements right?

It's important to make it clear since this helps the backporting process.

Thanks!
Leo