netdev - RE: [PATCH net] net: fec: tx processing does not call XDP APIs if budget is 0

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:
 <AM5PR04MB3139FC9C3FED1759E1160EAE8801A@AM5PR04MB3139.eurprd04.prod.outlook.com>
Date: Thu, 27 Jul 2023 02:08:32 +0000
From: Wei Fang <wei.fang@....com>
To: Alexander Duyck <alexander.duyck@...il.com>
CC: dl-linux-imx <linux-imx@....com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "davem@...emloft.net" <davem@...emloft.net>,
	"edumazet@...gle.com" <edumazet@...gle.com>, "kuba@...nel.org"
	<kuba@...nel.org>, Shenwei Wang <shenwei.wang@....com>, Clark Wang
	<xiaoning.wang@....com>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH net] net: fec: tx processing does not call XDP APIs if
 budget is 0

> > > This statement isn't correct. There are napi enabled and non-napi
> > > versions of these calls. This is the reason for things like the
> > > "allow_direct" parameter in page_pool_put_full_page and the
> > > "napi_direct" parameter in __xdp_return.
> > >
> > > By blocking on these cases you can end up hanging the Tx queue which is
> > > going to break netpoll as you are going to stall the ring on XDP
> > > packets if they are already in the queue.
> > >
> > > From what I can tell your driver is using xdp_return_frame in the case
> > > of an XDP frame which doesn't make use of the NAPI optimizations in
> > > freeing from what I can tell. The NAPI optimized version is
> > > xdp_return_frame_rx.
> > >
> > So you mean it is safe to use xdp_return_frame no matter in NAPI context
> > or non-NAPI context? And xdp_return_frame_rx_napi must be used in NAPI
> > context? If so, I think I must have misunderstood, then this patch is not
> necessary.
> 
> Actually after talking with Jakub a bit more there is an issue here,
> but not freeing the frames isn't the solution. We likely need to just
> fix the page pool code so that it doesn't attempt to recycle the
> frames if operating in IRQ context.
> 
> The way this is dealt with for skbs is that we queue skbs if we are in
> IRQ context so that it can be deferred to be freed by the
> net_tx_action. We likely need to look at doing something similar for
> page_pool pages or XDP frames.
> 
After reading your discussion with Jakub, I understand this issue a bit more.
But we are not sure when this issue will be fixed in page pool, currently we
can only tolerate a delay in sending of a netpoll message. So I think this patch
is necessary, and I will refine it in the future when the page pool has fixed the
issue. In addition, as you mentioned before, napi_consume_skb should be
used to instead of dev_kfree_skb_any, so I will improve this patch in version 2.
Thanks.