On Tue, Aug 26, 2025 at 8:24 PM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote:
>
> On 8/26/25 10:14 AM, Dan Carpenter wrote:
> > On Wed, Aug 20, 2025 at 05:44:16PM +0200, Maciej Fijalkowski wrote:
> >> return ERR_PTR(err);
> >>
> >> skb_reserve(skb, hr);
> >> +
> >> + addrs = kmem_cache_zalloc(xsk_tx_generic_cache, GFP_KERNEL);
> >> + if (!addrs) {
> >> + kfree(skb);
> >
> > This needs to be kfree_skb(skb);
>
> Oh well, good catch! Maciej, given this commit did not land yet in Linus' tree,
> I can toss the commit from bpf tree assuming you send a v7?
>
> Also, looking at xsk_build_skb(), do we similarly need to free that allocated
> skb when we hit the ERR_PTR(-EOVERFLOW) ? Mentioned function has the following
> in the free_err path:
>
> if (first_frag && skb)
> kfree_skb(skb);
>
> Pls double check.
Sorry to join the party late...
I have to bring bad news here. After I tested[1] on VM, I saw an
around 18.8% performance decrease (from 745,705 to 627,331) with this
patch applied.It can be reproduced stably :(
[1]: ./xdpsock -i eth1 -t -S -s 64
Thanks,
Jason
