netdev - RE: [Intel-wired-lan] [PATCH] ixgbe: Panic during XDP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <MN2PR11MB40459F3C2E0444FB7B211095EA849@MN2PR11MB4045.namprd11.prod.outlook.com>
Date:   Fri, 24 Mar 2023 08:52:04 +0000
From:   "Rout, ChandanX" <chandanx.rout@...el.com>
To:     Jiri Pirko <jiri@...nulli.us>, John Hickey <jjh@...dalian.us>,
        "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>
CC:     Jesper Dangaard Brouer <hawk@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        John Fastabend <john.fastabend@...il.com>,
        "Brandeburg, Jesse" <jesse.brandeburg@...el.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Jakub Kicinski <kuba@...nel.org>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        "David S. Miller" <davem@...emloft.net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Kuruvinakunnel, George" <george.kuruvinakunnel@...el.com>,
        "Nagaraju, Shwetha" <shwetha.nagaraju@...el.com>,
        "Nagraj, Shravan" <shravan.nagraj@...el.com>
Subject: RE: [Intel-wired-lan] [PATCH] ixgbe: Panic during XDP_TX with > 64
 CPUs



-----Original Message-----
From: Intel-wired-lan <intel-wired-lan-bounces@...osl.org> On Behalf Of Jiri Pirko
Sent: 30 January 2023 20:28
To: John Hickey <jjh@...dalian.us>
Cc: Jesper Dangaard Brouer <hawk@...nel.org>; Daniel Borkmann <daniel@...earbox.net>; intel-wired-lan@...ts.osuosl.org; John Fastabend <john.fastabend@...il.com>; Brandeburg, Jesse <jesse.brandeburg@...el.com>; Alexei Starovoitov <ast@...nel.org>; Eric Dumazet <edumazet@...gle.com>; Nguyen, Anthony L <anthony.l.nguyen@...el.com>; netdev@...r.kernel.org; Jakub Kicinski <kuba@...nel.org>; bpf@...r.kernel.org; Paolo Abeni <pabeni@...hat.com>; David S. Miller <davem@...emloft.net>; linux-kernel@...r.kernel.org
Subject: Re: [Intel-wired-lan] [PATCH] ixgbe: Panic during XDP_TX with > 64 CPUs

Sat, Jan 28, 2023 at 02:12:12AM CET, jjh@...dalian.us wrote:
>In commit 'ixgbe: let the xdpdrv work with more than 64 cpus'
>(4fe815850bdc8d4cc94e06fe1de069424a895826), support was added 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 via the Intel 5.18.6 out of tree driver 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.
>
>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 greater than 64 CPUs.
>
>For this patch, I have changed static_branch_inc to 
>static_branch_enable in ixgbe_setup_xdp.  We aren't counting references 
>and I don't see any reason to turn it off, since all the locking 
>appears to be in the XDP_TX path, which isn't run if a XDP program isn't loaded.
>
>Signed-off-by: John Hickey <jjh@...dalian.us>

This is missing "Fixes" tag and "net" keyword in "[patch]" subject section.


>---
> drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c  | 3 ---  
>drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
> 2 files changed, 1 insertion(+), 4 deletions(-)
>

Tested-by: Chandan Kumar Rout <chandanx.rout@...el.com> (A Contingent Worker at Intel)