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

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 6/27/19 7:04 AM, Matan Azrad wrote:
> 
> 
> From: Maxime Coquelin 
>>>>>> 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.
> 
> Looks like you are describing a new issue in the vhost PMD, it must close the connection when the PMD is closed\removed.
> So, every probe(hotplug add) it will start from scratch.
> 

No, you can close the application and restart it for example without
having to restart the guest. In this case, the feature negotiation is
not done again.

So I remain convinced we should not provide the possibility to disable
any feature that is not dependent on the application.

Megeable buffers is not dependent on the application, as it is only
related to the ring implementation, and it is supported by the DPDK
Vhost library.

Regards,
Maxime