On 02/20/2018 05:32 AM, Jesper Dangaard Brouer wrote:
> When a driver implements the ndo_xdp_xmit() function, there is
> (currently) no generic way to determine whether it is safe to call.
>
> It is e.g. unsafe to call the drivers ndo_xdp_xmit, if it have not
> allocated the needed XDP TX queues yet. This is the case for
> virtio_net, which first allocates the XDP TX queues once an XDP/bpf
> prog is attached (in virtnet_xdp_set()).
>
> Thus, a crash will occur for virtio_net when redirecting to another
> virtio_net device's ndo_xdp_xmit, which have not attached a XDP prog.
> The sample xdp_redirect_map tries to attach a dummy XDP prog to take
> this into account, but it can also easily fail if the virtio_net (or
> actually underlying vhost driver) have not allocated enough extra
> queues for the device.
>
> Allocating more queue this is currently a manual config.
> Hint for libvirt XML add:
>
> <driver name='vhost' queues='16'>
> <host mrg_rxbuf='off'/>
> <guest tso4='off' tso6='off' ecn='off' ufo='off'/>
> </driver>
>
> The solution in this patch is to check that the device have loaded an
> XDP/bpf prog before proceeding. This is similar to the check
> performed in driver ixgbe.
>
> Signed-off-by: Jesper Dangaard Brouer <bro...@redhat.com>
> ---
Yep, nice catch. I caught this in ixgbe but didn't manage to
pull it into virtio.
Acked-by: John Fastabend <john.fastab...@gmail.com>
> drivers/net/virtio_net.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 1e0e0fce3ab2..9bb9e562b893 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -452,8 +452,18 @@ static bool __virtnet_xdp_xmit(struct virtnet_info *vi,
> static int virtnet_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp)
> {
> struct virtnet_info *vi = netdev_priv(dev);
> - bool sent = __virtnet_xdp_xmit(vi, xdp);
> + struct receive_queue *rq = vi->rq;
> + struct bpf_prog *xdp_prog;
> + bool sent;
> +
> + /* 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);
> + if (!xdp_prog)
> + return -ENXIO;
>
> + sent = __virtnet_xdp_xmit(vi, xdp);
> if (!sent)
> return -ENOSPC;
> return 0;
>
