From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 8A9638D8E for ; Fri, 16 Oct 2015 08:20:23 +0200 (CEST) Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 5FB8D8F2E4; Fri, 16 Oct 2015 06:20:22 +0000 (UTC) Received: from redhat.com (ovpn-116-130.ams2.redhat.com [10.36.116.130]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with SMTP id t9G6KI9q004893; Fri, 16 Oct 2015 02:20:19 -0400 Date: Fri, 16 Oct 2015 09:20:18 +0300 From: "Michael S. Tsirkin" To: Yuanhan Liu Message-ID: <20151016091327-mutt-send-email-mst@redhat.com> References: <1444907319-26348-1-git-send-email-marcel@redhat.com> <20151015161150-mutt-send-email-mst@redhat.com> <20151016022438.GH3115@yliu-dev.sh.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151016022438.GH3115@yliu-dev.sh.intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 Cc: Marcel Apfelbaum , dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] vhost-user: enable virtio 1.0 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 16 Oct 2015 06:20:23 -0000 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, that's actually one of my TODOs in this quarter. So, thank > you! :) > > > > > Looks good to me > > > > Acked-by: Michael S. Tsirkin > > > > 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