From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 790888D8C for ; Fri, 16 Oct 2015 04:23:23 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga103.jf.intel.com with ESMTP; 15 Oct 2015 19:23:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,687,1437462000"; d="scan'208";a="581826448" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.66.49]) by FMSMGA003.fm.intel.com with ESMTP; 15 Oct 2015 19:23:20 -0700 Date: Fri, 16 Oct 2015 10:24:38 +0800 From: Yuanhan Liu To: "Michael S. Tsirkin" , Marcel Apfelbaum Message-ID: <20151016022438.GH3115@yliu-dev.sh.intel.com> References: <1444907319-26348-1-git-send-email-marcel@redhat.com> <20151015161150-mutt-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151015161150-mutt-send-email-mst@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: 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 02:23:24 -0000 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. > 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 | +-------------------+-----------------+------------------------------+ 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. 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. 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? 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. --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