Skip to content
Commit 913eda2b authored by Maciej Fijalkowski's avatar Maciej Fijalkowski Committed by Jakub Kicinski
Browse files

i40e: xsk: remove count_mask

Cited commit introduced a neat way of updating next_to_clean that does
not require boundary checks on each increment. This was done by masking
the new value with (ring length - 1) mask. Problem is that this is
applicable only for power of 2 ring sizes, for every other size this
assumption can not be made. In turn, it leads to cleaning descriptors
out of order as well as splats:

[ 1388.411915] Workqueue: events xp_release_deferred
[ 1388.411919] RIP: 0010:xp_free+0x1a/0x50
[ 1388.411921] Code: 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 0f 1f 44 00 00 55 48 8b 57 70 48 8d 47 70 48 89 e5 48 39 d0 74 06 <5d> c3 cc cc cc cc 48 8b 57 60 83 82 b8 00 00 00 01 48 8b 57 60 48
[ 1388.411922] RSP: 0018:ffa0000000a83cb0 EFLAGS: 00000206
[ 1388.411923] RAX: ff11000119aa5030 RBX: 000000000000001d RCX: ff110001129b6e50
[ 1388.411924] RDX: ff11000119aa4fa0 RSI: 0000000055555554 RDI: ff11000119aa4fc0
[ 1388.411925] RBP: ffa0000000a83cb0 R08: 0000000000000000 R09: 0000000000000000
[ 1388.411926] R10: 0000000000000001 R11: 0000000000000000 R12: ff11000115829b80
[ 1388.411927] R13: 000000000000005f R14: 0000000000000000 R15: ff11000119aa4fc0
[ 1388.411928] FS:  0000000000000000(0000) GS:ff11000277e00000(0000) knlGS:0000000000000000
[ 1388.411929] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1388.411930] CR2: 00007f1f564e6c14 CR3: 000000000783c005 CR4: 0000000000771ef0
[ 1388.411931] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 1388.411931] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 1388.411932] PKRU: 55555554
[ 1388.411933] Call Trace:
[ 1388.411934]  <IRQ>
[ 1388.411935]  ? show_regs+0x6e/0x80
[ 1388.411937]  ? watchdog_timer_fn+0x1d2/0x240
[ 1388.411939]  ? __pfx_watchdog_timer_fn+0x10/0x10
[ 1388.411941]  ? __hrtimer_run_queues+0x10e/0x290
[ 1388.411945]  ? clockevents_program_event+0xae/0x130
[ 1388.411947]  ? hrtimer_interrupt+0x105/0x240
[ 1388.411949]  ? __sysvec_apic_timer_interrupt+0x54/0x150
[ 1388.411952]  ? sysvec_apic_timer_interrupt+0x7f/0x90
[ 1388.411955]  </IRQ>
[ 1388.411955]  <TASK>
[ 1388.411956]  ? asm_sysvec_apic_timer_interrupt+0x1f/0x30
[ 1388.411958]  ? xp_free+0x1a/0x50
[ 1388.411960]  i40e_xsk_clean_rx_ring+0x5d/0x100 [i40e]
[ 1388.411968]  i40e_clean_rx_ring+0x14c/0x170 [i40e]
[ 1388.411977]  i40e_queue_pair_disable+0xda/0x260 [i40e]
[ 1388.411986]  i40e_xsk_pool_setup+0x192/0x1d0 [i40e]
[ 1388.411993]  i40e_reconfig_rss_queues+0x1f0/0x1450 [i40e]
[ 1388.412002]  xp_disable_drv_zc+0x73/0xf0
[ 1388.412004]  ? mutex_lock+0x17/0x50
[ 1388.412007]  xp_release_deferred+0x2b/0xc0
[ 1388.412010]  process_one_work+0x178/0x350
[ 1388.412011]  ? __pfx_worker_thread+0x10/0x10
[ 1388.412012]  worker_thread+0x2f7/0x420
[ 1388.412014]  ? __pfx_worker_thread+0x10/0x10
[ 1388.412015]  kthread+0xf8/0x130
[ 1388.412017]  ? __pfx_kthread+0x10/0x10
[ 1388.412019]  ret_from_fork+0x3d/0x60
[ 1388.412021]  ? __pfx_kthread+0x10/0x10
[ 1388.412023]  ret_from_fork_asm+0x1b/0x30
[ 1388.412026]  </TASK>

It comes from picking wrong ring entries when cleaning xsk buffers
during pool detach.

Remove the count_mask logic and use they boundary check when updating
next_to_process (which used to be a next_to_clean).

Fixes: c8a8ca34

 ("i40e: remove unnecessary memory writes of the next to clean pointer")
Reported-by: default avatarTushar Vyavahare <tushar.vyavahare@intel.com>
Tested-by: default avatarTushar Vyavahare <tushar.vyavahare@intel.com>
Signed-off-by: default avatarMaciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: default avatarJacob Keller <jacob.e.keller@intel.com>
Link: https://lore.kernel.org/r/20231018163908.40841-1-maciej.fijalkowski@intel.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent ce55c22e
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment