[ovs-dev] Vhost-user dequeue zero copy removal

[Kevin Traynor]

On 01/07/2020 13:46, Ilya Maximets wrote:
> On 7/1/20 1:46 PM, Kevin Traynor wrote:
>> On 01/07/2020 11:28, Stokes, Ian wrote:
>>> Hi All,
>>>
>>> While completing validation work for DPDK 18.11.9 and 19.11.3 it was 
>>> found that zero-copy for vhostuserclient devices is no longer possible. 
>>> Please see commit below from 18.11.9 (note this patch is also in DPDK 
>>> 19.11.3)
>>>
>>
>> Thanks catching this in your validation, it almost certainly wouldn't
>> have been caught otherwise.
> 
> Definitely a good catch.  Thanks.
> 
>>
>>>
>>> commit 81e025d7ed6a802845909df6fb90505508dc0fbf
>>> Author: Xuan Ding <xuan.ding at intel.com>
>>> Date:   Wed Apr 29 02:59:46 2020 +0000
>>>
>>>      vhost: prevent zero-copy with incompatible client mode
>>>
>>>      [ upstream commit 715070ea10e6da1169deef2a3ea77ae934b4c333 ]
>>>
>>>      In server mode, virtio-user inits under the assumption that vhost-user
>>>      supports a list of features. However, this could be problematic when
>>>      in_order feature is negotiated but not supported by vhost-user when
>>>      enables dequeue_zero_copy later.
>>>
>>>      Add handling when vhost-user enables dequeue_zero_copy as client.
> 
> IIUC, this patch basically drops the feature for perfectly fine cases
> with VMs.  While it was intended to forbid running zero-copy with virtio-user
> it breaks a different usecase blocking the feature entirely.
> 
> Isn't it an API breakage?  IMHO, it should not have been backported in the
> first place, since dropping the feature is not what usually expected in
> stable releases.  And this must be in release notes anyway.
> 
> I think, the right solution here should be to make a patch to handle specific
> virtio-user case and stop blocking valid cases and release new DPDK stable
> versions for already released ones.
> 
> If it's too hard to make a patch or no-one wants to work on this, just revert
> these changes from stable branches and release a new stable DPDK version
> for both 18.11 and 19.11.  But anyway, regression should be addressed in DPDK
> before 20.11 or it will block OVS upgrade to that version.
> 

It is not in a released 18.11. It was caught by Ian's team as part of
18.11.9-rc testing.

> 
>>>
>>> OVS only supports zero-copy to date as an experimental feature with 
>>> dpdkvhostuserclient port types.
>>>
>>> We were aiming to update the validated DPDK versions as follows and 
>>> recommend them as minimum versions due to the inclusion of CSE fixes.
>>>
>>> OVS 2.11 -> 18.11.6 -> 18.11.9
>>> OVS 2.12 -> 18.11.6 -> 18.11.9
>>> OVS 2.13 -> 19.11.0 -> 19.11.3
>>> OVS Master -> 19.11.0 -> 19.11.3
>>>
>>> However recommending these DPDK version will trigger the dpdk zero copy 
>>> functionality break in OVS.
>>>
>>
>> 18.11.9 is not released yet, so at least for it, I think we could
>> replace that patch(es) with a warning.
>>
>> That is not removing any functionality or causing a regression for users
>> of earlier 18.11.x or OVS 2.11/2.12, but it is letting them know there
>> may be an issue.
> 
> That might be an option.  But we likely need same change on 19.11 that
> will require one more stable release on it.
> 

Sent patches to remove restriction and add warning for 18.11 and 19.11
branches here:
http://inbox.dpdk.org/stable/20200701135250.20262-1-ktraynor@redhat.com/
http://inbox.dpdk.org/stable/20200701135305.20343-1-ktraynor@redhat.com/

If there is a better fix in the future, I can apply it in a future release.

>>
>>         if (vsocket->dequeue_zero_copy) {
>>                 if ((flags & RTE_VHOST_USER_CLIENT) != 0) {
>> -                       RTE_LOG(ERR, VHOST_CONFIG,
>> -                       "error: zero copy is incompatible with vhost
>> client mode\n");
>> -                       ret = -1;
>> -                       goto out_mutex;
>> +                       RTE_LOG(WARNING, VHOST_CONFIG,
>> +                       "zero copy may be incompatible with vhost client
>> mode\n");
>>                 }
>>
>>> What are peoples thoughts here on how to proceed?
>>> Are people aware if the feature is used in deployments to date? If not, 
>>> as it's experimental is it something that should be removed?
> 
> I don't think that we should remove the feature from OVS since it, IIUC,
> could be enabled back in DPDK.
> 
> 
>>>
>>> I'm aware that there is discussion ongoing about tagging for 2.13.1, 
>>> 2.12.1, 2.11.3 releases, I'm thinking the vhost zero copy issue should 
>>> be resolved within that time frame and before we update the recommended 
>>> DPDK versions.
>>>
>>> BR
>>> Ian
>>>
>>>
>>
>