Re: [dpdk-dev] [PATCH] examples/vhost: fix perf regression

["Tan, Jianfeng" <jianfeng.tan@intel.com>]

On 7/20/2016 12:38 PM, Yuanhan Liu wrote:
> On Tue, Jul 19, 2016 at 01:53:11PM +0000, Jianfeng Tan wrote:
>> We find significant perfermance drop introduced by below commit,
>> when vhost example is started with --mergeable 0 and inside vm,
>> kernel virtio-net driver is used to do ip based forwarding.
>>
>> The root cause is that below commit adds support for
>> VIRTIO_NET_F_GUEST_TSO4 and VIRTIO_NET_F_GUEST_TSO6, and when
>> mergeable is disabled, it triggers big_packets path of virtio-net
>> driver. In this path, virtio driver uses 19 desc with 18 4K-sized
>> pages to receive each packet, so that it can receive a big packet
>> with size of 64K. But QEMU only creates 256 desc entries for each
>> vq, which results in that only 13 packets can be received. VM
>> kernel can quickly handle those packets and go to sleep (HLT).
>>
>> As QEMU has no option to set the desc entries of a vq, so here,
>> we disable VIRTIO_NET_F_GUEST_TSO4 and VIRTIO_NET_F_GUEST_TSO6
>> with VIRTIO_NET_F_HOST_TSO4 and VIRTIO_NET_F_HOST_TSO6 when we
>> disable tso of vhost example, to avoid VM kernel virtio driver
>> go into big_packets path.
>>
>> Fixes: 859b480d5afd ("vhost: add guest offload setting")
> And here you are patching vhost example to try to fix an "issue"
> in vhost lib, this is __logically__ wrong.
>
> 	--yliu

This is not an issue from vhost lib's perspective, vhost lib should 
provide all features it supports by default. Applications can 
enable/disable features according to their own requirements. And the 
vhost example after this commit just triggers a slow path of virtio 
driver. So this fix just makes sure vhost example does not go into the 
slow path by default.

By the way, if a fix patch should only involve those commits it will change?

Thanks,
Jianfeng