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

Message-ID: <CAL+tcoCMQhaZdvbR1p50tuVk0RUdqAiRgjDrO0b+EO1XvM=2qw@mail.gmail.com>
Date: Thu, 17 Jul 2025 09:12:54 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: 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, 
	willemdebruijn.kernel@...il.com, 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 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())

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

> accesses? Maybe there's an optimization possible to the validation,
> which would apply more broadly, instead of skipping it for one trivial
> case.
>
> Third, I asked you to compare with AF_PACKET, because IIUC it should
> have similar properties as AF_XDP in copy mode. So why not use that?

I haven't run into AF_PACKET so far. At least, I can confirm that xsk
doesn't need it from my side. The whole logic of validation apparently
is not designed for xsk case...

>
> Lastly, the patch is not all that bad, sure. But the experience of
> supporting generic XDP is a very mixed. All the paths that pretend
> to do XDP on skbs have a bunch of quirks and bugs. I'd prefer that
> we push back more broadly on any sort of pretend XDP.

Well, sorry, I feel a bit upset when reading this because as I
insisted before not everyone can use the advanced zerocopy mode.

Thanks,
Jason