From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from cmailsend08.nm.naver.com (cmailsend08.nm.naver.com [125.209.208.217]) by dpdk.org (Postfix) with ESMTP id 443B5902 for ; Fri, 16 Oct 2015 08:33:42 +0200 (CEST) Received: (qmail 19849 invoked by uid 100); 16 Oct 2015 06:33:41 -0000 Received: from 10.114.49.74 (HELO cweb07.nm.nhnsystem.com) (10.114.49.74) by cmailsend08.nm.naver.com with SMTP;16 Oct 2015 06:33:41 -0000 Date: Fri, 16 Oct 2015 15:33:41 +0900 (KST) From: =?UTF-8?B?7LWc7J217ISx?= To: dev@dpdk.org Message-ID: MIME-Version: 1.0 Importance: normal X-Priority: 3 (Normal) X-Naver-CIP: 129.254.191.249 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: base64 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: [dpdk-dev] =?utf-8?q?_Would_you_check_if_there_is_deadlock_in_rte?= =?utf-8?q?=5Fdistributor=5Fprocess=28=29_librte=5Fdistributor=3F?= X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: =?UTF-8?B?7LWc7J217ISx?= 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:33:44 -0000 RGVhciBEUERLIGV4cGVydHMuCiAKVGhhbmsgeW91IHZlcnkgbXVjaCBmb3IgeW91ciBleGNlbGxl bnQgd29yayBhbmQgZ3JlYXQgY29udHJpYnV0aW9ucy4KIAogCkkgaGF2ZSBhIHF1ZXN0aW9uIGFi b3V0IGRpc3RyaWJ1dG9yIGxpYnJhcnkgc291cmNlIGNvZGUuICAgL2RwZGsvbGliL2xpYnJ0ZV9k aXN0cmlidXRvci9ydGVfZGlzdHJpYnV0b3IuYwogCkluIHJ0ZV9kaXN0cmlidXRvcl9wcm9jZXNz KCksCiAKcnRlX2Rpc3RyaWJ1dG9yX3Byb2Nlc3MoKSB7IG1idWZzW10gLSZndDsgZC0mZ3Q7YmFj a2xvZyAtJmd0OyBkLSZndDtidWZzIH0gICA9Jmd0OyBsY29yZV93b3JrZXIoKSByZWFkcyBkLSZn dDtidWZzICh3aXRoIDB4MCBlbXB0eSBmbGFnIHBhY2tldCkgYW5kIHNldHMgcmVzcG9uc2UgYml0 KFJURV9ESVNUUklCX0dFVF9CVUYpIG9mIGEgcmVjZWl2ZWQgcGFja2V0IChkLSZndDtidWZzKS4K IApXaGVuIHRoZSBmaXJzdCBwYWNrZXQgYXJyaXZlcyBhdCBydGVfZGlzdHJpYnV0b3JfcHJvY2Vz cygpLCB0aGUgbWF0Y2ggdmFsdWUgaXMgMC4KQW5kIGl0IGlzIG5vdCBlbnF1ZXVlZCBpbiBkLSZn dDtiYWNrbG9nIG5vciBpbiBkLSZndDtidWZzICAgIGJlY2F1c2UgdGhlIG1hdGNoIHZhbHVlIGVx dWFscyAwIC4KIApUaGUgcmVjZWl2ZWQgZmlyc3QgcGFja2V0IGlzIG5ldmVyIGVucXVldWVkIHRv IGQtJmd0O2J1ZnMgdW50aWwgdGhlIGxjb3JlX3dvcmtlcigpIHJldHVybnMgdGhlIHBhY2tldCBi dWZzICh3aXRoIHRoZSByZXNwb25zZSBiaXQoUlRFX0RJU1RSSUJfR0VUX0JVRikgb2YgYnVmcyBw YWNrZXQgaXMgc2V0KS4KIApTaW5jZSB0aGUgbGNvcmVfd29ya2VyIGNhbm5vdCByZXR1cm4gcGFj a2V0IGlmIGl0IGNhbm5vdCByZWNlaXZlIHBhY2tldCAoU2luY2UgdGhlIGxjb3JlX3dvcmtlciBv bmx5IHJlYWRzIHRoZSByZWNlaXZlZCBwYWNrZXQgd2l0aCAweDAgZmxhZykuCiAKSXQgc2VlbXMg dG8gYmUgYSBkZWFkbG9jay4KIApUaHVzLiBmaXJzdCBwYWNrZXQgaXMgbmV2ZXIgcHJvY2Vzc2Vk LCBJIHRoaW5rLgogCiAKV291bGQgeW91IGNoZWNrIHRoaXM/IAogCkFtIEkgd3Jvbmc/IAogCkRv ZXMgdGhlIGxjb3JlX3dvcmtlcigpIHJldHVybiBwYWNrZXQgZXZlbiB0aG91Z2ggaXQgZG9lcyBu b3QgaGF2ZSByZWNlaXZlZCBwYWNrZXQ/CiAKIApUaGFuayB5b3UgdmVyeSBtdWNoLiAKIApTaW5j ZXJlbHkgWW91cnMsCiAKSWNrLVN1bmcgQ2hvaS4KIAo= >From yuanhan.liu@linux.intel.com Fri Oct 16 08:36:57 2015 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 49595902 for ; Fri, 16 Oct 2015 08:36:57 +0200 (CEST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga103.fm.intel.com with ESMTP; 15 Oct 2015 23:36:56 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,687,1437462000"; d="scan'208";a="665524841" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.66.49]) by orsmga003.jf.intel.com with ESMTP; 15 Oct 2015 23:36:54 -0700 Date: Fri, 16 Oct 2015 14:38:14 +0800 From: Yuanhan Liu To: "Michael S. Tsirkin" Message-ID: <20151016063814.GI3115@yliu-dev.sh.intel.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> <20151016091327-mutt-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151016091327-mutt-send-email-mst@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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:36:58 -0000 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, 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. 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