From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 73C3AA046B for ; Wed, 26 Jun 2019 14:05:57 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 48FCF20BD; Wed, 26 Jun 2019 14:05:57 +0200 (CEST) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id C72611E25 for ; Wed, 26 Jun 2019 14:05:55 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 34A10316290F; Wed, 26 Jun 2019 12:05:55 +0000 (UTC) Received: from [10.36.112.46] (ovpn-112-46.ams2.redhat.com [10.36.112.46]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E16465D717; Wed, 26 Jun 2019 12:05:53 +0000 (UTC) To: Matan Azrad , Noa Ezra Cc: "dev@dpdk.org" , Tiwei Bie References: <1560924825-220648-1-git-send-email-noae@mellanox.com> <1560924825-220648-3-git-send-email-noae@mellanox.com> <29a8bedb-2363-d595-001b-577fdfc7318c@redhat.com> <59bce0c1-6f04-b743-c5fc-508b30e42075@redhat.com> <01a391c2-f68d-38fc-909b-3fc06100d1f7@redhat.com> From: Maxime Coquelin Message-ID: <81735ee1-645d-286b-4516-64755fd07c94@redhat.com> Date: Wed, 26 Jun 2019 14:05:52 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.41]); Wed, 26 Jun 2019 12:05:55 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH 2/2] net/vhost: support mrg-rxbuf disabling X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 6/26/19 1:18 PM, Matan Azrad wrote: > > > From: Maxime Coquelin >> On 6/26/19 9:50 AM, Matan Azrad wrote: >>> Hi Maxim >>> >>> Any response here? >>> >>> Besides that, >>> >>> Regarding the TSO and this patch: >>> I think we shouldn't be so strict to not take them for this version: >>> 1. The later time was a technical issue with the mailer - a mistake. >>> 2. The patches don't change any default and makes sense - will not hurt >> anyone. >>> >>> So I think we can do it beyond the letter of the law. >>> >>> From: Maxime Coquelin >>> > Sent: Thursday, June 20, 2019 10:19 AM >>> > To: Matan Azrad ; Noa Ezra >>> >>> > Cc: dev@dpdk.org >>> > Subject: Re: [PATCH 2/2] net/vhost: support mrg-rxbuf disabling >>> > >>> > >>> > >>> > On 6/20/19 8:52 AM, Matan Azrad wrote: >>> > > Hi all >>> > > >>> > >> -----Original Message----- >>> > >> From: Noa Ezra >>> > >> Sent: Thursday, June 20, 2019 8:58 AM >>> > >> To: Maxime Coquelin >>> > >> Cc: Matan Azrad ; dev@dpdk.org >>> > >> Subject: RE: [PATCH 2/2] net/vhost: support mrg-rxbuf disabling >>> > >> >>> > >> Hi Maxime, >>> > >> Thanks for your comment, please see below. >>> > >> >>> > >>> -----Original Message----- >>> > >>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com] >>> > >>> Sent: Wednesday, June 19, 2019 12:10 PM >>> > >>> To: Noa Ezra >>> > >>> Cc: Matan Azrad ; dev@dpdk.org >>> > >>> Subject: Re: [PATCH 2/2] net/vhost: support mrg-rxbuf disabling >>> > >>> >>> > >>> Hi Noa, >>> > >>> >>> > >>> On 6/19/19 8:13 AM, Noa Ezra wrote: >>> > >>>> Rx mergeable buffers is a virtio feature that allows chaining of >>> > >>>> multiple virtio descriptors to handle large packet size. >>> > >>>> This behavior is supported and enabled by default, however in >>> > >>>> case the user knows that rx mergeable buffers are not needed, he >>> > >>>> can disable the feature. >>> > >>>> The user should also set mrg_rxbuf=off in virtual machine's xml. >>> > >>> >>> > >>> I'm not sure to understand why it is needed, as the vhost-user >>> > >>> library supports the feature, it's better to let it being advertised. >>> > >>> >>> > >>> As you say, it is up to the user to disable it in the VM's XML. >>> > >>> Done this way, the feature won't be negotiated. >>> > >>> >>> > >> I agree with you, I'll remove this patch from the series. >>> > > >>> > > Are you sure that no performance impact exists for redundant >>> > > merg-rx-buf >>> > configuration here? >>> > >>> > I'm not sure to understand what you mean, could you please elaborate? >>> > >>> I guess that if this feature is enabled and the feature actually are not used >>> (no packets are scattered or merged) it will hurt the performance. >> >> Well, latest performance measurements does not show a big impact now on >> enabling mergeable buffers feature unconditionaly. > > Did you test small packets \ big? 64B packets, in non-vector mode on Virtio PMD side. > >>> So if one of the sides doesn't want to use it because of performance, it >> may >>> want to disable it. >> >> And even if there is an impact, the way to disable it is through >> Libvirt/Qemu. > > Not sure, as TSO application may decide to not do it in spite of it is configured in Qemu. > >>> > > What if the second side want it and the current side no? >>> > >>> > The feature won't be negotiated, assuming it has been disabled in >> QEMU >>> > cmdline (or via libvirt). >>> > > It may be that the vhost PMD user may want to disable it to save >>> > performance from some reasons, no? >>> > > >>> > >>> > Then this user should disable it at QEMU level. >>> > >>> So the vhost PMD is not one of the sides to decide? >>> If so, why do we need the APIs to configure the features? >> >> Are you talking about the rte_vhost_driver_set_features() and related >> APIs? > > Yes > >> This is used for example by the external backends that support features >> specific to the backend type (e.g. crypto), or also used by OVS-DPDK, to >> disable TSO. So these usages are for functional reasons, not tuning. > > Exactly, applications (like OVS) may decide to disable features because a lot of reasons. > >>> Looks like also the qemu is configured with the feature the VM\host sides >>> may decide in some cases to disable it. >> >> For functional reasons, I agree. So I that's why I agree with your tso >> patch as the application has to support it, but that's not the case of >> the mergeable buffers features. > > Performance reasons are not good enough? No, that's not what I mean. I mean that the application should be able to disable a feature when it does not meet the functional requirement. For performance tuning, the qemu way is available, and enough. > >> Tiwei, what's your opinion on this? >> >>> > Regards, >>> > Maxime >>>