[dpdk-dev] [PATCH] vhost-user: enable virtio 1.0

Andriy Berestovskyy aber at semihalf.com
Fri Oct 16 09:43:09 CEST 2015


Hi guys,
Just a minor note: ARM is bi-endian in fact. For instance, there are
both endians tool chains available on Linaro.

Andriy


On Fri, Oct 16, 2015 at 8:20 AM, Michael S. Tsirkin <mst at redhat.com> 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.
>
>> > 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.
>
>>     +-------------------+-----------------+------------------------------+
>> 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
>
>> 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?
>
>> 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. The trick is to find "legacy interface"
> sections and go over them, that compares 0.9 to 1.0.
>
>>
>> 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.
>
>>       --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



-- 
Andriy Berestovskyy


More information about the dev mailing list