Patch "ixgbe: Fix panic during XDP_TX with > 64 CPUs" has been added to the 6.1-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    ixgbe: Fix panic during XDP_TX with > 64 CPUs

to the 6.1-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     ixgbe-fix-panic-during-xdp_tx-with-64-cpus.patch
and it can be found in the queue-6.1 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit 9201b8cc44f66365ca2394d24f81388054735b28
Author: John Hickey <jjh@xxxxxxxxxxxx>
Date:   Tue Apr 25 10:03:08 2023 -0700

    ixgbe: Fix panic during XDP_TX with > 64 CPUs
    
    [ Upstream commit c23ae5091a8b3e50fe755257df020907e7c029bb ]
    
    Commit 4fe815850bdc ("ixgbe: let the xdpdrv work with more than 64 cpus")
    adds support to allow XDP programs to run on systems with more than
    64 CPUs by locking the XDP TX rings and indexing them using cpu % 64
    (IXGBE_MAX_XDP_QS).
    
    Upon trying this out patch on a system with more than 64 cores,
    the kernel paniced with an array-index-out-of-bounds at the return in
    ixgbe_determine_xdp_ring in ixgbe.h, which means ixgbe_determine_xdp_q_idx
    was just returning the cpu instead of cpu % IXGBE_MAX_XDP_QS.  An example
    splat:
    
     ==========================================================================
     UBSAN: array-index-out-of-bounds in
     /var/lib/dkms/ixgbe/5.18.6+focal-1/build/src/ixgbe.h:1147:26
     index 65 is out of range for type 'ixgbe_ring *[64]'
     ==========================================================================
     BUG: kernel NULL pointer dereference, address: 0000000000000058
     #PF: supervisor read access in kernel mode
     #PF: error_code(0x0000) - not-present page
     PGD 0 P4D 0
     Oops: 0000 [#1] SMP NOPTI
     CPU: 65 PID: 408 Comm: ksoftirqd/65
     Tainted: G          IOE     5.15.0-48-generic #54~20.04.1-Ubuntu
     Hardware name: Dell Inc. PowerEdge R640/0W23H8, BIOS 2.5.4 01/13/2020
     RIP: 0010:ixgbe_xmit_xdp_ring+0x1b/0x1c0 [ixgbe]
     Code: 3b 52 d4 cf e9 42 f2 ff ff 66 0f 1f 44 00 00 0f 1f 44 00 00 55 b9
     00 00 00 00 48 89 e5 41 57 41 56 41 55 41 54 53 48 83 ec 08 <44> 0f b7
     47 58 0f b7 47 5a 0f b7 57 54 44 0f b7 76 08 66 41 39 c0
     RSP: 0018:ffffbc3fcd88fcb0 EFLAGS: 00010282
     RAX: ffff92a253260980 RBX: ffffbc3fe68b00a0 RCX: 0000000000000000
     RDX: ffff928b5f659000 RSI: ffff928b5f659000 RDI: 0000000000000000
     RBP: ffffbc3fcd88fce0 R08: ffff92b9dfc20580 R09: 0000000000000001
     R10: 3d3d3d3d3d3d3d3d R11: 3d3d3d3d3d3d3d3d R12: 0000000000000000
     R13: ffff928b2f0fa8c0 R14: ffff928b9be20050 R15: 000000000000003c
     FS:  0000000000000000(0000) GS:ffff92b9dfc00000(0000)
     knlGS:0000000000000000
     CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
     CR2: 0000000000000058 CR3: 000000011dd6a002 CR4: 00000000007706e0
     DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
     DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
     PKRU: 55555554
     Call Trace:
      <TASK>
      ixgbe_poll+0x103e/0x1280 [ixgbe]
      ? sched_clock_cpu+0x12/0xe0
      __napi_poll+0x30/0x160
      net_rx_action+0x11c/0x270
      __do_softirq+0xda/0x2ee
      run_ksoftirqd+0x2f/0x50
      smpboot_thread_fn+0xb7/0x150
      ? sort_range+0x30/0x30
      kthread+0x127/0x150
      ? set_kthread_struct+0x50/0x50
      ret_from_fork+0x1f/0x30
      </TASK>
    
    I think this is how it happens:
    
    Upon loading the first XDP program on a system with more than 64 CPUs,
    ixgbe_xdp_locking_key is incremented in ixgbe_xdp_setup.  However,
    immediately after this, the rings are reconfigured by ixgbe_setup_tc.
    ixgbe_setup_tc calls ixgbe_clear_interrupt_scheme which calls
    ixgbe_free_q_vectors which calls ixgbe_free_q_vector in a loop.
    ixgbe_free_q_vector decrements ixgbe_xdp_locking_key once per call if
    it is non-zero.  Commenting out the decrement in ixgbe_free_q_vector
    stopped my system from panicing.
    
    I suspect to make the original patch work, I would need to load an XDP
    program and then replace it in order to get ixgbe_xdp_locking_key back
    above 0 since ixgbe_setup_tc is only called when transitioning between
    XDP and non-XDP ring configurations, while ixgbe_xdp_locking_key is
    incremented every time ixgbe_xdp_setup is called.
    
    Also, ixgbe_setup_tc can be called via ethtool --set-channels, so this
    becomes another path to decrement ixgbe_xdp_locking_key to 0 on systems
    with more than 64 CPUs.
    
    Since ixgbe_xdp_locking_key only protects the XDP_TX path and is tied
    to the number of CPUs present, there is no reason to disable it upon
    unloading an XDP program.  To avoid confusion, I have moved enabling
    ixgbe_xdp_locking_key into ixgbe_sw_init, which is part of the probe path.
    
    Fixes: 4fe815850bdc ("ixgbe: let the xdpdrv work with more than 64 cpus")
    Signed-off-by: John Hickey <jjh@xxxxxxxxxxxx>
    Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@xxxxxxxxx>
    Tested-by: Chandan Kumar Rout <chandanx.rout@xxxxxxxxx> (A Contingent Worker at Intel)
    Signed-off-by: Tony Nguyen <anthony.l.nguyen@xxxxxxxxx>
    Link: https://lore.kernel.org/r/20230425170308.2522429-1-anthony.l.nguyen@xxxxxxxxx
    Signed-off-by: Paolo Abeni <pabeni@xxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
index f8156fe4b1dc4..0ee943db3dc92 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
@@ -1035,9 +1035,6 @@ static void ixgbe_free_q_vector(struct ixgbe_adapter *adapter, int v_idx)
 	adapter->q_vector[v_idx] = NULL;
 	__netif_napi_del(&q_vector->napi);
 
-	if (static_key_enabled(&ixgbe_xdp_locking_key))
-		static_branch_dec(&ixgbe_xdp_locking_key);
-
 	/*
 	 * after a call to __netif_napi_del() napi may still be used and
 	 * ixgbe_get_stats64() might access the rings on this vector,
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index faf3a094ac540..9b8848daeb430 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6495,6 +6495,10 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter,
 	set_bit(0, adapter->fwd_bitmask);
 	set_bit(__IXGBE_DOWN, &adapter->state);
 
+	/* enable locking for XDP_TX if we have more CPUs than queues */
+	if (nr_cpu_ids > IXGBE_MAX_XDP_QS)
+		static_branch_enable(&ixgbe_xdp_locking_key);
+
 	return 0;
 }
 
@@ -10288,8 +10292,6 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
 	 */
 	if (nr_cpu_ids > IXGBE_MAX_XDP_QS * 2)
 		return -ENOMEM;
-	else if (nr_cpu_ids > IXGBE_MAX_XDP_QS)
-		static_branch_inc(&ixgbe_xdp_locking_key);
 
 	old_prog = xchg(&adapter->xdp_prog, prog);
 	need_reset = (!!prog != !!old_prog);



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux