On Wed, Apr 24, 2019 at 11:13:42AM -0600, David Ahern wrote:
seeing an RCU warning testing xdp with virtio net. net-next as of commit
b2f97f7de2f6a4df8e431330cf467576486651c5. No obvious changes so hoping
this rings a bell with someone else.
[ 121.990304] =============================
[ 121.991488] WARNING: suspicious RCU usage
[ 121.992392] 5.1.0-rc5+ #60 Not tainted
[ 121.993220] -----------------------------
[ 121.994158] /home/dsa/kernel-3.git/drivers/net/virtio_net.c:516
suspicious rcu_dereference_check() usage!
[ 121.996284]
other info that might help us debug this:
[ 121.997988]
rcu_scheduler_active = 2, debug_locks = 1
[ 121.999321] no locks held by swapper/1/0.
[ 122.000328]
stack backtrace:
[ 122.001253] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.1.0-rc5+ #60
[ 122.002474] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS 1.11.1-1 04/01/2014
[ 122.004141] Call Trace:
[ 122.004651] <IRQ>
[ 122.005082] dump_stack+0x7e/0xbb
[ 122.005757] lockdep_rcu_suspicious+0x102/0x10b
[ 122.006654] virtnet_xdp_xmit+0x104/0x4fe
[ 122.007447] ? kasan_check_read+0x11/0x13
[ 122.008267] ? mergeable_rx_buffer_size_show+0x163/0x163
[ 122.009299] ? __asan_loadN+0xf/0x11
[ 122.010010] ? pvclock_clocksource_read+0xfa/0x189
[ 122.010975] bq_xmit_all+0xdc/0x358
[ 122.011699] __dev_map_flush+0xc2/0xef
[ 122.012472] xdp_do_flush_map+0x5b/0x74
[ 122.013238] virtnet_poll+0x58f/0x679
Well virtnet_xdp_xmit seems to be called from .ndo_xdp_xmit
and that isn't in an RCU read-side critical section.
Looks like we just need to add RCU read lock/unlock.
Like the below perhaps?
This issue was introduced by 8dcc5b0ab0 however I find it
inelegant that we need to do checks in each driver,
and add RCU locks just for a startup initialization issue.
Can't XDP core make sure the callback isn't invoked
at an inappropriate time instead?
--->
virtio_net: call virtnet_xdp_xmit with RCU lock
This functions uses rcu_dereference so it needs to be
called in an RCU read-side critical section.
Fixes: 8dcc5b0ab0 ("virtio_net: fix ndo_xdp_xmit crash towards dev not ready for
XDP")
Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
--
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 9010938e2d71..ccc1bdd1bb1f 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -495,8 +495,8 @@ static struct send_queue *virtnet_xdp_sq(struct
virtnet_info *vi)
return &vi->sq[qp];
}
-static int virtnet_xdp_xmit(struct net_device *dev,
- int n, struct xdp_frame **frames, u32 flags)
+static int __virtnet_xdp_xmit(struct net_device *dev,
+ int n, struct xdp_frame **frames, u32 flags)
{
struct virtnet_info *vi = netdev_priv(dev);
struct receive_queue *rq = vi->rq;
@@ -569,6 +569,17 @@ static int virtnet_xdp_xmit(struct net_device *dev,
return ret;
}
+static int virtnet_xdp_xmit(struct net_device *dev,
+ int n, struct xdp_frame **frames, u32 flags)
+{
+ int r;
+
+ rcu_read_lock_bh();
+ r = __virtnet_xdp_xmit(dev, n, frames, flags);
+ rcu_read_unlock_bh();
+ return r;
+}
+
static unsigned int virtnet_get_headroom(struct virtnet_info *vi)
{
return vi->xdp_queue_pairs ? VIRTIO_XDP_HEADROOM : 0;
@@ -714,7 +725,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
xdpf = convert_to_xdp_frame(&xdp);
if (unlikely(!xdpf))
goto err_xdp;
- err = virtnet_xdp_xmit(dev, 1, &xdpf, 0);
+ err = __virtnet_xdp_xmit(dev, 1, &xdpf, 0);
if (unlikely(err < 0)) {
trace_xdp_exception(vi->dev, xdp_prog, act);
goto err_xdp;
@@ -887,7 +898,7 @@ static struct sk_buff *receive_mergeable(struct net_device
*dev,
xdpf = convert_to_xdp_frame(&xdp);
if (unlikely(!xdpf))
goto err_xdp;
- err = virtnet_xdp_xmit(dev, 1, &xdpf, 0);
+ err = __virtnet_xdp_xmit(dev, 1, &xdpf, 0);
if (unlikely(err < 0)) {
trace_xdp_exception(vi->dev, xdp_prog, act);
if (unlikely(xdp_page != page))
