From patchwork Wed Jun 23 11:07:19 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= X-Patchwork-Id: 1496070 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=osuosl.org (client-ip=140.211.166.138; helo=smtp1.osuosl.org; envelope-from=intel-wired-lan-bounces@osuosl.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=gtHQynnX; dkim-atps=neutral Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4G90yD39HYz9sWc for ; Wed, 23 Jun 2021 21:13:55 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id C5F6A82EB6; Wed, 23 Jun 2021 11:13:53 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id L-epqvRzD8jG; Wed, 23 Jun 2021 11:13:52 +0000 (UTC) Received: from ash.osuosl.org (ash.osuosl.org [140.211.166.34]) by smtp1.osuosl.org (Postfix) with ESMTP id 9D6D082EFC; Wed, 23 Jun 2021 11:13:52 +0000 (UTC) X-Original-To: intel-wired-lan@lists.osuosl.org Delivered-To: intel-wired-lan@lists.osuosl.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by ash.osuosl.org (Postfix) with ESMTP id E72BC1BF3E8 for ; Wed, 23 Jun 2021 11:13:50 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id D097E401EB for ; Wed, 23 Jun 2021 11:13:50 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Authentication-Results: smtp2.osuosl.org (amavisd-new); dkim=pass (1024-bit key) header.d=redhat.com Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id UnCALbURxuB6 for ; Wed, 23 Jun 2021 11:13:48 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by smtp2.osuosl.org (Postfix) with ESMTPS id A3DD1401E1 for ; Wed, 23 Jun 2021 11:13:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1624446827; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Qk5GBiM0tj1/P0/7gaLS65wrkya24n+AVH6Pdae+6a0=; b=gtHQynnXtxXcyczySwSLNs1njkQRoiXjlu2rOPJYUf8KbvfT8AhFR0RMXcg7BWprOkhbEH O+6haeGWW4QDoT79UkiHj/SOi82EZkb+hDqxCxxjTy25mw0V73Vj+Jm4AEscdia3kzlTED 4IRllsMZ1ajm4nOHYZ8rVF3yG4I/aPI= Received: from mail-ed1-f71.google.com (mail-ed1-f71.google.com [209.85.208.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-255-00wa7NknOc2I7HktgxaOqQ-1; Wed, 23 Jun 2021 07:13:45 -0400 X-MC-Unique: 00wa7NknOc2I7HktgxaOqQ-1 Received: by mail-ed1-f71.google.com with SMTP id j19-20020aa7c4130000b029039497d5cdbeso1097363edq.15 for ; Wed, 23 Jun 2021 04:13:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Qk5GBiM0tj1/P0/7gaLS65wrkya24n+AVH6Pdae+6a0=; b=stBwQsnXR6Rod0C0ndTmGJ3PUFRAoyNna3jq5HtGwj0tTRv6F39ptNgyBvu1xRckyG Nasj4sfG2xgjDOFs7x9+pPCzZXX02xnkHRh/5lkAkzAmJzRAs68yiqXLBuKojpHC0PJn kq+Ub101s5sks7M/zgKY/6seK6Me7+FUkoL2FzTm+dTs7xQdhOnpgz7LiVh3gjenLKhk vA6rg4Vs1j4Yhhz/XaG0cMjOYTgbIbnm9GF7OcoDwCJm1/d/S15uCfpU2yl8DvOqMyYl CIPRW6+KZeLzlK4i0TxceBBlIOR9zyp/G2hONFFlzIHwboeI9apmvMvMRznu6qx2Xu+y d7CQ== X-Gm-Message-State: AOAM532o5Nca/DRym0+WcIRiVZqTZ6P6KVLIiQ+LlxzKl2Q07t4/eGSz A0cZu5lsqWh3sLCgDrUGdTu2X0ArRKJp3JxKtGANaaOVYCF8XZxnZufRHnA0P9LmHozF6lD9TIA 5U6QsfiafvZYLbTfY8IIPOHmoHc8T/g== X-Received: by 2002:aa7:c50d:: with SMTP id o13mr11372740edq.9.1624446824013; Wed, 23 Jun 2021 04:13:44 -0700 (PDT) X-Google-Smtp-Source: ABdhPJztF5ae03ntoyILjp8pqHlp+iOfl1HNxJXBQz0XPbt2+lIDWho8ON30nAuFqqxYDRxnx9LSsA== X-Received: by 2002:aa7:c50d:: with SMTP id o13mr11372696edq.9.1624446823683; Wed, 23 Jun 2021 04:13:43 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([45.145.92.2]) by smtp.gmail.com with ESMTPSA id g8sm13567784edw.89.2021.06.23.04.13.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 23 Jun 2021 04:13:43 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 3EFD618073B; Wed, 23 Jun 2021 13:07:28 +0200 (CEST) From: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= To: bpf@vger.kernel.org, netdev@vger.kernel.org Date: Wed, 23 Jun 2021 13:07:19 +0200 Message-Id: <20210623110727.221922-12-toke@redhat.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210623110727.221922-1-toke@redhat.com> References: <20210623110727.221922-1-toke@redhat.com> MIME-Version: 1.0 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=toke@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [Intel-wired-lan] [PATCH bpf-next v4 11/19] net: intel: remove rcu_read_lock() around XDP program invocation X-BeenThere: intel-wired-lan@osuosl.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Wired Ethernet Linux Kernel Driver Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "Paul E . McKenney" , =?utf-8?q?Toke_H=C3=B8iland-J?= =?utf-8?q?=C3=B8rgensen?= , Hangbin Liu , Jesper Dangaard Brouer , Jakub Kicinski , intel-wired-lan@lists.osuosl.org, Martin KaFai Lau Errors-To: intel-wired-lan-bounces@osuosl.org Sender: "Intel-wired-lan" The Intel drivers all have rcu_read_lock()/rcu_read_unlock() pairs around XDP program invocations. However, the actual lifetime of the objects referred by the XDP program invocation is longer, all the way through to the call to xdp_do_flush(), making the scope of the rcu_read_lock() too small. This turns out to be harmless because it all happens in a single NAPI poll cycle (and thus under local_bh_disable()), but it makes the rcu_read_lock() misleading. Rather than extend the scope of the rcu_read_lock(), just get rid of it entirely. With the addition of RCU annotations to the XDP_REDIRECT map types that take bh execution into account, lockdep even understands this to be safe, so there's really no reason to keep it around. Cc: Jesse Brandeburg Cc: Tony Nguyen Cc: intel-wired-lan@lists.osuosl.org Tested-by: Jesper Dangaard Brouer # i40e Signed-off-by: Toke Høiland-Jørgensen --- drivers/net/ethernet/intel/i40e/i40e_txrx.c | 2 -- drivers/net/ethernet/intel/i40e/i40e_xsk.c | 6 +----- drivers/net/ethernet/intel/ice/ice_txrx.c | 6 +----- drivers/net/ethernet/intel/ice/ice_xsk.c | 6 +----- drivers/net/ethernet/intel/igb/igb_main.c | 2 -- drivers/net/ethernet/intel/igc/igc_main.c | 7 ++----- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 -- drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 6 +----- drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 2 -- 9 files changed, 6 insertions(+), 33 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c index de70c16ef619..ae3a64b6f5f8 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c @@ -2298,7 +2298,6 @@ static int i40e_run_xdp(struct i40e_ring *rx_ring, struct xdp_buff *xdp) struct bpf_prog *xdp_prog; u32 act; - rcu_read_lock(); xdp_prog = READ_ONCE(rx_ring->xdp_prog); if (!xdp_prog) @@ -2329,7 +2328,6 @@ static int i40e_run_xdp(struct i40e_ring *rx_ring, struct xdp_buff *xdp) break; } xdp_out: - rcu_read_unlock(); return result; } diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c index 46d884417c63..8dca53b7daff 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c @@ -153,7 +153,6 @@ static int i40e_run_xdp_zc(struct i40e_ring *rx_ring, struct xdp_buff *xdp) struct bpf_prog *xdp_prog; u32 act; - rcu_read_lock(); /* NB! xdp_prog will always be !NULL, due to the fact that * this path is enabled by setting an XDP program. */ @@ -162,9 +161,7 @@ static int i40e_run_xdp_zc(struct i40e_ring *rx_ring, struct xdp_buff *xdp) if (likely(act == XDP_REDIRECT)) { err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog); - result = !err ? I40E_XDP_REDIR : I40E_XDP_CONSUMED; - rcu_read_unlock(); - return result; + return !err ? I40E_XDP_REDIR : I40E_XDP_CONSUMED; } switch (act) { @@ -184,7 +181,6 @@ static int i40e_run_xdp_zc(struct i40e_ring *rx_ring, struct xdp_buff *xdp) result = I40E_XDP_CONSUMED; break; } - rcu_read_unlock(); return result; } diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c index e2b4b29ea207..1a311e91fb6d 100644 --- a/drivers/net/ethernet/intel/ice/ice_txrx.c +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c @@ -1129,15 +1129,11 @@ int ice_clean_rx_irq(struct ice_ring *rx_ring, int budget) xdp.frame_sz = ice_rx_frame_truesize(rx_ring, size); #endif - rcu_read_lock(); xdp_prog = READ_ONCE(rx_ring->xdp_prog); - if (!xdp_prog) { - rcu_read_unlock(); + if (!xdp_prog) goto construct_skb; - } xdp_res = ice_run_xdp(rx_ring, &xdp, xdp_prog); - rcu_read_unlock(); if (!xdp_res) goto construct_skb; if (xdp_res & (ICE_XDP_TX | ICE_XDP_REDIR)) { diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c index faa7b8d96adb..d6da377f5ac3 100644 --- a/drivers/net/ethernet/intel/ice/ice_xsk.c +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c @@ -463,7 +463,6 @@ ice_run_xdp_zc(struct ice_ring *rx_ring, struct xdp_buff *xdp) struct ice_ring *xdp_ring; u32 act; - rcu_read_lock(); /* ZC patch is enabled only when XDP program is set, * so here it can not be NULL */ @@ -473,9 +472,7 @@ ice_run_xdp_zc(struct ice_ring *rx_ring, struct xdp_buff *xdp) if (likely(act == XDP_REDIRECT)) { err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog); - result = !err ? ICE_XDP_REDIR : ICE_XDP_CONSUMED; - rcu_read_unlock(); - return result; + return !err ? ICE_XDP_REDIR : ICE_XDP_CONSUMED; } switch (act) { @@ -496,7 +493,6 @@ ice_run_xdp_zc(struct ice_ring *rx_ring, struct xdp_buff *xdp) break; } - rcu_read_unlock(); return result; } diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index 038a9fd1af44..8a11b7e55326 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -8387,7 +8387,6 @@ static struct sk_buff *igb_run_xdp(struct igb_adapter *adapter, struct bpf_prog *xdp_prog; u32 act; - rcu_read_lock(); xdp_prog = READ_ONCE(rx_ring->xdp_prog); if (!xdp_prog) @@ -8420,7 +8419,6 @@ static struct sk_buff *igb_run_xdp(struct igb_adapter *adapter, break; } xdp_out: - rcu_read_unlock(); return ERR_PTR(-result); } diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c index ea998d2defa4..2b666a6ec989 100644 --- a/drivers/net/ethernet/intel/igc/igc_main.c +++ b/drivers/net/ethernet/intel/igc/igc_main.c @@ -2175,18 +2175,15 @@ static struct sk_buff *igc_xdp_run_prog(struct igc_adapter *adapter, struct bpf_prog *prog; int res; - rcu_read_lock(); - prog = READ_ONCE(adapter->xdp_prog); if (!prog) { res = IGC_XDP_PASS; - goto unlock; + goto out; } res = __igc_xdp_run_prog(adapter, prog, xdp); -unlock: - rcu_read_unlock(); +out: return ERR_PTR(-res); } diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index c5ec17d19c59..27d7467534e0 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -2199,7 +2199,6 @@ static struct sk_buff *ixgbe_run_xdp(struct ixgbe_adapter *adapter, struct xdp_frame *xdpf; u32 act; - rcu_read_lock(); xdp_prog = READ_ONCE(rx_ring->xdp_prog); if (!xdp_prog) @@ -2237,7 +2236,6 @@ static struct sk_buff *ixgbe_run_xdp(struct ixgbe_adapter *adapter, break; } xdp_out: - rcu_read_unlock(); return ERR_PTR(-result); } diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c index 91ad5b902673..ffbf8a694362 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c @@ -100,15 +100,12 @@ static int ixgbe_run_xdp_zc(struct ixgbe_adapter *adapter, struct xdp_frame *xdpf; u32 act; - rcu_read_lock(); xdp_prog = READ_ONCE(rx_ring->xdp_prog); act = bpf_prog_run_xdp(xdp_prog, xdp); if (likely(act == XDP_REDIRECT)) { err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog); - result = !err ? IXGBE_XDP_REDIR : IXGBE_XDP_CONSUMED; - rcu_read_unlock(); - return result; + return !err ? IXGBE_XDP_REDIR : IXGBE_XDP_CONSUMED; } switch (act) { @@ -132,7 +129,6 @@ static int ixgbe_run_xdp_zc(struct ixgbe_adapter *adapter, result = IXGBE_XDP_CONSUMED; break; } - rcu_read_unlock(); return result; } diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c index ba2ed8a43d2d..fabada4ce315 100644 --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c @@ -1054,7 +1054,6 @@ static struct sk_buff *ixgbevf_run_xdp(struct ixgbevf_adapter *adapter, struct bpf_prog *xdp_prog; u32 act; - rcu_read_lock(); xdp_prog = READ_ONCE(rx_ring->xdp_prog); if (!xdp_prog) @@ -1079,7 +1078,6 @@ static struct sk_buff *ixgbevf_run_xdp(struct ixgbevf_adapter *adapter, break; } xdp_out: - rcu_read_unlock(); return ERR_PTR(-result); }