Re: [PATCH iwl-next 0/3] iavf: defer loading netmem_desc

[Alexander Lobakin <aleksander.lobakin@intel.com>]

From: Matt Vollrath <tactii@gmail.com>
Date: Fri, 31 Jul 2026 15:27:37 -0400

> On 7/31/26 11:13, Alexander Lobakin wrote:
>> From: Matt Vollrath <tactii@gmail.com>
>> Date: Fri, 31 Jul 2026 08:41:06 -0400
>>
>>> In a few places libeth and the iavf driver inspect which page pool is
>>> attached to a netmem_ref. This forces an immediate load of the
>>> netmem_desc struct for each buffer in the Rx loop.
>>>
>>> Defer or eliminate these loads of netmem_desc from the driver fast path
>>> by using the page_pool ref in the first cache line of iavf_ring instead.
>>
>> I was thinking of this when implementing the current design, but:
>>
>> Do you have any data to prove this actually helps performance?
> 
> The optimization was profile-driven, but not on an iavf. I was working on
> a libeth port of e1000e, because I'm an obsessive optimizer, and this quirk
> of the interface fell out. I'm working with a Xeon E3-1240 v3 and I218-V.
> 
> With a mirror of the changes in this patch series, on my platform, it's a
> net zero overall change in Rx performance. This is not surprising, because
> the cold read is just deferred until later. The time saved in
> eth_type_trans (where the payload is first read) is instead spent in
> skb_gro_receive and pool puts.
> 
> The additional thing that does make an improvement, which was omitted from
> this series, is prefetching netmem_desc early in the loop. This makes about
> 22 cycles/packet, 3% difference.
> 
> I left it where it is because I don't have hardware to perf this on, and
> didn't know if anyone would even be interested in looking at something that
> I can't produce data for on the actual hardware. For iavf the prefetch
> would be conditional, only when the EOP flag is set, because in this case
> netmem_desc will always be needed either for GRO merge or pool put.
> 
> I can't guarantee that this will be worth 3% on an iavf platform. In any
> case, it'll be a couple weeks before I can work on this again.

I asked because I was testing this approach on iavf and idpf when
developing the initial libeth code. But turned out that there was
[almost] no difference, while passing `struct page_pool *` to each
libeth Rx helper was very inconvenient and ugly. So I left it as it
is currently.

Moreover, most of libeth_xdp code couldn't take a separate PP pointer,
so it would make no sense at all since we're converting more and more
drivers to libeth_xdp helpers directly instead of raw libeth_rx. And
also, with XDP at least, the core either way accesses netmem_desc->pp.

So honestly I wouldn't make this change for some ghost benefit (the
current code is either way capable of handling 50-60 Mpps on XDP_DROP
despite that it reads netmem_desc->pp for each frame).

> 
>>
>>>
>>> There are only two paths out of the Rx loop where netmem_desc needs to
>>> be consumed:
>>> * The "very rare" case of libeth_rx_sync_for_cpu calling
>>>    libeth_rx_recycle_slow and indicating that there was no data. This
>>>    could be similarly factored out, but not by this series.
>>> * GRO merging a frame into an existing aggregate stream. In this case,
>>>    the cold load of netmem_desc may overlap the payload prefetch started
>>>    by iavf_build_skb, which is now no longer dependent on netmem_desc to
>>>    start.
> 
> I see now this isn't right, anything going to GRO will end up back in the
> pool in the same iteration, and netmem_desc will be loaded. It just
> happens later.

Thanks,
Olek