From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <maxime.coquelin@redhat.com> Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 221272C2B for <dev@dpdk.org>; Wed, 1 Mar 2017 08:44:23 +0100 (CET) Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2DCB9C057FA5; Wed, 1 Mar 2017 07:44:24 +0000 (UTC) Received: from [10.36.116.153] (ovpn-116-153.ams2.redhat.com [10.36.116.153]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v217iK9F023372 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 1 Mar 2017 02:44:22 -0500 To: Yuanhan Liu <yuanhan.liu@linux.intel.com> References: <20170213142820.8964-1-maxime.coquelin@redhat.com> <20170223071000.GY18844@yliu-dev.sh.intel.com> Cc: aconole@redhat.com, sodey@sonusnet.com, jianfeng.tan@intel.com, dev@dpdk.org From: Maxime Coquelin <maxime.coquelin@redhat.com> Message-ID: <ab3cfe53-91b5-1668-9e14-41d4978c0592@redhat.com> Date: Wed, 1 Mar 2017 08:44:18 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <20170223071000.GY18844@yliu-dev.sh.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Wed, 01 Mar 2017 07:44:24 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH 0/7] virtio/vhost: Add MTU feature support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> X-List-Received-Date: Wed, 01 Mar 2017 07:44:24 -0000 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