DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>, dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] vhost-user: enable virtio 1.0
Date: Fri, 16 Oct 2015 09:20:18 +0300	[thread overview]
Message-ID: <20151016091327-mutt-send-email-mst@redhat.com> (raw)
In-Reply-To: <20151016022438.GH3115@yliu-dev.sh.intel.com>

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@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@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

  reply	other threads:[~2015-10-16  6:20 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-15 11:08 Marcel Apfelbaum
2015-10-15 13:18 ` Michael S. Tsirkin
2015-10-16  2:24   ` Yuanhan Liu
2015-10-16  6:20     ` Michael S. Tsirkin [this message]
2015-10-16  7:43       ` Andriy Berestovskyy
2015-10-16  7:50         ` Yuanhan Liu
2015-10-16  7:56         ` Michael S. Tsirkin
2015-10-16 13:52   ` Bruce Richardson
2015-10-18  7:04     ` Michael S. Tsirkin
2015-10-30 17:48       ` Thomas Monjalon
2015-11-01  9:00         ` Marcel Apfelbaum
2015-11-01  9:53           ` Thomas Monjalon
2015-11-01 11:22             ` Marcel Apfelbaum
2015-11-09 19:55         ` Michael S. Tsirkin
2015-11-02 22:13   ` Thomas Monjalon
2015-11-03  3:45     ` Xu, Qian Q
2015-11-03  3:49       ` Xu, Qian Q
2015-11-03  8:03         ` Marcel Apfelbaum
2015-11-03  8:16           ` Xie, Huawei
2015-11-03  8:26             ` Thomas Monjalon
2015-11-03  8:30               ` Marcel Apfelbaum
2015-11-03  8:31             ` Marcel Apfelbaum
2015-10-16  6:39 ` Yuanhan Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151016091327-mutt-send-email-mst@redhat.com \
    --to=mst@redhat.com \
    --cc=dev@dpdk.org \
    --cc=marcel@redhat.com \
    --cc=yuanhan.liu@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).