Re: [dpdk-dev] [PATCH 2/2] net/vhost: support mrg-rxbuf disabling

[Matan Azrad <matan@mellanox.com>]

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 <matan@mellanox.com>; Noa Ezra
 <noae@mellanox.com>
 > 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 <maxime.coquelin@redhat.com>
 > >> Cc: Matan Azrad <matan@mellanox.com>; 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 <noae@mellanox.com>
 > >>> Cc: Matan Azrad <matan@mellanox.com>; 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.
 
 So if one of the sides doesn't want to use it because of performance, it may
 want to disable it.
 
 > > 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?
 
 Looks like also the qemu is configured with the feature the VM\host sides
 may decide in some cases to disable it.
 
 
 > Regards,
 > Maxime