Re: [RFC PATCHv2 1/1] idpf: Fix header clobber in IDPF with SWIOTLB enabled

[Alexander Lobakin]

From: Steve Rutherford <srutherford@xxxxxxxxxx>
Date: Tue, 3 Mar 2026 11:44:19 -0800

> On Tue, Mar 3, 2026 at 7:34 AM Alexander Lobakin
> <aleksander.lobakin@xxxxxxxxx> wrote:
>>
>> From: Steve Rutherford <srutherford@xxxxxxxxxx>
>> Date: Fri, 27 Feb 2026 20:34:57 +0000
>>
>>> When SWIOTLB and header split are enabled, IDPF sees empty packets in the
>>> rx queue.
>>>
>>> This is caused by libeth_rx_sync_for_cpu clobbering the synthesized header
>>> in the workaround (i.e. overflow) path. After the header is synthesized by
>>> idpf_rx_hsplit_wa, the sync call pulls from the empty SWIOTLB buffer,
>>> effectively zeroing out the buffer.
>>>
>>> This skips the extra sync in the workaround path in most cases. The one
>>> exception is that it calls sync to trigger a recycle the header buffer when
>>> it fails to find a header in the payload.
>>>
>>> Fixes: 90912f9f4f2d1 ("idpf: convert header split mode to libeth + napi_build_skb()")
>>> Signed-off-by: Steve Rutherford <srutherford@xxxxxxxxxx>
>>> ---
>>>  drivers/net/ethernet/intel/idpf/idpf_txrx.c | 9 +++++++--
>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
>>> index 3ddf7b1e85ef..946203a6bd86 100644
>>> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
>>> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
>>> @@ -3007,9 +3007,14 @@ static int idpf_rx_splitq_clean(struct idpf_rx_queue *rxq, int budget)
>>>                       u64_stats_update_begin(&rxq->stats_sync);
>>>                       u64_stats_inc(&rxq->q_stats.hsplit_buf_ovf);
>>>                       u64_stats_update_end(&rxq->stats_sync);
>>> -             }
>>>
>>> -             if (libeth_rx_sync_for_cpu(hdr, hdr_len)) {
>>> +                     /* Recycle the hdr buffer if unused.*/
>>> +                     if (!hdr_len)
>>> +                             libeth_rx_sync_for_cpu(hdr, 0);
>>> +             } else if (!libeth_rx_sync_for_cpu(hdr, hdr_len))
>>> +                     hdr_len = 0;
>>> +
>>> +             if (hdr_len) {
>>
>> This is for a very old tree I believe? We now have
>> libeth_xdp_process_buff() there for quite some time already.
> 
> It is, yeah. I thought I posted a cover letter with more of a description, but,
> frankly, I may have messed up the process of posting.
> 
> From the cover letter -
> Found an issue with the IDPF driver when SWIOTLB is enabled. The issue
> results in empty headers for packets that hit the split queue workaround
> path. It's caused by a spurious sync in that path. The header is synced
> from the SWIOTLB even when the header was shoved into the payload.
> 
> I cooked up a sample patch, but I'm not an expert in this driver, so I have
> no idea if it's the right solution. It did allow my QEMU VM to boot with a
> superficially functional passed-through IDPF NIC and SWIOTLB=force.
> 
> The patch was written against COS's 6.12, so I assume that it will not
> apply cleanly elsewhere, but I figured a wrong sample patch was better than
> a long paragraph describing the same thing. My read of more recent kernels
> is that this problem is still present, but could be mistaken.

Ooops, sorry, I haven't read the cover letter =\

Did I get it correctly that in case of SWIOTLB, we can't sync the same
buffer two times? But if the hsplit W/A was applied, then this double
sync corrupts the data?

I'll prepare a patch for the latest net (with you as Co-developed-by or
any other tag you prefer) once I find a way how to play this nicely with
libeth_xdp_process_buff(). It performs an unconditional sync and bails
out if it returned false.

> 
> Thanks,
> Steve

Thanks,
Olek