From patchwork Sun Jan 26 03:58:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 1229323 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Original-To: incoming-bpf@patchwork.ozlabs.org Delivered-To: patchwork-incoming-bpf@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=bpf-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20161025 header.b=TUfvWKI5; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 484zf461PRz9sP3 for ; Sun, 26 Jan 2020 14:59:24 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729134AbgAZD7X (ORCPT ); Sat, 25 Jan 2020 22:59:23 -0500 Received: from mail-pl1-f194.google.com ([209.85.214.194]:33164 "EHLO mail-pl1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729133AbgAZD7X (ORCPT ); Sat, 25 Jan 2020 22:59:23 -0500 Received: by mail-pl1-f194.google.com with SMTP id ay11so2477099plb.0; Sat, 25 Jan 2020 19:59:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=RuJpjVfRtmd6PBmsFbM+f7LVOKZDRhAzmDczWQcb+9w=; b=TUfvWKI5NBhNGh9WPdEDenZBKQQQdQCy2ky3j1mifZzMBngl+BMOqKmWpPpI9Mybnj J7fcHe/QgTwFgHN2hwmozTfPWgP44POVkrE/hkCDfRohfxBngrXfNlR9XnVLCYUaIC9N ReAXtknX/+RPT4Ehej6YiEc9q8g6ga1817HAQHL39oljPr9S4u/32171dhSOiXWPnIrC xqXVU4h6h2v141CuuaOw3QJDf7UXgwU1iI10sTPH7Edmh1vao0buQZPJaGZ9vjy6JbIF omUDm+94w1uG97Q4B3zi9OY8hQvllI/GpxXnGublk0v4ZU4sZgIdL+nBbdOayVbgK7tR l/Fg== 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=RuJpjVfRtmd6PBmsFbM+f7LVOKZDRhAzmDczWQcb+9w=; b=BtMFjXEYsXSJ4El0Ts2FEjXDCOpGMCn8IHjVVfsPwP7jONKKAvXMWcAVdhhG2avxcs AcR9jxroxOdMrf1sdeuFIP0LVq6qXNyyaRTULtWx7/KukfYfTxsHWlsiXU8Pqw4nVwQb gsA7YJebg4c+4BWf+7CgLTodnd3pbmMPvJm0cwhjlPh0nteva2Icmy9PDrMyZB4aNaK1 OQY5XqTn40YXnZ4WNe4xwzokkoKntc7P47V8RnJbKOl6UXq58ZaOSJZfkJ5aNHNNhwrS +yqwdGcPHh86COLEr9+38pgjW/+g+jl1DGYAH6hS5MEHNJol6kbCNq9NY/fjMpsaBtGO S7eg== X-Gm-Message-State: APjAAAVKmMZAbwPTHuAclancMwpv28yTRicIt9XJHV6cur9/TLs3LF0f +LSPEipOT1EJO84dMzrWZdZZeleh X-Google-Smtp-Source: APXvYqxuev402Y4Q6hYmF62LKOFfZzlnuXv8B87c3b4eL1Hz87XSk3E2yyYwE3ia+ZttSNfe5if8YA== X-Received: by 2002:a17:902:9a84:: with SMTP id w4mr11787996plp.324.1580011162479; Sat, 25 Jan 2020 19:59:22 -0800 (PST) Received: from localhost.localdomain ([184.63.162.180]) by smtp.gmail.com with ESMTPSA id 64sm11078650pfd.48.2020.01.25.19.59.12 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Sat, 25 Jan 2020 19:59:22 -0800 (PST) From: John Fastabend To: bpf@vger.kernel.org Cc: bjorn.topel@intel.com, songliubraving@fb.com, john.fastabend@gmail.com, ast@kernel.org, daniel@iogearbox.net, toke@redhat.com, maciej.fijalkowski@intel.com, netdev@vger.kernel.org Subject: [PATCH bpf-next v2 1/3] bpf: xdp, update devmap comments to reflect napi/rcu usage Date: Sat, 25 Jan 2020 19:58:51 -0800 Message-Id: <1580011133-17784-2-git-send-email-john.fastabend@gmail.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1580011133-17784-1-git-send-email-john.fastabend@gmail.com> References: <1580011133-17784-1-git-send-email-john.fastabend@gmail.com> MIME-Version: 1.0 Sender: bpf-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org Now that we rely on synchronize_rcu and call_rcu waiting to exit perempt-disable regions (NAPI) lets update the comments to reflect this. Fixes: 0536b85239b84 ("xdp: Simplify devmap cleanup") Acked-by: Björn Töpel Acked-by: Song Liu Signed-off-by: John Fastabend --- kernel/bpf/devmap.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c index da9c832..f0bf525 100644 --- a/kernel/bpf/devmap.c +++ b/kernel/bpf/devmap.c @@ -193,10 +193,12 @@ static void dev_map_free(struct bpf_map *map) /* At this point bpf_prog->aux->refcnt == 0 and this map->refcnt == 0, * so the programs (can be more than one that used this map) were - * disconnected from events. Wait for outstanding critical sections in - * these programs to complete. The rcu critical section only guarantees - * no further reads against netdev_map. It does __not__ ensure pending - * flush operations (if any) are complete. + * disconnected from events. The following synchronize_rcu() guarantees + * both rcu read critical sections complete and waits for + * preempt-disable regions (NAPI being the relavent context here) so we + * are certain there will be no further reads against the netdev_map and + * all flush operations are complete. Flush operations can only be done + * from NAPI context for this reason. */ spin_lock(&dev_map_lock); @@ -498,12 +500,11 @@ static int dev_map_delete_elem(struct bpf_map *map, void *key) return -EINVAL; /* Use call_rcu() here to ensure any rcu critical sections have - * completed, but this does not guarantee a flush has happened - * yet. Because driver side rcu_read_lock/unlock only protects the - * running XDP program. However, for pending flush operations the - * dev and ctx are stored in another per cpu map. 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. + * 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); if (old_dev) From patchwork Sun Jan 26 03:58:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 1229326 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20161025 header.b=b5xE6ckT; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 484zfJ6vCwz9sP3 for ; Sun, 26 Jan 2020 14:59:36 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729163AbgAZD7e (ORCPT ); Sat, 25 Jan 2020 22:59:34 -0500 Received: from mail-pj1-f67.google.com ([209.85.216.67]:36582 "EHLO mail-pj1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728842AbgAZD7e (ORCPT ); Sat, 25 Jan 2020 22:59:34 -0500 Received: by mail-pj1-f67.google.com with SMTP id gv17so1670902pjb.1; Sat, 25 Jan 2020 19:59:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=6ApWfuJ0D0g3QrelwnDPlSG8OOKZaWuA97rVS5nYc4w=; b=b5xE6ckT9q7fSwYL8kV8uFXEWZvFzT+xtwqoUiQ8iosIvkKb9R0wtLynHYpiRMTTZw 7LvHDc8INM/jI3yS4RoqcEFizA1kwmxSxZU8a6wlzEQmilrFIB7lV/vqJ9yQHFGQwQuh CXvxdqic5v7eikaP2Te9y93lV4FMUDmqBAwcgC6fjScQlMi4+8s0k/h9ED2z01J/mBM8 hJud0dEi5ff3b9DLOSIWzrjiTl6uz8GRcMqV0WChmfzrmrPoz+QBTd6c2avuhuQ8qiR4 sYqTLsbgT6ss+9AQAO9KZi2QPPkxw2kJshrsjxNS7kCrVy7WuD4zhYsrhP3nJhNlVeEP SlGQ== 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; bh=6ApWfuJ0D0g3QrelwnDPlSG8OOKZaWuA97rVS5nYc4w=; b=edrzuHGpl269YEeY3Qdr7h3PuQiX3sQd2OwCn61rGS2VB1CwaV1q+DUWKJhMLjwftf xPLbmkvOfbwvMx9XFPacF8lKCeATNj2GfagozSMEKtiSDnoKUmFPkpjfCjZnB1MnQsVv IiZBFfXVW+MBjCjzan2EtWWQ9ycCORPBZX0I9WLSNtjAvvWKMgKeJWwG+aSki/b3MmlP +Ed1eR/O3nPjgLdGD3DL8dCCaK6dpED1I+qDhCEWjuTu7MYZpgn+oTEoeOF8JIkTJDV8 2vcZgE5iapJzEw8F7sCV6Uv1nIcaIpX1imGzNudHmPc1wTzZQx18RONe4WyDhS8A8JI8 9EWg== X-Gm-Message-State: APjAAAX9LO+SgrzNzHgdHL0UgH0OKb3P6JWC3qOS5qxO2d9KVmGE5zfn PIonejWeMfhDNaKtM8LoMe8VEDJh X-Google-Smtp-Source: APXvYqwQvqQAxXTiDWY4asxbKBSqelPMmlT5UsNNQijJD9uQ1DtcN6V9TMS9RBEjgaRoB0Z57s1Yaw== X-Received: by 2002:a17:902:9a42:: with SMTP id x2mr12085789plv.194.1580011173480; Sat, 25 Jan 2020 19:59:33 -0800 (PST) Received: from localhost.localdomain ([184.63.162.180]) by smtp.gmail.com with ESMTPSA id 64sm11078650pfd.48.2020.01.25.19.59.23 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Sat, 25 Jan 2020 19:59:33 -0800 (PST) From: John Fastabend To: bpf@vger.kernel.org Cc: bjorn.topel@intel.com, songliubraving@fb.com, john.fastabend@gmail.com, ast@kernel.org, daniel@iogearbox.net, toke@redhat.com, maciej.fijalkowski@intel.com, netdev@vger.kernel.org Subject: [PATCH bpf-next v2 2/3] bpf: xdp, virtio_net use access ptr macro for xdp enable check Date: Sat, 25 Jan 2020 19:58:52 -0800 Message-Id: <1580011133-17784-3-git-send-email-john.fastabend@gmail.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1580011133-17784-1-git-send-email-john.fastabend@gmail.com> References: <1580011133-17784-1-git-send-email-john.fastabend@gmail.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org virtio_net currently relies on rcu critical section to access the xdp program in its xdp_xmit handler. However, the pointer to the xdp program is only used to do a NULL pointer comparison to determine if xdp is enabled or not. Use rcu_access_pointer() instead of rcu_dereference() to reflect this. Then later when we drop rcu_read critical section virtio_net will not need in special handling. Signed-off-by: John Fastabend Acked-by: Jesper Dangaard Brouer --- drivers/net/virtio_net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 4d7d5434..945eabc 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -501,7 +501,7 @@ static int virtnet_xdp_xmit(struct net_device *dev, /* Only allow ndo_xdp_xmit if XDP is loaded on dev, as this * indicate XDP resources have been successfully allocated. */ - xdp_prog = rcu_dereference(rq->xdp_prog); + xdp_prog = rcu_access_pointer(rq->xdp_prog); if (!xdp_prog) return -ENXIO; From patchwork Sun Jan 26 03:58:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 1229328 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Original-To: incoming-bpf@patchwork.ozlabs.org Delivered-To: patchwork-incoming-bpf@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=bpf-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20161025 header.b=kAH2Mk54; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 484zfV1hcDz9sRY for ; Sun, 26 Jan 2020 14:59:46 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729164AbgAZD7p (ORCPT ); Sat, 25 Jan 2020 22:59:45 -0500 Received: from mail-pl1-f193.google.com ([209.85.214.193]:39861 "EHLO mail-pl1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728842AbgAZD7o (ORCPT ); Sat, 25 Jan 2020 22:59:44 -0500 Received: by mail-pl1-f193.google.com with SMTP id g6so2464972plp.6; Sat, 25 Jan 2020 19:59:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=fEp2tBURROlhYKIVBLvlPnIxXcmQzUZcU2zhMK+Ny8k=; b=kAH2Mk54tpKfeIvs9dLDJlgOWFPzBScpMH9NWN+aDHXxUDPDDeuErMlLavKpwd5zKq mkeTSMB2uuUfMcHGjgaLsfNPW6P7uABX4pb3vettusUzlLzEvEM1HrhgR1PLzgOOOSyp jITNLWv/GDcfjekI2X1w6rNXk6hcxByGIMzrSCQE8Ysyu9ZBh2AgcMMQDVXGgzJZJ+W0 PXkK8+gSLRYDg9KpzGsXFLFqbqHKb2P6f4iHyzYpjb0WAQgRQHFH4xeklUIS0LLUBbAR 9O3u9LkTSQvV/rC3J4XoPKbcejDsNhI8ksax6l6CcZ4PY3cWyHJx3lSQ+c9V0arCg25/ L7qg== 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; bh=fEp2tBURROlhYKIVBLvlPnIxXcmQzUZcU2zhMK+Ny8k=; b=KtMEOoOt+66rGh0DW5xBOQEcPbkWlcRYhCmbEsM74ONi9YuUaRqe/VHMRye1iWbrT7 FOJ7Ardpoz4wkxs5ySLXxmFP51tjBxvR8LlHnl2fGo46f7l4DoxYSvGNQ/5goHRG+XDt h1MAh6jaRQJYYmtMrHRoqEJWXW/rP+0VKjLe6iaAQgdv8oGHvSLJB8+YXhfz+1CYE8aW uE93l1CYS1EQHruLnJivqMkEAJ+N/Vbyb74h90VC2cRGyBmGO7Hyt21eTRGl9QZMkWxr dmU2pc/y3YAw5x0LSSmyYqUnW5eaap2aLT6ETVil7SXgQCbyzmVrwOG/2ypxVlGDQEaJ OOLA== X-Gm-Message-State: APjAAAV61hbdHfAetq2zBDLPH3rayZXBWQxSmkpeoqOJuEuYMq8uJz8T BmICwoAYZAAsWtfiZxEy89gxKaf1 X-Google-Smtp-Source: APXvYqyHTXdBfKiLJkEnjWvxfqF4QBLDJW+YZ2cQ3ABDFcXiok5eZDGeypK/2ozN1BDcfkRZB+uB7g== X-Received: by 2002:a17:90b:3011:: with SMTP id hg17mr7607242pjb.90.1580011183786; Sat, 25 Jan 2020 19:59:43 -0800 (PST) Received: from localhost.localdomain ([184.63.162.180]) by smtp.gmail.com with ESMTPSA id 64sm11078650pfd.48.2020.01.25.19.59.34 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Sat, 25 Jan 2020 19:59:43 -0800 (PST) From: John Fastabend To: bpf@vger.kernel.org Cc: bjorn.topel@intel.com, songliubraving@fb.com, john.fastabend@gmail.com, ast@kernel.org, daniel@iogearbox.net, toke@redhat.com, maciej.fijalkowski@intel.com, netdev@vger.kernel.org Subject: [PATCH bpf-next v2 3/3] bpf: xdp, remove no longer required rcu_read_{un}lock() Date: Sat, 25 Jan 2020 19:58:53 -0800 Message-Id: <1580011133-17784-4-git-send-email-john.fastabend@gmail.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1580011133-17784-1-git-send-email-john.fastabend@gmail.com> References: <1580011133-17784-1-git-send-email-john.fastabend@gmail.com> Sender: bpf-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org Now that we depend on rcu_call() and synchronize_rcu() to also wait for preempt_disabled region to complete the rcu read critical section in __dev_map_flush() is no longer required. Except in a few special cases in drivers that need it for other reasons. These originally ensured the map reference was safe while a map was also being free'd. And additionally that bpf program updates via ndo_bpf did not happen while flush updates were in flight. But flush by new rules can only be called from preempt-disabled NAPI context. The synchronize_rcu from the map free path and the rcu_call from the delete path will ensure the reference there is safe. So lets remove the rcu_read_lock and rcu_read_unlock pair to avoid any confusion around how this is being protected. If the rcu_read_lock was required it would mean errors in the above logic and the original patch would also be wrong. Now that we have done above we put the rcu_read_lock in the driver code where it is needed in a driver dependent way. I think this helps readability of the code so we know where and why we are taking read locks. Most drivers will not need rcu_read_locks here and further XDP drivers already have rcu_read_locks in their code paths for reading xdp programs on RX side so this makes it symmetric where we don't have half of rcu critical sections define in driver and the other half in devmap. Signed-off-by: John Fastabend Acked-by: Jesper Dangaard Brouer --- drivers/net/veth.c | 6 +++++- kernel/bpf/devmap.c | 5 +++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/net/veth.c b/drivers/net/veth.c index a552df3..184e1b4 100644 --- a/drivers/net/veth.c +++ b/drivers/net/veth.c @@ -377,6 +377,7 @@ static int veth_xdp_xmit(struct net_device *dev, int n, unsigned int max_len; struct veth_rq *rq; + rcu_read_lock(); if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK)) { ret = -EINVAL; goto drop; @@ -418,11 +419,14 @@ static int veth_xdp_xmit(struct net_device *dev, int n, if (flags & XDP_XMIT_FLUSH) __veth_xdp_flush(rq); - if (likely(!drops)) + if (likely(!drops)) { + rcu_read_unlock(); return n; + } ret = n - drops; drop: + rcu_read_unlock(); atomic64_add(drops, &priv->dropped); return ret; diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c index f0bf525..d0ce2e2 100644 --- a/kernel/bpf/devmap.c +++ b/kernel/bpf/devmap.c @@ -372,16 +372,17 @@ static int bq_xmit_all(struct xdp_bulk_queue *bq, u32 flags) * 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. */ void __dev_map_flush(void) { struct list_head *flush_list = this_cpu_ptr(&dev_map_flush_list); struct xdp_bulk_queue *bq, *tmp; - rcu_read_lock(); list_for_each_entry_safe(bq, tmp, flush_list, flush_node) bq_xmit_all(bq, XDP_XMIT_FLUSH); - rcu_read_unlock(); } /* rcu_read_lock (from syscall and BPF contexts) ensures that if a delete and/or