| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20191127153253-mutt-send-email-mst@kernel.org>
Date: Wed, 27 Nov 2019 15:42:57 -0500
From: "Michael S. Tsirkin" <mst@...hat.com>
To: Prashant Bhole <prashantbhole.linux@...il.com>
Cc: "David S . Miller" <davem@...emloft.net>,
Jason Wang <jasowang@...hat.com>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Jakub Kicinski <jakub.kicinski@...ronome.com>,
Jesper Dangaard Brouer <hawk@...nel.org>,
John Fastabend <john.fastabend@...il.com>,
Martin KaFai Lau <kafai@...com>,
Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
Andrii Nakryiko <andriin@...com>, netdev@...r.kernel.org,
qemu-devel@...gnu.org, kvm@...r.kernel.org
Subject: Re: [RFC net-next 15/18] virtio_net: implement XDP prog offload
functionality
On Tue, Nov 26, 2019 at 07:07:41PM +0900, Prashant Bhole wrote:
> From: Jason Wang <jasowang@...hat.com>
>
> This patch implements bpf_prog_offload_ops callbacks and adds handling
> for XDP_SETUP_PROG_HW. Handling of XDP_SETUP_PROG_HW involves setting
> up struct virtio_net_ctrl_ebpf_prog and appending program instructions
> to it. This control buffer is sent to Qemu with class
> VIRTIO_NET_CTRL_EBPF and command VIRTIO_NET_BPF_CMD_SET_OFFLOAD.
> The expected behavior from Qemu is that it should try to load the
> program in host os and report the status.
That's not great e.g. for migration: different hosts might have
a different idea about what's allowed.
Device capabilities should be preferably exported through
feature bits or config space such that it's easy to
intercept and limit these as needed.
Also, how are we going to handle e.g. endian-ness here?
>
> It also adds restriction to have either driver or offloaded program
> at a time.
I'm not sure I understand what does the above say.
virtnet_xdp_offload_check?
Please add code comments so we remember what to do and when.
> This restriction can be removed later after proper testing.
What kind of testing is missing here?
> Signed-off-by: Jason Wang <jasowang@...hat.com>
> Co-developed-by: Prashant Bhole <prashantbhole.linux@...il.com>
> Signed-off-by: Prashant Bhole <prashantbhole.linux@...il.com>
Any UAPI changes need to be copied to virtio-dev@...ts.oasis-open.org
(subscriber only) list please.
> ---
> drivers/net/virtio_net.c | 114 +++++++++++++++++++++++++++++++-
> include/uapi/linux/virtio_net.h | 27 ++++++++
> 2 files changed, 139 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index a1088d0114f2..dddfbb2a2075 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -169,6 +169,7 @@ struct control_buf {
> u8 allmulti;
> __virtio16 vid;
> __virtio64 offloads;
> + struct virtio_net_ctrl_ebpf_prog prog_ctrl;
> };
>
> struct virtnet_info {
> @@ -272,6 +273,8 @@ struct virtnet_bpf_bound_prog {
> struct bpf_insn insnsi[0];
> };
>
> +#define VIRTNET_EA(extack, msg) NL_SET_ERR_MSG_MOD((extack), msg)
> +
> /* Converting between virtqueue no. and kernel tx/rx queue no.
> * 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
> */
> @@ -2427,6 +2430,11 @@ static int virtnet_xdp_set(struct net_device *dev, struct netdev_bpf *bpf)
> if (!xdp_attachment_flags_ok(&vi->xdp, bpf))
> return -EBUSY;
>
> + if (rtnl_dereference(vi->offload_xdp_prog)) {
> + VIRTNET_EA(bpf->extack, "program already attached in offload mode");
> + return -EINVAL;
> + }
> +
> if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS)
> && (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO4) ||
> virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO6) ||
> @@ -2528,17 +2536,114 @@ static int virtnet_bpf_verify_insn(struct bpf_verifier_env *env, int insn_idx,
>
> static void virtnet_bpf_destroy_prog(struct bpf_prog *prog)
> {
> + struct virtnet_bpf_bound_prog *state;
> +
> + state = prog->aux->offload->dev_priv;
> + list_del(&state->list);
> + kfree(state);
> +}
> +
> +static int virtnet_xdp_offload_check(struct virtnet_info *vi,
> + struct netdev_bpf *bpf)
> +{
> + if (!bpf->prog)
> + return 0;
> +
> + if (!bpf->prog->aux->offload) {
> + VIRTNET_EA(bpf->extack, "xdpoffload of non-bound program");
> + return -EINVAL;
> + }
> + if (bpf->prog->aux->offload->netdev != vi->dev) {
> + VIRTNET_EA(bpf->extack, "program bound to different dev");
> + return -EINVAL;
> + }
> +
> + if (rtnl_dereference(vi->xdp_prog)) {
> + VIRTNET_EA(bpf->extack, "program already attached in driver mode");
> + return -EINVAL;
> + }
> +
> + return 0;
> }
>
> static int virtnet_xdp_set_offload(struct virtnet_info *vi,
> struct netdev_bpf *bpf)
> {
> - return -EBUSY;
> + struct virtio_net_ctrl_ebpf_prog *ctrl;
> + struct virtnet_bpf_bound_prog *bound_prog = NULL;
> + struct virtio_device *vdev = vi->vdev;
> + struct bpf_prog *prog = bpf->prog;
> + void *ctrl_buf = NULL;
> + struct scatterlist sg;
> + int prog_len;
> + int err = 0;
> +
> + if (!xdp_attachment_flags_ok(&vi->xdp_hw, bpf))
> + return -EBUSY;
> +
> + if (prog) {
> + if (prog->type != BPF_PROG_TYPE_XDP)
> + return -EOPNOTSUPP;
> + bound_prog = prog->aux->offload->dev_priv;
> + prog_len = prog->len * sizeof(bound_prog->insnsi[0]);
> +
> + ctrl_buf = kmalloc(GFP_KERNEL, sizeof(*ctrl) + prog_len);
> + if (!ctrl_buf)
> + return -ENOMEM;
> + ctrl = ctrl_buf;
> + ctrl->cmd = cpu_to_virtio32(vi->vdev,
> + VIRTIO_NET_BPF_CMD_SET_OFFLOAD);
> + ctrl->len = cpu_to_virtio32(vi->vdev, prog_len);
> + ctrl->gpl_compatible = cpu_to_virtio16(vi->vdev,
> + prog->gpl_compatible);
> + memcpy(ctrl->insns, bound_prog->insnsi,
> + prog->len * sizeof(bound_prog->insnsi[0]));
> + sg_init_one(&sg, ctrl_buf, sizeof(*ctrl) + prog_len);
> + } else {
> + ctrl = &vi->ctrl->prog_ctrl;
> + ctrl->cmd = cpu_to_virtio32(vi->vdev,
> + VIRTIO_NET_BPF_CMD_UNSET_OFFLOAD);
> + sg_init_one(&sg, ctrl, sizeof(*ctrl));
> + }
> +
> + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_EBPF,
> + VIRTIO_NET_CTRL_EBPF_PROG,
> + &sg)) {
> + dev_warn(&vdev->dev, "Failed to set bpf offload prog\n");
> + err = -EFAULT;
> + goto out;
> + }
> +
> + rcu_assign_pointer(vi->offload_xdp_prog, prog);
> +
> + xdp_attachment_setup(&vi->xdp_hw, bpf);
> +
> +out:
> + kfree(ctrl_buf);
> + return err;
> }
>
> static int virtnet_bpf_verifier_setup(struct bpf_prog *prog)
> {
> - return -ENOMEM;
> + struct virtnet_info *vi = netdev_priv(prog->aux->offload->netdev);
> + size_t insn_len = prog->len * sizeof(struct bpf_insn);
> + struct virtnet_bpf_bound_prog *state;
> +
> + state = kzalloc(sizeof(*state) + insn_len, GFP_KERNEL);
> + if (!state)
> + return -ENOMEM;
> +
> + memcpy(&state->insnsi[0], prog->insnsi, insn_len);
> +
> + state->vi = vi;
> + state->prog = prog;
> + state->len = prog->len;
> +
> + list_add_tail(&state->list, &vi->bpf_bound_progs);
> +
> + prog->aux->offload->dev_priv = state;
> +
> + return 0;
> }
>
> static int virtnet_bpf_verifier_prep(struct bpf_prog *prog)
> @@ -2568,12 +2673,17 @@ static const struct bpf_prog_offload_ops virtnet_bpf_dev_ops = {
> static int virtnet_xdp(struct net_device *dev, struct netdev_bpf *xdp)
> {
> struct virtnet_info *vi = netdev_priv(dev);
> + int err;
> +
> switch (xdp->command) {
> case XDP_SETUP_PROG:
> return virtnet_xdp_set(dev, xdp);
> case XDP_QUERY_PROG:
> return xdp_attachment_query(&vi->xdp, xdp);
> case XDP_SETUP_PROG_HW:
> + err = virtnet_xdp_offload_check(vi, xdp);
> + if (err)
> + return err;
> return virtnet_xdp_set_offload(vi, xdp);
> case XDP_QUERY_PROG_HW:
> return xdp_attachment_query(&vi->xdp_hw, xdp);
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index a3715a3224c1..0ea2f7910a5a 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -261,4 +261,31 @@ struct virtio_net_ctrl_mq {
> #define VIRTIO_NET_CTRL_GUEST_OFFLOADS 5
> #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET 0
>
> +/*
> + * Control XDP offloads offloads
> + *
> + * When guest wants to offload XDP program to the device, it calls
> + * VIRTIO_NET_CTRL_EBPF_PROG along with VIRTIO_NET_BPF_CMD_SET_OFFLOAD
> + * subcommands. When offloading is successful, the device runs offloaded
> + * XDP program for each packet before sending it to the guest.
> + *
> + * VIRTIO_NET_BPF_CMD_UNSET_OFFLOAD removes the the offloaded program from
> + * the device, if exists.
> + */
> +
> +struct virtio_net_ctrl_ebpf_prog {
> + /* program length in bytes */
> + __virtio32 len;
> + __virtio16 cmd;
> + __virtio16 gpl_compatible;
> + __u8 insns[0];
> +};
> +
> +#define VIRTIO_NET_CTRL_EBPF 6
> + #define VIRTIO_NET_CTRL_EBPF_PROG 1
> +
> +/* Commands for VIRTIO_NET_CTRL_EBPF_PROG */
> +#define VIRTIO_NET_BPF_CMD_SET_OFFLOAD 1
> +#define VIRTIO_NET_BPF_CMD_UNSET_OFFLOAD 2
> +
> #endif /* _UAPI_LINUX_VIRTIO_NET_H */
> --
> 2.20.1
Powered by blists - more mailing lists