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

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

[Dragos Tatulea]

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

[ 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. ]

Fixes: db05815b36cb ("net/mlx5e: Add XSK zero-copy support")
Reported-by: Paul Saab <ps@mu.org>
Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20260513064613.334602-1-tariqt@nvidia.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
index 44547b22a536..799414c2cdc8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
@@ -245,7 +245,7 @@ int mlx5e_napi_poll(struct napi_struct *napi, int budget)
 	}
 
 	if (unlikely(aff_change && busy_xsk)) {
-		mlx5e_trigger_irq(&c->icosq);
+		mlx5e_trigger_napi_icosq(c);
 		ch_stats->force_irq++;
 	}
 
-- 
2.43.0

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

[Greg KH]

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 :(

> [ 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.

thanks,

greg k-h

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

[Dragos Tatulea]

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

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

[Greg KH]

On Wed, Sep 09, 2026 at 02:47:16PM +0200, Dragos Tatulea wrote:
> 
> 
> 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)

Yes please, v2 would be great.

thanks,

greg k-h

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

[Greg KH]

On Wed, Sep 09, 2026 at 02:47:16PM +0200, Dragos Tatulea wrote:
> 
> 
> 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.

No, that's ok, it's great to have, thanks for that.

greg k-h