From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-f50.google.com (mail-lf0-f50.google.com [209.85.215.50]) by dpdk.org (Postfix) with ESMTP id 1E92C8D8A for ; Fri, 16 Oct 2015 09:43:30 +0200 (CEST) Received: by lfeh64 with SMTP id h64so70493750lfe.3 for ; Fri, 16 Oct 2015 00:43:29 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type; bh=2CfKSrOHP8E8J2a4iCoySES+ZROxZg2YUIwJsQcAMVM=; b=aKlqeaKOHJx03pEdRTM4EaYd2QqpuBEfV5Mn9rAULgWdeivujKSaYo1dTVRW5G4gag Gfd5RhJY8J+dHy4DPS6dSUR47pIntOcbv3sBjc3Duyuz+ypN7SPAuGp3Afs/j6QTWTBe XlKztox6z7m7JFEKiBnY3uFIzuYICBX7iJhNwuQ4Tpn0MxBC47iqzaXDrMNAYqqTON56 1P/NI8wp1UGOOHRq/XLZTwMf0HYMN6wRYjl8QCw4yhC0H1pjbu8T26MaE02IbUIIZvWp i0BcetKv24u70NF+sDqzVx8WdaCYfsTERrhbgwWJ1AEkMjDxJ0m3+uDuTfH8AcDPQrZx bLPQ== X-Gm-Message-State: ALoCoQnHdXNQHJC3tT8oMNHvVNF6Bs3piZCd2gcEpJPHWmdrhOuljbvyIfQjqlZa9MC4SeWVWzlL X-Received: by 10.180.184.232 with SMTP id ex8mr3032941wic.15.1444981409074; Fri, 16 Oct 2015 00:43:29 -0700 (PDT) MIME-Version: 1.0 Received: by 10.27.16.65 with HTTP; Fri, 16 Oct 2015 00:43:09 -0700 (PDT) In-Reply-To: <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> <20151016091327-mutt-send-email-mst@redhat.com> From: Andriy Berestovskyy Date: Fri, 16 Oct 2015 09:43:09 +0200 Message-ID: To: "Michael S. Tsirkin" Content-Type: text/plain; charset=UTF-8 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 07:43:30 -0000 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 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. > >> > 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