[dpdk-dev] [PATCH] vhost-user: enable virtio 1.0
Yuanhan Liu
yuanhan.liu at linux.intel.com
Fri Oct 16 08:38:14 CEST 2015
On Fri, Oct 16, 2015 at 09:20:18AM +0300, Michael S. Tsirkin wrote:
> On Fri, Oct 16, 2015 at 10:24:38AM +0800, Yuanhan Liu wrote:
> > On Thu, Oct 15, 2015 at 04:18:59PM +0300, Michael S. Tsirkin wrote:
> > > On Thu, Oct 15, 2015 at 02:08:39PM +0300, Marcel Apfelbaum wrote:
> > > > Make vhost-user virtio 1.0 compatible by adding it to the
> > > > supported features and keeping the header length
> > > > the same as for mergeable RX buffers.
> > > >
> > > > Signed-off-by: Marcel Apfelbaum <marcel at redhat.com>
> >
> > Marcel, that's actually one of my TODOs in this quarter. So, thank
> > you! :)
> >
> > >
> > > Looks good to me
> > >
> > > Acked-by: Michael S. Tsirkin <mst at redhat.com>
> > >
> > > Just one question: dpdk is only supported on little-endian
> > > platforms at the moment, right?
> >
> > AFAIK, yes. But you might also see that there are some patch to add
> > ARM arch support showed up in the mailing list few weeks ago.
>
> Luckily, that's also little-endian.
Oops, I never had hands on experience on ARM and I was always thinking
it's big endian. Sigh ... :-/
Anyway, good to know.
>
> > > virtio 1 spec requires little endian.
> >
> > I made a quick list of the difference between virtio v0.95 and v1.0
> > months ago just by reading your kernel commits of adding v1.0 support:
> >
> > +-------------------+-----------------+------------------------------+
> > | | v0.95 | v1.0 |
> > +-------------------+-----------------+------------------------------+
> > 1) | features bits | 32 | 64 |
> > +-------------------+-----------------+------------------------------+
> > 2) | Endianness | nature | Little Endian |
> > +-------------------+-----------------+------------------------------+
> > 3) | vring space | contiguous | avail and used buffer could |
> > | | memory | be on a separate memory |
>
> And desc buffer, too.
Thanks for the remind.
>
> > +-------------------+-----------------+------------------------------+
> > 4) | FEATURE_OK status | No | Yes |
> > +-------------------+-----------------+------------------------------+
> >
> >
> >
> > For 1), I guess we have been using 64 bit for storing features bits
> > for vhost since long time ago. So, there should be no extra effort.
> >
> > For 2), as stated, there might be no issue as far as DPDK is little
> > endian only. But we'd better add a wrapper for that, as it seems
> > adding big endian support would come in near future.
>
> OK, but it probably doesn't
Yeah, let's handle that later.
>
> > For 3), are we actually doing that? I just saw that there is a kernel
> > patch to introduce two functions for getting the avail and used buffer
> > address, respectively. But I didn't see that the two buffer are
> > allocated in non-contiguous memory.
>
> That will soon happen, anyone claiming support for virtio 1
>
> But vhost user already sends each ring part separately.
> Does dpdk assume they are contigious?
Oh, right, we don't assume that. See set_vring_addr() ib/librte_vhost/virtio-net.c.
We get the address of avail buffer, used buffere and desc buffer one by
one.
>
> > For 4), it's a work we should do at virtio PMD driver. And it seems
> > that there are far more work need to be done at virtio PDM driver than
> > at vhost lib, say, adding the virtio morden PCI support.
> >
> > Besides those 4 differs, did I miss anyting?
>
>
> >From virtio PMD point of view? There are more
> differences.
That's true.
> The trick is to find "legacy interface"
> sections and go over them, that compares 0.9 to 1.0.
Thanks for the tip!
>
> >
> > BTW, since we already have same TODOs, I guess it'd be better to
> > share what we have in our TODO list. Here are what I got till the
> > time writing this email (in order of priority):
> >
> > - a vhost performance issue (it might last long; it might not).
> >
> > - vhost-user live migration support
> >
> > - virtio 1.0 support, including PMD and vhost lib (and you guys have
> > already done that :)
> >
> > Thanks.
>
> This patch only touches the vhost lib, though.
Yes, and this patch also looks good to me. I will add Reviewed-by in
another email, to make it oustanding so that Thomas can see that clearly
and pick it up.
--yliu
> >
> >
> > > > ---
> > > >
> > > > To be applied on top of:
> > > > [dpdk-dev] [PATCH v6 00/13] vhost-user multiple queues enabling
> > > >
> > > > Thanks,
> > > > Marcel
> > > >
> > > > lib/librte_vhost/virtio-net.c | 15 ++++++++-------
> > > > 1 file changed, 8 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
> > > > index a51327d..ee4650e 100644
> > > > --- a/lib/librte_vhost/virtio-net.c
> > > > +++ b/lib/librte_vhost/virtio-net.c
> > > > @@ -75,6 +75,7 @@ static struct virtio_net_config_ll *ll_root;
> > > > (1ULL << VIRTIO_NET_F_CTRL_VQ) | \
> > > > (1ULL << VIRTIO_NET_F_CTRL_RX) | \
> > > > (1ULL << VIRTIO_NET_F_MQ) | \
> > > > + (1ULL << VIRTIO_F_VERSION_1) | \
> > > > (1ULL << VHOST_F_LOG_ALL) | \
> > > > (1ULL << VHOST_USER_F_PROTOCOL_FEATURES))
> > > > static uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES;
> > > > @@ -477,17 +478,17 @@ set_features(struct vhost_device_ctx ctx, uint64_t *pu)
> > > > return -1;
> > > >
> > > > dev->features = *pu;
> > > > - if (dev->features & (1 << VIRTIO_NET_F_MRG_RXBUF)) {
> > > > - LOG_DEBUG(VHOST_CONFIG,
> > > > - "(%"PRIu64") Mergeable RX buffers enabled\n",
> > > > - dev->device_fh);
> > > > + if (dev->features &
> > > > + ((1 << VIRTIO_NET_F_MRG_RXBUF) | (1ULL << VIRTIO_F_VERSION_1))) {
> > > > vhost_hlen = sizeof(struct virtio_net_hdr_mrg_rxbuf);
> > > > } else {
> > > > - LOG_DEBUG(VHOST_CONFIG,
> > > > - "(%"PRIu64") Mergeable RX buffers disabled\n",
> > > > - dev->device_fh);
> > > > vhost_hlen = sizeof(struct virtio_net_hdr);
> > > > }
> > > > + LOG_DEBUG(VHOST_CONFIG,
> > > > + "(%"PRIu64") Mergeable RX buffers %s, virtio 1 %s\n",
> > > > + dev->device_fh,
> > > > + (dev->features & (1 << VIRTIO_NET_F_MRG_RXBUF)) ? "on" : "off",
> > > > + (dev->features & (1ULL << VIRTIO_F_VERSION_1)) ? "on" : "off");
> > > >
> > > > for (i = 0; i < dev->virt_qp_nb; i++) {
> > > > uint16_t base_idx = i * VIRTIO_QNUM;
> > > > --
> > > > 2.1.0
More information about the dev
mailing list