DPDK patches and discussions
 help / color / mirror / Atom feed
From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Cc: aconole@redhat.com, sodey@sonusnet.com, jianfeng.tan@intel.com,
	dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 0/7] virtio/vhost: Add MTU feature support
Date: Wed, 1 Mar 2017 08:44:18 +0100	[thread overview]
Message-ID: <ab3cfe53-91b5-1668-9e14-41d4978c0592@redhat.com> (raw)
In-Reply-To: <20170223071000.GY18844@yliu-dev.sh.intel.com>



On 02/23/2017 08:10 AM, Yuanhan Liu wrote:
> On Mon, Feb 13, 2017 at 03:28:13PM +0100, Maxime Coquelin wrote:
>> This series adds support to new Virtio's MTU feature[1].
>
> Seems you missed a link here?
Oops, I meant this link:
https://www.spinics.net/lists/linux-virtualization/msg28908.html

Will fix in v2.

>
>> The MTU
>> value is set via QEMU parameters.
>>
>> If the feature is negotiated (i.e supported by both host andcguest,
>> and valid MTU value is set in QEMU via its host_mtu parameter), QEMU
>> shares the configured MTU value throught dedicated Vhost protocol
>> feature.
>>
>> On vhost side, the value is stored in the virtio_net structure, and
>> made available to the application thanks to new vhost lib's
>> rte_vhost_mtu_get() function.
>>
>> rte_vhost_mtu_set() functions is implemented, but only succeed if the
>> application sets the same value as the one set in QEMU. Idea is that
>> it would be used for the application to ensure configured MTU value
>> is consistent, but maybe the mtu_get() API is enough, and mtu_set()
>> could just be dropped.
>
> If the vhost MTU is designed to be read-only, then we may should drop
> the set_mtu function.

Ok, I will remove it in next revision.

>> Vhost PMD mtu_set callback is also implemented
>> in the same spirit.
>>
>> To be able to set eth_dev's MTU value at the right time, i.e. to call
>> rte_vhost_mtu_get() just after Virtio features have been negotiated
>> and before the device is really started, a new vhost flag has been
>> introduced (VIRTIO_DEV_READY), because the VIRTIO_DEV_RUNNING flag is
>> set too late (after .new_device() ops is called).
>
> Okay, and I think this kind of info should be in corresponding commit log.
Sure.
>
>> Regarding valid MTU values, the maximum MTU value accepted on vhost
>> side is 65535 bytes, as defined in Virtio Spec and supported in
>> Virtio-net Kernel driver. But in Virtio PMD, current maximum frame
>> size is 9728 bytes (~9700 bytes MTU). So maximum MTU size accepted in
>> Virtio PMD is the minimum between ~9700 bytes and host's MTU.
>>
>> Initially, we thought about disabling the rx-mergeable feature when
>> MTU value was low enough to ensure all received packets would fit in
>> receive buffers (when offloads are disabled). Doing this, we would
>> save one cache-miss in the receive path. Problem is that we don't
>> know the buffers size at Virtio feature neogotiation time.
>> It might be possible for the application to call the configure
>> callback again once the Rx queue is set up, but it seems a bit hacky.
>
> Worse, if multiple queue is involved, one queue could have it's own
> mempool, meaning the buffer size could be different, whereas the
> MTU feature is global.
Indeed, we should ensure that for every queues, the corresponding buf
size can handle a full packet to be able to disable the mergeable
feature.

Let's skip this for now and focus on improving the rx-mergeable path.

