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

On 6/27/19 7:04 AM, Matan Azrad wrote:

From: Maxime Coquelin 

quoted

quoted

quoted

quoted

quoted

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.

quoted

So, it makes sense IMO to let the application to decide what it wants

without any concern about the QEMU configuration.

quoted

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