Re: [PATCH 0/5] vDPA/ifcvf: implement immediate initialization mechanism
On Fri, Mar 31, 2023 at 8:49 PM Zhu Lingshan wrote: > > Formerly, ifcvf driver has implemented a lazy-initialization mechanism > for the virtqueues and other config space contents, > it would store all configurations that passed down from the userspace, > then load them to the device config space upon DRIVER_OK. > > This can not serve live migration, so this series implement an > immediate initialization mechanism, which means rather than the > former store-load process, the virtio operations like vq ops > would take immediate actions by access the virtio registers. Is there any chance that ifcvf can use virtio_pci_modern_dev library? Then we don't need to duplicate the codes. Note that pds_vdpa will be the second user for virtio_pci_modern_dev library (and the first vDPA parent to use that library). Thanks > > This series also implement irq synchronization in the reset > routine > > Zhu Lingshan (5): > virt queue ops take immediate actions > get_driver_features from virito registers > retire ifcvf_start_datapath and ifcvf_add_status > synchronize irqs in the reset routine > a vendor driver should not set _CONFIG_S_FAILED > > drivers/vdpa/ifcvf/ifcvf_base.c | 162 +++- > drivers/vdpa/ifcvf/ifcvf_base.h | 16 ++-- > drivers/vdpa/ifcvf/ifcvf_main.c | 97 --- > 3 files changed, 122 insertions(+), 153 deletions(-) > > -- > 2.39.1 > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH resend 1/2] vdpa/snet: support getting and setting VQ state
On Sun, Apr 2, 2023 at 8:52 PM Alvaro Karsz wrote:
>
> This patch adds the get_vq_state and set_vq_state vDPA callbacks.
>
> In order to get the VQ state, the state needs to be read from the DPU.
> In order to allow that, the old messaging mechanism is replaced with a new,
> flexible control mechanism.
> This mechanism allows to read data from the DPU.
>
> The mechanism can be used if the negotiated config version is 2 or
> higher.
>
> If the new mechanism is used when the config version is 1, it will call
> snet_send_ctrl_msg_old, which is config 1 compatible.
>
> Signed-off-by: Alvaro Karsz
> ---
> drivers/vdpa/solidrun/Makefile | 1 +
> drivers/vdpa/solidrun/snet_ctrl.c | 318 +
> drivers/vdpa/solidrun/snet_hwmon.c | 2 +-
> drivers/vdpa/solidrun/snet_main.c | 111 --
> drivers/vdpa/solidrun/snet_vdpa.h | 17 +-
> 5 files changed, 378 insertions(+), 71 deletions(-)
> create mode 100644 drivers/vdpa/solidrun/snet_ctrl.c
>
> diff --git a/drivers/vdpa/solidrun/Makefile b/drivers/vdpa/solidrun/Makefile
> index c0aa3415bf7..9116252cd5f 100644
> --- a/drivers/vdpa/solidrun/Makefile
> +++ b/drivers/vdpa/solidrun/Makefile
> @@ -1,6 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_SNET_VDPA) += snet_vdpa.o
> snet_vdpa-$(CONFIG_SNET_VDPA) += snet_main.o
> +snet_vdpa-$(CONFIG_SNET_VDPA) += snet_ctrl.o
> ifdef CONFIG_HWMON
> snet_vdpa-$(CONFIG_SNET_VDPA) += snet_hwmon.o
> endif
> diff --git a/drivers/vdpa/solidrun/snet_ctrl.c
> b/drivers/vdpa/solidrun/snet_ctrl.c
> new file mode 100644
> index 000..54909549a00
> --- /dev/null
> +++ b/drivers/vdpa/solidrun/snet_ctrl.c
> @@ -0,0 +1,318 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * SolidRun DPU driver for control plane
> + *
> + * Copyright (C) 2022-2023 SolidRun
> + *
> + * Author: Alvaro Karsz
> + *
> + */
> +
> +#include
> +
> +#include "snet_vdpa.h"
> +
> +enum snet_ctrl_opcodes {
> + SNET_CTRL_OP_DESTROY = 1,
> + SNET_CTRL_OP_READ_VQ_STATE,
> +};
> +
> +#define SNET_CTRL_TIMEOUT 200
> +
> +#define SNET_CTRL_DATA_SIZE_MASK 0x
> +#define SNET_CTRL_IN_PROCESS_MASK 0x0001
> +#define SNET_CTRL_CHUNK_RDY_MASK 0x0002
> +#define SNET_CTRL_ERROR_MASK 0x0FFC
> +
> +#define SNET_VAL_TO_ERR(val) (-(((val) & SNET_CTRL_ERROR_MASK) >>
> 18))
> +#define SNET_EMPTY_CTRL(val) (((val) & SNET_CTRL_ERROR_MASK) || \
> + !((val) &
> SNET_CTRL_IN_PROCESS_MASK))
> +#define SNET_DATA_READY(val) ((val) & (SNET_CTRL_ERROR_MASK |
> SNET_CTRL_CHUNK_RDY_MASK))
> +
> +/* Control register used to read data from the DPU */
> +struct snet_ctrl_reg_ctrl {
> + /* Chunk size in 4B words */
> + u16 data_size;
> + /* We are in the middle of a command */
> + u16 in_process:1;
> + /* A data chunk is ready and can be consumed */
> + u16 chunk_ready:1;
> + /* Error code */
> + u16 error:10;
> + /* Saved for future usage */
> + u16 rsvd:4;
> +};
> +
> +/* Opcode register */
> +struct snet_ctrl_reg_op {
> + u16 opcode;
> + /* Only if VQ index is relevant for the command */
> + u16 vq_idx;
> +};
> +
> +struct snet_ctrl_regs {
> + struct snet_ctrl_reg_op op;
> + struct snet_ctrl_reg_ctrl ctrl;
> + u32 rsvd;
> + u32 data[];
> +};
> +
> +static struct snet_ctrl_regs __iomem *snet_get_ctrl(struct snet *snet)
> +{
> + return snet->bar + snet->psnet->cfg.ctrl_off;
> +}
> +
> +static int snet_wait_for_empty_ctrl(struct snet_ctrl_regs __iomem *regs)
> +{
> + u32 val;
> +
> + return readx_poll_timeout(ioread32, >ctrl, val,
> SNET_EMPTY_CTRL(val), 10,
> + SNET_CTRL_TIMEOUT);
> +}
> +
> +static int snet_wait_for_empty_op(struct snet_ctrl_regs __iomem *regs)
> +{
> + u32 val;
> +
> + return readx_poll_timeout(ioread32, >op, val, !val, 10,
> SNET_CTRL_TIMEOUT);
> +}
> +
> +static int snet_wait_for_data(struct snet_ctrl_regs __iomem *regs)
> +{
> + u32 val;
> +
> + return readx_poll_timeout(ioread32, >ctrl, val,
> SNET_DATA_READY(val), 10,
> + SNET_CTRL_TIMEOUT);
> +}
> +
> +static u32 snet_read32_word(struct snet_ctrl_regs __iomem *ctrl_regs, u16
> word_idx)
> +{
> + return ioread32(_regs->data[word_idx]);
> +}
> +
> +static u32 snet_read_ctrl(struct snet_ctrl_regs __iomem *ctrl_regs)
> +{
> + return ioread32(_regs->ctrl);
> +}
> +
> +static void snet_write_ctrl(struct snet_ctrl_regs __iomem *ctrl_regs, u32
> val)
> +{
> + iowrite32(val, _regs->ctrl);
> +}
> +
> +static void snet_write_op(struct snet_ctrl_regs __iomem *ctrl_regs, u32 val)
> +{
> + iowrite32(val, _regs->op);
> +}
> +
> +static int snet_wait_for_dpu_completion(struct snet_ctrl_regs __iomem
> *ctrl_regs)
> +{
> + /* Wait until the DPU finishes completely.
> +* It
Re: [Patch v3] vdpa/mlx5: Avoid losing link state updates
On Sun, Apr 2, 2023 at 10:15 PM Eli Cohen wrote:
>
> Current code ignores link state updates if VIRTIO_NET_F_STATUS was not
> negotiated. However, link state updates could be received before feature
> negotiation was completed , therefore causing link state events to be
> lost, possibly leaving the link state down.
>
> Modify the code so link state notifier is registered only when
> VIRTIO_NET_F_STATUS flips from 0 to 1 and unregister it on driver reset
> or suspend.
>
> Fixes: 033779a708f0 ("vdpa/mlx5: make MTU/STATUS presence conditional on
> feature bits")
> Signed-off-by: Eli Cohen
> ---
> v2 -> v3
> Only register the link event notifier when VIRTIO_NET_F_STATUS is
> negotiated.
>
> drivers/vdpa/mlx5/net/mlx5_vnet.c | 200 +-
> 1 file changed, 112 insertions(+), 88 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 317cef9b7813..9b1432e22540 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -2322,10 +2322,115 @@ static void update_cvq_info(struct mlx5_vdpa_dev
> *mvdev)
> }
> }
>
> +static u8 query_vport_state(struct mlx5_core_dev *mdev, u8 opmod, u16 vport)
> +{
> + u32 out[MLX5_ST_SZ_DW(query_vport_state_out)] = {};
> + u32 in[MLX5_ST_SZ_DW(query_vport_state_in)] = {};
> + int err;
> +
> + MLX5_SET(query_vport_state_in, in, opcode,
> MLX5_CMD_OP_QUERY_VPORT_STATE);
> + MLX5_SET(query_vport_state_in, in, op_mod, opmod);
> + MLX5_SET(query_vport_state_in, in, vport_number, vport);
> + if (vport)
> + MLX5_SET(query_vport_state_in, in, other_vport, 1);
> +
> + err = mlx5_cmd_exec_inout(mdev, query_vport_state, in, out);
> + if (err)
> + return 0;
> +
> + return MLX5_GET(query_vport_state_out, out, state);
> +}
> +
> +static bool get_link_state(struct mlx5_vdpa_dev *mvdev)
> +{
> + if (query_vport_state(mvdev->mdev,
> MLX5_VPORT_STATE_OP_MOD_VNIC_VPORT, 0) ==
> + VPORT_STATE_UP)
> + return true;
> +
> + return false;
> +}
> +
> +static void update_carrier(struct work_struct *work)
> +{
> + struct mlx5_vdpa_wq_ent *wqent;
> + struct mlx5_vdpa_dev *mvdev;
> + struct mlx5_vdpa_net *ndev;
> +
> + wqent = container_of(work, struct mlx5_vdpa_wq_ent, work);
> + mvdev = wqent->mvdev;
> + ndev = to_mlx5_vdpa_ndev(mvdev);
> + if (get_link_state(mvdev))
> + ndev->config.status |= cpu_to_mlx5vdpa16(mvdev,
> VIRTIO_NET_S_LINK_UP);
> + else
> + ndev->config.status &= cpu_to_mlx5vdpa16(mvdev,
> ~VIRTIO_NET_S_LINK_UP);
> +
> + if (ndev->nb_registered && ndev->config_cb.callback)
It looks to me nb_registered is accessed without synchronization. Or
we don't even need to check that if we do:
unregister();
flush_workqueue();
which has been done in unregister_link_notifier().
> + ndev->config_cb.callback(ndev->config_cb.private);
> +
> + kfree(wqent);
> +}
> +
> +static int queue_link_work(struct mlx5_vdpa_net *ndev)
> +{
> + struct mlx5_vdpa_wq_ent *wqent;
> +
> + wqent = kzalloc(sizeof(*wqent), GFP_ATOMIC);
> + if (!wqent)
> + return -ENOMEM;
> +
> + wqent->mvdev = >mvdev;
> + INIT_WORK(>work, update_carrier);
> + queue_work(ndev->mvdev.wq, >work);
> + return 0;
> +}
> +
> +static int event_handler(struct notifier_block *nb, unsigned long event,
> void *param)
> +{
> + struct mlx5_vdpa_net *ndev = container_of(nb, struct mlx5_vdpa_net,
> nb);
> + struct mlx5_eqe *eqe = param;
> + int ret = NOTIFY_DONE;
> +
> + if (event == MLX5_EVENT_TYPE_PORT_CHANGE) {
> + switch (eqe->sub_type) {
> + case MLX5_PORT_CHANGE_SUBTYPE_DOWN:
> + case MLX5_PORT_CHANGE_SUBTYPE_ACTIVE:
> + if (queue_link_work(ndev))
> + return NOTIFY_DONE;
> +
> + ret = NOTIFY_OK;
> + break;
> + default:
> + return NOTIFY_DONE;
> + }
> + return ret;
> + }
> + return ret;
> +}
> +
> +static void register_link_notifier(struct mlx5_vdpa_net *ndev)
> +{
> + ndev->nb.notifier_call = event_handler;
> + mlx5_notifier_register(ndev->mvdev.mdev, >nb);
> + ndev->nb_registered = true;
> + queue_link_work(ndev);
> +}
> +
> +static void unregister_link_notifier(struct mlx5_vdpa_net *ndev)
> +{
> + if (!ndev->nb_registered)
> + return;
> +
> + ndev->nb_registered = false;
> + mlx5_notifier_unregister(ndev->mvdev.mdev, >nb);
> + if (ndev->mvdev.wq)
Under which case could we hit mvdev.wq = NULL?
(We call unregister_link_notifier() before setting mq to NULL during
device del).
> + flush_workqueue(ndev->mvdev.wq);
> +}
> +
> static int
Re: [PATCH net-next 6/8] virtio_net: auto release xdp shinfo
On Mon, 3 Apr 2023 11:18:02 +0800, Jason Wang wrote:
>
> 在 2023/3/28 20:04, Xuan Zhuo 写道:
> > virtnet_build_xdp_buff_mrg() and virtnet_xdp_handler() auto
>
>
> I think you meant virtnet_xdp_handler() actually?
>
>
> > release xdp shinfo then the caller no need to careful the xdp shinfo.
> >
> > Signed-off-by: Xuan Zhuo
> > ---
> > drivers/net/virtio_net.c | 29 +
> > 1 file changed, 17 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index a3f2bcb3db27..136131a7868a 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -833,14 +833,14 @@ static int virtnet_xdp_handler(struct bpf_prog
> > *xdp_prog, struct xdp_buff *xdp,
> > stats->xdp_tx++;
> > xdpf = xdp_convert_buff_to_frame(xdp);
> > if (unlikely(!xdpf))
> > - return VIRTNET_XDP_RES_DROP;
> > + goto drop;
> >
> > err = virtnet_xdp_xmit(dev, 1, , 0);
> > if (unlikely(!err)) {
> > xdp_return_frame_rx_napi(xdpf);
> > } else if (unlikely(err < 0)) {
> > trace_xdp_exception(dev, xdp_prog, act);
> > - return VIRTNET_XDP_RES_DROP;
> > + goto drop;
> > }
> >
> > *xdp_xmit |= VIRTIO_XDP_TX;
> > @@ -850,7 +850,7 @@ static int virtnet_xdp_handler(struct bpf_prog
> > *xdp_prog, struct xdp_buff *xdp,
> > stats->xdp_redirects++;
> > err = xdp_do_redirect(dev, xdp, xdp_prog);
> > if (err)
> > - return VIRTNET_XDP_RES_DROP;
> > + goto drop;
> >
> > *xdp_xmit |= VIRTIO_XDP_REDIR;
> > return VIRTNET_XDP_RES_CONSUMED;
> > @@ -862,8 +862,12 @@ static int virtnet_xdp_handler(struct bpf_prog
> > *xdp_prog, struct xdp_buff *xdp,
> > trace_xdp_exception(dev, xdp_prog, act);
> > fallthrough;
> > case XDP_DROP:
> > - return VIRTNET_XDP_RES_DROP;
> > + goto drop;
>
>
> This goto is kind of meaningless.
Will fix.
Thanks.
>
> Thanks
>
>
> > }
> > +
> > +drop:
> > + put_xdp_frags(xdp);
> > + return VIRTNET_XDP_RES_DROP;
> > }
> >
> > static unsigned int virtnet_get_headroom(struct virtnet_info *vi)
> > @@ -1199,7 +1203,7 @@ static int virtnet_build_xdp_buff_mrg(struct
> > net_device *dev,
> > dev->name, *num_buf,
> > virtio16_to_cpu(vi->vdev, hdr->num_buffers));
> > dev->stats.rx_length_errors++;
> > - return -EINVAL;
> > + goto err;
> > }
> >
> > stats->bytes += len;
> > @@ -1218,7 +1222,7 @@ static int virtnet_build_xdp_buff_mrg(struct
> > net_device *dev,
> > pr_debug("%s: rx error: len %u exceeds truesize %lu\n",
> > dev->name, len, (unsigned long)(truesize -
> > room));
> > dev->stats.rx_length_errors++;
> > - return -EINVAL;
> > + goto err;
> > }
> >
> > frag = >frags[shinfo->nr_frags++];
> > @@ -1233,6 +1237,10 @@ static int virtnet_build_xdp_buff_mrg(struct
> > net_device *dev,
> >
> > *xdp_frags_truesize = xdp_frags_truesz;
> > return 0;
> > +
> > +err:
> > + put_xdp_frags(xdp);
> > + return -EINVAL;
> > }
> >
> > static void *mergeable_xdp_prepare(struct virtnet_info *vi,
> > @@ -1361,7 +1369,7 @@ static struct sk_buff *receive_mergeable(struct
> > net_device *dev,
> > err = virtnet_build_xdp_buff_mrg(dev, vi, rq, , data, len,
> > frame_sz,
> > _buf, _frags_truesz,
> > stats);
> > if (unlikely(err))
> > - goto err_xdp_frags;
> > + goto err_xdp;
> >
> > act = virtnet_xdp_handler(xdp_prog, , dev, xdp_xmit, stats);
> >
> > @@ -1369,7 +1377,7 @@ static struct sk_buff *receive_mergeable(struct
> > net_device *dev,
> > case VIRTNET_XDP_RES_PASS:
> > head_skb = build_skb_from_xdp_buff(dev, vi, ,
> > xdp_frags_truesz);
> > if (unlikely(!head_skb))
> > - goto err_xdp_frags;
> > + goto err_xdp;
> >
> > rcu_read_unlock();
> > return head_skb;
> > @@ -1379,11 +1387,8 @@ static struct sk_buff *receive_mergeable(struct
> > net_device *dev,
> > goto xdp_xmit;
> >
> > case VIRTNET_XDP_RES_DROP:
> > - goto err_xdp_frags;
> > + goto err_xdp;
> > }
> > -err_xdp_frags:
> > - put_xdp_frags();
> > - goto err_xdp;
> > }
> > rcu_read_unlock();
> >
>
___
Virtualization mailing list
Re: [PATCH net-next 3/8] virtio_net: introduce virtnet_xdp_handler() to seprate the logic of run xdp
On Mon, 3 Apr 2023 10:43:03 +0800, Jason Wang wrote:
> On Tue, Mar 28, 2023 at 8:04 PM Xuan Zhuo wrote:
> >
> > At present, we have two similar logic to perform the XDP prog.
> >
> > Therefore, this PATCH separates the code of executing XDP, which is
> > conducive to later maintenance.
> >
> > Signed-off-by: Xuan Zhuo
> > ---
> > drivers/net/virtio_net.c | 142 +--
> > 1 file changed, 75 insertions(+), 67 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index bb426958cdd4..72b9d6ee4024 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -301,6 +301,15 @@ struct padded_vnet_hdr {
> > char padding[12];
> > };
> >
> > +enum {
> > + /* xdp pass */
> > + VIRTNET_XDP_RES_PASS,
> > + /* drop packet. the caller needs to release the page. */
> > + VIRTNET_XDP_RES_DROP,
> > + /* packet is consumed by xdp. the caller needs to do nothing. */
> > + VIRTNET_XDP_RES_CONSUMED,
> > +};
>
> I'd prefer this to be done on top unless it is a must. But I don't see
> any advantage of introducing this, it's partial mapping of XDP action
> and it needs to be extended when XDP action is extended. (And we've
> already had: VIRTIO_XDP_REDIR and VIRTIO_XDP_TX ...)
No, these are the three states of buffer after XDP processing.
* PASS: goto make skb
* DROP: we should release buffer
* CUNSUMED: xdp prog used the buffer, we do nothing
The latter two are not particularly related to XDP ACTION. And it does not need
to extend when XDP action is extended. At least I have not thought of this
situation.
>
> > +
> > static void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf);
> > static void virtnet_sq_free_unused_buf(struct virtqueue *vq, void *buf);
> >
> > @@ -789,6 +798,59 @@ static int virtnet_xdp_xmit(struct net_device *dev,
> > return ret;
> > }
> >
> > +static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff
> > *xdp,
> > + struct net_device *dev,
> > + unsigned int *xdp_xmit,
> > + struct virtnet_rq_stats *stats)
> > +{
> > + struct xdp_frame *xdpf;
> > + int err;
> > + u32 act;
> > +
> > + act = bpf_prog_run_xdp(xdp_prog, xdp);
> > + stats->xdp_packets++;
> > +
> > + switch (act) {
> > + case XDP_PASS:
> > + return VIRTNET_XDP_RES_PASS;
> > +
> > + case XDP_TX:
> > + stats->xdp_tx++;
> > + xdpf = xdp_convert_buff_to_frame(xdp);
> > + if (unlikely(!xdpf))
> > + return VIRTNET_XDP_RES_DROP;
> > +
> > + err = virtnet_xdp_xmit(dev, 1, , 0);
> > + if (unlikely(!err)) {
> > + xdp_return_frame_rx_napi(xdpf);
> > + } else if (unlikely(err < 0)) {
> > + trace_xdp_exception(dev, xdp_prog, act);
> > + return VIRTNET_XDP_RES_DROP;
> > + }
> > +
> > + *xdp_xmit |= VIRTIO_XDP_TX;
> > + return VIRTNET_XDP_RES_CONSUMED;
> > +
> > + case XDP_REDIRECT:
> > + stats->xdp_redirects++;
> > + err = xdp_do_redirect(dev, xdp, xdp_prog);
> > + if (err)
> > + return VIRTNET_XDP_RES_DROP;
> > +
> > + *xdp_xmit |= VIRTIO_XDP_REDIR;
> > + return VIRTNET_XDP_RES_CONSUMED;
> > +
> > + default:
> > + bpf_warn_invalid_xdp_action(dev, xdp_prog, act);
> > + fallthrough;
> > + case XDP_ABORTED:
> > + trace_xdp_exception(dev, xdp_prog, act);
> > + fallthrough;
> > + case XDP_DROP:
> > + return VIRTNET_XDP_RES_DROP;
> > + }
> > +}
> > +
> > static unsigned int virtnet_get_headroom(struct virtnet_info *vi)
> > {
> > return vi->xdp_enabled ? VIRTIO_XDP_HEADROOM : 0;
> > @@ -876,7 +938,6 @@ static struct sk_buff *receive_small(struct net_device
> > *dev,
> > struct page *page = virt_to_head_page(buf);
> > unsigned int delta = 0;
> > struct page *xdp_page;
> > - int err;
> > unsigned int metasize = 0;
> >
> > len -= vi->hdr_len;
> > @@ -898,7 +959,6 @@ static struct sk_buff *receive_small(struct net_device
> > *dev,
> > xdp_prog = rcu_dereference(rq->xdp_prog);
> > if (xdp_prog) {
> > struct virtio_net_hdr_mrg_rxbuf *hdr = buf + header_offset;
> > - struct xdp_frame *xdpf;
> > struct xdp_buff xdp;
> > void *orig_data;
> > u32 act;
> > @@ -931,46 +991,22 @@ static struct sk_buff *receive_small(struct
> > net_device *dev,
> > xdp_prepare_buff(, buf + VIRTNET_RX_PAD + vi->hdr_len,
> > xdp_headroom, len, true);
> >
Re: [PATCH net-next 2/8] virtio_net: mergeable xdp: introduce mergeable_xdp_prepare
On Fri, 31 Mar 2023 17:14:33 +0800, Jason Wang wrote:
>
> 在 2023/3/28 20:04, Xuan Zhuo 写道:
> > Separating the logic of preparation for xdp from receive_mergeable.
> >
> > The purpose of this is to simplify the logic of execution of XDP.
> >
> > The main logic here is that when headroom is insufficient, we need to
> > allocate a new page and calculate offset. It should be noted that if
> > there is new page, the variable page will refer to the new page.
> >
> > Signed-off-by: Xuan Zhuo
> > ---
> > drivers/net/virtio_net.c | 135 ++-
> > 1 file changed, 77 insertions(+), 58 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 4d2bf1ce0730..bb426958cdd4 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -1162,6 +1162,79 @@ static int virtnet_build_xdp_buff_mrg(struct
> > net_device *dev,
> > return 0;
> > }
> >
> > +static void *mergeable_xdp_prepare(struct virtnet_info *vi,
> > + struct receive_queue *rq,
> > + struct bpf_prog *xdp_prog,
> > + void *ctx,
> > + unsigned int *frame_sz,
> > + int *num_buf,
> > + struct page **page,
> > + int offset,
> > + unsigned int *len,
> > + struct virtio_net_hdr_mrg_rxbuf *hdr)
> > +{
> > + unsigned int truesize = mergeable_ctx_to_truesize(ctx);
> > + unsigned int headroom = mergeable_ctx_to_headroom(ctx);
> > + struct page *xdp_page;
> > + unsigned int xdp_room;
> > +
> > + /* Transient failure which in theory could occur if
> > +* in-flight packets from before XDP was enabled reach
> > +* the receive path after XDP is loaded.
> > +*/
> > + if (unlikely(hdr->hdr.gso_type))
> > + return NULL;
> > +
> > + /* Now XDP core assumes frag size is PAGE_SIZE, but buffers
> > +* with headroom may add hole in truesize, which
> > +* make their length exceed PAGE_SIZE. So we disabled the
> > +* hole mechanism for xdp. See add_recvbuf_mergeable().
> > +*/
> > + *frame_sz = truesize;
> > +
> > + /* This happens when headroom is not enough because
> > +* of the buffer was prefilled before XDP is set.
> > +* This should only happen for the first several packets.
> > +* In fact, vq reset can be used here to help us clean up
> > +* the prefilled buffers, but many existing devices do not
> > +* support it, and we don't want to bother users who are
> > +* using xdp normally.
> > +*/
> > + if (!xdp_prog->aux->xdp_has_frags &&
> > + (*num_buf > 1 || headroom < virtnet_get_headroom(vi))) {
> > + /* linearize data for XDP */
> > + xdp_page = xdp_linearize_page(rq, num_buf,
> > + *page, offset,
> > + VIRTIO_XDP_HEADROOM,
> > + len);
> > +
> > + if (!xdp_page)
> > + return NULL;
> > + } else if (unlikely(headroom < virtnet_get_headroom(vi))) {
> > + xdp_room = SKB_DATA_ALIGN(VIRTIO_XDP_HEADROOM +
> > + sizeof(struct skb_shared_info));
> > + if (*len + xdp_room > PAGE_SIZE)
> > + return NULL;
> > +
> > + xdp_page = alloc_page(GFP_ATOMIC);
> > + if (!xdp_page)
> > + return NULL;
> > +
> > + memcpy(page_address(xdp_page) + VIRTIO_XDP_HEADROOM,
> > + page_address(*page) + offset, *len);
> > + } else {
> > + return page_address(*page) + offset;
>
>
> This makes the code a little harder to be read than the original code.
>
> Why not do a verbatim moving without introducing new logic? (Or
> introducing new logic on top?)
Yes. Will fix.
Thanks.
>
> Thanks
>
>
> > + }
> > +
> > + *frame_sz = PAGE_SIZE;
> > +
> > + put_page(*page);
> > +
> > + *page = xdp_page;
> > +
> > + return page_address(xdp_page) + VIRTIO_XDP_HEADROOM;
> > +}
> > +
> > static struct sk_buff *receive_mergeable(struct net_device *dev,
> > struct virtnet_info *vi,
> > struct receive_queue *rq,
> > @@ -1181,7 +1254,7 @@ static struct sk_buff *receive_mergeable(struct
> > net_device *dev,
> > unsigned int headroom = mergeable_ctx_to_headroom(ctx);
> > unsigned int tailroom = headroom ? sizeof(struct skb_shared_info) : 0;
> > unsigned int room = SKB_DATA_ALIGN(headroom + tailroom);
> > - unsigned int frame_sz, xdp_room;
> > + unsigned int frame_sz;
> > int err;
> >
> > head_skb = NULL;
> > @@ -1211,65 +1284,11 @@ static struct sk_buff *receive_mergeable(struct
> > net_device *dev,
> > u32 act;
> > int i;
> >
> > -
Re: [PATCH net-next 1/8] virtio_net: mergeable xdp: put old page immediately
On Fri, 31 Mar 2023 17:01:54 +0800, Jason Wang wrote:
> On Tue, Mar 28, 2023 at 8:04 PM Xuan Zhuo wrote:
> >
> > In the xdp implementation of virtio-net mergeable, it always checks
> > whether two page is used and a page is selected to release. This is
> > complicated for the processing of action, and be careful.
> >
> > In the entire process, we have such principles:
> > * If xdp_page is used (PASS, TX, Redirect), then we release the old
> > page.
> > * If it is a drop case, we will release two. The old page obtained from
> > buf is release inside err_xdp, and xdp_page needs be relased by us.
> >
> > But in fact, when we allocate a new page, we can release the old page
> > immediately. Then just one is using, we just need to release the new
> > page for drop case. On the drop path, err_xdp will release the variable
> > "page", so we only need to let "page" point to the new xdp_page in
> > advance.
> >
> > Signed-off-by: Xuan Zhuo
> > ---
> > drivers/net/virtio_net.c | 15 ++-
> > 1 file changed, 6 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index e2560b6f7980..4d2bf1ce0730 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -1245,6 +1245,9 @@ static struct sk_buff *receive_mergeable(struct
> > net_device *dev,
> > if (!xdp_page)
> > goto err_xdp;
> > offset = VIRTIO_XDP_HEADROOM;
> > +
> > + put_page(page);
> > + page = xdp_page;
> > } else if (unlikely(headroom < virtnet_get_headroom(vi))) {
> > xdp_room = SKB_DATA_ALIGN(VIRTIO_XDP_HEADROOM +
> > sizeof(struct
> > skb_shared_info));
> > @@ -1259,6 +1262,9 @@ static struct sk_buff *receive_mergeable(struct
> > net_device *dev,
> >page_address(page) + offset, len);
> > frame_sz = PAGE_SIZE;
> > offset = VIRTIO_XDP_HEADROOM;
> > +
> > + put_page(page);
> > + page = xdp_page;
> > } else {
> > xdp_page = page;
> > }
>
> While at this I would go a little further, just remove the above
> assignment then we can use:
>
> data = page_address(page) + offset;
>
> Which seems cleaner?
I will try.
Thanks.
>
> Thanks
>
> > @@ -1278,8 +1284,6 @@ static struct sk_buff *receive_mergeable(struct
> > net_device *dev,
> > if (unlikely(!head_skb))
> > goto err_xdp_frags;
> >
> > - if (unlikely(xdp_page != page))
> > - put_page(page);
> > rcu_read_unlock();
> > return head_skb;
> > case XDP_TX:
> > @@ -1297,8 +1301,6 @@ static struct sk_buff *receive_mergeable(struct
> > net_device *dev,
> > goto err_xdp_frags;
> > }
> > *xdp_xmit |= VIRTIO_XDP_TX;
> > - if (unlikely(xdp_page != page))
> > - put_page(page);
> > rcu_read_unlock();
> > goto xdp_xmit;
> > case XDP_REDIRECT:
> > @@ -1307,8 +1309,6 @@ static struct sk_buff *receive_mergeable(struct
> > net_device *dev,
> > if (err)
> > goto err_xdp_frags;
> > *xdp_xmit |= VIRTIO_XDP_REDIR;
> > - if (unlikely(xdp_page != page))
> > - put_page(page);
> > rcu_read_unlock();
> > goto xdp_xmit;
> > default:
> > @@ -1321,9 +1321,6 @@ static struct sk_buff *receive_mergeable(struct
> > net_device *dev,
> > goto err_xdp_frags;
> > }
> > err_xdp_frags:
> > - if (unlikely(xdp_page != page))
> > - __free_pages(xdp_page, 0);
> > -
> > if (xdp_buff_has_frags()) {
> > shinfo = xdp_get_shared_info_from_buff();
> > for (i = 0; i < shinfo->nr_frags; i++) {
> > --
> > 2.32.0.3.g01195cf9f
> >
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH net-next 6/8] virtio_net: auto release xdp shinfo
在 2023/3/28 20:04, Xuan Zhuo 写道:
virtnet_build_xdp_buff_mrg() and virtnet_xdp_handler() auto
I think you meant virtnet_xdp_handler() actually?
release xdp shinfo then the caller no need to careful the xdp shinfo.
Signed-off-by: Xuan Zhuo
---
drivers/net/virtio_net.c | 29 +
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index a3f2bcb3db27..136131a7868a 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -833,14 +833,14 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog,
struct xdp_buff *xdp,
stats->xdp_tx++;
xdpf = xdp_convert_buff_to_frame(xdp);
if (unlikely(!xdpf))
- return VIRTNET_XDP_RES_DROP;
+ goto drop;
err = virtnet_xdp_xmit(dev, 1, , 0);
if (unlikely(!err)) {
xdp_return_frame_rx_napi(xdpf);
} else if (unlikely(err < 0)) {
trace_xdp_exception(dev, xdp_prog, act);
- return VIRTNET_XDP_RES_DROP;
+ goto drop;
}
*xdp_xmit |= VIRTIO_XDP_TX;
@@ -850,7 +850,7 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog,
struct xdp_buff *xdp,
stats->xdp_redirects++;
err = xdp_do_redirect(dev, xdp, xdp_prog);
if (err)
- return VIRTNET_XDP_RES_DROP;
+ goto drop;
*xdp_xmit |= VIRTIO_XDP_REDIR;
return VIRTNET_XDP_RES_CONSUMED;
@@ -862,8 +862,12 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog,
struct xdp_buff *xdp,
trace_xdp_exception(dev, xdp_prog, act);
fallthrough;
case XDP_DROP:
- return VIRTNET_XDP_RES_DROP;
+ goto drop;
This goto is kind of meaningless.
Thanks
}
+
+drop:
+ put_xdp_frags(xdp);
+ return VIRTNET_XDP_RES_DROP;
}
static unsigned int virtnet_get_headroom(struct virtnet_info *vi)
@@ -1199,7 +1203,7 @@ static int virtnet_build_xdp_buff_mrg(struct net_device
*dev,
dev->name, *num_buf,
virtio16_to_cpu(vi->vdev, hdr->num_buffers));
dev->stats.rx_length_errors++;
- return -EINVAL;
+ goto err;
}
stats->bytes += len;
@@ -1218,7 +1222,7 @@ static int virtnet_build_xdp_buff_mrg(struct net_device
*dev,
pr_debug("%s: rx error: len %u exceeds truesize %lu\n",
dev->name, len, (unsigned long)(truesize -
room));
dev->stats.rx_length_errors++;
- return -EINVAL;
+ goto err;
}
frag = >frags[shinfo->nr_frags++];
@@ -1233,6 +1237,10 @@ static int virtnet_build_xdp_buff_mrg(struct net_device
*dev,
*xdp_frags_truesize = xdp_frags_truesz;
return 0;
+
+err:
+ put_xdp_frags(xdp);
+ return -EINVAL;
}
static void *mergeable_xdp_prepare(struct virtnet_info *vi,
@@ -1361,7 +1369,7 @@ static struct sk_buff *receive_mergeable(struct
net_device *dev,
err = virtnet_build_xdp_buff_mrg(dev, vi, rq, , data, len,
frame_sz,
_buf, _frags_truesz,
stats);
if (unlikely(err))
- goto err_xdp_frags;
+ goto err_xdp;
act = virtnet_xdp_handler(xdp_prog, , dev, xdp_xmit, stats);
@@ -1369,7 +1377,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
case VIRTNET_XDP_RES_PASS:
head_skb = build_skb_from_xdp_buff(dev, vi, ,
xdp_frags_truesz);
if (unlikely(!head_skb))
- goto err_xdp_frags;
+ goto err_xdp;
rcu_read_unlock();
return head_skb;
@@ -1379,11 +1387,8 @@ static struct sk_buff *receive_mergeable(struct
net_device *dev,
goto xdp_xmit;
case VIRTNET_XDP_RES_DROP:
- goto err_xdp_frags;
+ goto err_xdp;
}
-err_xdp_frags:
- put_xdp_frags();
- goto err_xdp;
}
rcu_read_unlock();
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH net-next 5/8] virtio_net: separate the logic of freeing the rest mergeable buf
On Tue, Mar 28, 2023 at 8:04 PM Xuan Zhuo wrote:
>
> This patch introduce a new function that frees the rest mergeable buf.
> The subsequent patch will reuse this function.
>
> Signed-off-by: Xuan Zhuo
Acked-by: Jason Wang
Thanks
> ---
> drivers/net/virtio_net.c | 36
> 1 file changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 09aed60e2f51..a3f2bcb3db27 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1076,6 +1076,28 @@ static struct sk_buff *receive_big(struct net_device
> *dev,
> return NULL;
> }
>
> +static void mergeable_buf_free(struct receive_queue *rq, int num_buf,
> + struct net_device *dev,
> + struct virtnet_rq_stats *stats)
> +{
> + struct page *page;
> + void *buf;
> + int len;
> +
> + while (num_buf-- > 1) {
> + buf = virtqueue_get_buf(rq->vq, );
> + if (unlikely(!buf)) {
> + pr_debug("%s: rx error: %d buffers missing\n",
> +dev->name, num_buf);
> + dev->stats.rx_length_errors++;
> + break;
> + }
> + stats->bytes += len;
> + page = virt_to_head_page(buf);
> + put_page(page);
> + }
> +}
> +
> /* Why not use xdp_build_skb_from_frame() ?
> * XDP core assumes that xdp frags are PAGE_SIZE in length, while in
> * virtio-net there are 2 points that do not match its requirements:
> @@ -1436,18 +1458,8 @@ static struct sk_buff *receive_mergeable(struct
> net_device *dev,
> stats->xdp_drops++;
> err_skb:
> put_page(page);
> - while (num_buf-- > 1) {
> - buf = virtqueue_get_buf(rq->vq, );
> - if (unlikely(!buf)) {
> - pr_debug("%s: rx error: %d buffers missing\n",
> -dev->name, num_buf);
> - dev->stats.rx_length_errors++;
> - break;
> - }
> - stats->bytes += len;
> - page = virt_to_head_page(buf);
> - put_page(page);
> - }
> + mergeable_buf_free(rq, num_buf, dev, stats);
> +
> err_buf:
> stats->drops++;
> dev_kfree_skb(head_skb);
> --
> 2.32.0.3.g01195cf9f
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH net-next 4/8] virtio_net: separate the logic of freeing xdp shinfo
On Tue, Mar 28, 2023 at 8:04 PM Xuan Zhuo wrote:
>
> This patch introduce a new function that releases the
> xdp shinfo. The subsequent patch will reuse this function.
>
> Signed-off-by: Xuan Zhuo
Acked-by: Jason Wang
Thanks
> ---
> drivers/net/virtio_net.c | 27 ---
> 1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 72b9d6ee4024..09aed60e2f51 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -798,6 +798,21 @@ static int virtnet_xdp_xmit(struct net_device *dev,
> return ret;
> }
>
> +static void put_xdp_frags(struct xdp_buff *xdp)
> +{
> + struct skb_shared_info *shinfo;
> + struct page *xdp_page;
> + int i;
> +
> + if (xdp_buff_has_frags(xdp)) {
> + shinfo = xdp_get_shared_info_from_buff(xdp);
> + for (i = 0; i < shinfo->nr_frags; i++) {
> + xdp_page = skb_frag_page(>frags[i]);
> + put_page(xdp_page);
> + }
> + }
> +}
> +
> static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff
> *xdp,
>struct net_device *dev,
>unsigned int *xdp_xmit,
> @@ -1312,12 +1327,9 @@ static struct sk_buff *receive_mergeable(struct
> net_device *dev,
> xdp_prog = rcu_dereference(rq->xdp_prog);
> if (xdp_prog) {
> unsigned int xdp_frags_truesz = 0;
> - struct skb_shared_info *shinfo;
> - struct page *xdp_page;
> struct xdp_buff xdp;
> void *data;
> u32 act;
> - int i;
>
> data = mergeable_xdp_prepare(vi, rq, xdp_prog, ctx,
> _sz, _buf, ,
> offset, , hdr);
> @@ -1348,14 +1360,7 @@ static struct sk_buff *receive_mergeable(struct
> net_device *dev,
> goto err_xdp_frags;
> }
> err_xdp_frags:
> - if (xdp_buff_has_frags()) {
> - shinfo = xdp_get_shared_info_from_buff();
> - for (i = 0; i < shinfo->nr_frags; i++) {
> - xdp_page = skb_frag_page(>frags[i]);
> - put_page(xdp_page);
> - }
> - }
> -
> + put_xdp_frags();
> goto err_xdp;
> }
> rcu_read_unlock();
> --
> 2.32.0.3.g01195cf9f
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH net-next 3/8] virtio_net: introduce virtnet_xdp_handler() to seprate the logic of run xdp
On Tue, Mar 28, 2023 at 8:04 PM Xuan Zhuo wrote:
>
> At present, we have two similar logic to perform the XDP prog.
>
> Therefore, this PATCH separates the code of executing XDP, which is
> conducive to later maintenance.
>
> Signed-off-by: Xuan Zhuo
> ---
> drivers/net/virtio_net.c | 142 +--
> 1 file changed, 75 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index bb426958cdd4..72b9d6ee4024 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -301,6 +301,15 @@ struct padded_vnet_hdr {
> char padding[12];
> };
>
> +enum {
> + /* xdp pass */
> + VIRTNET_XDP_RES_PASS,
> + /* drop packet. the caller needs to release the page. */
> + VIRTNET_XDP_RES_DROP,
> + /* packet is consumed by xdp. the caller needs to do nothing. */
> + VIRTNET_XDP_RES_CONSUMED,
> +};
I'd prefer this to be done on top unless it is a must. But I don't see
any advantage of introducing this, it's partial mapping of XDP action
and it needs to be extended when XDP action is extended. (And we've
already had: VIRTIO_XDP_REDIR and VIRTIO_XDP_TX ...)
> +
> static void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf);
> static void virtnet_sq_free_unused_buf(struct virtqueue *vq, void *buf);
>
> @@ -789,6 +798,59 @@ static int virtnet_xdp_xmit(struct net_device *dev,
> return ret;
> }
>
> +static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff
> *xdp,
> + struct net_device *dev,
> + unsigned int *xdp_xmit,
> + struct virtnet_rq_stats *stats)
> +{
> + struct xdp_frame *xdpf;
> + int err;
> + u32 act;
> +
> + act = bpf_prog_run_xdp(xdp_prog, xdp);
> + stats->xdp_packets++;
> +
> + switch (act) {
> + case XDP_PASS:
> + return VIRTNET_XDP_RES_PASS;
> +
> + case XDP_TX:
> + stats->xdp_tx++;
> + xdpf = xdp_convert_buff_to_frame(xdp);
> + if (unlikely(!xdpf))
> + return VIRTNET_XDP_RES_DROP;
> +
> + err = virtnet_xdp_xmit(dev, 1, , 0);
> + if (unlikely(!err)) {
> + xdp_return_frame_rx_napi(xdpf);
> + } else if (unlikely(err < 0)) {
> + trace_xdp_exception(dev, xdp_prog, act);
> + return VIRTNET_XDP_RES_DROP;
> + }
> +
> + *xdp_xmit |= VIRTIO_XDP_TX;
> + return VIRTNET_XDP_RES_CONSUMED;
> +
> + case XDP_REDIRECT:
> + stats->xdp_redirects++;
> + err = xdp_do_redirect(dev, xdp, xdp_prog);
> + if (err)
> + return VIRTNET_XDP_RES_DROP;
> +
> + *xdp_xmit |= VIRTIO_XDP_REDIR;
> + return VIRTNET_XDP_RES_CONSUMED;
> +
> + default:
> + bpf_warn_invalid_xdp_action(dev, xdp_prog, act);
> + fallthrough;
> + case XDP_ABORTED:
> + trace_xdp_exception(dev, xdp_prog, act);
> + fallthrough;
> + case XDP_DROP:
> + return VIRTNET_XDP_RES_DROP;
> + }
> +}
> +
> static unsigned int virtnet_get_headroom(struct virtnet_info *vi)
> {
> return vi->xdp_enabled ? VIRTIO_XDP_HEADROOM : 0;
> @@ -876,7 +938,6 @@ static struct sk_buff *receive_small(struct net_device
> *dev,
> struct page *page = virt_to_head_page(buf);
> unsigned int delta = 0;
> struct page *xdp_page;
> - int err;
> unsigned int metasize = 0;
>
> len -= vi->hdr_len;
> @@ -898,7 +959,6 @@ static struct sk_buff *receive_small(struct net_device
> *dev,
> xdp_prog = rcu_dereference(rq->xdp_prog);
> if (xdp_prog) {
> struct virtio_net_hdr_mrg_rxbuf *hdr = buf + header_offset;
> - struct xdp_frame *xdpf;
> struct xdp_buff xdp;
> void *orig_data;
> u32 act;
> @@ -931,46 +991,22 @@ static struct sk_buff *receive_small(struct net_device
> *dev,
> xdp_prepare_buff(, buf + VIRTNET_RX_PAD + vi->hdr_len,
> xdp_headroom, len, true);
> orig_data = xdp.data;
> - act = bpf_prog_run_xdp(xdp_prog, );
> - stats->xdp_packets++;
> +
> + act = virtnet_xdp_handler(xdp_prog, , dev, xdp_xmit,
> stats);
>
> switch (act) {
> - case XDP_PASS:
> + case VIRTNET_XDP_RES_PASS:
> /* Recalculate length in case bpf program changed it
> */
> delta = orig_data - xdp.data;
> len = xdp.data_end - xdp.data;
> metasize = xdp.data - xdp.data_meta;
> break;
> - case XDP_TX:
> -
[PATCH resend 1/2] vdpa/snet: support getting and setting VQ state
This patch adds the get_vq_state and set_vq_state vDPA callbacks.
In order to get the VQ state, the state needs to be read from the DPU.
In order to allow that, the old messaging mechanism is replaced with a new,
flexible control mechanism.
This mechanism allows to read data from the DPU.
The mechanism can be used if the negotiated config version is 2 or
higher.
If the new mechanism is used when the config version is 1, it will call
snet_send_ctrl_msg_old, which is config 1 compatible.
Signed-off-by: Alvaro Karsz
---
drivers/vdpa/solidrun/Makefile | 1 +
drivers/vdpa/solidrun/snet_ctrl.c | 318 +
drivers/vdpa/solidrun/snet_hwmon.c | 2 +-
drivers/vdpa/solidrun/snet_main.c | 111 --
drivers/vdpa/solidrun/snet_vdpa.h | 17 +-
5 files changed, 378 insertions(+), 71 deletions(-)
create mode 100644 drivers/vdpa/solidrun/snet_ctrl.c
diff --git a/drivers/vdpa/solidrun/Makefile b/drivers/vdpa/solidrun/Makefile
index c0aa3415bf7..9116252cd5f 100644
--- a/drivers/vdpa/solidrun/Makefile
+++ b/drivers/vdpa/solidrun/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_SNET_VDPA) += snet_vdpa.o
snet_vdpa-$(CONFIG_SNET_VDPA) += snet_main.o
+snet_vdpa-$(CONFIG_SNET_VDPA) += snet_ctrl.o
ifdef CONFIG_HWMON
snet_vdpa-$(CONFIG_SNET_VDPA) += snet_hwmon.o
endif
diff --git a/drivers/vdpa/solidrun/snet_ctrl.c
b/drivers/vdpa/solidrun/snet_ctrl.c
new file mode 100644
index 000..54909549a00
--- /dev/null
+++ b/drivers/vdpa/solidrun/snet_ctrl.c
@@ -0,0 +1,318 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * SolidRun DPU driver for control plane
+ *
+ * Copyright (C) 2022-2023 SolidRun
+ *
+ * Author: Alvaro Karsz
+ *
+ */
+
+#include
+
+#include "snet_vdpa.h"
+
+enum snet_ctrl_opcodes {
+ SNET_CTRL_OP_DESTROY = 1,
+ SNET_CTRL_OP_READ_VQ_STATE,
+};
+
+#define SNET_CTRL_TIMEOUT 200
+
+#define SNET_CTRL_DATA_SIZE_MASK 0x
+#define SNET_CTRL_IN_PROCESS_MASK 0x0001
+#define SNET_CTRL_CHUNK_RDY_MASK 0x0002
+#define SNET_CTRL_ERROR_MASK 0x0FFC
+
+#define SNET_VAL_TO_ERR(val) (-(((val) & SNET_CTRL_ERROR_MASK) >>
18))
+#define SNET_EMPTY_CTRL(val) (((val) & SNET_CTRL_ERROR_MASK) || \
+ !((val) &
SNET_CTRL_IN_PROCESS_MASK))
+#define SNET_DATA_READY(val) ((val) & (SNET_CTRL_ERROR_MASK |
SNET_CTRL_CHUNK_RDY_MASK))
+
+/* Control register used to read data from the DPU */
+struct snet_ctrl_reg_ctrl {
+ /* Chunk size in 4B words */
+ u16 data_size;
+ /* We are in the middle of a command */
+ u16 in_process:1;
+ /* A data chunk is ready and can be consumed */
+ u16 chunk_ready:1;
+ /* Error code */
+ u16 error:10;
+ /* Saved for future usage */
+ u16 rsvd:4;
+};
+
+/* Opcode register */
+struct snet_ctrl_reg_op {
+ u16 opcode;
+ /* Only if VQ index is relevant for the command */
+ u16 vq_idx;
+};
+
+struct snet_ctrl_regs {
+ struct snet_ctrl_reg_op op;
+ struct snet_ctrl_reg_ctrl ctrl;
+ u32 rsvd;
+ u32 data[];
+};
+
+static struct snet_ctrl_regs __iomem *snet_get_ctrl(struct snet *snet)
+{
+ return snet->bar + snet->psnet->cfg.ctrl_off;
+}
+
+static int snet_wait_for_empty_ctrl(struct snet_ctrl_regs __iomem *regs)
+{
+ u32 val;
+
+ return readx_poll_timeout(ioread32, >ctrl, val,
SNET_EMPTY_CTRL(val), 10,
+ SNET_CTRL_TIMEOUT);
+}
+
+static int snet_wait_for_empty_op(struct snet_ctrl_regs __iomem *regs)
+{
+ u32 val;
+
+ return readx_poll_timeout(ioread32, >op, val, !val, 10,
SNET_CTRL_TIMEOUT);
+}
+
+static int snet_wait_for_data(struct snet_ctrl_regs __iomem *regs)
+{
+ u32 val;
+
+ return readx_poll_timeout(ioread32, >ctrl, val,
SNET_DATA_READY(val), 10,
+ SNET_CTRL_TIMEOUT);
+}
+
+static u32 snet_read32_word(struct snet_ctrl_regs __iomem *ctrl_regs, u16
word_idx)
+{
+ return ioread32(_regs->data[word_idx]);
+}
+
+static u32 snet_read_ctrl(struct snet_ctrl_regs __iomem *ctrl_regs)
+{
+ return ioread32(_regs->ctrl);
+}
+
+static void snet_write_ctrl(struct snet_ctrl_regs __iomem *ctrl_regs, u32 val)
+{
+ iowrite32(val, _regs->ctrl);
+}
+
+static void snet_write_op(struct snet_ctrl_regs __iomem *ctrl_regs, u32 val)
+{
+ iowrite32(val, _regs->op);
+}
+
+static int snet_wait_for_dpu_completion(struct snet_ctrl_regs __iomem
*ctrl_regs)
+{
+ /* Wait until the DPU finishes completely.
+* It will clear the opcode register.
+*/
+ return snet_wait_for_empty_op(ctrl_regs);
+}
+
+/* Reading ctrl from the DPU:
+ * buf_size must be 4B aligned
+ *
+ * Steps:
+ *
+ * (1) Verify that the DPU is not in the middle of another operation by
+ * reading the in_process and error bits in the control register.
+ * (2) Write the request opcode and the VQ idx in
[PATCH resend 2/2] vdpa/snet: support the suspend vDPA callback
When suspend is called, the driver sends a suspend command to the DPU
through the control mechanism.
Signed-off-by: Alvaro Karsz
---
drivers/vdpa/solidrun/snet_ctrl.c | 6 ++
drivers/vdpa/solidrun/snet_main.c | 15 +++
drivers/vdpa/solidrun/snet_vdpa.h | 1 +
3 files changed, 22 insertions(+)
diff --git a/drivers/vdpa/solidrun/snet_ctrl.c
b/drivers/vdpa/solidrun/snet_ctrl.c
index 54909549a00..7d08e6f 100644
--- a/drivers/vdpa/solidrun/snet_ctrl.c
+++ b/drivers/vdpa/solidrun/snet_ctrl.c
@@ -15,6 +15,7 @@
enum snet_ctrl_opcodes {
SNET_CTRL_OP_DESTROY = 1,
SNET_CTRL_OP_READ_VQ_STATE,
+ SNET_CTRL_OP_SUSPEND,
};
#define SNET_CTRL_TIMEOUT 200
@@ -316,3 +317,8 @@ int snet_read_vq_state(struct snet *snet, u16 idx, struct
vdpa_vq_state *state)
return snet_ctrl_read_from_dpu(snet, SNET_CTRL_OP_READ_VQ_STATE, idx,
state,
sizeof(*state));
}
+
+int snet_suspend_dev(struct snet *snet)
+{
+ return snet_send_ctrl_msg(snet, SNET_CTRL_OP_SUSPEND, 0);
+}
diff --git a/drivers/vdpa/solidrun/snet_main.c
b/drivers/vdpa/solidrun/snet_main.c
index daeb69d00ed..96830e58211 100644
--- a/drivers/vdpa/solidrun/snet_main.c
+++ b/drivers/vdpa/solidrun/snet_main.c
@@ -483,6 +483,20 @@ static void snet_set_config(struct vdpa_device *vdev,
unsigned int offset,
iowrite8(*buf_ptr++, cfg_ptr + i);
}
+static int snet_suspend(struct vdpa_device *vdev)
+{
+ struct snet *snet = vdpa_to_snet(vdev);
+ int ret;
+
+ ret = snet_suspend_dev(snet);
+ if (ret)
+ SNET_ERR(snet->pdev, "SNET[%u] suspend failed, err: %d\n",
snet->sid, ret);
+ else
+ SNET_DBG(snet->pdev, "Suspend SNET[%u] device\n", snet->sid);
+
+ return ret;
+}
+
static const struct vdpa_config_ops snet_config_ops = {
.set_vq_address = snet_set_vq_address,
.set_vq_num = snet_set_vq_num,
@@ -508,6 +522,7 @@ static const struct vdpa_config_ops snet_config_ops = {
.set_status = snet_set_status,
.get_config = snet_get_config,
.set_config = snet_set_config,
+ .suspend= snet_suspend,
};
static int psnet_open_pf_bar(struct pci_dev *pdev, struct psnet *psnet)
diff --git a/drivers/vdpa/solidrun/snet_vdpa.h
b/drivers/vdpa/solidrun/snet_vdpa.h
index a4213e97cfc..6dbb95fe2b7 100644
--- a/drivers/vdpa/solidrun/snet_vdpa.h
+++ b/drivers/vdpa/solidrun/snet_vdpa.h
@@ -201,5 +201,6 @@ void psnet_create_hwmon(struct pci_dev *pdev);
void snet_ctrl_clear(struct snet *snet);
int snet_destroy_dev(struct snet *snet);
int snet_read_vq_state(struct snet *snet, u16 idx, struct vdpa_vq_state
*state);
+int snet_suspend_dev(struct snet *snet);
#endif //_SNET_VDPA_H_
--
2.34.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH resend 0/2] vdpa/snet: support [s/g]et_vq_state and suspend
Add more vDPA callbacks. [s/g]et_vq_state is added in patch 1, including a new control mechanism to read data from the DPU. suspend is added in patch 2. Alvaro Karsz (2): vdpa/snet: support getting and setting VQ state vdpa/snet: support the suspend vDPA callback drivers/vdpa/solidrun/Makefile | 1 + drivers/vdpa/solidrun/snet_ctrl.c | 324 + drivers/vdpa/solidrun/snet_hwmon.c | 2 +- drivers/vdpa/solidrun/snet_main.c | 126 ++- drivers/vdpa/solidrun/snet_vdpa.h | 18 +- 5 files changed, 400 insertions(+), 71 deletions(-) create mode 100644 drivers/vdpa/solidrun/snet_ctrl.c -- 2.34.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 0/2] vdpa/snet: support [s/g]et_vq_state and suspend
On Sun, Apr 02, 2023 at 08:36:20AM +, Alvaro Karsz wrote: > > tagged. in the future pls CC everyone on the cover letter too. > > Ok, thanks. > > I'm not getting responses, should I resend it? Hmm maybe. Remember to add "PATCH resend" in the subject. git format-patch has a --subject-prefix for this. -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 0/2] vdpa/snet: support [s/g]et_vq_state and suspend
> tagged. in the future pls CC everyone on the cover letter too. Ok, thanks. I'm not getting responses, should I resend it? ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v6] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support
Hi Viktor,
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 4c3bb0ddeb9b..f9c6604352b4 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -2752,6 +2752,23 @@ void vring_del_virtqueue(struct virtqueue *_vq)
> }
> EXPORT_SYMBOL_GPL(vring_del_virtqueue);
>
> +u32 vring_notification_data(struct virtqueue *_vq)
> +{
> + struct vring_virtqueue *vq = to_vvq(_vq);
> + u16 next;
> +
> + if (vq->packed_ring)
> + next = (vq->packed.next_avail_idx &
> + ~(-(1 << VRING_PACKED_EVENT_F_WRAP_CTR))) |
> + vq->packed.avail_wrap_counter <<
> + VRING_PACKED_EVENT_F_WRAP_CTR;
> + else
> + next = vq->split.avail_idx_shadow;
> +
> + return next << 16 | _vq->index;
> +}
> +EXPORT_SYMBOL_GPL(vring_notification_data);
> +
> /* Manipulates transport-specific feature bits. */
> void vring_transport_features(struct virtio_device *vdev)
> {
> @@ -2771,6 +2788,8 @@ void vring_transport_features(struct virtio_device
> *vdev)
> break;
> case VIRTIO_F_ORDER_PLATFORM:
> break;
> + case VIRTIO_F_NOTIFICATION_DATA:
> + break;
This function is used by virtio_vdpa as well
(drivers/virtio/virtio_vdpa.c:virtio_vdpa_finalize_features).
A vDPA device can offer this feature and it will be accepted, even though
VIRTIO_F_NOTIFICATION_DATA is not a thing for the vDPA transport at the moment.
I don't know if this is bad, since offering VIRTIO_F_NOTIFICATION_DATA is
meaningless for a vDPA device at the moment.
I submitted a patch adding support for vDPA transport.
https://lore.kernel.org/virtualization/20230402081034.1021886-1-alvaro.ka...@solid-run.com/T/#u
> default:
> /* We don't understand this bit. */
> __virtio_clear_bit(vdev, i);
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] virtio-vdpa: add VIRTIO_F_NOTIFICATION_DATA feature support
Add VIRTIO_F_NOTIFICATION_DATA support for vDPA transport.
If this feature is negotiated, the driver passes extra data when kicking
a virtqueue.
A device that offers this feature needs to implement the
kick_vq_with_data callback.
kick_vq_with_data receives the vDPA device and data.
data includes the vqn, next_off and next_wrap for packed virtqueues.
This patch follows a patch [1] by Viktor Prutyanov which adds support
for the MMIO, channel I/O and modern PCI transports.
This patch needs to be applied on top of Viktor's patch.
[1] https://lore.kernel.org/lkml/20230324195029.2410503-1-vik...@daynix.com/
Signed-off-by: Alvaro Karsz
---
drivers/virtio/virtio_vdpa.c | 20 ++--
include/linux/vdpa.h | 6 ++
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
index d7f5af62dda..bdaf30f7fbf 100644
--- a/drivers/virtio/virtio_vdpa.c
+++ b/drivers/virtio/virtio_vdpa.c
@@ -112,6 +112,17 @@ static bool virtio_vdpa_notify(struct virtqueue *vq)
return true;
}
+static bool virtio_vdpa_notify_with_data(struct virtqueue *vq)
+{
+ struct vdpa_device *vdpa = vd_get_vdpa(vq->vdev);
+ const struct vdpa_config_ops *ops = vdpa->config;
+ u32 data = vring_notification_data(vq);
+
+ ops->kick_vq_with_data(vdpa, data);
+
+ return true;
+}
+
static irqreturn_t virtio_vdpa_config_cb(void *private)
{
struct virtio_vdpa_device *vd_dev = private;
@@ -138,6 +149,7 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned
int index,
struct device *dma_dev;
const struct vdpa_config_ops *ops = vdpa->config;
struct virtio_vdpa_vq_info *info;
+ bool (*notify)(struct virtqueue *vq);
struct vdpa_callback cb;
struct virtqueue *vq;
u64 desc_addr, driver_addr, device_addr;
@@ -154,6 +166,11 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned
int index,
if (index >= vdpa->nvqs)
return ERR_PTR(-ENOENT);
+ if (__virtio_test_bit(vdev, VIRTIO_F_NOTIFICATION_DATA))
+ notify = virtio_vdpa_notify_with_data;
+ else
+ notify = virtio_vdpa_notify;
+
/* Queue shouldn't already be set up. */
if (ops->get_vq_ready(vdpa, index))
return ERR_PTR(-ENOENT);
@@ -183,8 +200,7 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned
int index,
dma_dev = vdpa_get_dma_dev(vdpa);
vq = vring_create_virtqueue_dma(index, max_num, align, vdev,
true, may_reduce_num, ctx,
- virtio_vdpa_notify, callback,
- name, dma_dev);
+ notify, callback, name, dma_dev);
if (!vq) {
err = -ENOMEM;
goto error_new_virtqueue;
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 43f59ef10cc..a83bb0501c5 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -143,6 +143,11 @@ struct vdpa_map_file {
* @kick_vq: Kick the virtqueue
* @vdev: vdpa device
* @idx: virtqueue index
+ * @kick_vq_with_data: Kick the virtqueue and supply extra data
+ * (only if VIRTIO_F_NOTIFICATION_DATA is
negotiated)
+ * @vdev: vdpa device
+ * @data: includes vqn, next_off and next_wrap for
+ * packed virtqueues
* @set_vq_cb: Set the interrupt callback function for
* a virtqueue
* @vdev: vdpa device
@@ -300,6 +305,7 @@ struct vdpa_config_ops {
u64 device_area);
void (*set_vq_num)(struct vdpa_device *vdev, u16 idx, u32 num);
void (*kick_vq)(struct vdpa_device *vdev, u16 idx);
+ void (*kick_vq_with_data)(struct vdpa_device *vdev, u32 data);
void (*set_vq_cb)(struct vdpa_device *vdev, u16 idx,
struct vdpa_callback *cb);
void (*set_vq_ready)(struct vdpa_device *vdev, u16 idx, bool ready);
--
2.34.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
