From patchwork Tue Jun 15 14:54: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: 460896 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 91CB8C48BDF for ; Tue, 15 Jun 2021 14:55:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 79F24615A0 for ; Tue, 15 Jun 2021 14:55:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231438AbhFOO5R (ORCPT ); Tue, 15 Jun 2021 10:57:17 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:31078 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231274AbhFOO5M (ORCPT ); Tue, 15 Jun 2021 10:57:12 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623768907; 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=Gxp4+onf8wuO87Iii4KRL2PGeBHDLINdkwBz3RXDQ8I=; b=Ro3mOVqYwuMlkAgcIBfxI+x/BnGTgITw4VNuRC0gU71ZSYGO5Yqemed8MoLNL1CHMLZfGI w9KxJQnNBOMBn2N/UCAak/1STlVlMuGgtPdBPHcVqLw99rediUGL0+tsjxxkrZuCDQb0Ln 2SBpA4aO6uclw1VhTNnjOdKyHyoKKxM= 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-151-qe8IcxMmPo-oJCMCML7_Pw-1; Tue, 15 Jun 2021 10:55:06 -0400 X-MC-Unique: qe8IcxMmPo-oJCMCML7_Pw-1 Received: by mail-ed1-f72.google.com with SMTP id t11-20020a056402524bb029038ffacf1cafso13027471edd.5 for ; Tue, 15 Jun 2021 07:55:06 -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=Gxp4+onf8wuO87Iii4KRL2PGeBHDLINdkwBz3RXDQ8I=; b=fY5908mlwg7PEtgWYYd5PDiz/oCz20t24WQ/YGV8K6bC4kzZyby2tTgfmUJXt8QlBT HMKCE7a/s+K/ZuMkuJQLB7us09Wgvma7+PMQ75kwh1RqcsYT2FDmP/dNCPOiz+jTBRFp snkNJM63P4vWF7NaTfjl6e0lC0nvCnbSgA8mOllTyodBHK6FBIjkeCzjI7+veFOX456Y Iq1dFlhigvdWS1lghsYOeRYh5VWpcUWNGBeg8l2gNQk5V6XU1UJ10NsqvmOzPRTsjgvU XUL84a/jtxTGKRgb4XG5eKcSkbVgFL4ftyp+ICetQEGwkeW6o7YIdDA4WaqsqFlnP6QY +Lig== X-Gm-Message-State: AOAM532UTsh41DWtdolPc0TRLa+oDYbCDKlgrHskDzrV29mm81AHbvJI ++0OdMIr6uijdnJRK5iXvjruYR9h3nyQuUz2CImLXTUeZP0+qirVTfJMYSOaNSlb6IJCrMSGyql v7Bqzw9b6V7WAdZ3u X-Received: by 2002:a50:b2c5:: with SMTP id p63mr10055607edd.5.1623768905657; Tue, 15 Jun 2021 07:55:05 -0700 (PDT) X-Google-Smtp-Source: ABdhPJz/YwZa9hwA5T/K0ZvJNmG1D6JAT7VKQUI+n0jma5WK9p1jIeM6YWlaebazh4ioDqp7gZhVVw== X-Received: by 2002:a50:b2c5:: with SMTP id p63mr10055582edd.5.1623768905390; Tue, 15 Jun 2021 07:55:05 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([2a0c:4d80:42:443::2]) by smtp.gmail.com with ESMTPSA id ci12sm10333745ejc.17.2021.06.15.07.55.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 15 Jun 2021 07:55:04 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id A7CD31802AB; Tue, 15 Jun 2021 16:54:58 +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-?= =?utf-8?q?J=C3=B8rgensen?= Subject: [PATCH bpf-next v2 01/16] rcu: Create an unrcu_pointer() to remove __rcu from a pointer Date: Tue, 15 Jun 2021 16:54:40 +0200 Message-Id: <20210615145455.564037-2-toke@redhat.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210615145455.564037-1-toke@redhat.com> References: <20210615145455.564037-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 Tue Jun 15 14:54: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: 460897 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 407B9C48BDF for ; Tue, 15 Jun 2021 14:55:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 22EDB615A0 for ; Tue, 15 Jun 2021 14:55:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231357AbhFOO5N (ORCPT ); Tue, 15 Jun 2021 10:57:13 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:40521 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231276AbhFOO5L (ORCPT ); Tue, 15 Jun 2021 10:57:11 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623768906; 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=XauQxIidgZWqKYwupDfOjsOZffqWyiEa+6VH4Dkvxc8=; b=GG5mJkvMg0FD2ozmM3S9z4D4J8V7qVLG0x2IKs0UhzT86GYGW2QHUVsWUZbnQB82zwfqeG v6SoqbPAfhUSRFQBkNqGs97bjUBpK0GN7UMje+r13n6S27ZpdDhwUYX6l+IOpbA589/VRU 1N8xVPkNhFMJacPAmxkHFMOKRwbpb98= 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-262-pa5E2ndZOr2itvzYNZuM2Q-1; Tue, 15 Jun 2021 10:55:05 -0400 X-MC-Unique: pa5E2ndZOr2itvzYNZuM2Q-1 Received: by mail-ed1-f70.google.com with SMTP id j3-20020aa7c3430000b0290393f7aad447so8301591edr.18 for ; Tue, 15 Jun 2021 07:55: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=XauQxIidgZWqKYwupDfOjsOZffqWyiEa+6VH4Dkvxc8=; b=jCpZbLiGgqwHT12cVEOwo+8cycSJLz7t5ZNErUqAsZIOG2JkG808LutZMidH2vllEt VDkEpsIO7x0fqu8xdtLnxC/MU6C5SRdNCeZUeTUSpHsREfZhhqzr1YHOk6lzTU8KHtsM 1P86z6kHk4zL04FO0XTDi1xLluJcsz7TtpoZ7sAdVZZBDzMNPVZa3Jb8mq07nw1otaVz ISDHf09gGFxfgqzLi48UxjeoVx3Uh08fwjM4+PsMo+PVA7hnthJCmgQeCnXSo3cRyRRj DHkW1bKzNuwdqY+d82wn2uRmS4a7lpHOtgBAUQ7D8bEytVXoN11AYI3EaRyuy5lGb37S BkFw== X-Gm-Message-State: AOAM53174b6xzwBrQzRjVONHRHKT0hPfO4A/dFKqaP4bIBkInFgZrMN7 HOmLDwHfZobsiWV4YyraZ4lL9kDhgY0ELK/4ixsm8lpRGQ/nLJ1Ar/Cv2d23aW/LSPbsdQCAvQT C+eoYpf4VzPxHJzf+ X-Received: by 2002:a05:6402:4c5:: with SMTP id n5mr23639459edw.322.1623768900927; Tue, 15 Jun 2021 07:55:00 -0700 (PDT) X-Google-Smtp-Source: ABdhPJz+UVP8pxC8w7iB/YuB9hqs/WlODKOZA/Y8SXwOER+whNMJCEFEtE059xFN1e1YScbbjgXvLQ== X-Received: by 2002:a05:6402:4c5:: with SMTP id n5mr23639436edw.322.1623768900673; Tue, 15 Jun 2021 07:55:00 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([45.145.92.2]) by smtp.gmail.com with ESMTPSA id t5sm10267139eje.29.2021.06.15.07.54.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 15 Jun 2021 07:54:59 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id B4D5C1802FF; Tue, 15 Jun 2021 16:54:58 +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-?= =?utf-8?q?J=C3=B8rgensen?= Subject: [PATCH bpf-next v2 03/16] xdp: add proper __rcu annotations to redirect map entries Date: Tue, 15 Jun 2021 16:54:42 +0200 Message-Id: <20210615145455.564037-4-toke@redhat.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210615145455.564037-1-toke@redhat.com> References: <20210615145455.564037-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 --- include/net/xdp_sock.h | 2 +- kernel/bpf/cpumap.c | 13 +++++++---- kernel/bpf/devmap.c | 52 +++++++++++++++++++----------------------- net/core/filter.c | 28 +++++++++++++++++++++++ net/xdp/xsk.c | 4 ++-- net/xdp/xsk.h | 4 ++-- net/xdp/xskmap.c | 29 +++++++++++++---------- 7 files changed, 82 insertions(+), 50 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..ae6d9bfeae06 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); @@ -266,7 +270,8 @@ static void *__dev_map_hash_lookup_elem(struct bpf_map *map, u32 key) struct bpf_dtab_netdev *dev; hlist_for_each_entry_rcu(dev, head, index_hlist, - lockdep_is_held(&dtab->index_lock)) + (lockdep_is_held(&dtab->index_lock) || + rcu_read_lock_bh_held())) if (dev->idx == key) return dev; @@ -410,15 +415,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 +432,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 +444,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 +736,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 +845,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 +1025,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 Tue Jun 15 14:54: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: 460898 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 38222C49361 for ; Tue, 15 Jun 2021 14:55:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 08C9D61584 for ; Tue, 15 Jun 2021 14:55:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231346AbhFOO5L (ORCPT ); Tue, 15 Jun 2021 10:57:11 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:22129 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231274AbhFOO5J (ORCPT ); Tue, 15 Jun 2021 10:57:09 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623768904; 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=bmidfz0+CJmOWuJ1Ds56KwPBuWWIUyMrF1VbqTWEQjA=; b=LQ8sfZHrWMzCACzx9XHzBzUoD19ELMK2n2/Q9PmVSJWfvFySgUJlY55dXmN++MEjXjuirx i8VRmRn+E8PuWnJZd0AGdUjHiItbah2rt7uAdg8kF121CGAWakOpTpgLq2PH/PeXzThK10 sL7RC5oyRKJo1tcY6InRAiuttdnLZ5I= 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-167-zrCV6V5yPMidgSl97lFm9Q-1; Tue, 15 Jun 2021 10:55:01 -0400 X-MC-Unique: zrCV6V5yPMidgSl97lFm9Q-1 Received: by mail-ed1-f69.google.com with SMTP id df3-20020a05640230a3b029039179c0f290so20694042edb.13 for ; Tue, 15 Jun 2021 07:55: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=bmidfz0+CJmOWuJ1Ds56KwPBuWWIUyMrF1VbqTWEQjA=; b=Ug2FZCKDWI7y48pzp72Llw1oV6G43hCT1GPBWM1BEAZci5OpO7yB5iBcgop8LSX8zG fms7c/MIAnXI5upuvENc23olhSqMT6OnWAOkNxpmTbFQ4clZx7izoAQgjVSZBIfOrTKM InNnqOoIGzzzVSrCcY1rNO5eKhtCi9sl9MFjCSbunRQevP/2iQe/v4QaIr3DIcDzhrkq t8V/ICzOJ9WTIup7XDVrsBCkacgnBH15H/rp4s6d3eLVsud0IIQKGiSkVTfYcVoF2Z95 kWO1wZXFc8oAYl2IKpq5nsiVKmFZhLmCDCEnXJjSmo7YoW6As+CXqhhCZYbzAIQbUkzz haVg== X-Gm-Message-State: AOAM5316+N0svGwQl3t206qNCzQQEIrscVM6r10pdNUzUPFGBx05jU+b dh+SyH7ufk2fJcHSGW6SQTNJU01wFq6KN7BJdDip+KrLot4MMIJeNzHsxwhgEFTbO9umk5bMD5w Ef2/aMhoNp85YVr3c X-Received: by 2002:a05:6402:702:: with SMTP id w2mr23790300edx.189.1623768900337; Tue, 15 Jun 2021 07:55:00 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzNRBKZlwf2ReOVkRIekxwBZ2n5A2DWRM9fshz8kMlqUDSUxWOoQYe/r7X+b9O8Xp4q8OQ9cQ== X-Received: by 2002:a05:6402:702:: with SMTP id w2mr23790285edx.189.1623768900198; Tue, 15 Jun 2021 07:55:00 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([45.145.92.2]) by smtp.gmail.com with ESMTPSA id kf3sm9320945ejc.8.2021.06.15.07.54.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 15 Jun 2021 07:54:59 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id C4240180727; Tue, 15 Jun 2021 16:54:58 +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-?= =?utf-8?q?J=C3=B8rgensen?= , Guy Tzalik , Saeed Bishara Subject: [PATCH bpf-next v2 04/16] ena: remove rcu_read_lock() around XDP program invocation Date: Tue, 15 Jun 2021 16:54:43 +0200 Message-Id: <20210615145455.564037-5-toke@redhat.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210615145455.564037-1-toke@redhat.com> References: <20210615145455.564037-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 Tue Jun 15 14:54: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: 460892 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 99AF5C48BDF for ; Tue, 15 Jun 2021 14:55:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 805C261581 for ; Tue, 15 Jun 2021 14:55:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231513AbhFOO5i (ORCPT ); Tue, 15 Jun 2021 10:57:38 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:37186 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231421AbhFOO5R (ORCPT ); Tue, 15 Jun 2021 10:57:17 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623768912; 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=g/1dUDyNZZrs33xk0mFJ4m/UiP5685UlgffhChjEESM=; b=DOEJnelVpruc384QI/NHgh8z/aFuVM7UIsdirE1T2JeKTowfD1uhDLqwuNYd/Uh3cMUSXl 4EZbqfYaE5W5DY5Mi181iRC0rV+GBllMaUXFn2gOZJnPACb5OoTVzo9f68lAkwYGb1Z8rk F8LQD1AXy7p2okwCP08XDSqdYxv1yiE= 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-604-N2tt06V-P6-pj6dmtn30hQ-1; Tue, 15 Jun 2021 10:55:11 -0400 X-MC-Unique: N2tt06V-P6-pj6dmtn30hQ-1 Received: by mail-ed1-f71.google.com with SMTP id z16-20020aa7d4100000b029038feb83da57so22348413edq.4 for ; Tue, 15 Jun 2021 07:55:11 -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=g/1dUDyNZZrs33xk0mFJ4m/UiP5685UlgffhChjEESM=; b=M+GC/aP3565lmX/3TOOzUMVIlyJh9d+21ClrG7uWfwYdqAxRUt+7mnkQA8OYozQPai UizzunUUKAl8QS7ZCPcEtOyHh4USSzdiv9kE0K9iW/FmJerwQUw0Smcy2djucvBW7002 OZVV181iqtmzKgo6TNlu6SoIcl1I3g0O6rSsDF0SY5kt1X8HgyYVzeBA87HsR9Ovx69P oJl2/b33c30TvjCIXEqYQ3+gdElH61501PW5z9pwKhfIodsAGtEjJ8x0fz9gVhS3u4Bo 2fTfvbiW7bY851YUlWzcJjUsXpDYB6lEZBPj3Qf1faG1N7y1i4ubwdwd3oiBJS8zt0aT SQjw== X-Gm-Message-State: AOAM531A1iw0uPOlJWTzasYVfMIUlFJCIx4ThFK7dQAri3+NYU+lW2xq M/U2RcM3s6LU8Rwu/nb4HmdRpRZXs/GRVim9XwtnmzVrjujqbmMAY1vgqu30W9CkuT24IEq8V0k DR9HnREE4eyRfcmxa X-Received: by 2002:a05:6402:128d:: with SMTP id w13mr24062077edv.38.1623768910450; Tue, 15 Jun 2021 07:55:10 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzCDxZA5428OnKpa9GZb7yrE5vaWiO0ZAS3v/w0w55ygu8BMKcn1QLYrEOPu83LXEhSAk3gSw== X-Received: by 2002:a05:6402:128d:: with SMTP id w13mr24062041edv.38.1623768910200; Tue, 15 Jun 2021 07:55:10 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([2a0c:4d80:42:443::2]) by smtp.gmail.com with ESMTPSA id w24sm8609477eju.73.2021.06.15.07.55.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 15 Jun 2021 07:55:06 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id E1C88180731; Tue, 15 Jun 2021 16:54:58 +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-?= =?utf-8?q?J=C3=B8rgensen?= , Jesse Brandeburg , Tony Nguyen , intel-wired-lan@lists.osuosl.org Subject: [PATCH bpf-next v2 08/16] net: intel: remove rcu_read_lock() around XDP program invocation Date: Tue, 15 Jun 2021 16:54:47 +0200 Message-Id: <20210615145455.564037-9-toke@redhat.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210615145455.564037-1-toke@redhat.com> References: <20210615145455.564037-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 Tue Jun 15 14:54: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: 460894 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 560CCC48BDF for ; Tue, 15 Jun 2021 14:55:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 41B8F61584 for ; Tue, 15 Jun 2021 14:55:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231627AbhFOO51 (ORCPT ); Tue, 15 Jun 2021 10:57:27 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:42767 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231486AbhFOO5Q (ORCPT ); Tue, 15 Jun 2021 10:57:16 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623768911; 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=rY2VfBWw6NJ0/51nfkfzpQbN8Kg/IiRrFOK+qEg2pA4=; b=fJGgf0fmYBJ/yu0/oXh4QqvwosS682jmUbvzFLAYILs+04z/ZSo+vIWOx3rQRiOnxmQiHj CVePXs21ZwsBkDr+QwU1sRz+RlvDxV8gv0kGFi65fwRtybe/n/qr34v4Mwz7yX8WadHDHb GtryHSWYT6U9TJ8tzaEaDSsBnZqB6lQ= 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-526-49Q8_mNDMYCUBCAY2mZF5A-1; Tue, 15 Jun 2021 10:55:09 -0400 X-MC-Unique: 49Q8_mNDMYCUBCAY2mZF5A-1 Received: by mail-ed1-f69.google.com with SMTP id s25-20020aa7c5590000b0290392e051b029so17410110edr.11 for ; Tue, 15 Jun 2021 07:55:09 -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=rY2VfBWw6NJ0/51nfkfzpQbN8Kg/IiRrFOK+qEg2pA4=; b=nw+cegES9CuYA4wIjVZpreTY0+6IavH6unmtR10qHJfOpHwdNsrtZLHIgBBmYR6+gF rzoFLAJ3cyAE1ntACGYIf720SqBAnFqvjIgA0hQ8fMHm+zO15RNPthFIe6EDRW8YPLzr EZCQE8/7Is5kMNe31KjxKx3nRexWMISR2Wi4XDoNs1AVA2QWi5g1vruMlaN1iqXUzYgB 4LuTSjVkEVRUJlugSUpX07y1FtCRhTUwNyWG04ZmCqzfxN3KL6+c2Ee7Cqn5Mm7uj5+v l8ot0fPh1NLN/1CD+tqHBKXVPChz8hDlu/GfCkO/s54BGWpN0od7hBt3dAxnQI1nGAUY FKaQ== X-Gm-Message-State: AOAM530FhDK5On1m8WAOwz3Jusng9fUlnsySHO0HqQt9YfKDidQFMmtc rjq+8UhhIF4V12Q0x4nTIL1NDi2y0A1+R55895HQi/J4cc8pqHUpwE0itJCyHLRb9gJd7PNCY04 GxsW37cSV0MDCOSwD X-Received: by 2002:a17:906:dfd1:: with SMTP id jt17mr21253047ejc.486.1623768908510; Tue, 15 Jun 2021 07:55:08 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxUc77COnG0WIdO1tAjDrhLpnKaKhnq5sFxyuzHGltCcw1hnUBlYA0poEo+mX++Ywz8VM61xg== X-Received: by 2002:a17:906:dfd1:: with SMTP id jt17mr21253020ejc.486.1623768908212; Tue, 15 Jun 2021 07:55:08 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([2a0c:4d80:42:443::2]) by smtp.gmail.com with ESMTPSA id cf26sm10281358ejb.38.2021.06.15.07.55.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 15 Jun 2021 07:55:06 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id E6CA1180732; Tue, 15 Jun 2021 16:54:58 +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-?= =?utf-8?q?J=C3=B8rgensen?= , Thomas Petazzoni , Marcin Wojtas , Russell King Subject: [PATCH bpf-next v2 09/16] marvell: remove rcu_read_lock() around XDP program invocation Date: Tue, 15 Jun 2021 16:54:48 +0200 Message-Id: <20210615145455.564037-10-toke@redhat.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210615145455.564037-1-toke@redhat.com> References: <20210615145455.564037-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 Tue Jun 15 14:54:52 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: 460891 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 4315AC49361 for ; Tue, 15 Jun 2021 15:04:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2818961607 for ; Tue, 15 Jun 2021 15:04:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231421AbhFOPGd (ORCPT ); Tue, 15 Jun 2021 11:06:33 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:59557 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231311AbhFOPGc (ORCPT ); Tue, 15 Jun 2021 11:06:32 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623769467; 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=nZyBRJPoB1j5/g6EWUWSPPNl7ozTkFFFt8p6irUhI/M=; b=DuKb4ZAv55LI5PWv3k3xbZMN3oEaVoTx6K2D7R56EcdkIIAyW8uWtZp6TmEBENBkCy4mcf 5xryy5tloNrs7ct6lwj+uIOsT8EX3DuiIq/kYVV8YmXJsssKWFiu8qwQZGC4/wVvY9icfw bYI6nsjTT7MjTj2oyuEHOx4EX5VbWXo= 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-356-oBOgQghXNAGwGqR8PYz-oQ-1; Tue, 15 Jun 2021 11:04:25 -0400 X-MC-Unique: oBOgQghXNAGwGqR8PYz-oQ-1 Received: by mail-ed1-f70.google.com with SMTP id q7-20020aa7cc070000b029038f59dab1c5so22543642edt.23 for ; Tue, 15 Jun 2021 08:04: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=nZyBRJPoB1j5/g6EWUWSPPNl7ozTkFFFt8p6irUhI/M=; b=b0Tl58LAQCaiPln2kI/p4TPxZRcnZ6+F/xqQtd4UMvm9qXqqnETQxUqsqip8Uhiq1s fAK5qfBx4hI6xSs6iaofwIYCHMOheJ+O0K87IYC4BgiI4UjXrgBIGxgU+UUJZ88xg9JL nAPL7eeu5Lu6V6R3j7P/+H71SJoaVPqHp9fVx/jpCYb4Cn7Kk9I0SYMkS4WUvRaK7bii oA+pqvJ60BudFM9/15xm8C/MFKdDvv+q/yBO9LKbMDryP5JWqF5FCV0GtBhk+UO5iLuB cbSdhwoRG6PMrY6VcxyrCCqju3pu3UdLmyQr5mTUiOG14w/W0O/0W7VFGKU/uqrwSPFi RyDg== X-Gm-Message-State: AOAM5330rnhCGyiFVTBMALDC/mkaH3vPbA1E6lCwfDOZaEWk9sCXCYbB 4Wol5Wb1mOt5xTBEkhm56sXsCNb3YCEc66RGLWgLjC39OwYqxb7NqNHsEx+HkgyMp3iYKCSG4d8 MhhIhehfwxKt9MZA5 X-Received: by 2002:a50:fb8f:: with SMTP id e15mr23315229edq.46.1623769464512; Tue, 15 Jun 2021 08:04:24 -0700 (PDT) X-Google-Smtp-Source: ABdhPJytWmHoc5NSv2Ml6IB/zXKszT4ABcEjPhTM0+V3qJGdbm2cn7dpYji1+8/ONPwzQWZqoX25Dg== X-Received: by 2002:a50:fb8f:: with SMTP id e15mr23315193edq.46.1623769464293; Tue, 15 Jun 2021 08:04:24 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([45.145.92.2]) by smtp.gmail.com with ESMTPSA id u21sm7514919eja.59.2021.06.15.08.04.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 15 Jun 2021 08:04:23 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 1744F180736; Tue, 15 Jun 2021 16:54:59 +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-?= =?utf-8?q?J=C3=B8rgensen?= , Edward Cree , Martin Habets Subject: [PATCH bpf-next v2 13/16] sfc: remove rcu_read_lock() around XDP program invocation Date: Tue, 15 Jun 2021 16:54:52 +0200 Message-Id: <20210615145455.564037-14-toke@redhat.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210615145455.564037-1-toke@redhat.com> References: <20210615145455.564037-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 Tue Jun 15 14:54:53 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: 460890 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 2C146C48BDF for ; Tue, 15 Jun 2021 15:04:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 176DF61607 for ; Tue, 15 Jun 2021 15:04:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231496AbhFOPGf (ORCPT ); Tue, 15 Jun 2021 11:06:35 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:37950 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231401AbhFOPGd (ORCPT ); Tue, 15 Jun 2021 11:06:33 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623769468; 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=YfPOjpqs+xPPviaak2ogNv+B986oj5TZMO/NC3oJo44=; b=fduab+WxX+D8d0k6z/Mxc4/GxuftIzkKOareUkN4iWcpD1lJPbQWiK2Sn27XzZj39gbrkz 41EcJTnTYKmP0KQj/6HfOhLYQeg6rtZIqgJBIGHwfeyDCTWU38tRXWFsJJcA6czJWl9bL2 j55TZwPaPuUCEomKBSdFCALHj9nJm3M= 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-561-pdlO72V7OpeoG5U3U8XeIQ-1; Tue, 15 Jun 2021 11:04:26 -0400 X-MC-Unique: pdlO72V7OpeoG5U3U8XeIQ-1 Received: by mail-ed1-f70.google.com with SMTP id df3-20020a05640230a3b029039179c0f290so20716694edb.13 for ; Tue, 15 Jun 2021 08:04: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=YfPOjpqs+xPPviaak2ogNv+B986oj5TZMO/NC3oJo44=; b=PraAz4ZOyJg3Z6131PGw3hH3axVo9dyANbPgKk6zF9eV3EDbjQ1Y27xEuAYFe5n2+9 wrF9JaIOOHlmPGGkFXYFkFf0iCjHiO0lUusGllI9jPgJ9dccu08CHcZVJxfVCyDhBlHB sZWYGbGM643ndmc7uIefEDCNCwOIvDcqdvmp4H+QQTJdMncW48WShQlWZNbWgemfR/uM tqBE/HIfOuKoKcpdp1ypF8SmNUMaAtl+yXwUYVhhVsGOxlvSWKSKgOCmCs8ZGObLaBog 6rAffM+nMqokFXo2eCMFwZaua9JKh3MOrH5VrLSxBA9/KnCEs42DnK/RXGqaVXM2JCoH jS3Q== X-Gm-Message-State: AOAM531IHYxbpRPkx15RT7GU7G/3bq1F5p/ZbQ3A7HV/qch7I1qvndyw lnICBQumBbIndGmTMQkWv0n00mnI5MuwLmfKCl8FYO180UfxNZMGNNI0yz3ZoI/SJvFe1DaxXNS f/tkpd2Lw0bA8cDsE X-Received: by 2002:a50:fc9a:: with SMTP id f26mr23484747edq.216.1623769465616; Tue, 15 Jun 2021 08:04:25 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyy8TVWB3Ab3G+Ed06XwW3QccJKn8HJhXMLG8fARccSuyWGMMDysWF3YtrcYvvSs+y6ZDLVcw== X-Received: by 2002:a50:fc9a:: with SMTP id f26mr23484707edq.216.1623769465281; Tue, 15 Jun 2021 08:04:25 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([2a0c:4d80:42:443::2]) by smtp.gmail.com with ESMTPSA id k21sm12184299edo.41.2021.06.15.08.04.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 15 Jun 2021 08:04:23 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 21558180737; Tue, 15 Jun 2021 16:54:59 +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-?= =?utf-8?q?J=C3=B8rgensen?= , Jassi Brar , Ilias Apalodimas Subject: [PATCH bpf-next v2 14/16] netsec: remove rcu_read_lock() around XDP program invocation Date: Tue, 15 Jun 2021 16:54:53 +0200 Message-Id: <20210615145455.564037-15-toke@redhat.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210615145455.564037-1-toke@redhat.com> References: <20210615145455.564037-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 Tue Jun 15 14:54:54 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: 460893 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 03453C48BE5 for ; Tue, 15 Jun 2021 14:55:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E4F936141D for ; Tue, 15 Jun 2021 14:55:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231743AbhFOO5g (ORCPT ); Tue, 15 Jun 2021 10:57:36 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:29013 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231504AbhFOO5R (ORCPT ); Tue, 15 Jun 2021 10:57:17 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623768912; 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=ByKk7NdGTOHi1KbabnH0lRUOaQxO13c2A34mff7i6os=; b=OjIoqgUQVtfpdvF0fbiChcm25mhoMwTtE8rPKyWuUdk0fHctE1Uq2n3g+ApSc9SPnJcTSr NuhSVLM5mLexabc4ZPpjckPpfnwMjT2XZ/Zjx5IxhEzlFY1SUItPqX8lnD8Kl9g+nezA3O TzNmZimB557A/9DjVU9LDhvHyVkPSc8= 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-568-xKUcp1h0Nhuv68O-n3K4EQ-1; Tue, 15 Jun 2021 10:55:10 -0400 X-MC-Unique: xKUcp1h0Nhuv68O-n3K4EQ-1 Received: by mail-ed1-f69.google.com with SMTP id v12-20020aa7dbcc0000b029038fc8e57037so4214701edt.0 for ; Tue, 15 Jun 2021 07:55:09 -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=ByKk7NdGTOHi1KbabnH0lRUOaQxO13c2A34mff7i6os=; b=buXGQWZfsrPXBEN2Xwt21i48SlM9NHgWc2TRhsxxXph8iPsQOobeRJVxWCBb/N6WtF dLgeHFTQUYteKoNEY1aBq2nJEoQKcj4e93dg+L5reQp1s59gMv3tcD6oezuQnbTBSSed k2CxWsk7AxtwkyL+zQYW2Sz8HDiiUoInygT3YZ+HTlEpPJmgfUYuJp3spGdgAX3l5Ljn lWr3/cuYltxRiRMV90a/TXQA2d5ZI+4/CAPXY2VVBUAeRRRegtM9mYSi0yBWumNkWm5w umxfUChUpEs2k9A2eBPDMRC0tM6MZLbIwObDAcL0xI0IkSC5Y8ig6QHfJGsqCepWFAsy a4Tw== X-Gm-Message-State: AOAM532fWnc8zRcNeL5II/v0eQTAoJmoNu84Qf/TOGVUcwJ8Q0832uWq Xm+f3TfxRq5L+ndpDQf1eMYysc28lIuepRcPtj9oFEQa6BRxkB5NCGc1xg3k+nvYOzgJOrc3YsT 3f3VEc7LdKAdWfI++ X-Received: by 2002:a17:906:1790:: with SMTP id t16mr21140734eje.203.1623768908968; Tue, 15 Jun 2021 07:55:08 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzO7NzkHVdn3I9zDxJvs+KHKQGP1gNNIkOvx0CUNjvYdIP8qMYlgja+FT1bnm1L0s5u+E2uKg== X-Received: by 2002:a17:906:1790:: with SMTP id t16mr21140717eje.203.1623768908806; Tue, 15 Jun 2021 07:55:08 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([45.145.92.2]) by smtp.gmail.com with ESMTPSA id o20sm11770169eds.20.2021.06.15.07.55.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 15 Jun 2021 07:55:06 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 28118180738; Tue, 15 Jun 2021 16:54:59 +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-?= =?utf-8?q?J=C3=B8rgensen?= , Giuseppe Cavallaro , Alexandre Torgue , Jose Abreu Subject: [PATCH bpf-next v2 15/16] stmmac: remove rcu_read_lock() around XDP program invocation Date: Tue, 15 Jun 2021 16:54:54 +0200 Message-Id: <20210615145455.564037-16-toke@redhat.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210615145455.564037-1-toke@redhat.com> References: <20210615145455.564037-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 --- 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 Tue Jun 15 14:54:55 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: 460895 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 9681CC49EB7 for ; Tue, 15 Jun 2021 14:55:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7F21761613 for ; Tue, 15 Jun 2021 14:55:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231496AbhFOO5W (ORCPT ); Tue, 15 Jun 2021 10:57:22 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:42389 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231442AbhFOO5P (ORCPT ); Tue, 15 Jun 2021 10:57:15 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623768910; 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=7zXHSL7RwyAJWnz3yfpZ7/QXhLd143qN0GShu6MwfqY=; b=GFGug33ocwRA9NG0Gg6g6zaTYvemTjQmMlyWA2Mk4JLjhLLaq0FmnRq18KZ90sErhVrj7w 4PJs96x9T7bvstC5aaIv0+TEuS3rmPvmxG38WOTlAgUXSgF3VdxTKjBE+8AnVrTIASGS+k P2g0IVMZUY0zKUVfYQjrQkU7WlkvA4o= 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-280-pKL3zz_4OWqbDL_-Q40Owg-1; Tue, 15 Jun 2021 10:55:09 -0400 X-MC-Unique: pKL3zz_4OWqbDL_-Q40Owg-1 Received: by mail-ed1-f71.google.com with SMTP id h23-20020aa7c5d70000b029038fed7b27d5so22254199eds.21 for ; Tue, 15 Jun 2021 07:55:09 -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=7zXHSL7RwyAJWnz3yfpZ7/QXhLd143qN0GShu6MwfqY=; b=dfl/usrlsB4L6rip7bvp1UhKdvKHb4ZEoKRUn09VNLgGDaA7CNZGUEnrGunl3wZ1IQ TzUKVZ/tj+PG/Ra2z+50b88E8PSlMGQPc7V32UzxUUbslsMFXDC86/U4/fM+n6aOvcBj VNoA459MOwYx3V9bOwo3k6AdRtLKgCw9GbMqdEsz/ifrutNoe/bvSCKkbJ/76aYK3TyD Xwh2oxpRDccpKkiKRfN+0BqgRO6OpGXQpMakIurQJ/kmjZuFlCyn4tvzu8hTDhzXr+3F EUfrFYQm1wXmm72s3ybLkxxALWZhpnVUJV/casCJRok4tSQPkLTCXHLqp2j65q6MPswi ftMQ== X-Gm-Message-State: AOAM5321XE5iVJoUbA7waIG36Q+i5p5rd7Qi7kgcxW33Q+2DmJIjWBw7 Sxm5DxGYIOJAPKydKDeeJng5IX+uI4YlrKi+HAAOjNPoUXkIkI+tPBQRdD8OjcFUK0vFz9PHTSg monaMNmHViK2w9Soh X-Received: by 2002:a05:6402:1652:: with SMTP id s18mr23048013edx.131.1623768908124; Tue, 15 Jun 2021 07:55:08 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzAy/n5Es33Uj8+GjOWq3YRmuEbjhuD5rzEEzzfpqUH7cASDVtLd3bVhCfFDT4UAd0JjxkZnQ== X-Received: by 2002:a05:6402:1652:: with SMTP id s18mr23047988edx.131.1623768907804; Tue, 15 Jun 2021 07:55:07 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([2a0c:4d80:42:443::2]) by smtp.gmail.com with ESMTPSA id f8sm9985632ejw.75.2021.06.15.07.55.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 15 Jun 2021 07:55:05 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 354DC180739; Tue, 15 Jun 2021 16:54:59 +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-?= =?utf-8?q?J=C3=B8rgensen?= , Grygorii Strashko , linux-omap@vger.kernel.org Subject: [PATCH bpf-next v2 16/16] net: ti: remove rcu_read_lock() around XDP program invocation Date: Tue, 15 Jun 2021 16:54:55 +0200 Message-Id: <20210615145455.564037-17-toke@redhat.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210615145455.564037-1-toke@redhat.com> References: <20210615145455.564037-1-toke@redhat.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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; }