From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
To: linux-kernel@vger.kernel.org, rcu@vger.kernel.org
Cc: Joel Fernandes <joel@joelfernandes.org>, paulmck@linux.ibm.com
Subject: [RFC 2/5] ixgbe: Fix incorrect RCU API usage
Date: Tue, 19 Feb 2019 23:08:24 -0500 [thread overview]
Message-ID: <20190220040827.136184-3-joel@joelfernandes.org> (raw)
In-Reply-To: <20190220040827.136184-1-joel@joelfernandes.org>
From: Joel Fernandes <joel@joelfernandes.org>
Recently, I added an RCU annotation check in rcu_assign_pointer. This
caused a sparse error to be reported by the ixgbe driver.
Further looking, it seems the adapter->xdp_prog pointer is not annotated
with __rcu. Annonating it fixed the error, but caused a bunch of other
warnings.
This patch tries to fix all warnings by using RCU API properly. This
makes sense to do because not using RCU properly can result in various
hard to find bugs. This is a best effort fix and is only build tested.
The sparse errors and warnings go away with the change. I request
maintainers / developers in this area to test it properly.
Signed-off-by: Joel Fernandes <joel@joelfernandes.org>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 16 +++++++++++-----
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 08d85e336bd4..3b14daf27516 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -311,7 +311,7 @@ struct ixgbe_ring {
struct ixgbe_ring *next; /* pointer to next ring in q_vector */
struct ixgbe_q_vector *q_vector; /* backpointer to host q_vector */
struct net_device *netdev; /* netdev ring belongs to */
- struct bpf_prog *xdp_prog;
+ struct bpf_prog __rcu *xdp_prog;
struct device *dev; /* device for DMA mapping */
void *desc; /* descriptor ring memory */
union {
@@ -560,7 +560,7 @@ struct ixgbe_adapter {
unsigned long active_vlans[BITS_TO_LONGS(VLAN_N_VID)];
/* OS defined structs */
struct net_device *netdev;
- struct bpf_prog *xdp_prog;
+ struct bpf_prog __rcu *xdp_prog;
struct pci_dev *pdev;
struct mii_bus *mii_bus;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index daff8183534b..aad7b800aacd 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2199,7 +2199,7 @@ static struct sk_buff *ixgbe_run_xdp(struct ixgbe_adapter *adapter,
u32 act;
rcu_read_lock();
- xdp_prog = READ_ONCE(rx_ring->xdp_prog);
+ xdp_prog = rcu_dereference(rx_ring->xdp_prog);
if (!xdp_prog)
goto xdp_out;
@@ -6547,7 +6547,7 @@ int ixgbe_setup_rx_resources(struct ixgbe_adapter *adapter,
rx_ring->queue_index) < 0)
goto err;
- rx_ring->xdp_prog = adapter->xdp_prog;
+ rcu_assign_pointer(rx_ring->xdp_prog, adapter->xdp_prog);
return 0;
err:
@@ -10246,7 +10246,8 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
if (nr_cpu_ids > MAX_XDP_QUEUES)
return -ENOMEM;
- old_prog = xchg(&adapter->xdp_prog, prog);
+ old_prog = rcu_dereference(adapter->xdp_prog);
+ rcu_assign_pointer(adapter->xdp_prog, prog);
/* If transitioning XDP modes reconfigure rings */
if (!!prog != !!old_prog) {
@@ -10271,13 +10272,18 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
static int ixgbe_xdp(struct net_device *dev, struct netdev_bpf *xdp)
{
struct ixgbe_adapter *adapter = netdev_priv(dev);
+ struct bpf_prog *prog;
+ int ret;
switch (xdp->command) {
case XDP_SETUP_PROG:
return ixgbe_xdp_setup(dev, xdp->prog);
case XDP_QUERY_PROG:
- xdp->prog_id = adapter->xdp_prog ?
- adapter->xdp_prog->aux->id : 0;
+ rcu_read_lock();
+ prog = rcu_dereference(adapter->xdp_prog);
+ xdp->prog_id = prog ? prog->aux->id : 0;
+ rcu_read_unlock();
+
return 0;
case XDP_QUERY_XSK_UMEM:
return ixgbe_xsk_umem_query(adapter, &xdp->xsk.umem,
--
2.21.0.rc0.258.g878e2cd30e-goog
next prev parent reply other threads:[~2019-02-20 4:09 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-20 4:08 [RFC 0/5] RCU fixes for rcu_assign_pointer usage Joel Fernandes (Google)
2019-02-20 4:08 ` [RFC 1/5] net: rtnetlink: Fix incorrect RCU API usage Joel Fernandes (Google)
2019-02-20 16:40 ` Paul E. McKenney
2019-02-20 18:08 ` Joel Fernandes
2019-02-20 4:08 ` Joel Fernandes (Google) [this message]
2019-02-20 4:08 ` [RFC 3/5] sched/cpufreq: " Joel Fernandes (Google)
2019-02-20 4:08 ` [RFC 4/5] sched/toplogy: Use smp_store_release() instead of rcu_assign_pointer Joel Fernandes (Google)
2019-02-20 4:08 ` [RFC 5/5] rcuwait: Replace rcu_assign_pointer with smp_store_release Joel Fernandes (Google)
2019-02-20 4:11 ` [RFC 0/5] RCU fixes for rcu_assign_pointer usage Joel Fernandes
2019-02-20 16:42 ` Paul E. McKenney
2019-02-20 18:09 ` Joel Fernandes
2019-02-20 18:28 ` Paul E. McKenney
2019-02-21 16:50 ` Joel Fernandes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190220040827.136184-3-joel@joelfernandes.org \
--to=joel@joelfernandes.org \
--cc=linux-kernel@vger.kernel.org \
--cc=paulmck@linux.ibm.com \
--cc=rcu@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).