Thanks,
Maxime
>
> 	--yliu
>
>> So I decided to skip this optimization for now, even if feedback and
>> are of course appreciated.
>>
>> Finally, this series also adds MTU value printing  in testpmd's
>> "show port info" command when non-zero.
>>
>> This series target v17.05 release.
>>
>> Cheers,
>> Maxime
>>
>> Maxime Coquelin (7):
>>   vhost: Enable VIRTIO_NET_F_MTU feature
>>   vhost: vhost-user: Add MTU protocol feature support
>>   vhost: Add new ready status flag
>>   vhost: Add API to get/set MTU value
>>   net/vhost: Implement mtu_set callback
>>   net/virtio: Add MTU feature support
>>   app/testpmd: print MTU value in show port info
>>
>>  app/test-pmd/config.c               |  5 +++++
>>  doc/guides/nics/features/vhost.ini  |  1 +
>>  doc/guides/nics/features/virtio.ini |  1 +
>>  drivers/net/vhost/rte_eth_vhost.c   | 18 +++++++++++++++
>>  drivers/net/virtio/virtio_ethdev.c  | 22 +++++++++++++++++--
>>  drivers/net/virtio/virtio_ethdev.h  |  3 ++-
>>  drivers/net/virtio/virtio_pci.h     |  3 +++
>>  lib/librte_vhost/rte_virtio_net.h   | 31 ++++++++++++++++++++++++++
>>  lib/librte_vhost/vhost.c            | 42 ++++++++++++++++++++++++++++++++++-
>>  lib/librte_vhost/vhost.h            |  9 +++++++-
>>  lib/librte_vhost/vhost_user.c       | 44 +++++++++++++++++++++++++++++++------
>>  lib/librte_vhost/vhost_user.h       |  5 ++++-
>>  12 files changed, 171 insertions(+), 13 deletions(-)
>>
>> --
>> 2.9.3

  reply	other threads:[~2017-03-01  7:44 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-13 14:28 Maxime Coquelin
2017-02-13 14:28 ` [dpdk-dev] [PATCH 1/7] vhost: Enable VIRTIO_NET_F_MTU feature Maxime Coquelin
2017-02-13 14:28 ` [dpdk-dev] [PATCH 2/7] vhost: vhost-user: Add MTU protocol feature support Maxime Coquelin
2017-02-13 14:28 ` [dpdk-dev] [PATCH 3/7] vhost: Add new ready status flag Maxime Coquelin
2017-02-13 14:28 ` [dpdk-dev] [PATCH 4/7] vhost: Add API to get/set MTU value Maxime Coquelin
2017-02-13 14:28 ` [dpdk-dev] [PATCH 5/7] net/vhost: Implement mtu_set callback Maxime Coquelin
2017-02-13 14:28 ` [dpdk-dev] [PATCH 6/7] net/virtio: Add MTU feature support Maxime Coquelin
2017-02-16 19:31   ` Aaron Conole
2017-02-16 21:17     ` Maxime Coquelin
2017-02-13 14:28 ` [dpdk-dev] [PATCH 7/7] app/testpmd: print MTU value in show port info Maxime Coquelin
2017-02-23  7:10 ` [dpdk-dev] [PATCH 0/7] virtio/vhost: Add MTU feature support Yuanhan Liu
2017-03-01  7:44   ` Maxime Coquelin [this message]
2017-03-06  8:27 ` [dpdk-dev] [PATCH v2 " Maxime Coquelin
2017-03-06  8:27   ` [dpdk-dev] [PATCH v2 1/7] vhost: Enable VIRTIO_NET_F_MTU feature Maxime Coquelin
2017-03-06  8:27   ` [dpdk-dev] [PATCH v2 2/7] vhost: vhost-user: Add MTU protocol feature support Maxime Coquelin
2017-03-08  2:31     ` Yuanhan Liu
2017-03-12 10:24       ` Maxime Coquelin
2017-03-06  8:27   ` [dpdk-dev] [PATCH v2 3/7] vhost: Add new ready status flag Maxime Coquelin
2017-03-06  8:27   ` [dpdk-dev] [PATCH v2 4/7] vhost: Add API to get MTU value Maxime Coquelin
2017-03-08  2:45     ` Yuanhan Liu
2017-03-12 10:23       ` Maxime Coquelin
2017-03-06  8:27   ` [dpdk-dev] [PATCH v2 5/7] net/vhost: Fill rte_eth_dev's MTU property Maxime Coquelin
2017-03-06  8:27   ` [dpdk-dev] [PATCH v2 6/7] net/virtio: Add MTU feature support Maxime Coquelin
2017-03-06  8:27   ` [dpdk-dev] [PATCH v2 7/7] app/testpmd: print MTU value in show port info Maxime Coquelin
2017-03-07 21:57   ` [dpdk-dev] [PATCH v2 0/7] virtio/vhost: Add MTU feature support Thomas Monjalon
2017-03-12 10:00     ` Maxime Coquelin
2017-03-12 16:33 ` [dpdk-dev] [PATCH v3 0/9] " Maxime Coquelin
2017-03-12 16:33   ` [dpdk-dev] [PATCH v3 1/9] vhost: Enable VIRTIO_NET_F_MTU feature Maxime Coquelin
2017-03-12 16:33   ` [dpdk-dev] [PATCH v3 2/9] vhost: vhost-user: Add MTU protocol feature support Maxime Coquelin
2017-03-12 16:34   ` [dpdk-dev] [PATCH v3 3/9] vhost: Add new ready status flag Maxime Coquelin
2017-03-12 16:34   ` [dpdk-dev] [PATCH v3 4/9] vhost: Add API to get MTU value Maxime Coquelin
2017-03-16  8:00     ` Yuanhan Liu
2017-03-16 11:37       ` Maxime Coquelin
2017-03-17  5:32         ` Yuanhan Liu
2017-03-17  9:59           ` Maxime Coquelin
2017-03-20  8:42             ` Yuanhan Liu
2017-03-12 16:34   ` [dpdk-dev] [PATCH v3 5/9] vhost: export " Maxime Coquelin
2017-03-12 16:34   ` [dpdk-dev] [PATCH v3 6/9] net/vhost: Fill rte_eth_dev's MTU property Maxime Coquelin
2017-03-12 16:34   ` [dpdk-dev] [PATCH v3 7/9] net/virtio: Add MTU feature support Maxime Coquelin
2017-04-05  4:52     ` Tan, Jianfeng
2017-04-05  7:11       ` Maxime Coquelin
2017-04-05  9:42         ` Tan, Jianfeng
2017-04-05 13:54           ` Maxime Coquelin
2017-04-05 14:50             ` Tan, Jianfeng
2017-04-07 16:40               ` Maxime Coquelin
2017-04-10  6:48                 ` Tan, Jianfeng
2017-03-12 16:34   ` [dpdk-dev] [PATCH v3 8/9] doc: announce Virtio and Vhost MTU support Maxime Coquelin
2017-03-12 16:34   ` [dpdk-dev] [PATCH v3 9/9] app/testpmd: print MTU value in show port info Maxime Coquelin
2017-03-22  8:58   ` [dpdk-dev] [PATCH v3 0/9] virtio/vhost: Add MTU feature support Yuanhan Liu
2017-03-22  9:03     ` Maxime Coquelin
2017-03-28  5:39   ` Yao, Lei A
2017-03-30 11:34     ` Maxime Coquelin

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=ab3cfe53-91b5-1668-9e14-41d4978c0592@redhat.com \
    --to=maxime.coquelin@redhat.com \
    --cc=aconole@redhat.com \
    --cc=dev@dpdk.org \
    --cc=jianfeng.tan@intel.com \
    --cc=sodey@sonusnet.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).