Re: [PATCH 6.1.y 6.6.y 6.12.y 6.18.y] net/mlx5e: xsk: Fix unlocked writing to ICOSQ

[Dragos Tatulea <dtatulea@nvidia.com>]

On 09.09.26 14:21, Greg KH wrote:
> On Wed, Sep 09, 2026 at 10:03:10AM +0000, Dragos Tatulea wrote:
>> commit c326f9c68921e2f14dfcecb2f6b4216313d50248 upstream.
>>
>> During napi poll, when the affinity changes and there's still XSK work
>> to be done, we trigger an ICOSQ interrupt on the new CPU. However, this
>> triggering on the ICOSQ is done unprotected.
>>
>> mlx5e_trigger_irq() is called while mlx5e_xsk_alloc_rx_mpwqe() is
>> running from a different CPU due to affinity change. This can happen
>> because IRQ triggering is done after napi_complete_done(). At this point
>> the NAPI can be scheduled on a different CPU. Like this:
>>
>>   CPU A (old affinity, NAPI tail)    CPU B (new affinity, fresh NAPI)
>>   -------------------------------    --------------------------------
>>   napi_complete_done()  clears SCHED
>>   mlx5e_cq_arm(...)
>>                                      napi_schedule_prep() sets SCHED
>>                                      mlx5e_napi_poll()
>>                                        mlx5e_xsk_alloc_rx_mpwqe()
>>                                          memcpy 640 B UMR body
>>                                          advance sq->pc by 10
>>   mlx5e_trigger_irq(&c->icosq)
>>     wqe_info[pi] = {NOP, 1}
>>     mlx5e_post_nop() advances sq->pc
>>
>> The obvious fix would be to lock the ICOSQ. But the ICOSQ is expected to
>> be accessed only from the channel's NAPI and has no locking. Kick the
>> async ICOSQ instead which is always locked.
>>
>> This issue was noticed in the wild with the following splat:
>>
>>   netdevice: ge-0-0-1: Bad OP in ICOSQ CQE: 0xd
>>   WARNING: drivers/net/ethernet/mellanox/mlx5/core/en_rx.c:826 [...]
>>   [...]
>>   Call Trace:
>>    <IRQ>
>>    mlx5e_napi_poll+0x11d/0x7f0 [mlx5_core]
>>    __napi_poll+0x30/0x200
>>    ? skb_defer_free_flush+0x9c/0xc0
>>    net_rx_action+0x2fe/0x3f0
>>    handle_softirqs+0xd8/0x340
>>    __irq_exit_rcu+0xbc/0xe0
>>    common_interrupt+0x85/0xa0
>>    </IRQ>
>>    <TASK>
>>    asm_common_interrupt+0x26/0x40
>>   [...]
>>   ---[ end trace 0000000000000000 ]---
>>   mlx5_core 0000:08:00.0 ge-0-0-1: Error cqe on cqn 0x548, ci 0x2022, qn 0x8f4,
>>   opcode 0xd, syndrome 0x2, vendor syndrome 0x68
> 
> Why does the changelog here differ from what is in Linus's tree?
> 
> Please don't do that :(
> 
Apologies. Will fix.

>> [ Backport to 6.18.y and older: upstream commit calls
>>   mlx5e_trigger_napi_async_icosq(), which was introduced by commit
>>   0da1dba72616 ("net/mlx5e: XSK, Fix unintended ICOSQ change") and is not
>>   present here. In these trees mlx5e_trigger_napi_icosq() is the
>>   equivalent helper: it takes c->async_icosq_lock and triggers
>>   c->async_icosq, which is unconditionally opened, activated, polled and
>>   armed for every channel. Race B of the upstream commit message does not
>>   apply, as it concerns the sync-ICOSQ variant of
>>   mlx5e_trigger_napi_icosq() that only exists upstream. ]
> 
> That part is ok, but changing the overall changelog text for no obvious
> reson isn't ok.
> 
Do you prefer that I drop it in v2.

Also, should I prefix the next patch with v2? (First time sending a stable
patch like this)

Thanks,
Dragos