netdev - Re: [PATCH net-next v2] xsk: skip validating skb list in xmit path

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: <CAL+tcoAEtJ43UKR6rDsLV9s9U31KmT_sE2+xpYnvFRQjsSAu5w@mail.gmail.com>
Date: Thu, 17 Jul 2025 11:10:48 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: Jakub Kicinski <kuba@...nel.org>, davem@...emloft.net, edumazet@...gle.com, 
	pabeni@...hat.com, bjorn@...nel.org, magnus.karlsson@...el.com, 
	maciej.fijalkowski@...el.com, jonathan.lemon@...il.com, sdf@...ichev.me, 
	ast@...nel.org, daniel@...earbox.net, hawk@...nel.org, 
	john.fastabend@...il.com, joe@...a.to, bpf@...r.kernel.org, 
	netdev@...r.kernel.org, Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net-next v2] xsk: skip validating skb list in xmit path

On Thu, Jul 17, 2025 at 10:52 AM Willem de Bruijn
<willemdebruijn.kernel@...il.com> wrote:
>
> Jason Xing wrote:
> > On Thu, Jul 17, 2025 at 8:52 AM Jakub Kicinski <kuba@...nel.org> wrote:
> > >
> > > On Thu, 17 Jul 2025 08:06:48 +0800 Jason Xing wrote:
> > > > To be honest, this patch really only does one thing as the commit
> > > > says. It might look very complex, but if readers take a deep look they
> > > > will find only one removal of that validation for xsk in the hot path.
> > > > Nothing more and nothing less. So IMHO, it doesn't bring more complex
> > > > codes here.
> > > >
> > > > And removal of one validation indeed contributes to the transmission.
> > > > I believe there remain a number of applications using copy mode
> > > > currently. And maintainers of xsk don't regard copy mode as orphaned,
> > > > right?
> > >
> > > First of all, I'm not sure the patch is correct. The XSK skbs can have
> > > frags, if device doesn't support or clears _SG we should linearize,
> > > right?
> >
> > But note that there is one more function __skb_linearize() after
> > skb_needs_linearize() in the validate_xmit_skb(). __skb_linearize()
> > tests many members of skbs, which are not used to check the skbs from
> > xsk. For xsk, it's very simple (please see xsk_build_skb())
>
> For single frame xsk skb_needs_linearize will be false and thus
> __skb_linearize is not called?
>
> More generally, I would also think that the cost of the
> validate_xmit_skb checks are quite cheap in the xsk case where they
> are all false. On the assumption that the touched cachelines are
> likely warm.
>
> > >
> > > Second, we don't understand where the win is coming from, the numbers
> > > you share are a bit vague. What's so expensive about a few skbs
> >
> > To be more accurate, it's not "a few" but "so many" because of the
> > high pps reaching more than 1,000,000. So if people run the xdpsock to
> > test it, it's not hard to see most of time is spent during the skb
> > allocation process.
>
> Right, the alloc or memcpy more than the validate?

Thanks for chiming in.

Sure thing. Validate only takes 4% total time, which could be easily
observed by using perf.

The story behind the patch is that I was scanning the code and found
the validation is not necessary based on the theory instead of
experiment, _then_ I tried xdpsock to see if any performance impact
and used perf to capture the hot spot.

And I don't think I can find any other useful improvement whether in
copy mode or zc mode after finishing investigation so far.

Last time, I mentioned that I tried two out-of-thin-air approaches[1].
But everything didn't go as well as expected...

[1]: https://lore.kernel.org/all/CAL+tcoAn8ADUGARSzZB=5dGoa+Kh7HnNBLxyqTa3W6tOhUK-sg@mail.gmail.com/

Thanks,
Jason