From: "Wang, Xiao W" <xiao.w.wang@intel.com>
To: "Bie, Tiwei" <tiwei.bie@intel.com>
Cc: "maxime.coquelin@redhat.com" <maxime.coquelin@redhat.com>,
"dev@dpdk.org" <dev@dpdk.org>,
"Wang, Zhihong" <zhihong.wang@intel.com>,
"Ye, Xiaolong" <xiaolong.ye@intel.com>
Subject: Re: [dpdk-dev] [PATCH 2/9] vhost: provide helpers for virtio ring relay
Date: Wed, 12 Dec 2018 06:51:28 +0000 [thread overview]
Message-ID: <B7F2E978279D1D49A3034B7786DACF407A4B5FA5@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <20181204062241.GA19852@btwcube1>
Hi,
> -----Original Message-----
> From: Bie, Tiwei
> Sent: Monday, December 3, 2018 10:23 PM
> To: Wang, Xiao W <xiao.w.wang@intel.com>
> Cc: maxime.coquelin@redhat.com; dev@dpdk.org; Wang, Zhihong
> <zhihong.wang@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>
> Subject: Re: [PATCH 2/9] vhost: provide helpers for virtio ring relay
>
> On Wed, Nov 28, 2018 at 05:46:00PM +0800, Xiao Wang wrote:
> [...]
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice
> > + *
> > + * Synchronize the available ring from guest to mediate ring, help to
> > + * check desc validity to protect against malicious guest driver.
> > + *
> > + * @param vid
> > + * vhost device id
> > + * @param qid
> > + * vhost queue id
> > + * @param m_vring
> > + * mediate virtio ring pointer
> > + * @return
> > + * number of synced available entries on success, -1 on failure
> > + */
> > +int __rte_experimental
> > +rte_vdpa_relay_avail_ring(int vid, int qid, struct vring *m_vring);
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice
> > + *
> > + * Synchronize the used ring from mediate ring to guest, log dirty
> > + * page for each Rx buffer used.
> > + *
> > + * @param vid
> > + * vhost device id
> > + * @param qid
> > + * vhost queue id
> > + * @param m_vring
> > + * mediate virtio ring pointer
> > + * @return
> > + * number of synced used entries on success, -1 on failure
> > + */
> > +int __rte_experimental
> > +rte_vdpa_relay_used_ring(int vid, int qid, struct vring *m_vring);
>
> Above APIs are split ring specific. We also need to take
> packed ring into consideration.
After some study on the current packed ring description, several ideas:
1. These APIs are used as helpers to setup a mediate relay layer to help do dirty page logging, we may not need
this kind of ring relay for packed ring at all. The target of a mediate SW layer is to help device do dirty page
logging, so this SW-assisted VDPA tries to find a way to intercept the frontend-backend communication, as you
can see in this patch set, SW captures the device interrupt and then parse the vring and log dirty page
afterwards. We set up this mediate vring to make sure the relay SW can intercept the device interrupt, as you
know, this way we can control the mediate vring's interrupt suppression structure.
2.One new point about the packed ring is that it separates out the event suppression structure from the
description ring. So in this case, we can just set up a mediate event suppression structure to intercept event
notification.
BTW, I find one troublesome point about the packed ring is that it's hard for a mediate SW to quickly handle the
"buffer id", guest virtio driver understands this id well, it keeps some internal info about each id, e.g. chain list
length, but the relay SW has to parse the packed ring again, which is not efficient.
3. In the split vring, relay SW reuses the guest desc vring, and desc is not writed by DMA, so no log for the desc.
But in the packed vring, desc is writed by DMA, desc ring's logging is a new thing.
Packed ring is quite different, it could be a very different mechanism, other than following a vring relay API. Also
from testing point of view, if we come out with a new efficient implementation for packed ring VDPA, it's hard to
test it with HW. Testing need a HW supporting packed ring DMA and the get_vring_base/set_vring_base
interface.
>
> > #endif /* _RTE_VDPA_H_ */
> [...]
> > diff --git a/lib/librte_vhost/vdpa.c b/lib/librte_vhost/vdpa.c
> > index e7d849ee0..e41117776 100644
> > --- a/lib/librte_vhost/vdpa.c
> > +++ b/lib/librte_vhost/vdpa.c
> > @@ -122,3 +122,176 @@ rte_vdpa_get_device_num(void)
> > {
> > return vdpa_device_num;
> > }
> > +
> > +static int
> > +invalid_desc_check(struct virtio_net *dev, struct vhost_virtqueue *vq,
> > + uint64_t desc_iova, uint64_t desc_len, uint8_t perm)
> > +{
> > + uint64_t desc_addr, desc_chunck_len;
> > +
> > + while (desc_len) {
> > + desc_chunck_len = desc_len;
> > + desc_addr = vhost_iova_to_vva(dev, vq,
> > + desc_iova,
> > + &desc_chunck_len,
> > + perm);
> > +
> > + if (!desc_addr)
> > + return -1;
> > +
> > + desc_len -= desc_chunck_len;
> > + desc_iova += desc_chunck_len;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int
> > +rte_vdpa_relay_avail_ring(int vid, int qid, struct vring *m_vring)
> > +{
> > + struct virtio_net *dev = get_device(vid);
> > + uint16_t idx, idx_m, desc_id;
> > + struct vring_desc desc;
> > + struct vhost_virtqueue *vq;
> > + struct vring_desc *desc_ring;
> > + struct vring_desc *idesc = NULL;
> > + uint64_t dlen;
> > + int ret;
> > +
> > + if (!dev)
> > + return -1;
> > +
> > + vq = dev->virtqueue[qid];
>
> Better to also validate qid.
>
> > + idx = vq->avail->idx;
> > + idx_m = m_vring->avail->idx;
> > + ret = idx - idx_m;
>
> Need to cast (idx - idx_m) to uint16_t.
>
> > +
> > + while (idx_m != idx) {
> > + /* avail entry copy */
> > + desc_id = vq->avail->ring[idx_m % vq->size];
>
> idx_m & (vq->size - 1) should be faster.
>
> > + m_vring->avail->ring[idx_m % vq->size] = desc_id;
> > + desc_ring = vq->desc;
> > +
> > + if (vq->desc[desc_id].flags & VRING_DESC_F_INDIRECT) {
> > + dlen = vq->desc[desc_id].len;
> > + desc_ring = (struct vring_desc *)(uintptr_t)
> > + vhost_iova_to_vva(dev, vq, vq->desc[desc_id].addr,
>
> The indent needs to be fixed.
>
> > + &dlen,
> > + VHOST_ACCESS_RO);
> > + if (unlikely(!desc_ring))
> > + return -1;
> > +
> > + if (unlikely(dlen < vq->desc[idx].len)) {
> > + idesc = alloc_copy_ind_table(dev, vq,
> > + vq->desc[idx].addr, vq->desc[idx].len);
> > + if (unlikely(!idesc))
> > + return -1;
> > +
> > + desc_ring = idesc;
> > + }
> > +
> > + desc_id = 0;
> > + }
> > +
> > + /* check if the buf addr is within the guest memory */
> > + do {
> > + desc = desc_ring[desc_id];
> > + if (invalid_desc_check(dev, vq, desc.addr, desc.len,
> > + VHOST_ACCESS_RW))
>
> Should check with < 0, otherwise should return bool.
>
> We may just have RO access.
The desc may refers to a transmit buffer as well as receive buffer. Agree on the comments and nice catches elsewhere above, will send new version.
[...]
BRs,
Xiao
next prev parent reply other threads:[~2018-12-12 6:51 UTC|newest]
Thread overview: 86+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-28 9:45 [dpdk-dev] [PATCH 0/9] support SW assisted VDPA live migration Xiao Wang
2018-11-28 9:45 ` [dpdk-dev] [PATCH 1/9] vhost: provide helper for host notifier ctrl Xiao Wang
2018-11-28 9:46 ` [dpdk-dev] [PATCH 2/9] vhost: provide helpers for virtio ring relay Xiao Wang
2018-12-04 6:22 ` Tiwei Bie
2018-12-12 6:51 ` Wang, Xiao W [this message]
2018-12-13 1:10 ` [dpdk-dev] [PATCH v2 0/9] support SW assisted VDPA live migration Xiao Wang
2018-12-13 1:10 ` [dpdk-dev] [PATCH v2 1/9] vhost: provide helper for host notifier ctrl Xiao Wang
2018-12-13 1:10 ` [dpdk-dev] [PATCH v2 2/9] vhost: provide helpers for virtio ring relay Xiao Wang
2018-12-13 10:09 ` [dpdk-dev] [PATCH v3 0/9] support SW assisted VDPA live migration Xiao Wang
2018-12-13 10:09 ` [dpdk-dev] [PATCH v3 1/9] vhost: provide helper for host notifier ctrl Xiao Wang
2018-12-14 13:33 ` Maxime Coquelin
2018-12-14 19:05 ` Wang, Xiao W
2018-12-14 21:16 ` [dpdk-dev] [PATCH v4 00/10] support SW assisted VDPA live migration Xiao Wang
2018-12-14 21:16 ` [dpdk-dev] [PATCH v4 01/10] vhost: remove unused internal API Xiao Wang
2018-12-16 8:58 ` Maxime Coquelin
2018-12-14 21:16 ` [dpdk-dev] [PATCH v4 02/10] vhost: provide helper for host notifier ctrl Xiao Wang
2018-12-16 9:00 ` Maxime Coquelin
2018-12-14 21:16 ` [dpdk-dev] [PATCH v4 03/10] vhost: provide helpers for virtio ring relay Xiao Wang
2018-12-16 9:10 ` Maxime Coquelin
2018-12-17 8:51 ` Wang, Xiao W
2018-12-17 11:02 ` Maxime Coquelin
2018-12-17 14:41 ` Wang, Xiao W
2018-12-17 19:00 ` Maxime Coquelin
2018-12-18 8:27 ` Wang, Xiao W
2018-12-18 8:44 ` Thomas Monjalon
2018-12-18 8:01 ` [dpdk-dev] [PATCH v5 00/10] support SW assisted VDPA live migration Xiao Wang
2018-12-18 8:01 ` [dpdk-dev] [PATCH v5 01/10] vhost: remove unused internal API Xiao Wang
2018-12-18 8:01 ` [dpdk-dev] [PATCH v5 02/10] vhost: provide helper for host notifier ctrl Xiao Wang
2018-12-18 15:37 ` Ferruh Yigit
2018-12-18 8:02 ` [dpdk-dev] [PATCH v5 03/10] vhost: provide helpers for virtio ring relay Xiao Wang
2018-12-18 8:02 ` [dpdk-dev] [PATCH v5 04/10] net/ifc: dump debug message for error Xiao Wang
2018-12-18 8:02 ` [dpdk-dev] [PATCH v5 05/10] net/ifc: store only registered device instance Xiao Wang
2018-12-18 8:02 ` [dpdk-dev] [PATCH v5 06/10] net/ifc: detect if VDPA mode is specified Xiao Wang
2018-12-18 8:02 ` [dpdk-dev] [PATCH v5 07/10] net/ifc: add devarg for LM mode Xiao Wang
2018-12-18 11:23 ` Maxime Coquelin
2018-12-18 8:02 ` [dpdk-dev] [PATCH v5 08/10] net/ifc: use lib API for used ring logging Xiao Wang
2018-12-18 8:02 ` [dpdk-dev] [PATCH v5 09/10] net/ifc: support SW assisted VDPA live migration Xiao Wang
2018-12-18 11:33 ` Maxime Coquelin
2018-12-18 8:02 ` [dpdk-dev] [PATCH v5 10/10] doc: update ifc NIC document Xiao Wang
2018-12-18 11:35 ` Maxime Coquelin
2018-12-14 21:16 ` [dpdk-dev] [PATCH v4 04/10] net/ifc: dump debug message for error Xiao Wang
2018-12-16 9:11 ` Maxime Coquelin
2018-12-14 21:16 ` [dpdk-dev] [PATCH v4 05/10] net/ifc: store only registered device instance Xiao Wang
2018-12-16 9:12 ` Maxime Coquelin
2018-12-14 21:16 ` [dpdk-dev] [PATCH v4 06/10] net/ifc: detect if VDPA mode is specified Xiao Wang
2018-12-16 9:17 ` Maxime Coquelin
2018-12-17 8:54 ` Wang, Xiao W
2018-12-14 21:16 ` [dpdk-dev] [PATCH v4 07/10] net/ifc: add devarg for LM mode Xiao Wang
2018-12-16 9:21 ` Maxime Coquelin
2018-12-17 9:00 ` Wang, Xiao W
2018-12-14 21:16 ` [dpdk-dev] [PATCH v4 08/10] net/ifc: use lib API for used ring logging Xiao Wang
2018-12-16 9:24 ` Maxime Coquelin
2018-12-14 21:16 ` [dpdk-dev] [PATCH v4 09/10] net/ifc: support SW assisted VDPA live migration Xiao Wang
2018-12-16 9:35 ` Maxime Coquelin
2018-12-17 9:12 ` Wang, Xiao W
2018-12-17 11:08 ` Maxime Coquelin
2018-12-14 21:16 ` [dpdk-dev] [PATCH v4 10/10] doc: update ifc NIC document Xiao Wang
2018-12-16 9:36 ` Maxime Coquelin
2018-12-17 9:15 ` Wang, Xiao W
2018-12-18 14:01 ` [dpdk-dev] [PATCH v4 00/10] support SW assisted VDPA live migration Maxime Coquelin
2018-12-13 10:09 ` [dpdk-dev] [PATCH v3 2/9] vhost: provide helpers for virtio ring relay Xiao Wang
2018-12-13 10:09 ` [dpdk-dev] [PATCH v3 3/9] net/ifc: dump debug message for error Xiao Wang
2018-12-13 10:09 ` [dpdk-dev] [PATCH v3 4/9] net/ifc: store only registered device instance Xiao Wang
2018-12-13 10:09 ` [dpdk-dev] [PATCH v3 5/9] net/ifc: detect if VDPA mode is specified Xiao Wang
2018-12-13 10:09 ` [dpdk-dev] [PATCH v3 6/9] net/ifc: add devarg for LM mode Xiao Wang
2018-12-13 10:09 ` [dpdk-dev] [PATCH v3 7/9] net/ifc: use lib API for used ring logging Xiao Wang
2018-12-13 10:09 ` [dpdk-dev] [PATCH v3 8/9] net/ifc: support SW assisted VDPA live migration Xiao Wang
2018-12-13 10:09 ` [dpdk-dev] [PATCH v3 9/9] doc: update ifc NIC document Xiao Wang
2018-12-13 1:10 ` [dpdk-dev] [PATCH v2 3/9] net/ifc: dump debug message for error Xiao Wang
2018-12-13 1:10 ` [dpdk-dev] [PATCH v2 4/9] net/ifc: store only registered device instance Xiao Wang
2018-12-13 1:10 ` [dpdk-dev] [PATCH v2 5/9] net/ifc: detect if VDPA mode is specified Xiao Wang
2018-12-13 1:10 ` [dpdk-dev] [PATCH v2 6/9] net/ifc: add devarg for LM mode Xiao Wang
2018-12-13 1:10 ` [dpdk-dev] [PATCH v2 7/9] net/ifc: use lib API for used ring logging Xiao Wang
2018-12-13 1:10 ` [dpdk-dev] [PATCH v2 8/9] net/ifc: support SW assisted VDPA live migration Xiao Wang
2018-12-13 1:10 ` [dpdk-dev] [PATCH v2 9/9] doc: update ifc NIC document Xiao Wang
2018-11-28 9:46 ` [dpdk-dev] [PATCH 3/9] net/ifc: dump debug message for error Xiao Wang
2018-11-28 9:46 ` [dpdk-dev] [PATCH 4/9] net/ifc: store only registered device instance Xiao Wang
2018-11-28 9:46 ` [dpdk-dev] [PATCH 5/9] net/ifc: detect if VDPA mode is specified Xiao Wang
2018-11-28 9:46 ` [dpdk-dev] [PATCH 6/9] net/ifc: add devarg for LM mode Xiao Wang
2018-12-04 6:31 ` Tiwei Bie
2018-12-12 6:53 ` Wang, Xiao W
2018-12-12 10:15 ` Alejandro Lucero
2018-12-12 10:23 ` Wang, Xiao W
2018-11-28 9:46 ` [dpdk-dev] [PATCH 7/9] net/ifc: use lib API for used ring logging Xiao Wang
2018-11-28 9:46 ` [dpdk-dev] [PATCH 8/9] net/ifc: support SW assisted VDPA live migration Xiao Wang
2018-11-28 9:46 ` [dpdk-dev] [PATCH 9/9] doc: update ifc NIC document Xiao Wang
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=B7F2E978279D1D49A3034B7786DACF407A4B5FA5@SHSMSX101.ccr.corp.intel.com \
--to=xiao.w.wang@intel.com \
--cc=dev@dpdk.org \
--cc=maxime.coquelin@redhat.com \
--cc=tiwei.bie@intel.com \
--cc=xiaolong.ye@intel.com \
--cc=zhihong.wang@intel.com \
/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).