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 D123CA046B for ; Wed, 26 Jun 2019 16:18:01 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 73AA925D9; Wed, 26 Jun 2019 16:18:00 +0200 (CEST) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id F058A1D7 for ; Wed, 26 Jun 2019 16:17:57 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 24130C05168F; Wed, 26 Jun 2019 14:17:44 +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 D1F095C22F; Wed, 26 Jun 2019 14:17:42 +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> <81735ee1-645d-286b-4516-64755fd07c94@redhat.com> From: Maxime Coquelin Message-ID: <00a5fe75-4df7-5bf7-55f9-1b18aba4506c@redhat.com> Date: Wed, 26 Jun 2019 16:17:41 +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.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 26 Jun 2019 14:17:52 +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 3:24 PM, Matan Azrad wrote: > > >> -----Original Message----- >> From: Maxime Coquelin >> Sent: Wednesday, June 26, 2019 3:06 PM >> To: Matan Azrad ; Noa Ezra >> Cc: dev@dpdk.org; Tiwei Bie >> Subject: Re: [PATCH 2/2] net/vhost: support mrg-rxbuf disabling >> >> >> >> 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. >> > > I think that this is the point we are not agree on. > > I think that application may want to disable the feature in some cases because of performance reasons (maybe others too), > And in some other cases to work with the feature. > > So, it makes sense IMO to let the application to decide what it wants without any concern about the QEMU configuration. > > Why to not allow to the PMD user to do it by the application (using prob parameters)? I think we should restrict the Virtio features from the Vhost PMD parameter at as min as possible, only to ensure compatibility with the application (iommu, postcopy, tso, ...). One problem I see with providing the possibility to change any Virtio feature at runtime is reconnection. For example, you start your application with enabling mergeable buffers, stop it and restart it without the feature enabled by the application. As the negotiation with the driver is not done again at reconnect time, Qemu will fail. > > >>> >>>> Tiwei, what's your opinion on this? >>>> >>>>> > Regards, >>>>> > Maxime >>>>>