Re: [PATCH net v3 8/8] xsk: don't support AF_XDP on 32-bit architectures

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

From: Jason Xing <kerneljasonxing@gmail.com>
Date: Thu, 23 Apr 2026 07:49:58 +0800

> On Thu, Apr 23, 2026 at 12:58 AM Maciej Fijalkowski
> <maciej.fijalkowski@intel.com> wrote:
>>
>> On Thu, Apr 23, 2026 at 12:37:07AM +0800, Jason Xing wrote:
>>> On Thu, Apr 23, 2026 at 12:10 AM Alexander Lobakin
>>> <aleksander.lobakin@intel.com> wrote:
>>>>
>>>> From: Jason Xing <kerneljasonxing@gmail.com>
>>>> Date: Wed, 22 Apr 2026 11:36:50 +0800
>>>>
>>>>> From: Jason Xing <kernelxing@tencent.com>
>>>>>
>>>>> In copy mode TX, xsk_skb_destructor_set_addr() stores the 64-bit
>>>>> descriptor address into skb_shinfo(skb)->destructor_arg (void *) via a
>>>>> uintptr_t cast:
>>>>>
>>>>>     skb_shinfo(skb)->destructor_arg = (void *)((uintptr_t)addr | 0x1UL);
>>>>>
>>>>> On 32-bit architectures uintptr_t is 32 bits, so the upper 32 bits of
>>>>> the descriptor address are silently dropped. In XDP_ZEROCOPY unaligned
>>>>> mode the chunk offset is encoded in bits 48-63 of the descriptor
>>>>> address (XSK_UNALIGNED_BUF_OFFSET_SHIFT = 48), meaning the offset is
>>>>> lost entirely. The completion queue then returns a truncated address to
>>>>> userspace, making buffer recycling impossible.
>>>>
>>>> What if we relax the restriction a bit? For example, refuse to configure
>>>
>>> As to the bug itself, yes, It only affects the unaligned mode.
>>>
>>> I wonder if we can support this after someone requires us to support
>>> 32-bit arch and use it in the real world, then we can use the previous
>>> patch to complete the full support (which doesn't harm the path on
>>> 64-bit arch).
>>>
>>> The code looks like this based on your suggestion. Just for the record.
>>> diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
>>> index 58da2f4f4397..03417b04592f 100644
>>> --- a/net/xdp/xdp_umem.c
>>> +++ b/net/xdp/xdp_umem.c
>>> @@ -177,6 +177,9 @@ static int xdp_umem_reg(struct xdp_umem *umem,
>>> struct xdp_umem_reg *mr)
>>> if (mr->flags & ~XDP_UMEM_FLAGS_VALID)
>>> return -EINVAL;
>>> + if (!IS_ENABLED(CONFIG_64BIT) && unaligned_chunks)
>>> + return -EOPNOTSUPP;
>>> +
>>> if (!unaligned_chunks && !is_power_of_2(chunk_size))
>>> return -EINVAL;
>>>
>>> Actually I'm fine with either of them. Right now I'm not so sure which
>>> direction this patch should take :)
>>
>> Abstracting from discussion, that is not a patch we would want against
>> -net. We should not forbid 32bit on stable kernels, maybe there is someone
>> in the 'basement' using 32bit xsk on stable kernel. Plus the fixes tag vs
>> patch's content looks weird ;)
> 
> Sorry, I don't follow this. Any specific suggestions on how to move forward?

Maciej just says that if we go the first route (forbid 32-bit arches),
then this commit should not go to the stable/LTS kernels.
But

> 
> Now we have three approaches w/o the fixes tag:
> 1. completely forbid 32-bit arch
> 2. only forbid unaligned mode on 32-bit arch
> 3. completely fix/modify the allocation logic of the first frag on
> 32-bit (see the v2-0008 patch)
> 
> I'm wondering how to proceed with that?

I've just reread the v2 thread (I realized I missed it earlier) and to
me, the v2-0008 (route 3) solution looks good. It introduces a bit of
overhead, but only for 32-bit arches, where the XSk performance will
never be push-it-to-the-limit anyway (esp. given that this is skb mode).

So I'd personally vote for number 3. 47 insertions(+), 7 deletions(-) is
not a big deal. And for 64-bit arches, there won't be any object code
changes at all (unconditional `return 0` and checking the return value
of xsk_skb_init_misc() will be optimized out by the compiler).

Number 2 is also okay-ish, if we're sure that addr is always 32-bit on
32-bit arches in the aligned mode. But given that you already has a
proper fix (n. 3) which allows to not forbid anything at all, I believe
it's more preferred to fix rather than disable.

+ as mentioned above, if you go route number 3, the series could be sent
to -net and the fix would eventually hit the stable/LTS trees, but I
don't think we can disable/forbid something in the stable kernels.

> 
> Thanks,
> Jason
Thanks,
Olek