From patchwork Thu Jun 17 21:27:33 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: 12329561 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 839FDC48BE5 for ; Thu, 17 Jun 2021 21:28:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6DB3161375 for ; Thu, 17 Jun 2021 21:28:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233009AbhFQVaI (ORCPT ); Thu, 17 Jun 2021 17:30:08 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:39164 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230288AbhFQVaH (ORCPT ); Thu, 17 Jun 2021 17:30:07 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623965278; 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=ESlzoBrCtV1p5Bz5L61JWXgmG5edOzrWTpf13f6lwsQ=; b=EUNzvsFw1gUIvG2m24sl/ooKd0gr/8H4zQ7xV3X+3nUQMQ3pemtMyGQYnUwe0UpCDk+MAb hrTFNdCiHlBjvTPSE5Yec49g3fVC1LcqQzabvfS12SIH/1RxIONYQMhGgHNX6e7r39KGVC 5h9UwmpcYgvw5XrRG41kQdwJ/sc4C4E= Received: from mail-ed1-f69.google.com (mail-ed1-f69.google.com [209.85.208.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-448-mtOtoRsbN5aofYJlzOb7bA-1; Thu, 17 Jun 2021 17:27:57 -0400 X-MC-Unique: mtOtoRsbN5aofYJlzOb7bA-1 Received: by mail-ed1-f69.google.com with SMTP id v8-20020a0564023488b0290393873961f6so2373405edc.17 for ; Thu, 17 Jun 2021 14:27:56 -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=ESlzoBrCtV1p5Bz5L61JWXgmG5edOzrWTpf13f6lwsQ=; b=IRVI+cej4MrIOIeQ2a7jzZFcCJcKeaaSvwCqfIIhnisyVql+peC+NY31LFhbKRuT0S zp60oxBFE/pMZpCmEvQwhyifVddXtlpqig4EQvoHR99Nx2CJXlItM4KmVIpRoMAees4c wX9eYHye3KaWrWXlGvp0oJvQsibr/QBtAwubXVTeYtjqMwjFklIVbUo2hFT9uz5piNqp eaCmymAFvT4quCeKfFHnz9KeO1rPOOIbPAHzlAwytX7+eJzX8EYcgLuP5/NnocWJA9AI 7OPwsiij60GCK8UAw3UTMPBKgE0PrBjcPGbwplj+NBkE3xMupkzqIIh1nzpCq44f7V8n N10w== X-Gm-Message-State: AOAM532XZd/G64JrGEZbrl6bmAOAnsCdPxlJSOsr8/14NWBED6ydzJi/ EbfyFxGjibFZMX0tNyQx6ZoAlFonhL1ZpCWWKUBZJngRuhoc+JduTVZxXCEoRlEZV+PiROaX3md XuFd6KEEOm/V5 X-Received: by 2002:a17:906:b03:: with SMTP id u3mr7263312ejg.95.1623965275938; Thu, 17 Jun 2021 14:27:55 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxVq90bXdFT/ATvZR+KbPRvFhmdGeDC2xS3RJdzmjp7zYHS+Yy5ZefFxwZeuZ2Lr3aFcVh7Nw== X-Received: by 2002:a17:906:b03:: with SMTP id u3mr7263295ejg.95.1623965275782; Thu, 17 Jun 2021 14:27:55 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([45.145.92.2]) by smtp.gmail.com with ESMTPSA id h9sm5028627edt.18.2021.06.17.14.27.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Jun 2021 14:27:55 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 775961802A5; Thu, 17 Jun 2021 23:27:54 +0200 (CEST) From: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= To: bpf@vger.kernel.org, netdev@vger.kernel.org Cc: Martin KaFai Lau , Hangbin Liu , Jesper Dangaard Brouer , Magnus Karlsson , "Paul E . McKenney" , Jakub Kicinski , =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= Subject: [PATCH bpf-next v3 01/16] rcu: Create an unrcu_pointer() to remove __rcu from a pointer Date: Thu, 17 Jun 2021 23:27:33 +0200 Message-Id: <20210617212748.32456-2-toke@redhat.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210617212748.32456-1-toke@redhat.com> References: <20210617212748.32456-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net From: "Paul E. McKenney" The xchg() and cmpxchg() functions are sometimes used to carry out RCU updates. Unfortunately, this can result in sparse warnings for both the old-value and new-value arguments, as well as for the return value. The arguments can be dealt with using RCU_INITIALIZER(): old_p = xchg(&p, RCU_INITIALIZER(new_p)); But a sparse warning still remains due to assigning the __rcu pointer returned from xchg to the (most likely) non-__rcu pointer old_p. This commit therefore provides an unrcu_pointer() macro that strips the __rcu. This macro can be used as follows: old_p = unrcu_pointer(xchg(&p, RCU_INITIALIZER(new_p))); Reported-by: Toke Høiland-Jørgensen Signed-off-by: Paul E. McKenney Signed-off-by: Toke Høiland-Jørgensen --- include/linux/rcupdate.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h index 9455476c5ba2..d7895b81264e 100644 --- a/include/linux/rcupdate.h +++ b/include/linux/rcupdate.h @@ -363,6 +363,20 @@ static inline void rcu_preempt_sleep_check(void) { } #define rcu_check_sparse(p, space) #endif /* #else #ifdef __CHECKER__ */ +/** + * unrcu_pointer - mark a pointer as not being RCU protected + * @p: pointer needing to lose its __rcu property + * + * Converts @p from an __rcu pointer to a __kernel pointer. + * This allows an __rcu pointer to be used with xchg() and friends. + */ +#define unrcu_pointer(p) \ +({ \ + typeof(*p) *_________p1 = (typeof(*p) *__force)(p); \ + rcu_check_sparse(p, __rcu); \ + ((typeof(*p) __force __kernel *)(_________p1)); \ +}) + #define __rcu_access_pointer(p, space) \ ({ \ typeof(*p) *_________p1 = (typeof(*p) *__force)READ_ONCE(p); \ From patchwork Thu Jun 17 21:27:34 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: 12329555 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, UNWANTED_LANGUAGE_BODY,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4A1EEC2B9F4 for ; Thu, 17 Jun 2021 21:28:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1E9A460D07 for ; Thu, 17 Jun 2021 21:28:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232939AbhFQVaH (ORCPT ); Thu, 17 Jun 2021 17:30:07 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:31862 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230334AbhFQVaH (ORCPT ); Thu, 17 Jun 2021 17:30:07 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623965278; 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=x+C1vvBih2DDvMDumUaYzwO9FAV6D2CItQN3YHn0G7w=; b=UyVLYk+R40TPN2gjxxJ6AnCrY6BqKrtvb/0N0vPT7aiuiIw/e9n0o+5Jhq00VvueZDnc5v ANOLoNNYA3sOIM+A0ZDWza8+EMrc8CWERzq9bOFS9Vw2Lt2nZxhSYw4Q910PAv0E9Fc2J4 /bZRNSO5Pd3OMB3LK6crZ5N4dmaDIm0= Received: from mail-ed1-f69.google.com (mail-ed1-f69.google.com [209.85.208.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-394-ZpfDd0uFMj-HNA11kmiSeA-1; Thu, 17 Jun 2021 17:27:57 -0400 X-MC-Unique: ZpfDd0uFMj-HNA11kmiSeA-1 Received: by mail-ed1-f69.google.com with SMTP id q7-20020aa7cc070000b029038f59dab1c5so2368957edt.23 for ; Thu, 17 Jun 2021 14:27:57 -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=x+C1vvBih2DDvMDumUaYzwO9FAV6D2CItQN3YHn0G7w=; b=M/aAkeQ0arlmZlAXoorx02jx7W5MJQwcbudXuvKyLGB9et9swjo9DQaL2YW1g+qBU9 UqaSMZSORozFdoZhRzkfmvaCfYXSltk2Q/8FHsNNqhFILTkVF6yasFVQdVa7uxaOME2z dqezskooGmnGptWvo+zrG4XAu6SC9S9qK79SHhRROpf7CyTVxO2/0EasiWH//sAAsKWn dQior7JVgoVR2DNJrafpVftlANhKN3lVS+yNTi9McPtHTjrhw2W5+YcMYxbmyV1rFarI 051NUOCe1rz15ompx6DW8/Nz66pNSxJEhzFs2g6GvxSlSd6Y0MWeJLr8nmSFdyG5XB+E Dw3Q== X-Gm-Message-State: AOAM53105L7sron5XNpnVs4j+W+QTqWAbHy8Hs9iiGS5HHBfNlTBv8bD on/hO3c+OgnfD83eVvW3+qe5JjB2L2uU8xdra9CsSvXfuAS4BTJvLhpdr9iO6AxTqTahI/LY7HS +myDy+m4+8+id X-Received: by 2002:a05:6402:51d1:: with SMTP id r17mr451085edd.91.1623965276166; Thu, 17 Jun 2021 14:27:56 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzuJMoUy1hXydwhfMQBzqzr6BrEg7lbvkkFSNeTBiotlbBoHDl0qp+bpeu9UXkB60eR8jNOsQ== X-Received: by 2002:a05:6402:51d1:: with SMTP id r17mr451071edd.91.1623965276019; Thu, 17 Jun 2021 14:27:56 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([45.145.92.2]) by smtp.gmail.com with ESMTPSA id t18sm5186222edw.47.2021.06.17.14.27.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Jun 2021 14:27:55 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 7E6BC18071E; Thu, 17 Jun 2021 23:27:54 +0200 (CEST) From: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= To: bpf@vger.kernel.org, netdev@vger.kernel.org Cc: Martin KaFai Lau , Hangbin Liu , Jesper Dangaard Brouer , Magnus Karlsson , "Paul E . McKenney" , Jakub Kicinski , =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= Subject: [PATCH bpf-next v3 02/16] bpf: allow RCU-protected lookups to happen from bh context Date: Thu, 17 Jun 2021 23:27:34 +0200 Message-Id: <20210617212748.32456-3-toke@redhat.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210617212748.32456-1-toke@redhat.com> References: <20210617212748.32456-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net XDP programs are called from a NAPI poll context, which means the RCU reference liveness is ensured by local_bh_disable(). Add rcu_read_lock_bh_held() as a condition to the RCU checks for map lookups so lockdep understands that the dereferences are safe from inside *either* an rcu_read_lock() section *or* a local_bh_disable() section. While both bh_disabled and rcu_read_lock() provide RCU protection, they are semantically distinct, so we need both conditions to prevent lockdep complaints. This change is done in preparation for removing the redundant rcu_read_lock()s from drivers. Acked-by: Martin KaFai Lau Signed-off-by: Toke Høiland-Jørgensen --- kernel/bpf/hashtab.c | 21 ++++++++++++++------- kernel/bpf/helpers.c | 6 +++--- kernel/bpf/lpm_trie.c | 6 ++++-- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c index 6f6681b07364..72c58cc516a3 100644 --- a/kernel/bpf/hashtab.c +++ b/kernel/bpf/hashtab.c @@ -596,7 +596,8 @@ static void *__htab_map_lookup_elem(struct bpf_map *map, void *key) struct htab_elem *l; u32 hash, key_size; - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held()); + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() && + !rcu_read_lock_bh_held()); key_size = map->key_size; @@ -989,7 +990,8 @@ static int htab_map_update_elem(struct bpf_map *map, void *key, void *value, /* unknown flags */ return -EINVAL; - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held()); + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() && + !rcu_read_lock_bh_held()); key_size = map->key_size; @@ -1082,7 +1084,8 @@ static int htab_lru_map_update_elem(struct bpf_map *map, void *key, void *value, /* unknown flags */ return -EINVAL; - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held()); + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() && + !rcu_read_lock_bh_held()); key_size = map->key_size; @@ -1148,7 +1151,8 @@ static int __htab_percpu_map_update_elem(struct bpf_map *map, void *key, /* unknown flags */ return -EINVAL; - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held()); + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() && + !rcu_read_lock_bh_held()); key_size = map->key_size; @@ -1202,7 +1206,8 @@ static int __htab_lru_percpu_map_update_elem(struct bpf_map *map, void *key, /* unknown flags */ return -EINVAL; - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held()); + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() && + !rcu_read_lock_bh_held()); key_size = map->key_size; @@ -1276,7 +1281,8 @@ static int htab_map_delete_elem(struct bpf_map *map, void *key) u32 hash, key_size; int ret; - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held()); + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() && + !rcu_read_lock_bh_held()); key_size = map->key_size; @@ -1311,7 +1317,8 @@ static int htab_lru_map_delete_elem(struct bpf_map *map, void *key) u32 hash, key_size; int ret; - WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held()); + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() && + !rcu_read_lock_bh_held()); key_size = map->key_size; diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 544773970dbc..e880f6bb6f28 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -28,7 +28,7 @@ */ BPF_CALL_2(bpf_map_lookup_elem, struct bpf_map *, map, void *, key) { - WARN_ON_ONCE(!rcu_read_lock_held()); + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_bh_held()); return (unsigned long) map->ops->map_lookup_elem(map, key); } @@ -44,7 +44,7 @@ const struct bpf_func_proto bpf_map_lookup_elem_proto = { BPF_CALL_4(bpf_map_update_elem, struct bpf_map *, map, void *, key, void *, value, u64, flags) { - WARN_ON_ONCE(!rcu_read_lock_held()); + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_bh_held()); return map->ops->map_update_elem(map, key, value, flags); } @@ -61,7 +61,7 @@ const struct bpf_func_proto bpf_map_update_elem_proto = { BPF_CALL_2(bpf_map_delete_elem, struct bpf_map *, map, void *, key) { - WARN_ON_ONCE(!rcu_read_lock_held()); + WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_bh_held()); return map->ops->map_delete_elem(map, key); } diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c index 1b7b8a6f34ee..423549d2c52e 100644 --- a/kernel/bpf/lpm_trie.c +++ b/kernel/bpf/lpm_trie.c @@ -232,7 +232,8 @@ static void *trie_lookup_elem(struct bpf_map *map, void *_key) /* Start walking the trie from the root node ... */ - for (node = rcu_dereference(trie->root); node;) { + for (node = rcu_dereference_check(trie->root, rcu_read_lock_bh_held()); + node;) { unsigned int next_bit; size_t matchlen; @@ -264,7 +265,8 @@ static void *trie_lookup_elem(struct bpf_map *map, void *_key) * traverse down. */ next_bit = extract_bit(key->data, node->prefixlen); - node = rcu_dereference(node->child[next_bit]); + node = rcu_dereference_check(node->child[next_bit], + rcu_read_lock_bh_held()); } if (!found) From patchwork Thu Jun 17 21:27:35 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: 12329567 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 83B5FC2B9F4 for ; Thu, 17 Jun 2021 21:28:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6D8E4613E7 for ; Thu, 17 Jun 2021 21:28:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233068AbhFQVaM (ORCPT ); Thu, 17 Jun 2021 17:30:12 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:26169 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233070AbhFQVaL (ORCPT ); Thu, 17 Jun 2021 17:30:11 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623965283; 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=ydDHLo7h+/kRhIHhd9hy6CQrP4iJ7Ox8MAnTy1XBZCM=; b=i/af2G6TyClCjgwSup0t7KcTG0HKt1rfZZzJu8kRJMAlxkpxY8pwo6UDcq4WCstV52l+od NSINBf6TMipBz3NXkcOBnBhfnf3thWoz1lxP/fGapiE4hm15P8ivTA7l83IrnZ1w55T+59 H7ee+KKj8Qz8DW0airYf4nP3KVDkDvY= Received: from mail-ed1-f72.google.com (mail-ed1-f72.google.com [209.85.208.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-378-Kw08GVw3MUKZ4bCSPPtmAQ-1; Thu, 17 Jun 2021 17:28:01 -0400 X-MC-Unique: Kw08GVw3MUKZ4bCSPPtmAQ-1 Received: by mail-ed1-f72.google.com with SMTP id i19-20020a05640200d3b02903948b71f25cso321436edu.4 for ; Thu, 17 Jun 2021 14:28:01 -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=ydDHLo7h+/kRhIHhd9hy6CQrP4iJ7Ox8MAnTy1XBZCM=; b=Ws0iyqV+fbIO1gXRqncocHAj/06CfbQ47Ul/nC+3m7pZE3jJKGptCSJ33fN5zWvnNf crt1vBvla4ZqCv4x8Pv+4hFPCYz3MlcZO9IR/HV8Z9gnn1ULJ/mCw9kbYLvlSLAx21Dw w+QtPHKk8vSGYjm1mgKkHm+HwY0F34T4XI21zIRVxhW1muZU342ewEll1bVSMrWm86oo 9rI6jY8J+SflhWxoVVBzVndY/QGjJDqHzN2AZ7oDL3SvMm7qD7RSNSi16AY5Za03wKXx PzmkGsV6PuBXEfa3k2RE36EPQppn4tvXCO9RYOmH/UbnLDpXfPQ50c0GWNxZ4aXMfMU0 J8ag== X-Gm-Message-State: AOAM5323PAVrEQcgEXckRkAdk6pMcYw/lTQBeDNq5qNmnv/E5kP0f5On gCQ282HB/tLA1eNgQmg8fR4EgDAxDEWYpgDEitbsYWQNGL4DFVpuksRXfckcGN8YrMHfvNWVQhL AkybNKvS0TNdp X-Received: by 2002:a05:6402:49:: with SMTP id f9mr476513edu.178.1623965278903; Thu, 17 Jun 2021 14:27:58 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzm2DB8/xNEXj3vCRwcZxq38p7u54o/cR+dzc/pDN9w/NJ0aojiieR04HtOKnyxUwPN47VZyg== X-Received: by 2002:a05:6402:49:: with SMTP id f9mr476463edu.178.1623965278445; Thu, 17 Jun 2021 14:27:58 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([2a0c:4d80:42:443::2]) by smtp.gmail.com with ESMTPSA id c18sm5025757edt.97.2021.06.17.14.27.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Jun 2021 14:27:55 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 860B3180727; Thu, 17 Jun 2021 23:27:54 +0200 (CEST) From: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= To: bpf@vger.kernel.org, netdev@vger.kernel.org Cc: Martin KaFai Lau , Hangbin Liu , Jesper Dangaard Brouer , Magnus Karlsson , "Paul E . McKenney" , Jakub Kicinski , =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= Subject: [PATCH bpf-next v3 03/16] xdp: add proper __rcu annotations to redirect map entries Date: Thu, 17 Jun 2021 23:27:35 +0200 Message-Id: <20210617212748.32456-4-toke@redhat.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210617212748.32456-1-toke@redhat.com> References: <20210617212748.32456-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net XDP_REDIRECT works by a three-step process: the bpf_redirect() and bpf_redirect_map() helpers will lookup the target of the redirect and store it (along with some other metadata) in a per-CPU struct bpf_redirect_info. Next, when the program returns the XDP_REDIRECT return code, the driver will call xdp_do_redirect() which will use the information thus stored to actually enqueue the frame into a bulk queue structure (that differs slightly by map type, but shares the same principle). Finally, before exiting its NAPI poll loop, the driver will call xdp_do_flush(), which will flush all the different bulk queues, thus completing the redirect. Pointers to the map entries will be kept around for this whole sequence of steps, protected by RCU. However, there is no top-level rcu_read_lock() in the core code; instead drivers add their own rcu_read_lock() around the XDP portions of the code, but somewhat inconsistently as Martin discovered[0]. However, things still work because everything happens inside a single NAPI poll sequence, which means it's between a pair of calls to local_bh_disable()/local_bh_enable(). So Paul suggested[1] that we could document this intention by using rcu_dereference_check() with rcu_read_lock_bh_held() as a second parameter, thus allowing sparse and lockdep to verify that everything is done correctly. This patch does just that: we add an __rcu annotation to the map entry pointers and remove the various comments explaining the NAPI poll assurance strewn through devmap.c in favour of a longer explanation in filter.c. The goal is to have one coherent documentation of the entire flow, and rely on the RCU annotations as a "standard" way of communicating the flow in the map code (which can additionally be understood by sparse and lockdep). The RCU annotation replacements result in a fairly straight-forward replacement where READ_ONCE() becomes rcu_dereference_check(), WRITE_ONCE() becomes rcu_assign_pointer() and xchg() and cmpxchg() gets wrapped in the proper constructs to cast the pointer back and forth between __rcu and __kernel address space (for the benefit of sparse). The one complication is that xskmap has a few constructions where double-pointers are passed back and forth; these simply all gain __rcu annotations, and only the final reference/dereference to the inner-most pointer gets changed. With this, everything can be run through sparse without eliciting complaints, and lockdep can verify correctness even without the use of rcu_read_lock() in the drivers. Subsequent patches will clean these up from the drivers. [0] https://lore.kernel.org/bpf/20210415173551.7ma4slcbqeyiba2r@kafai-mbp.dhcp.thefacebook.com/ [1] https://lore.kernel.org/bpf/20210419165837.GA975577@paulmck-ThinkPad-P17-Gen-1/ Signed-off-by: Toke Høiland-Jørgensen Acked-by: Martin KaFai Lau --- include/net/xdp_sock.h | 2 +- kernel/bpf/cpumap.c | 13 +++++++---- kernel/bpf/devmap.c | 49 ++++++++++++++++++------------------------ net/core/filter.c | 28 ++++++++++++++++++++++++ net/xdp/xsk.c | 4 ++-- net/xdp/xsk.h | 4 ++-- net/xdp/xskmap.c | 29 ++++++++++++++----------- 7 files changed, 80 insertions(+), 49 deletions(-) diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h index 9c0722c6d7ac..fff069d2ed1b 100644 --- a/include/net/xdp_sock.h +++ b/include/net/xdp_sock.h @@ -37,7 +37,7 @@ struct xdp_umem { struct xsk_map { struct bpf_map map; spinlock_t lock; /* Synchronize map updates */ - struct xdp_sock *xsk_map[]; + struct xdp_sock __rcu *xsk_map[]; }; struct xdp_sock { diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c index a1a0c4e791c6..480e936c54d0 100644 --- a/kernel/bpf/cpumap.c +++ b/kernel/bpf/cpumap.c @@ -74,7 +74,7 @@ struct bpf_cpu_map_entry { struct bpf_cpu_map { struct bpf_map map; /* Below members specific for map type */ - struct bpf_cpu_map_entry **cpu_map; + struct bpf_cpu_map_entry __rcu **cpu_map; }; static DEFINE_PER_CPU(struct list_head, cpu_map_flush_list); @@ -469,7 +469,7 @@ static void __cpu_map_entry_replace(struct bpf_cpu_map *cmap, { struct bpf_cpu_map_entry *old_rcpu; - old_rcpu = xchg(&cmap->cpu_map[key_cpu], rcpu); + old_rcpu = unrcu_pointer(xchg(&cmap->cpu_map[key_cpu], RCU_INITIALIZER(rcpu))); if (old_rcpu) { call_rcu(&old_rcpu->rcu, __cpu_map_entry_free); INIT_WORK(&old_rcpu->kthread_stop_wq, cpu_map_kthread_stop); @@ -551,7 +551,7 @@ static void cpu_map_free(struct bpf_map *map) for (i = 0; i < cmap->map.max_entries; i++) { struct bpf_cpu_map_entry *rcpu; - rcpu = READ_ONCE(cmap->cpu_map[i]); + rcpu = rcu_dereference_raw(cmap->cpu_map[i]); if (!rcpu) continue; @@ -562,6 +562,10 @@ static void cpu_map_free(struct bpf_map *map) kfree(cmap); } +/* Elements are kept alive by RCU; either by rcu_read_lock() (from syscall) or + * by local_bh_disable() (from XDP calls inside NAPI). The + * rcu_read_lock_bh_held() below makes lockdep accept both. + */ static void *__cpu_map_lookup_elem(struct bpf_map *map, u32 key) { struct bpf_cpu_map *cmap = container_of(map, struct bpf_cpu_map, map); @@ -570,7 +574,8 @@ static void *__cpu_map_lookup_elem(struct bpf_map *map, u32 key) if (key >= map->max_entries) return NULL; - rcpu = READ_ONCE(cmap->cpu_map[key]); + rcpu = rcu_dereference_check(cmap->cpu_map[key], + rcu_read_lock_bh_held()); return rcpu; } diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c index 2a75e6c2d27d..2f6bd75cd682 100644 --- a/kernel/bpf/devmap.c +++ b/kernel/bpf/devmap.c @@ -73,7 +73,7 @@ struct bpf_dtab_netdev { struct bpf_dtab { struct bpf_map map; - struct bpf_dtab_netdev **netdev_map; /* DEVMAP type only */ + struct bpf_dtab_netdev __rcu **netdev_map; /* DEVMAP type only */ struct list_head list; /* these are only used for DEVMAP_HASH type maps */ @@ -226,7 +226,7 @@ static void dev_map_free(struct bpf_map *map) for (i = 0; i < dtab->map.max_entries; i++) { struct bpf_dtab_netdev *dev; - dev = dtab->netdev_map[i]; + dev = rcu_dereference_raw(dtab->netdev_map[i]); if (!dev) continue; @@ -259,6 +259,10 @@ static int dev_map_get_next_key(struct bpf_map *map, void *key, void *next_key) return 0; } +/* Elements are kept alive by RCU; either by rcu_read_lock() (from syscall) or + * by local_bh_disable() (from XDP calls inside NAPI). The + * rcu_read_lock_bh_held() below makes lockdep accept both. + */ static void *__dev_map_hash_lookup_elem(struct bpf_map *map, u32 key) { struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map); @@ -410,15 +414,9 @@ static void bq_xmit_all(struct xdp_dev_bulk_queue *bq, u32 flags) trace_xdp_devmap_xmit(bq->dev_rx, dev, sent, cnt - sent, err); } -/* __dev_flush is called from xdp_do_flush() which _must_ be signaled - * from the driver before returning from its napi->poll() routine. The poll() - * routine is called either from busy_poll context or net_rx_action signaled - * from NET_RX_SOFTIRQ. Either way the poll routine must complete before the - * net device can be torn down. On devmap tear down we ensure the flush list - * is empty before completing to ensure all flush operations have completed. - * When drivers update the bpf program they may need to ensure any flush ops - * are also complete. Using synchronize_rcu or call_rcu will suffice for this - * because both wait for napi context to exit. +/* __dev_flush is called from xdp_do_flush() which _must_ be signalled from the + * driver before returning from its napi->poll() routine. See the comment above + * xdp_do_flush() in filter.c. */ void __dev_flush(void) { @@ -433,9 +431,9 @@ void __dev_flush(void) } } -/* rcu_read_lock (from syscall and BPF contexts) ensures that if a delete and/or - * update happens in parallel here a dev_put won't happen until after reading - * the ifindex. +/* Elements are kept alive by RCU; either by rcu_read_lock() (from syscall) or + * by local_bh_disable() (from XDP calls inside NAPI). The + * rcu_read_lock_bh_held() below makes lockdep accept both. */ static void *__dev_map_lookup_elem(struct bpf_map *map, u32 key) { @@ -445,12 +443,14 @@ static void *__dev_map_lookup_elem(struct bpf_map *map, u32 key) if (key >= map->max_entries) return NULL; - obj = READ_ONCE(dtab->netdev_map[key]); + obj = rcu_dereference_check(dtab->netdev_map[key], + rcu_read_lock_bh_held()); return obj; } -/* Runs under RCU-read-side, plus in softirq under NAPI protection. - * Thus, safe percpu variable access. +/* Runs in NAPI, i.e., softirq under local_bh_disable(). Thus, safe percpu + * variable access, and map elements stick around. See comment above + * xdp_do_flush() in filter.c. */ static void bq_enqueue(struct net_device *dev, struct xdp_frame *xdpf, struct net_device *dev_rx, struct bpf_prog *xdp_prog) @@ -735,14 +735,7 @@ static int dev_map_delete_elem(struct bpf_map *map, void *key) if (k >= map->max_entries) return -EINVAL; - /* Use call_rcu() here to ensure any rcu critical sections have - * completed as well as any flush operations because call_rcu - * will wait for preempt-disable region to complete, NAPI in this - * context. And additionally, the driver tear down ensures all - * soft irqs are complete before removing the net device in the - * case of dev_put equals zero. - */ - old_dev = xchg(&dtab->netdev_map[k], NULL); + old_dev = unrcu_pointer(xchg(&dtab->netdev_map[k], NULL)); if (old_dev) call_rcu(&old_dev->rcu, __dev_map_entry_free); return 0; @@ -851,7 +844,7 @@ static int __dev_map_update_elem(struct net *net, struct bpf_map *map, * Remembering the driver side flush operation will happen before the * net device is removed. */ - old_dev = xchg(&dtab->netdev_map[i], dev); + old_dev = unrcu_pointer(xchg(&dtab->netdev_map[i], RCU_INITIALIZER(dev))); if (old_dev) call_rcu(&old_dev->rcu, __dev_map_entry_free); @@ -1031,10 +1024,10 @@ static int dev_map_notification(struct notifier_block *notifier, for (i = 0; i < dtab->map.max_entries; i++) { struct bpf_dtab_netdev *dev, *odev; - dev = READ_ONCE(dtab->netdev_map[i]); + dev = rcu_dereference(dtab->netdev_map[i]); if (!dev || netdev != dev->dev) continue; - odev = cmpxchg(&dtab->netdev_map[i], dev, NULL); + odev = unrcu_pointer(cmpxchg(&dtab->netdev_map[i], RCU_INITIALIZER(dev), NULL)); if (dev == odev) call_rcu(&dev->rcu, __dev_map_entry_free); diff --git a/net/core/filter.c b/net/core/filter.c index caa88955562e..0b7db5c70385 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -3922,6 +3922,34 @@ static const struct bpf_func_proto bpf_xdp_adjust_meta_proto = { .arg2_type = ARG_ANYTHING, }; +/* XDP_REDIRECT works by a three-step process, implemented in the functions + * below: + * + * 1. The bpf_redirect() and bpf_redirect_map() helpers will lookup the target + * of the redirect and store it (along with some other metadata) in a per-CPU + * struct bpf_redirect_info. + * + * 2. When the program returns the XDP_REDIRECT return code, the driver will + * call xdp_do_redirect() which will use the information in struct + * bpf_redirect_info to actually enqueue the frame into a map type-specific + * bulk queue structure. + * + * 3. Before exiting its NAPI poll loop, the driver will call xdp_do_flush(), + * which will flush all the different bulk queues, thus completing the + * redirect. + * + * Pointers to the map entries will be kept around for this whole sequence of + * steps, protected by RCU. However, there is no top-level rcu_read_lock() in + * the core code; instead, the RCU protection relies on everything happening + * inside a single NAPI poll sequence, which means it's between a pair of calls + * to local_bh_disable()/local_bh_enable(). + * + * The map entries are marked as __rcu and the map code makes sure to + * dereference those pointers with rcu_dereference_check() in a way that works + * for both sections that to hold an rcu_read_lock() and sections that are + * called from NAPI without a separate rcu_read_lock(). The code below does not + * use RCU annotations, but relies on those in the map code. + */ void xdp_do_flush(void) { __dev_flush(); diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c index cd62d4ba87a9..996da915f520 100644 --- a/net/xdp/xsk.c +++ b/net/xdp/xsk.c @@ -749,7 +749,7 @@ static void xsk_unbind_dev(struct xdp_sock *xs) } static struct xsk_map *xsk_get_map_list_entry(struct xdp_sock *xs, - struct xdp_sock ***map_entry) + struct xdp_sock __rcu ***map_entry) { struct xsk_map *map = NULL; struct xsk_map_node *node; @@ -785,7 +785,7 @@ static void xsk_delete_from_maps(struct xdp_sock *xs) * might be updates to the map between * xsk_get_map_list_entry() and xsk_map_try_sock_delete(). */ - struct xdp_sock **map_entry = NULL; + struct xdp_sock __rcu **map_entry = NULL; struct xsk_map *map; while ((map = xsk_get_map_list_entry(xs, &map_entry))) { diff --git a/net/xdp/xsk.h b/net/xdp/xsk.h index edcf249ad1f1..a4bc4749faac 100644 --- a/net/xdp/xsk.h +++ b/net/xdp/xsk.h @@ -31,7 +31,7 @@ struct xdp_mmap_offsets_v1 { struct xsk_map_node { struct list_head node; struct xsk_map *map; - struct xdp_sock **map_entry; + struct xdp_sock __rcu **map_entry; }; static inline struct xdp_sock *xdp_sk(struct sock *sk) @@ -40,7 +40,7 @@ static inline struct xdp_sock *xdp_sk(struct sock *sk) } void xsk_map_try_sock_delete(struct xsk_map *map, struct xdp_sock *xs, - struct xdp_sock **map_entry); + struct xdp_sock __rcu **map_entry); void xsk_clear_pool_at_qid(struct net_device *dev, u16 queue_id); int xsk_reg_pool_at_qid(struct net_device *dev, struct xsk_buff_pool *pool, u16 queue_id); diff --git a/net/xdp/xskmap.c b/net/xdp/xskmap.c index 9df75ea4a567..2e48d0e094d9 100644 --- a/net/xdp/xskmap.c +++ b/net/xdp/xskmap.c @@ -12,7 +12,7 @@ #include "xsk.h" static struct xsk_map_node *xsk_map_node_alloc(struct xsk_map *map, - struct xdp_sock **map_entry) + struct xdp_sock __rcu **map_entry) { struct xsk_map_node *node; @@ -42,7 +42,7 @@ static void xsk_map_sock_add(struct xdp_sock *xs, struct xsk_map_node *node) } static void xsk_map_sock_delete(struct xdp_sock *xs, - struct xdp_sock **map_entry) + struct xdp_sock __rcu **map_entry) { struct xsk_map_node *n, *tmp; @@ -124,6 +124,10 @@ static int xsk_map_gen_lookup(struct bpf_map *map, struct bpf_insn *insn_buf) return insn - insn_buf; } +/* Elements are kept alive by RCU; either by rcu_read_lock() (from syscall) or + * by local_bh_disable() (from XDP calls inside NAPI). The + * rcu_read_lock_bh_held() below makes lockdep accept both. + */ static void *__xsk_map_lookup_elem(struct bpf_map *map, u32 key) { struct xsk_map *m = container_of(map, struct xsk_map, map); @@ -131,12 +135,11 @@ static void *__xsk_map_lookup_elem(struct bpf_map *map, u32 key) if (key >= map->max_entries) return NULL; - return READ_ONCE(m->xsk_map[key]); + return rcu_dereference_check(m->xsk_map[key], rcu_read_lock_bh_held()); } static void *xsk_map_lookup_elem(struct bpf_map *map, void *key) { - WARN_ON_ONCE(!rcu_read_lock_held()); return __xsk_map_lookup_elem(map, *(u32 *)key); } @@ -149,7 +152,8 @@ static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value, u64 map_flags) { struct xsk_map *m = container_of(map, struct xsk_map, map); - struct xdp_sock *xs, *old_xs, **map_entry; + struct xdp_sock __rcu **map_entry; + struct xdp_sock *xs, *old_xs; u32 i = *(u32 *)key, fd = *(u32 *)value; struct xsk_map_node *node; struct socket *sock; @@ -179,7 +183,7 @@ static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value, } spin_lock_bh(&m->lock); - old_xs = READ_ONCE(*map_entry); + old_xs = rcu_dereference_protected(*map_entry, lockdep_is_held(&m->lock)); if (old_xs == xs) { err = 0; goto out; @@ -191,7 +195,7 @@ static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value, goto out; } xsk_map_sock_add(xs, node); - WRITE_ONCE(*map_entry, xs); + rcu_assign_pointer(*map_entry, xs); if (old_xs) xsk_map_sock_delete(old_xs, map_entry); spin_unlock_bh(&m->lock); @@ -208,7 +212,8 @@ static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value, static int xsk_map_delete_elem(struct bpf_map *map, void *key) { struct xsk_map *m = container_of(map, struct xsk_map, map); - struct xdp_sock *old_xs, **map_entry; + struct xdp_sock __rcu **map_entry; + struct xdp_sock *old_xs; int k = *(u32 *)key; if (k >= map->max_entries) @@ -216,7 +221,7 @@ static int xsk_map_delete_elem(struct bpf_map *map, void *key) spin_lock_bh(&m->lock); map_entry = &m->xsk_map[k]; - old_xs = xchg(map_entry, NULL); + old_xs = unrcu_pointer(xchg(map_entry, NULL)); if (old_xs) xsk_map_sock_delete(old_xs, map_entry); spin_unlock_bh(&m->lock); @@ -231,11 +236,11 @@ static int xsk_map_redirect(struct bpf_map *map, u32 ifindex, u64 flags) } void xsk_map_try_sock_delete(struct xsk_map *map, struct xdp_sock *xs, - struct xdp_sock **map_entry) + struct xdp_sock __rcu **map_entry) { spin_lock_bh(&map->lock); - if (READ_ONCE(*map_entry) == xs) { - WRITE_ONCE(*map_entry, NULL); + if (rcu_access_pointer(*map_entry) == xs) { + rcu_assign_pointer(*map_entry, NULL); xsk_map_sock_delete(xs, map_entry); } spin_unlock_bh(&map->lock); From patchwork Thu Jun 17 21:27:36 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: 12329559 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BF529C49EA6 for ; Thu, 17 Jun 2021 21:28:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A9D8F60D07 for ; Thu, 17 Jun 2021 21:28:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233056AbhFQVaK (ORCPT ); Thu, 17 Jun 2021 17:30:10 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:39869 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232079AbhFQVaK (ORCPT ); Thu, 17 Jun 2021 17:30:10 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623965281; 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=gpw0jr2etcJQOtwBBETmSUpHlhu94Ou8Nardir8qwr8=; b=CMai4sOtFpXBo6gptTIoEADYAbeiSYp3t1qHoeeBXkmdt+U+mQxqupsM8MgAkqixY/+dMq dvuyCEXOz0LqhXagVWh/CqcVF/aoDXHuDtZKqsfRastsPD07nuBmW/+HWE0FXh6Z8QRwW9 QQfcnvP1/LXr6GZjk/lm+oFEMdMfoCc= Received: from mail-ej1-f71.google.com (mail-ej1-f71.google.com [209.85.218.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-272-LmTqsbu1M6adUUxb4VLfQA-1; Thu, 17 Jun 2021 17:28:00 -0400 X-MC-Unique: LmTqsbu1M6adUUxb4VLfQA-1 Received: by mail-ej1-f71.google.com with SMTP id br12-20020a170906d14cb02904311c0f32adso3026706ejb.9 for ; Thu, 17 Jun 2021 14:28:00 -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=gpw0jr2etcJQOtwBBETmSUpHlhu94Ou8Nardir8qwr8=; b=sv/9MyNywdNk4dnd6v4Dj1JEebKCD9rz3Cj49KauNNgWOqTJsxfbfKA3EWjyUX3n3z jF7c8vMxMsRZF/gRlGIjRmDkMYFAyh3GSEy/kC7WKXmlMA74OWA9/tf7pz5dzbaLh5O9 98Y2jOlJ46vZDWo0JGI5NcH1zY+k+1rlnhprUpgZlXwMfQZiRtbvhqReAqTIPzUyTxdn 2qfRQpQIQxsglS3HZBq56V/zGjOFlAJLfJIhVkt+LxDRyRk2yaYm8T2jyO3ReLbIiwMD MWJD9HbKIEoKcHyKZUuYdp2MoPuoA2dGokB3flsaJ4h2JzktZc4zvYIm80poVEWgUwLD zg8w== X-Gm-Message-State: AOAM530iKfRgPmyX5ylbdl0Ed/iFsaPsOzuGh7sqLZEb6D85JWTtVZhi 4RrfbDEdP5ENOH0/L1p2v2Hs4/vZk1ep8QDjeq/VHARoER6/xgFgbKkVzJen4GH/+x7rrVZoA/b 42pn9ksh3DM+4 X-Received: by 2002:a17:906:22c7:: with SMTP id q7mr7355896eja.547.1623965279114; Thu, 17 Jun 2021 14:27:59 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwJ0vfR8rrkBi2MA3Q9968M4U2OVMwrReOPcnhagocHm36uIqqyWEHWUNmgXDLaWB74kPUh2Q== X-Received: by 2002:a17:906:22c7:: with SMTP id q7mr7355863eja.547.1623965278752; Thu, 17 Jun 2021 14:27:58 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([2a0c:4d80:42:443::2]) by smtp.gmail.com with ESMTPSA id g16sm93259ejh.92.2021.06.17.14.27.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Jun 2021 14:27:55 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 8C8D1180728; Thu, 17 Jun 2021 23:27:54 +0200 (CEST) From: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= To: bpf@vger.kernel.org, netdev@vger.kernel.org Cc: Martin KaFai Lau , Hangbin Liu , Jesper Dangaard Brouer , Magnus Karlsson , "Paul E . McKenney" , Jakub Kicinski , =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= , Guy Tzalik , Saeed Bishara Subject: [PATCH bpf-next v3 04/16] ena: remove rcu_read_lock() around XDP program invocation Date: Thu, 17 Jun 2021 23:27:36 +0200 Message-Id: <20210617212748.32456-5-toke@redhat.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210617212748.32456-1-toke@redhat.com> References: <20210617212748.32456-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net The ena driver has 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: Guy Tzalik Cc: Saeed Bishara Signed-off-by: Toke Høiland-Jørgensen --- drivers/net/ethernet/amazon/ena/ena_netdev.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c index 881f88754bf6..cd5f03691b4c 100644 --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c @@ -385,7 +385,9 @@ static int ena_xdp_execute(struct ena_ring *rx_ring, struct xdp_buff *xdp) u64 *xdp_stat; int qid; - rcu_read_lock(); + /* This code is invoked within a single NAPI poll cycle and thus under + * local_bh_disable(), which provides the needed RCU protection. + */ xdp_prog = READ_ONCE(rx_ring->xdp_bpf_prog); if (!xdp_prog) @@ -443,8 +445,6 @@ static int ena_xdp_execute(struct ena_ring *rx_ring, struct xdp_buff *xdp) ena_increase_stat(xdp_stat, 1, &rx_ring->syncp); out: - rcu_read_unlock(); - return verdict; } From patchwork Thu Jun 17 21:27:37 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: 12329601 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 84811C48BE5 for ; Thu, 17 Jun 2021 21:37:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 68A92610A0 for ; Thu, 17 Jun 2021 21:37:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231241AbhFQVjf (ORCPT ); Thu, 17 Jun 2021 17:39:35 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:29944 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230076AbhFQVje (ORCPT ); Thu, 17 Jun 2021 17:39:34 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623965846; 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=bxe/jYWJoRuqS9AZsVd+t2yebUIqnn8PaQiPBsd+Vzc=; b=EoHiXRDHQl9YnGk6PXC5c2atASgs+Aw6ymEbYxGTc8ao74k05KYxutK91pT+SpwDVFM7F4 te7YufXbFcKPS9v1ogBtA416gIottaEq78m29LaODXftwGzqqZ4/E0ySWJCCDDJSr3wHNH jmjikqM5HqFqrUSZXCmX9OOoD9v3ITY= Received: from mail-ed1-f69.google.com (mail-ed1-f69.google.com [209.85.208.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-233-e3BDpmbBOSmFuT6oNp-HLA-1; Thu, 17 Jun 2021 17:37:25 -0400 X-MC-Unique: e3BDpmbBOSmFuT6oNp-HLA-1 Received: by mail-ed1-f69.google.com with SMTP id ch5-20020a0564021bc5b029039389929f28so2377039edb.16 for ; Thu, 17 Jun 2021 14:37:25 -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=bxe/jYWJoRuqS9AZsVd+t2yebUIqnn8PaQiPBsd+Vzc=; b=DZgUyHCLjAQUXnsZIouG15d2KLzc7F7kSLRJ3BjYbew9nUMdGqqHO3RDbV9oydPROQ S5pbvQjnuSY7ru2zt3tuT/dQccaZOqmGfNFduz/wfh5zDIZJ6WgjUVGglSLiHAURVCtX C+xAsStE15yGdynixz6t5BS4WWTIpgRSBfFbqKE/kFjtaNUcouoI5fQ6HyR//jLI0V21 6eAuMzpEosec7vQ5xE3DBhJU/I8bLrRtIU1cA2Ze738fO4ylqXVIfH9PRt8PFXv4gX4K 0+JOSp1AJMyNog63KBZTZEokOL0wzkNQmkx0zaJJZJwDiNfM02sCJn4EfCap9eQa3XSU iltg== X-Gm-Message-State: AOAM532kaYXBoPdmkRoYKf2cYEm3pA8961uO580clqfiFkufNnolSey6 t6UhaB8BewXD69YPDZ8LUrleV6EVsHkUa/vhGoxvgosQnoH6KXWt4gzgnF5FEX5GqllynZVGeYg dtGHnA9NX0v0B X-Received: by 2002:a17:906:180a:: with SMTP id v10mr7410958eje.22.1623965843971; Thu, 17 Jun 2021 14:37:23 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwyKGuzt6VkoFoUlAWaJe8evaIA1QIU3+GOM1Ofc6+tjgndrMmCGSDwsqBN+7ahsRYZN7fO0Q== X-Received: by 2002:a17:906:180a:: with SMTP id v10mr7410928eje.22.1623965843601; Thu, 17 Jun 2021 14:37:23 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([2a0c:4d80:42:443::2]) by smtp.gmail.com with ESMTPSA id y20sm122399ejm.44.2021.06.17.14.37.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Jun 2021 14:37:22 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 938D118072C; Thu, 17 Jun 2021 23:27:54 +0200 (CEST) From: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= To: bpf@vger.kernel.org, netdev@vger.kernel.org Cc: Martin KaFai Lau , Hangbin Liu , Jesper Dangaard Brouer , Magnus Karlsson , "Paul E . McKenney" , Jakub Kicinski , =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= , Michael Chan Subject: [PATCH bpf-next v3 05/16] bnxt: remove rcu_read_lock() around XDP program invocation Date: Thu, 17 Jun 2021 23:27:37 +0200 Message-Id: <20210617212748.32456-6-toke@redhat.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210617212748.32456-1-toke@redhat.com> References: <20210617212748.32456-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net The bnxt driver has 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: Michael Chan Signed-off-by: Toke Høiland-Jørgensen --- drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c index ec9564e584e0..f38e6ce9b40e 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c @@ -138,9 +138,10 @@ bool bnxt_rx_xdp(struct bnxt *bp, struct bnxt_rx_ring_info *rxr, u16 cons, xdp_prepare_buff(&xdp, *data_ptr - offset, offset, *len, false); orig_data = xdp.data; - rcu_read_lock(); + /* This code is invoked within a single NAPI poll cycle and thus under + * local_bh_disable(), which provides the needed RCU protection. + */ act = bpf_prog_run_xdp(xdp_prog, &xdp); - rcu_read_unlock(); tx_avail = bnxt_tx_avail(bp, txr); /* If the tx ring is not full, we must not update the rx producer yet From patchwork Thu Jun 17 21:27:38 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: 12329605 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 702F8C49361 for ; Thu, 17 Jun 2021 21:37:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4D751613B4 for ; Thu, 17 Jun 2021 21:37:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231434AbhFQVjh (ORCPT ); Thu, 17 Jun 2021 17:39:37 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:53711 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229816AbhFQVjf (ORCPT ); Thu, 17 Jun 2021 17:39:35 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623965846; 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=V+K4Rzii9x9DcPIEElTMZYJsTVD8g+UmhuIcex1BLcU=; b=iy0RQ42oSxyAWuNLaDe3zANnlZ1ty8ssmtHdf/4yPwKpfDqIeZpgtrri2V8A+GL93FKRN7 yCYhK3a3914EGO1eT/4X3M0ipNyrKnifNd0kBd7r2u2d6wYOP3HzMX/ahxRay7uOROEU/3 VSTCaf+HRTIY1FoxY6ZDrmKao22g714= Received: from mail-ed1-f70.google.com (mail-ed1-f70.google.com [209.85.208.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-194-IvEl53gRPZS9U5eLGI9WXw-1; Thu, 17 Jun 2021 17:37:25 -0400 X-MC-Unique: IvEl53gRPZS9U5eLGI9WXw-1 Received: by mail-ed1-f70.google.com with SMTP id q7-20020aa7cc070000b029038f59dab1c5so2390511edt.23 for ; Thu, 17 Jun 2021 14:37:25 -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=V+K4Rzii9x9DcPIEElTMZYJsTVD8g+UmhuIcex1BLcU=; b=oH7VLFEqh6t7AzQFAn0jQB/lFAUhbeG+qqcMnTwTGDl7nZ+qg4T+3kug9H6qEWYpZO NJQqjjZCAsicZlJbE1DJ/fTtpDRggp/FWkUuHfCA2uMeP9zaK7W3tT3wOBPUoQ6MOaZ7 JzrzQ1niyez3TwSU7hpR0atg4OQ1KiiD/KB/QIFowt8M4Yql1zKMbUwGr4lPAGxla2TB 5REUX7biSFcUnvhkzHvXLHrMcyQzFlJqDLOebydu0NYF1ix3WZxcDjNKInpXg/iBrD9I JJjcTmGDLoc+VNTxbQdZnah6vSyUxmN2kqH9SQtDjGnhPlSeqMVok95uF1ZUxd++xVZ9 pyfw== X-Gm-Message-State: AOAM5303p1yjro78zLU7TNcP6tb0MqNvqh+8paC5dQZcIZaIV6jUnDHi TM1KyI93APs6rBY6n6K6JrdUVZ0fuXSQU3jWQXnB1d8ie9XdNE+s/JjH+VKsTHUnGXTvgsU4mn0 Y9Ed6vcVD+CH5 X-Received: by 2002:a17:907:7b9d:: with SMTP id ne29mr7692190ejc.167.1623965844476; Thu, 17 Jun 2021 14:37:24 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyT3fEGEihEGarB3fpS2Al0EXij4hKluNygNZjVN8leQ1wAJxPYopgkOlxKigP3A1jBe4lPvA== X-Received: by 2002:a17:907:7b9d:: with SMTP id ne29mr7692177ejc.167.1623965844289; Thu, 17 Jun 2021 14:37:24 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([45.145.92.2]) by smtp.gmail.com with ESMTPSA id q9sm5018510edw.51.2021.06.17.14.37.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Jun 2021 14:37:22 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 9A79C18072F; Thu, 17 Jun 2021 23:27:54 +0200 (CEST) From: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= To: bpf@vger.kernel.org, netdev@vger.kernel.org Cc: Martin KaFai Lau , Hangbin Liu , Jesper Dangaard Brouer , Magnus Karlsson , "Paul E . McKenney" , Jakub Kicinski , =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= , Sunil Goutham , linux-arm-kernel@lists.infradead.org Subject: [PATCH bpf-next v3 06/16] thunderx: remove rcu_read_lock() around XDP program invocation Date: Thu, 17 Jun 2021 23:27:38 +0200 Message-Id: <20210617212748.32456-7-toke@redhat.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210617212748.32456-1-toke@redhat.com> References: <20210617212748.32456-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net The thunderx driver has 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: Sunil Goutham Cc: linux-arm-kernel@lists.infradead.org Signed-off-by: Toke Høiland-Jørgensen --- drivers/net/ethernet/cavium/thunder/nicvf_main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c index c33b4e837515..1d752815c69a 100644 --- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c +++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c @@ -555,9 +555,10 @@ static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog, xdp_prepare_buff(&xdp, hard_start, data - hard_start, len, false); orig_data = xdp.data; - rcu_read_lock(); + /* This code is invoked within a single NAPI poll cycle and thus under + * local_bh_disable(), which provides the needed RCU protection. + */ action = bpf_prog_run_xdp(prog, &xdp); - rcu_read_unlock(); len = xdp.data_end - xdp.data; /* Check if XDP program has changed headers */ From patchwork Thu Jun 17 21:27:39 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: 12329607 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 57077C49EA5 for ; Thu, 17 Jun 2021 21:37:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3DE39613B4 for ; Thu, 17 Jun 2021 21:37:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231468AbhFQVjh (ORCPT ); Thu, 17 Jun 2021 17:39:37 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:24051 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231346AbhFQVjg (ORCPT ); Thu, 17 Jun 2021 17:39:36 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623965848; 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=cJz59YB8g7oLdL+mJzpIs/zDJlYd7kwgcbU7x/ViItE=; b=OLrvLcJPRWOqKsrReXwUvbhvPVmJY+iNlWFuzI7TTPyvIQhfaA+ZF1YO6EzX+H9bbAKLhl 1yK/DyhW89yKI2rF3Wk4B42PzReCX3N+TqM50kOq+Z03IZftjVB4BHRqb3aIJaxm8QHNtZ lIjr2YfHRf9J0tqoS/SGVT28h+cchEs= Received: from mail-ed1-f70.google.com (mail-ed1-f70.google.com [209.85.208.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-251-pLLRDiEeNVqkD-_KZdAvbg-1; Thu, 17 Jun 2021 17:37:27 -0400 X-MC-Unique: pLLRDiEeNVqkD-_KZdAvbg-1 Received: by mail-ed1-f70.google.com with SMTP id t11-20020a056402524bb029038ffacf1cafso2417008edd.5 for ; Thu, 17 Jun 2021 14:37:26 -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=cJz59YB8g7oLdL+mJzpIs/zDJlYd7kwgcbU7x/ViItE=; b=cXaP4S7jenz7mqWOJaJyhxkaROrtrYsv1HpoYyGKn0d3qe3/UvkKSeRDlOdPnIRhmW q3UGJXrSa+6eqLlRh8twkiqu4GoxhYpyHm7gQLQOzy8087co1qyxHNdRbi9GbqimnrdL mq9cZdlj0xzWBfEOY7KnIifbj+a9SYHcOBDTqHV/hA4NVkCMZ/3EQ4DBLJV1amUdt8py MU075Ap73zsShAoZV6r6dMSiqwRgXOoKHSRuXcclugOQswlHtuFTVH65cExa06KlKs6W i2yZbkEJ8Pla/uAj1FVkteljZ4YcUPQFqCtxyes+JzyHpLxBE9/lJ/yx3Bt2zxthdlXK bZjg== X-Gm-Message-State: AOAM532eSn5sC7yVbHyHvNG4BNVfI9hlpfgvBrkQQP5Pnv60TUnexpoM B6CGgmK28JM+PbMjayBtQg3pcnB3g01oxxOuP8l9UnYpjBNLu0/vmYezoG5Op9sj28olHkQ80bB hGS/4kAtu1hM/ X-Received: by 2002:a17:906:6bd8:: with SMTP id t24mr7529617ejs.501.1623965845819; Thu, 17 Jun 2021 14:37:25 -0700 (PDT) X-Google-Smtp-Source: ABdhPJx7MiYndSZPPnikrer9lLYqdDjZqLkcm6ZdagJQugiaU6quK0vkJPkwzDn0Y2udQcNQ/YaluA== X-Received: by 2002:a17:906:6bd8:: with SMTP id t24mr7529608ejs.501.1623965845672; Thu, 17 Jun 2021 14:37:25 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([45.145.92.2]) by smtp.gmail.com with ESMTPSA id f4sm125915ejj.49.2021.06.17.14.37.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Jun 2021 14:37:22 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id A1B4A180730; Thu, 17 Jun 2021 23:27:54 +0200 (CEST) From: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= To: bpf@vger.kernel.org, netdev@vger.kernel.org Cc: Martin KaFai Lau , Hangbin Liu , Jesper Dangaard Brouer , Magnus Karlsson , "Paul E . McKenney" , Jakub Kicinski , =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= , Madalin Bucur , Ioana Ciornei , Ioana Radulescu , Camelia Groza Subject: [PATCH bpf-next v3 07/16] freescale: remove rcu_read_lock() around XDP program invocation Date: Thu, 17 Jun 2021 23:27:39 +0200 Message-Id: <20210617212748.32456-8-toke@redhat.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210617212748.32456-1-toke@redhat.com> References: <20210617212748.32456-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net The dpaa and dpaa2 drivers 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: Madalin Bucur Cc: Ioana Ciornei Cc: Ioana Radulescu Reviewed-by: Camelia Groza Signed-off-by: Toke Høiland-Jørgensen --- drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 11 ++++------- drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 6 +++--- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c index 177c020bf34a..98fdcbde687a 100644 --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c @@ -2558,13 +2558,9 @@ static u32 dpaa_run_xdp(struct dpaa_priv *priv, struct qm_fd *fd, void *vaddr, u32 xdp_act; int err; - rcu_read_lock(); - xdp_prog = READ_ONCE(priv->xdp_prog); - if (!xdp_prog) { - rcu_read_unlock(); + if (!xdp_prog) return XDP_PASS; - } xdp_init_buff(&xdp, DPAA_BP_RAW_SIZE - DPAA_TX_PRIV_DATA_SIZE, &dpaa_fq->xdp_rxq); @@ -2585,6 +2581,9 @@ static u32 dpaa_run_xdp(struct dpaa_priv *priv, struct qm_fd *fd, void *vaddr, } #endif + /* This code is invoked within a single NAPI poll cycle and thus under + * local_bh_disable(), which provides the needed RCU protection. + */ xdp_act = bpf_prog_run_xdp(xdp_prog, &xdp); /* Update the length and the offset of the FD */ @@ -2638,8 +2637,6 @@ static u32 dpaa_run_xdp(struct dpaa_priv *priv, struct qm_fd *fd, void *vaddr, break; } - rcu_read_unlock(); - return xdp_act; } diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c index 8433aa730c42..964d85c9e37d 100644 --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c @@ -352,8 +352,6 @@ static u32 dpaa2_eth_run_xdp(struct dpaa2_eth_priv *priv, u32 xdp_act = XDP_PASS; int err, offset; - rcu_read_lock(); - xdp_prog = READ_ONCE(ch->xdp.prog); if (!xdp_prog) goto out; @@ -363,6 +361,9 @@ static u32 dpaa2_eth_run_xdp(struct dpaa2_eth_priv *priv, xdp_prepare_buff(&xdp, vaddr + offset, XDP_PACKET_HEADROOM, dpaa2_fd_get_len(fd), false); + /* This code is invoked within a single NAPI poll cycle and thus under + * local_bh_disable(), which provides the needed RCU protection. + */ xdp_act = bpf_prog_run_xdp(xdp_prog, &xdp); /* xdp.data pointer may have changed */ @@ -414,7 +415,6 @@ static u32 dpaa2_eth_run_xdp(struct dpaa2_eth_priv *priv, ch->xdp.res |= xdp_act; out: - rcu_read_unlock(); return xdp_act; } From patchwork Thu Jun 17 21:27:40 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: 12329563 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EF4C2C49EA7 for ; Thu, 17 Jun 2021 21:28:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DBDF561375 for ; Thu, 17 Jun 2021 21:28:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233062AbhFQVaL (ORCPT ); Thu, 17 Jun 2021 17:30:11 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:58203 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233045AbhFQVaK (ORCPT ); Thu, 17 Jun 2021 17:30:10 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623965282; 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=BrDykLhiGSf6xXkjN+JUrFWMpiV7dzYyZbBTLShnH1w=; b=fcC6mJKVUS8MeJKuL3dtGKeJO53YzPBwteu9zeVkoub0oKz8RlXTf0WfmrnCYuoe575aJA nYNuiZ6J3YlIwPuj+scZnxmVtgY1QfNmgmU6XGTmv5qCLrXt0lfkWZbU7o3UO40I4xxvV3 7nWTupPAq8HY4VNdQAIQQqgPrL+S68w= 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-232-Eeq7R-aAMiiINt4qpQYGbA-1; Thu, 17 Jun 2021 17:28:00 -0400 X-MC-Unique: Eeq7R-aAMiiINt4qpQYGbA-1 Received: by mail-ed1-f71.google.com with SMTP id h23-20020aa7c5d70000b029038fed7b27d5so2348555eds.21 for ; Thu, 17 Jun 2021 14:28:00 -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=BrDykLhiGSf6xXkjN+JUrFWMpiV7dzYyZbBTLShnH1w=; b=MBhd1OY5eaZuZ/ZnrSY0Z8BhpKR2HhS4+Gd/ydHSl5oOj5B+46ppHU8FxxL5LJCJ8Q w9377Mrccp3+W6SLsaubUdOuJpoeONb85YOsbMLuiRynkM/Os5YYnEv25Pz2G7FokV9n nV48ozyx7yFaviv3LvTEBwczFSuodYSNSV+miPtzgNv53xd/y6Y3DUGBmduNshpz90dY WaZJrbl1K5R4/TweSqH0boztHVE/FiGWRcL0ut66EKoitc5jmeHGjskQM7EPkg55wKH8 wux05pMiPPZUsJGEYn/R8TMfxdHaXF+a9cxkIEtsg+4NwqK5hu1miZ/Mo6ZBjHEm6V8D 1mMg== X-Gm-Message-State: AOAM530e9BzgZBOjOJQbntSYArjkizzebgbyQvX8hRfq5XtzOvjpjOaf iUo7uyXblZwHlCpygqqPtm09iUYGSL7cHiA+I8LaD0jHKTauIL9YM900rGqOxNhTybFg4DF/JLW FnFjn/HcBrU3T X-Received: by 2002:a17:906:29c8:: with SMTP id y8mr7433858eje.312.1623965279723; Thu, 17 Jun 2021 14:27:59 -0700 (PDT) X-Google-Smtp-Source: ABdhPJw2cjGWsNm7HTPrQRwMQa1xBLKp0M4CHdCL2Y/WsZaVaFHpBIn5mKLkfJDTlSnxEUOYcRWEHg== X-Received: by 2002:a17:906:29c8:: with SMTP id y8mr7433846eje.312.1623965279559; Thu, 17 Jun 2021 14:27:59 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([45.145.92.2]) by smtp.gmail.com with ESMTPSA id v26sm111414ejk.70.2021.06.17.14.27.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Jun 2021 14:27:58 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id A8776180731; Thu, 17 Jun 2021 23:27:54 +0200 (CEST) From: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= To: bpf@vger.kernel.org, netdev@vger.kernel.org Cc: Martin KaFai Lau , Hangbin Liu , Jesper Dangaard Brouer , Magnus Karlsson , "Paul E . McKenney" , Jakub Kicinski , =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= , Jesse Brandeburg , Tony Nguyen , intel-wired-lan@lists.osuosl.org Subject: [PATCH bpf-next v3 08/16] net: intel: remove rcu_read_lock() around XDP program invocation Date: Thu, 17 Jun 2021 23:27:40 +0200 Message-Id: <20210617212748.32456-9-toke@redhat.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210617212748.32456-1-toke@redhat.com> References: <20210617212748.32456-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net 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 | 5 +++-- drivers/net/ethernet/intel/i40e/i40e_xsk.c | 11 +++++------ 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 | 5 +++-- drivers/net/ethernet/intel/igc/igc_main.c | 10 +++++----- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 +++-- drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 9 ++++----- drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 5 +++-- 9 files changed, 28 insertions(+), 34 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c index de70c16ef619..7fc5bdb5cd9e 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) @@ -2306,6 +2305,9 @@ static int i40e_run_xdp(struct i40e_ring *rx_ring, struct xdp_buff *xdp) prefetchw(xdp->data_hard_start); /* xdp_frame write */ + /* This code is invoked within a single NAPI poll cycle and thus under + * local_bh_disable(), which provides the needed RCU protection. + */ act = bpf_prog_run_xdp(xdp_prog, xdp); switch (act) { case XDP_PASS: @@ -2329,7 +2331,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..a5982cd112be 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c @@ -153,8 +153,10 @@ 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 code is invoked within a single NAPI poll cycle and thus under + * local_bh_disable(), which provides the needed RCU protection. + * + * NB! xdp_prog will always be !NULL, due to the fact that * this path is enabled by setting an XDP program. */ xdp_prog = READ_ONCE(rx_ring->xdp_prog); @@ -162,9 +164,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 +184,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..0b68d589218a 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) @@ -8395,6 +8394,9 @@ static struct sk_buff *igb_run_xdp(struct igb_adapter *adapter, prefetchw(xdp->data_hard_start); /* xdp_frame write */ + /* This code is invoked within a single NAPI poll cycle and thus under + * local_bh_disable(), which provides the needed RCU protection. + */ act = bpf_prog_run_xdp(xdp_prog, xdp); switch (act) { case XDP_PASS: @@ -8420,7 +8422,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..333057ce60c7 100644 --- a/drivers/net/ethernet/intel/igc/igc_main.c +++ b/drivers/net/ethernet/intel/igc/igc_main.c @@ -2175,18 +2175,18 @@ 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; } + /* This code is invoked within a single NAPI poll cycle and thus under + * local_bh_disable(), which provides the needed RCU protection. + */ 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..9cebe7894111 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) @@ -2207,6 +2206,9 @@ static struct sk_buff *ixgbe_run_xdp(struct ixgbe_adapter *adapter, prefetchw(xdp->data_hard_start); /* xdp_frame write */ + /* This code is invoked within a single NAPI poll cycle and thus under + * local_bh_disable(), which provides the needed RCU protection. + */ act = bpf_prog_run_xdp(xdp_prog, xdp); switch (act) { case XDP_PASS: @@ -2237,7 +2239,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..4a075e667082 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c @@ -100,15 +100,15 @@ static int ixgbe_run_xdp_zc(struct ixgbe_adapter *adapter, struct xdp_frame *xdpf; u32 act; - rcu_read_lock(); + /* This code is invoked within a single NAPI poll cycle and thus under + * local_bh_disable(), which provides the needed RCU protection. + */ 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 +132,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..ab05ebc3d350 100644 --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c @@ -1054,12 +1054,14 @@ 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) goto xdp_out; + /* This code is invoked within a single NAPI poll cycle and thus under + * local_bh_disable(), which provides the needed RCU protection. + */ act = bpf_prog_run_xdp(xdp_prog, xdp); switch (act) { case XDP_PASS: @@ -1079,7 +1081,6 @@ static struct sk_buff *ixgbevf_run_xdp(struct ixgbevf_adapter *adapter, break; } xdp_out: - rcu_read_unlock(); return ERR_PTR(-result); } From patchwork Thu Jun 17 21:27:41 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: 12329603 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 09F9FC49EA2 for ; Thu, 17 Jun 2021 21:37:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E8C1F610A0 for ; Thu, 17 Jun 2021 21:37:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231252AbhFQVjg (ORCPT ); Thu, 17 Jun 2021 17:39:36 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:41521 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231203AbhFQVjf (ORCPT ); Thu, 17 Jun 2021 17:39:35 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623965846; 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=b/T3H4vaV8F+Q+x5m1dW8YU/RP90nVjw+/NyR+B0cto=; b=RwTWpJxetiam5oVIzIWEXQj8BpTAo/DJPK3GC2CVlO/TNGpHrrJOpk3q9z6DDuRKf0WlCN kKf1cy7PlGzVvqnDIZGxgXLViga0IE47kFpyqOu2Eebvva5aE/MqyKmQHH3BAhr5YU8Zsy ul6BfmvcRzQ39ndPuGV02O20b48FIMs= Received: from mail-ej1-f72.google.com (mail-ej1-f72.google.com [209.85.218.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-170-Q3AHFvRwMYe_l1hsPDJb1g-1; Thu, 17 Jun 2021 17:37:25 -0400 X-MC-Unique: Q3AHFvRwMYe_l1hsPDJb1g-1 Received: by mail-ej1-f72.google.com with SMTP id n19-20020a1709067253b029043b446e4a03so3035499ejk.23 for ; Thu, 17 Jun 2021 14:37:25 -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=b/T3H4vaV8F+Q+x5m1dW8YU/RP90nVjw+/NyR+B0cto=; b=UHC7cHtRpRu2FIFMPxqpl5fgu5BmcC9U/ukP1MGe91GUz1Prx+GNsLnbRElpn1jxiZ YDNMBqCUdjt0yLWu0O89syVUjiYh9S2YPnYPwQRM/dkw7ffxmL0L/x4uzq5uESUbY/j9 EHFUjuxHnOknCLWfAy7cOAXcAiD8xTNja1HNufUzjKdUgRs2hu+GrLyNG29gq1Mh4dc3 Lcw/pFEk9NPfxtEbX36HGN8MwOMa++X03e7dLNiYJK//T2Zf6WXUPa9lEq9ExgMxs+fR zZazTy8op2zNlyPcObHcPqVsVbt160xdGr/mlcpYWRXitkXAlZwdBeVDIph0HLxg88Qn 7EYg== X-Gm-Message-State: AOAM530VCrCzI5/9397lZBHVaEOZhNQSmyI/I6qB4uxQjq9JyZeOLhQo O9Ki7Bb59cJfebKl2Lix+IHBbkXMmt8bN0OaazQNJ8lJD5P5TbzQzNjxWCrSCIOwxyp+2r72DZ4 7ssUVvp6oS+eP X-Received: by 2002:a05:6402:b5a:: with SMTP id bx26mr493956edb.81.1623965844135; Thu, 17 Jun 2021 14:37:24 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzRYMJ1okfOU42zzGSz1z3QdJrJk3hRHhSFfun2L1fNVGPvJLVoSQn8Ec15Jt6SRMeYoFq9xQ== X-Received: by 2002:a05:6402:b5a:: with SMTP id bx26mr493935edb.81.1623965843903; Thu, 17 Jun 2021 14:37:23 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([2a0c:4d80:42:443::2]) by smtp.gmail.com with ESMTPSA id i10sm123544eja.3.2021.06.17.14.37.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Jun 2021 14:37:22 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id AF9FF180732; Thu, 17 Jun 2021 23:27:54 +0200 (CEST) From: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= To: bpf@vger.kernel.org, netdev@vger.kernel.org Cc: Martin KaFai Lau , Hangbin Liu , Jesper Dangaard Brouer , Magnus Karlsson , "Paul E . McKenney" , Jakub Kicinski , =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= , Thomas Petazzoni , Marcin Wojtas , Russell King Subject: [PATCH bpf-next v3 09/16] marvell: remove rcu_read_lock() around XDP program invocation Date: Thu, 17 Jun 2021 23:27:41 +0200 Message-Id: <20210617212748.32456-10-toke@redhat.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210617212748.32456-1-toke@redhat.com> References: <20210617212748.32456-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net The mvneta and mvpp2 drivers 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: Thomas Petazzoni Cc: Marcin Wojtas Cc: Russell King Signed-off-by: Toke Høiland-Jørgensen --- drivers/net/ethernet/marvell/mvneta.c | 6 ++++-- drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 8 ++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c index 7d5cd9bc6c99..c2e9cbebc8d1 100644 --- a/drivers/net/ethernet/marvell/mvneta.c +++ b/drivers/net/ethernet/marvell/mvneta.c @@ -2370,7 +2370,6 @@ static int mvneta_rx_swbm(struct napi_struct *napi, /* Get number of received packets */ rx_todo = mvneta_rxq_busy_desc_num_get(pp, rxq); - rcu_read_lock(); xdp_prog = READ_ONCE(pp->xdp_prog); /* Fairness NAPI loop */ @@ -2421,6 +2420,10 @@ static int mvneta_rx_swbm(struct napi_struct *napi, goto next; } + /* This code is invoked within a single NAPI poll cycle and thus + * under local_bh_disable(), which provides the needed RCU + * protection. + */ if (xdp_prog && mvneta_run_xdp(pp, rxq, xdp_prog, &xdp_buf, frame_sz, &ps)) goto next; @@ -2448,7 +2451,6 @@ static int mvneta_rx_swbm(struct napi_struct *napi, xdp_buf.data_hard_start = NULL; sinfo.nr_frags = 0; } - rcu_read_unlock(); if (xdp_buf.data_hard_start) mvneta_xdp_put_buff(pp, rxq, &xdp_buf, &sinfo, -1); diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c index b2259bf1d299..658db1720826 100644 --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c @@ -3852,8 +3852,6 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi, int rx_done = 0; u32 xdp_ret = 0; - rcu_read_lock(); - xdp_prog = READ_ONCE(port->xdp_prog); /* Get number of received packets and clamp the to-do */ @@ -3925,6 +3923,10 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi, MVPP2_MH_SIZE + MVPP2_SKB_HEADROOM, rx_bytes, false); + /* This code is invoked within a single NAPI poll cycle + * and thus under local_bh_disable(), which provides the + * needed RCU protection. + */ ret = mvpp2_run_xdp(port, xdp_prog, &xdp, pp, &ps); if (ret) { @@ -3988,8 +3990,6 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi, mvpp2_bm_pool_put(port, pool, dma_addr, phys_addr); } - rcu_read_unlock(); - if (xdp_ret & MVPP2_XDP_REDIR) xdp_do_flush_map(); From patchwork Thu Jun 17 21:27:42 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: 12329599 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 14140C49361 for ; Thu, 17 Jun 2021 21:37:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E675D61249 for ; Thu, 17 Jun 2021 21:37:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231193AbhFQVje (ORCPT ); Thu, 17 Jun 2021 17:39:34 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:29085 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229816AbhFQVje (ORCPT ); Thu, 17 Jun 2021 17:39:34 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623965846; 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=c8O4Yimc+TSjNewaWd3nzsqZvk1ap2sDHxPur5LQuO8=; b=ioRxytb/5DCB2jAHr9ziFsycYpmEm88IUSPjOLYkzn1c3oCueUM9hwuOLXbPkMuhgQWE+J t16xxFaEZz5bn8tGW9N9nHr6JcPBklxHXJJ5Mvu9mGROEb9i6Imi9Qpy6pDw25yosZV0p8 xF410h9XhW1QQRMxa88EoRMHaJYbmV0= Received: from mail-ed1-f69.google.com (mail-ed1-f69.google.com [209.85.208.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-187-Lu7umEYaOz-vtkUsYSDsSg-1; Thu, 17 Jun 2021 17:37:24 -0400 X-MC-Unique: Lu7umEYaOz-vtkUsYSDsSg-1 Received: by mail-ed1-f69.google.com with SMTP id y16-20020a0564024410b0290394293f6816so2383733eda.20 for ; Thu, 17 Jun 2021 14:37:24 -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=c8O4Yimc+TSjNewaWd3nzsqZvk1ap2sDHxPur5LQuO8=; b=A8IdVk6MBpJ17wyM09XUP/GXZn2kVqo1H9S/YOGkbB2gLgeOXSa1ic3SbWlAX2n82Z CYog4gbxmoRk4327q0QZGtqhzDxRl4Apa+R5K7cNmDukWdg5/pL2rQqqE1jeQ2F0S7x5 C/cicOYpTIQYIhaDW1kkZzoKZYvOq9yDEGy+jcLbh+4nZumcbHCAANmyrdZwDn8X4Zd/ /ymYflxtMKw4xYr71cr9ZSDKtkReQLGK0qHOBH+4cvarprj/amTG1RvirHD03pAl/Zh9 +WFTdpDeePAX/JU9h1nzTm3SGznlcfgHmWiawiMaa5yF00827b77FidN+4h1EcvUzdQA 4aZw== X-Gm-Message-State: AOAM533CcizA6lCa9DPfiFmMU9dti4fYbnXA6g7ueMM2delZYK4K3H/5 qYCMHW88A2FVO4IDGmWLFZoTGS7ccIwYCjnRyLlMs4ZzQe/CRxiX115YNQ5HHsZf/q9meFf/aPi Cs9IqTKJ00O3/ X-Received: by 2002:a17:906:b857:: with SMTP id ga23mr7467084ejb.296.1623965843565; Thu, 17 Jun 2021 14:37:23 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyncqixvPI2i/r8fEKf9J5GXPyMqsF90pHqtqhIlR+8XIzCseASzejpmol2cBYRKDrxwm28Mg== X-Received: by 2002:a17:906:b857:: with SMTP id ga23mr7467067ejb.296.1623965843412; Thu, 17 Jun 2021 14:37:23 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([45.145.92.2]) by smtp.gmail.com with ESMTPSA id cf26sm121229ejb.38.2021.06.17.14.37.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Jun 2021 14:37:22 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id B62E9180733; Thu, 17 Jun 2021 23:27:54 +0200 (CEST) From: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= To: bpf@vger.kernel.org, netdev@vger.kernel.org Cc: Martin KaFai Lau , Hangbin Liu , Jesper Dangaard Brouer , Magnus Karlsson , "Paul E . McKenney" , Jakub Kicinski , =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= , Tariq Toukan Subject: [PATCH bpf-next v3 10/16] mlx4: remove rcu_read_lock() around XDP program invocation Date: Thu, 17 Jun 2021 23:27:42 +0200 Message-Id: <20210617212748.32456-11-toke@redhat.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210617212748.32456-1-toke@redhat.com> References: <20210617212748.32456-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net The mlx4 driver has 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. Also switch the RCU dereferences in the driver loop itself to the _bh variants. Cc: Tariq Toukan Reviewed-by: Tariq Toukan Signed-off-by: Toke Høiland-Jørgensen --- drivers/net/ethernet/mellanox/mlx4/en_rx.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c index e35e4d7ef4d1..3f08c14d0441 100644 --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c @@ -679,9 +679,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud ring = priv->rx_ring[cq_ring]; - /* Protect accesses to: ring->xdp_prog, priv->mac_hash list */ - rcu_read_lock(); - xdp_prog = rcu_dereference(ring->xdp_prog); + xdp_prog = rcu_dereference_bh(ring->xdp_prog); xdp_init_buff(&xdp, priv->frag_info[0].frag_stride, &ring->xdp_rxq); doorbell_pending = false; @@ -744,7 +742,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud /* Drop the packet, since HW loopback-ed it */ mac_hash = ethh->h_source[MLX4_EN_MAC_HASH_IDX]; bucket = &priv->mac_hash[mac_hash]; - hlist_for_each_entry_rcu(entry, bucket, hlist) { + hlist_for_each_entry_rcu_bh(entry, bucket, hlist) { if (ether_addr_equal_64bits(entry->mac, ethh->h_source)) goto next; @@ -899,8 +897,6 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud break; } - rcu_read_unlock(); - if (likely(polled)) { if (doorbell_pending) { priv->tx_cq[TX_XDP][cq_ring]->xdp_busy = true; From patchwork Thu Jun 17 21:27:43 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: 12329575 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E4FD4C49361 for ; Thu, 17 Jun 2021 21:28:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CFA04613E2 for ; Thu, 17 Jun 2021 21:28:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233113AbhFQVaW (ORCPT ); Thu, 17 Jun 2021 17:30:22 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:26602 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233125AbhFQVaO (ORCPT ); Thu, 17 Jun 2021 17:30:14 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623965285; 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=dAdah8VGuQn72TJlVoq7ahYZ+lMNJR+3mmSVKWZMutE=; b=jT3oPPxpdUesieYuMOXbqqnft7SsufHlevpPhfFazFqeDEMQcv66Reg+ULQ0rC1jcxAtxv lx1HTwlp+h9l0XmVrRyPlsEPEG7EvPnDQMWK1HSKbTjvOmDt5wbdVIUSb2aJ3Xl22DbxRj u3qnsfdA3g5WOcv5+p784A/yGHljj2Q= Received: from mail-ed1-f69.google.com (mail-ed1-f69.google.com [209.85.208.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-538-NmCxkeuEOeiehp8BAoV9nw-1; Thu, 17 Jun 2021 17:28:04 -0400 X-MC-Unique: NmCxkeuEOeiehp8BAoV9nw-1 Received: by mail-ed1-f69.google.com with SMTP id y18-20020a0564022712b029038ffac1995eso2399529edd.12 for ; Thu, 17 Jun 2021 14:28:04 -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=dAdah8VGuQn72TJlVoq7ahYZ+lMNJR+3mmSVKWZMutE=; b=eoyduCaQcQipGkO6vq+fh3tiPxRetiXLASJ6tGCSJPLXJ7GExDdtxgITolpmi/pyOy N+dECvB5RUInn9qwp/EjgCA8pWDz3JHRR6OTv68pQz1tKEqz3+5bcPDQBchZBtZDI3M8 2dYisVN5wQx1g7tdWlrWJMIgjiRlqXRnWz79wJKKXmCFpyQ8IcIKulcvdB2rl4jZcWWs XkULzXIV3W1bAEbFAgWksV8dTM+1XeLOGMrkiLyYSRHpFgfVZ8J5mZMWAgpOF1wpUOt0 L90X7a8U4OfW08gEwValIdb9IlYyOlrLZi6d6x8t3r24Fr+tFtp3kc23sbBo6I8OCe/R qAKQ== X-Gm-Message-State: AOAM530rrUeuDDa9501HD8oqQ43hpEDrWMMIB+5GiRy2gNeHWgXo9mV6 BDgkSn90dkBHryA98p0QFYEPOrnYPHQF1ozcdyCkgD8hoMFlFYvdi4C0n6+2JodRcX9982yELj7 hVSckFXI8DWdo X-Received: by 2002:a50:ff0a:: with SMTP id a10mr436470edu.273.1623965282741; Thu, 17 Jun 2021 14:28:02 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzAR7MBrW4DjSCrWUTnO3yJRhsZzoQfEPwCrPjITjLEgp/KVyK6TzDvfdi+b7OXK36PkMlVfA== X-Received: by 2002:a50:ff0a:: with SMTP id a10mr436435edu.273.1623965282380; Thu, 17 Jun 2021 14:28:02 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([2a0c:4d80:42:443::2]) by smtp.gmail.com with ESMTPSA id mm25sm100204ejb.125.2021.06.17.14.27.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Jun 2021 14:27:59 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id BD795180734; Thu, 17 Jun 2021 23:27:54 +0200 (CEST) From: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= To: bpf@vger.kernel.org, netdev@vger.kernel.org Cc: Martin KaFai Lau , Hangbin Liu , Jesper Dangaard Brouer , Magnus Karlsson , "Paul E . McKenney" , Jakub Kicinski , =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= , Simon Horman , oss-drivers@netronome.com Subject: [PATCH bpf-next v3 11/16] nfp: remove rcu_read_lock() around XDP program invocation Date: Thu, 17 Jun 2021 23:27:43 +0200 Message-Id: <20210617212748.32456-12-toke@redhat.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210617212748.32456-1-toke@redhat.com> References: <20210617212748.32456-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net The nfp driver has 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. While this is not actually an issue for the nfp driver because it doesn't support XDP_REDIRECT (and thus doesn't call xdp_do_flush()), the rcu_read_lock() is still unneeded. And 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: Simon Horman Cc: oss-drivers@netronome.com Reviewed-by: Simon Horman Signed-off-by: Toke Høiland-Jørgensen --- drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c index eeb30680b4dc..a3d59abed6ae 100644 --- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c @@ -1819,7 +1819,6 @@ static int nfp_net_rx(struct nfp_net_rx_ring *rx_ring, int budget) struct xdp_buff xdp; int idx; - rcu_read_lock(); xdp_prog = READ_ONCE(dp->xdp_prog); true_bufsz = xdp_prog ? PAGE_SIZE : dp->fl_bufsz; xdp_init_buff(&xdp, PAGE_SIZE - NFP_NET_RX_BUF_HEADROOM, @@ -1919,6 +1918,10 @@ static int nfp_net_rx(struct nfp_net_rx_ring *rx_ring, int budget) pkt_off - NFP_NET_RX_BUF_HEADROOM, pkt_len, true); + /* This code is invoked within a single NAPI poll cycle + * and thus under local_bh_disable(), which provides the + * needed RCU protection. + */ act = bpf_prog_run_xdp(xdp_prog, &xdp); pkt_len = xdp.data_end - xdp.data; @@ -2036,7 +2039,6 @@ static int nfp_net_rx(struct nfp_net_rx_ring *rx_ring, int budget) if (!nfp_net_xdp_complete(tx_ring)) pkts_polled = budget; } - rcu_read_unlock(); return pkts_polled; } From patchwork Thu Jun 17 21:27:44 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: 12329573 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 255CEC48BE5 for ; Thu, 17 Jun 2021 21:28:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1087760D07 for ; Thu, 17 Jun 2021 21:28:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233217AbhFQVaU (ORCPT ); Thu, 17 Jun 2021 17:30:20 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:59137 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233106AbhFQVaN (ORCPT ); Thu, 17 Jun 2021 17:30:13 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623965284; 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=nO6s/4BqIfWQwa6yGTA6YR9i3EYkNdVjQ6416QGe4OM=; b=ZrHGbjWQxb2wDBGtCsc0oox+Ls3J9Mx8lg4/5gUBAyhStSsk0Qq5NlurmE7CUdWT06kLaW YDxIc9GVIWqskMXmGNIOWGIv0/MKsfjX8Pzg1Q+HRbJ8abSx6T5dhZ6PGgJm3xKi9Vvi1z HDhCGVfvg6ZdNPmahoEMfxeJASNkxu8= Received: from mail-ed1-f69.google.com (mail-ed1-f69.google.com [209.85.208.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-324-vlIMDKgXOwaO7qku8GhG2g-1; Thu, 17 Jun 2021 17:28:03 -0400 X-MC-Unique: vlIMDKgXOwaO7qku8GhG2g-1 Received: by mail-ed1-f69.google.com with SMTP id j3-20020aa7c3430000b0290393f7aad447so226219edr.18 for ; Thu, 17 Jun 2021 14:28:03 -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=nO6s/4BqIfWQwa6yGTA6YR9i3EYkNdVjQ6416QGe4OM=; b=n8NZml38CWU5Ifv2lawsFnWeDwXvPd9AZPSJYvkpJbcED21paumvUOLSGd+fAcp2Z9 8H9ixRjo765r7fY1XK56uOtH2DgOLXbfLF7DZKJseytvv2cywTxC5rafgCYjieCMOTHe MmsnyYKXAwxIqLgksq1TMzyyEjamJmtsqGRuL2KGHFWsLLjrmlvo5ydhbjlmcPDSckSB Fo47vNjSvOnn3BDhvucbOwFgYow2nffcXCzEWi7G3ArB98PXSxxz1cV6EiR1SGvq6W7j DID4/C9ohkaX+oAOT3KyRQ2fz89eZwk9j4qBvYD9bmcPabW0zOyRdQ8PNEEarSFzJAPB nx2Q== X-Gm-Message-State: AOAM532/AjGXwGEeU2X4Saa0MDkBv3q2Hr68+sFeWBo9IGl8uKAev6c2 XxVotkYuA6U3mC/PpRQPqhUs3Sjpk937erBlGyl05pJuj/gqEHwWZsqRnJ8L9AtFKeKo2Psuibq mpN8SBj+X9lT6 X-Received: by 2002:a17:906:b55:: with SMTP id v21mr7480626ejg.88.1623965282187; Thu, 17 Jun 2021 14:28:02 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxMnbPrCh3ymsX9SRGg5ZxHfU1Rt4PKuNVke7r/Sv+gD0iKkUHnyz2chwe6+FBIOwlgHOcrZw== X-Received: by 2002:a17:906:b55:: with SMTP id v21mr7480611ejg.88.1623965282049; Thu, 17 Jun 2021 14:28:02 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([45.145.92.2]) by smtp.gmail.com with ESMTPSA id c23sm5072627eds.57.2021.06.17.14.27.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Jun 2021 14:27:59 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id C520F180735; Thu, 17 Jun 2021 23:27:54 +0200 (CEST) From: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= To: bpf@vger.kernel.org, netdev@vger.kernel.org Cc: Martin KaFai Lau , Hangbin Liu , Jesper Dangaard Brouer , Magnus Karlsson , "Paul E . McKenney" , Jakub Kicinski , =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= , Ariel Elior , GR-everest-linux-l2@marvell.com Subject: [PATCH bpf-next v3 12/16] qede: remove rcu_read_lock() around XDP program invocation Date: Thu, 17 Jun 2021 23:27:44 +0200 Message-Id: <20210617212748.32456-13-toke@redhat.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210617212748.32456-1-toke@redhat.com> References: <20210617212748.32456-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net The qede driver has 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: Ariel Elior Cc: GR-everest-linux-l2@marvell.com Signed-off-by: Toke Høiland-Jørgensen --- drivers/net/ethernet/qlogic/qede/qede_fp.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c b/drivers/net/ethernet/qlogic/qede/qede_fp.c index 8e150dd4f899..d806ab925cee 100644 --- a/drivers/net/ethernet/qlogic/qede/qede_fp.c +++ b/drivers/net/ethernet/qlogic/qede/qede_fp.c @@ -1089,13 +1089,10 @@ static bool qede_rx_xdp(struct qede_dev *edev, xdp_prepare_buff(&xdp, page_address(bd->data), *data_offset, *len, false); - /* Queues always have a full reset currently, so for the time - * being until there's atomic program replace just mark read - * side for map helpers. + /* This code is invoked within a single NAPI poll cycle and thus under + * local_bh_disable(), which provides the needed RCU protection. */ - rcu_read_lock(); act = bpf_prog_run_xdp(prog, &xdp); - rcu_read_unlock(); /* Recalculate, as XDP might have changed the headers */ *data_offset = xdp.data - xdp.data_hard_start; From patchwork Thu Jun 17 21:27:45 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: 12329577 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E1F07C48BE5 for ; Thu, 17 Jun 2021 21:28:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CB662611BE for ; Thu, 17 Jun 2021 21:28:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233125AbhFQVaZ (ORCPT ); Thu, 17 Jun 2021 17:30:25 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:43762 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233131AbhFQVaO (ORCPT ); Thu, 17 Jun 2021 17:30:14 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623965286; 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=4E0BLtcqynOO11TlkwRy31SmQKgoTi4+6vdURBBEN5E=; b=F+hgpmXLTPhxl0vwX11AGC9DUnN1AX6ORLgFeDaOBnVfb6QmSBEcI0iy8EFqlsMUe8nI74 Qw79JhaFE+ezIP4yt5fb4Wf5nfG1pBbE792oYYd220MTDzFDp06mutxk52HWc9kS5/IwyV 6nxjWGHSkVM/Y24Z8lkuqRfIXkeq/ZY= Received: from mail-ed1-f72.google.com (mail-ed1-f72.google.com [209.85.208.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-213-EYRR2OAxNbuFfuR0sWeSqQ-1; Thu, 17 Jun 2021 17:28:04 -0400 X-MC-Unique: EYRR2OAxNbuFfuR0sWeSqQ-1 Received: by mail-ed1-f72.google.com with SMTP id a16-20020aa7cf100000b0290391819a774aso802975edy.8 for ; Thu, 17 Jun 2021 14:28:04 -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=4E0BLtcqynOO11TlkwRy31SmQKgoTi4+6vdURBBEN5E=; b=XOECHVLYetIfYfpb8Hxw/aRu7Y3mjHVkdpsOn10kPZKLcYcBd0xcXQCY8K7G4eiF+5 hqD4pZBXl3eiRrNEBVcafwzQ2iaLpXN8pSa3YXS991uQJBuPN3uuepU//YebmOJMPJR2 PEU6SJ7IODkxCMajrGO0cv1cBJjlgL5Jci6lf5qnXKTvmM7yIWJwHfaOPvufNi0B4yLW eO8WoT4iLu/My/Rek3MgGtdhGQMNpI15hIiI5cmn9nLJ44VXuPSF6otAiphI5NGX2CIc hf1Tr0bNUnAL/qAST9CZqLgweCfAeha6AL1zu0U5Muob/BBT1IcrD4fIfN3WQkV+d/Si c3eA== X-Gm-Message-State: AOAM531+8lMcdReH53uQAra5UNIHt7Lr0haEfT34XPxBT4tMEWrqY/9r 2F/R5xc6GtGet3C/rGNJh17G5gQlbjQUzX1+aPzvBa2pls53W9t2oaKTSAkQMCZeR32HoomiUP8 7aCIMM9j5hc5F X-Received: by 2002:aa7:dc0a:: with SMTP id b10mr480326edu.134.1623965283765; Thu, 17 Jun 2021 14:28:03 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwm3xB3xkhsrXomg6vyS9qjizIuVw9HAdz+x5dP1AwlFGrfqO5qySP880NPEtyTVT7G1MUUUw== X-Received: by 2002:aa7:dc0a:: with SMTP id b10mr480311edu.134.1623965283649; Thu, 17 Jun 2021 14:28:03 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([45.145.92.2]) by smtp.gmail.com with ESMTPSA id e6sm111671ejm.35.2021.06.17.14.27.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Jun 2021 14:28:01 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id CCF49180736; Thu, 17 Jun 2021 23:27:54 +0200 (CEST) From: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= To: bpf@vger.kernel.org, netdev@vger.kernel.org Cc: Martin KaFai Lau , Hangbin Liu , Jesper Dangaard Brouer , Magnus Karlsson , "Paul E . McKenney" , Jakub Kicinski , =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= , Edward Cree , Martin Habets Subject: [PATCH bpf-next v3 13/16] sfc: remove rcu_read_lock() around XDP program invocation Date: Thu, 17 Jun 2021 23:27:45 +0200 Message-Id: <20210617212748.32456-14-toke@redhat.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210617212748.32456-1-toke@redhat.com> References: <20210617212748.32456-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net The sfc driver has 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: Edward Cree Cc: Martin Habets Acked-by: Edward Cree Signed-off-by: Toke Høiland-Jørgensen --- drivers/net/ethernet/sfc/rx.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c index 17b8119c48e5..3e5b88ab42db 100644 --- a/drivers/net/ethernet/sfc/rx.c +++ b/drivers/net/ethernet/sfc/rx.c @@ -260,18 +260,14 @@ static bool efx_do_xdp(struct efx_nic *efx, struct efx_channel *channel, s16 offset; int err; - rcu_read_lock(); - xdp_prog = rcu_dereference(efx->xdp_prog); - if (!xdp_prog) { - rcu_read_unlock(); + xdp_prog = rcu_dereference_bh(efx->xdp_prog); + if (!xdp_prog) return true; - } rx_queue = efx_channel_get_rx_queue(channel); if (unlikely(channel->rx_pkt_n_frags > 1)) { /* We can't do XDP on fragmented packets - drop. */ - rcu_read_unlock(); efx_free_rx_buffers(rx_queue, rx_buf, channel->rx_pkt_n_frags); if (net_ratelimit()) @@ -295,8 +291,10 @@ static bool efx_do_xdp(struct efx_nic *efx, struct efx_channel *channel, xdp_prepare_buff(&xdp, *ehp - EFX_XDP_HEADROOM, EFX_XDP_HEADROOM, rx_buf->len, false); + /* This code is invoked within a single NAPI poll cycle and thus under + * local_bh_disable(), which provides the needed RCU protection. + */ xdp_act = bpf_prog_run_xdp(xdp_prog, &xdp); - rcu_read_unlock(); offset = (u8 *)xdp.data - *ehp; From patchwork Thu Jun 17 21:27:46 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: 12329571 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 51380C49EA5 for ; Thu, 17 Jun 2021 21:28:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3222D613EC for ; Thu, 17 Jun 2021 21:28:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233167AbhFQVaQ (ORCPT ); Thu, 17 Jun 2021 17:30:16 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:27356 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233090AbhFQVaN (ORCPT ); Thu, 17 Jun 2021 17:30:13 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623965284; 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=2HHxZ70enPSVJrHvmI2lNUfsNsQ5hyZDG+eWNYytmQM=; b=PMT4XWzUBZDWG2f+2uMkyfZd5fEyla64cz85KHk9bNYJ45s1rCJVt6ZVhd2SLNDSGOAQyC +JNoHEPL8QCsnMpVOcUuE8fH8HZ/cOF780EmEPhNQsKN8XhXS6Cq3bqMbw1oD6x8gOzxVj YabyHRcShouDE6Gk9l/nZR9sWvE7I3Y= Received: from mail-ed1-f70.google.com (mail-ed1-f70.google.com [209.85.208.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-290-6hoGTueMPtufSuP8ieEn3g-1; Thu, 17 Jun 2021 17:28:03 -0400 X-MC-Unique: 6hoGTueMPtufSuP8ieEn3g-1 Received: by mail-ed1-f70.google.com with SMTP id y16-20020a0564024410b0290394293f6816so2362379eda.20 for ; Thu, 17 Jun 2021 14:28:02 -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=2HHxZ70enPSVJrHvmI2lNUfsNsQ5hyZDG+eWNYytmQM=; b=eoRU+EQoOn3scy9J39cPUZUOIG3DZv1TJS2xRx5XNnfaYh2gBXqN9GGYhmH1OMzoI2 wP8Jb63feOlUOv9XgvMFYV88Xg1aSn4ajHv+erFp1HbAR94h3T+ab4MNNV3gG26H+SD9 l/UplF2JRfSBuXiOCmh3mStAhwTl03REjGbTaYvTd/TCX1XclxraQjyLddPCfcWBtinY HqM8MSw7N1Im6IJHlCxy9EKgl4LITHKLrp9hkHl8N/GE48vCdHFYlLHd2lmuLuG0Po9i 7ijWuzUd0rFScR0VAzxwFVp21ZyVijllqsYIK8O2v08lhXdW/JwaEhiCA/HdzWldB3GR na7w== X-Gm-Message-State: AOAM5327rru9eNAgTeBqvTLhCvni5nOQhOIQny1xD0HR/jCuC1heqoMp NgMhAi+ricLjEOYULTMioQwMkyGuO+UQGIPslWlD0dNYI/JlExizHu3U9b6Yk1w3LlVxe9n9HJJ 67QwvNCzpdlTb X-Received: by 2002:a05:6402:27d0:: with SMTP id c16mr475910ede.60.1623965282046; Thu, 17 Jun 2021 14:28:02 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzo+0no19/AfrC5WVLy9yZXt8Bo09UqWLQ/taswl5RGBhcCnNAeCwZslfDXtE9wkSElMgquYg== X-Received: by 2002:a05:6402:27d0:: with SMTP id c16mr475898ede.60.1623965281944; Thu, 17 Jun 2021 14:28:01 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([45.145.92.2]) by smtp.gmail.com with ESMTPSA id q15sm5324872edr.84.2021.06.17.14.27.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Jun 2021 14:28:01 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id D5B79180737; Thu, 17 Jun 2021 23:27:54 +0200 (CEST) From: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= To: bpf@vger.kernel.org, netdev@vger.kernel.org Cc: Martin KaFai Lau , Hangbin Liu , Jesper Dangaard Brouer , Magnus Karlsson , "Paul E . McKenney" , Jakub Kicinski , =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= , Jassi Brar , Ilias Apalodimas Subject: [PATCH bpf-next v3 14/16] netsec: remove rcu_read_lock() around XDP program invocation Date: Thu, 17 Jun 2021 23:27:46 +0200 Message-Id: <20210617212748.32456-15-toke@redhat.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210617212748.32456-1-toke@redhat.com> References: <20210617212748.32456-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net The netsec driver has a rcu_read_lock()/rcu_read_unlock() pair around the full RX loop, covering everything up to and including xdp_do_flush(). This is actually the correct behaviour, but because it all happens in a single NAPI poll cycle (and thus under local_bh_disable()), it is also technically redundant. 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 the rcu_read_lock() around anymore, so let's just remove it. Cc: Jassi Brar Cc: Ilias Apalodimas Acked-by: Ilias Apalodimas Signed-off-by: Toke Høiland-Jørgensen --- drivers/net/ethernet/socionext/netsec.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c index dfc85cc68173..e07b7c870046 100644 --- a/drivers/net/ethernet/socionext/netsec.c +++ b/drivers/net/ethernet/socionext/netsec.c @@ -958,7 +958,6 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget) xdp_init_buff(&xdp, PAGE_SIZE, &dring->xdp_rxq); - rcu_read_lock(); xdp_prog = READ_ONCE(priv->xdp_prog); dma_dir = page_pool_get_dma_dir(dring->page_pool); @@ -1019,6 +1018,10 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget) pkt_len, false); if (xdp_prog) { + /* This code is invoked within a single NAPI poll cycle + * and thus under local_bh_disable(), which provides the + * needed RCU protection. + */ xdp_result = netsec_run_xdp(priv, xdp_prog, &xdp); if (xdp_result != NETSEC_XDP_PASS) { xdp_act |= xdp_result; @@ -1069,8 +1072,6 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget) } netsec_finalize_xdp_rx(priv, xdp_act, xdp_xmit); - rcu_read_unlock(); - return done; } From patchwork Thu Jun 17 21:27:47 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: 12329565 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CE2CAC49EAB for ; Thu, 17 Jun 2021 21:28:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B71E9613E2 for ; Thu, 17 Jun 2021 21:28:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233100AbhFQVaN (ORCPT ); Thu, 17 Jun 2021 17:30:13 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:56967 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233076AbhFQVaL (ORCPT ); Thu, 17 Jun 2021 17:30:11 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623965283; 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=h2Vi9XceSPVZfLpkffqV3pCVibBmY0ByVpnQRv2lfdE=; b=YS+1cSJxvroIY0LLcif7PJxPOT5xDNu+a1+KYWHTjA4lbIf9RJlCEXH6Ik5rzBa8aB4xL6 xtDtwo1a8kuT5kguHB3gAC6nzRs8xLREqI02OkwjHGLQ3sGnczXLHCVcIIgk6m/IvS7MLU 6ByvBVcrlQUiau+Ng7snyZ+hbTKsTP8= Received: from mail-ej1-f72.google.com (mail-ej1-f72.google.com [209.85.218.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-465-JrQVayaTNdW2QKsbm-fy6g-1; Thu, 17 Jun 2021 17:28:02 -0400 X-MC-Unique: JrQVayaTNdW2QKsbm-fy6g-1 Received: by mail-ej1-f72.google.com with SMTP id 16-20020a1709063010b029037417ca2d43so3025643ejz.5 for ; Thu, 17 Jun 2021 14:28:01 -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=h2Vi9XceSPVZfLpkffqV3pCVibBmY0ByVpnQRv2lfdE=; b=SGT/VCmR1TJcWjlssGilRbpq+So8ySH2tYMxKsklJiW9p8mkH//RLBDx/5yGRix0DY cYHsDdKtA4Bp2zWOL0ez6GSneLWvqjcntN968DxGr4lMC5FrdadyXO4ECvbPY5uLZ+nF Er7rk0LmhO9VCNT0AaIvIk1YZAZFt48q3KwtRrYOEYOwar9fmpa2m8Hli7RkEM1gBjK9 sNwQEfHOXafUbeFEYLpYJhcDMzbHz+pyUj4YT8RBL8LRQHXkyuxvZjXywRLnOuWZpLqf 96OnqOaazHEX0+NbCNgaAk+vzC/dbHVHwM5lbGpTKzIraqtvewd0D/N6ICGgQUqYBrRg CD7Q== X-Gm-Message-State: AOAM5310RaqAfds399rvJSpwvTGHwcFaNUnITD68FeZfxL1RgkxdkqOU gW0LVXhRJFMLDZa7GBIxlROiCkcLkEopRSmkJkWhbjXWTXKC0ga7mxUDLlA6tXtOJ9Mpr1Nxap0 K6FD4V4N2o6c0 X-Received: by 2002:a05:6402:42cc:: with SMTP id i12mr418816edc.345.1623965280723; Thu, 17 Jun 2021 14:28:00 -0700 (PDT) X-Google-Smtp-Source: ABdhPJx7GdrC2OmFDGjTuzCjTi7XENVhneXkF+uyTA4qSW3+SnXwzyHh4ZUMgJUipj8lkd9TnOv6ZA== X-Received: by 2002:a05:6402:42cc:: with SMTP id i12mr418777edc.345.1623965280307; Thu, 17 Jun 2021 14:28:00 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([2a0c:4d80:42:443::2]) by smtp.gmail.com with ESMTPSA id hg25sm108948ejc.51.2021.06.17.14.27.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Jun 2021 14:27:59 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id DC3F4180738; Thu, 17 Jun 2021 23:27:54 +0200 (CEST) From: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= To: bpf@vger.kernel.org, netdev@vger.kernel.org Cc: Martin KaFai Lau , Hangbin Liu , Jesper Dangaard Brouer , Magnus Karlsson , "Paul E . McKenney" , Jakub Kicinski , =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= , Giuseppe Cavallaro , Alexandre Torgue , Jose Abreu Subject: [PATCH bpf-next v3 15/16] stmmac: remove rcu_read_lock() around XDP program invocation Date: Thu, 17 Jun 2021 23:27:47 +0200 Message-Id: <20210617212748.32456-16-toke@redhat.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210617212748.32456-1-toke@redhat.com> References: <20210617212748.32456-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net The stmmac driver has 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: Giuseppe Cavallaro Cc: Alexandre Torgue Cc: Jose Abreu Signed-off-by: Toke Høiland-Jørgensen Acked-by: Wong Vee Khee Tested-by: Song, Yoong Siang --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index bf9fe25fed69..5dcc8a42abf9 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -4654,7 +4654,6 @@ static int stmmac_xdp_xmit_back(struct stmmac_priv *priv, return res; } -/* This function assumes rcu_read_lock() is held by the caller. */ static int __stmmac_xdp_run_prog(struct stmmac_priv *priv, struct bpf_prog *prog, struct xdp_buff *xdp) @@ -4662,6 +4661,9 @@ static int __stmmac_xdp_run_prog(struct stmmac_priv *priv, u32 act; int res; + /* This code is invoked within a single NAPI poll cycle and thus under + * local_bh_disable(), which provides the needed RCU protection. + */ act = bpf_prog_run_xdp(prog, xdp); switch (act) { case XDP_PASS: @@ -4696,17 +4698,14 @@ static struct sk_buff *stmmac_xdp_run_prog(struct stmmac_priv *priv, struct bpf_prog *prog; int res; - rcu_read_lock(); - prog = READ_ONCE(priv->xdp_prog); if (!prog) { res = STMMAC_XDP_PASS; - goto unlock; + goto out; } res = __stmmac_xdp_run_prog(priv, prog, xdp); -unlock: - rcu_read_unlock(); +out: return ERR_PTR(-res); } @@ -4976,10 +4975,8 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue) buf->xdp->data_end = buf->xdp->data + buf1_len; xsk_buff_dma_sync_for_cpu(buf->xdp, rx_q->xsk_pool); - rcu_read_lock(); prog = READ_ONCE(priv->xdp_prog); res = __stmmac_xdp_run_prog(priv, prog, buf->xdp); - rcu_read_unlock(); switch (res) { case STMMAC_XDP_PASS: From patchwork Thu Jun 17 21:27:48 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: 12329569 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8BC3DC48BE5 for ; Thu, 17 Jun 2021 21:28:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6ED26613E2 for ; Thu, 17 Jun 2021 21:28:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233159AbhFQVaQ (ORCPT ); Thu, 17 Jun 2021 17:30:16 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:26210 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233083AbhFQVaM (ORCPT ); Thu, 17 Jun 2021 17:30:12 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623965284; 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=URoBjUEpro9U64xROBMZeKvvFlHbDEw9khuYvYfphMY=; b=OCfiPrSpkjdBK1L4Ao3s928FwmsIZVCdW9AO5EhBIKy6GmqNMUSm8PeyjqWZEI2hAUAzOZ MR2gtbONZjk9txF7CYtNuZ3MwK3WALyMtnsiGGo/N8peUTiXcurGK98FifEnxoEGYQEhbB YdecEcgyJZnlV92uxvTk1ZNV9xPSvr4= Received: from mail-ej1-f69.google.com (mail-ej1-f69.google.com [209.85.218.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-454-VezdYB3-PPutLWdtEI7bow-1; Thu, 17 Jun 2021 17:28:02 -0400 X-MC-Unique: VezdYB3-PPutLWdtEI7bow-1 Received: by mail-ej1-f69.google.com with SMTP id z6-20020a17090665c6b02903700252d1ccso3011491ejn.10 for ; Thu, 17 Jun 2021 14:28:02 -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=URoBjUEpro9U64xROBMZeKvvFlHbDEw9khuYvYfphMY=; b=I0grLI8Uds0Pfg+l2QG+3cwfv7/gSRqywS4tvxHG8Y7cTgocn+XEITwP5fJobMJqLQ /oC0UnE2JCsSvF2P6O2qUcaL2B8Yb/e9ZOEmE4MvWEid4HwDX3InwT+YRnRF4ogudQ3y QpiBe8LQd9mFdgy1077JwgJA5seVOBH/7r8xDjhjoUI2CjjzgypipuhSUHanI/vOeIgC rD6tuSSjEf6FFAsKkBVIoWrgVEQxBfU5N7gbyqPWmegdUdebDcGjn5V72ohJkZdoZKwL GsbBC3caJq8rNwjYTLrXsD+030N/PwDipapdU87RAWvSEW8YX/Qbxootiueb3xGVkC+C wMkg== X-Gm-Message-State: AOAM533dlyCMqPdridicSZKGlBaPaZnzg34rYYKG4Yn8BfTAbx5DJZOd B/8sM34bD8g6xr40wZJCfQLuOeDPn5l35g71iXB/Dt9aWwQmCcrAwBihVVQyEMpYliO67Z6Fth5 OZPH/0mGMMfUM X-Received: by 2002:a17:906:1c4e:: with SMTP id l14mr7493114ejg.172.1623965281348; Thu, 17 Jun 2021 14:28:01 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxBED07LUChvl53lkxSTodZZSwtVeK7ggiDn0AD9irgpsF1jIRBNNK0HPoUFkO1vHM4IXH08A== X-Received: by 2002:a17:906:1c4e:: with SMTP id l14mr7493097ejg.172.1623965281090; Thu, 17 Jun 2021 14:28:01 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([2a0c:4d80:42:443::2]) by smtp.gmail.com with ESMTPSA id og6sm94201ejc.108.2021.06.17.14.27.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Jun 2021 14:27:59 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id E3BD4180739; Thu, 17 Jun 2021 23:27:54 +0200 (CEST) From: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= To: bpf@vger.kernel.org, netdev@vger.kernel.org Cc: Martin KaFai Lau , Hangbin Liu , Jesper Dangaard Brouer , Magnus Karlsson , "Paul E . McKenney" , Jakub Kicinski , =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= , Grygorii Strashko , linux-omap@vger.kernel.org Subject: [PATCH bpf-next v3 16/16] net: ti: remove rcu_read_lock() around XDP program invocation Date: Thu, 17 Jun 2021 23:27:48 +0200 Message-Id: <20210617212748.32456-17-toke@redhat.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210617212748.32456-1-toke@redhat.com> References: <20210617212748.32456-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net The cpsw driver has 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: Grygorii Strashko Cc: linux-omap@vger.kernel.org Tested-by: Grygorii Strashko Reviewed-by: Grygorii Strashko Signed-off-by: Toke Høiland-Jørgensen --- drivers/net/ethernet/ti/cpsw_priv.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/ti/cpsw_priv.c b/drivers/net/ethernet/ti/cpsw_priv.c index 5862f0a4a975..25fa7304a542 100644 --- a/drivers/net/ethernet/ti/cpsw_priv.c +++ b/drivers/net/ethernet/ti/cpsw_priv.c @@ -1328,14 +1328,13 @@ int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp, struct bpf_prog *prog; u32 act; - rcu_read_lock(); - prog = READ_ONCE(priv->xdp_prog); - if (!prog) { - ret = CPSW_XDP_PASS; - goto out; - } + if (!prog) + return CPSW_XDP_PASS; + /* This code is invoked within a single NAPI poll cycle and thus under + * local_bh_disable(), which provides the needed RCU protection. + */ act = bpf_prog_run_xdp(prog, xdp); /* XDP prog might have changed packet data and boundaries */ *len = xdp->data_end - xdp->data; @@ -1378,10 +1377,8 @@ int cpsw_run_xdp(struct cpsw_priv *priv, int ch, struct xdp_buff *xdp, ndev->stats.rx_bytes += *len; ndev->stats.rx_packets++; out: - rcu_read_unlock(); return ret; drop: - rcu_read_unlock(); page_pool_recycle_direct(cpsw->page_pool[ch], page); return ret; }