Jason Xing wrote: > On Thu, Jul 17, 2025 at 8:52 AM Jakub Kicinski <kuba@xxxxxxxxxx> 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? > > 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
