On 02/07/2020 12:18, Stokes, Ian wrote: > > > On 7/1/2020 3:08 PM, Kevin Traynor wrote: >> 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 <[email protected]> >>>>> 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. >> > > OK so it seems like we can use 18.11.9 for the 2.11 and 2.13 branches as > it will have this patch reverted. > > Is there an 18.11.9 RC3 planned? We can test it if it's of use. >
I've just pushed it to the 18:11 branch, can you test with that? I wasn't planning on an rc3 at this stage for just this patch, but if you need a tarball for testing automation or something, let me know and I can create it. >>> >>>>> >>>>> 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/[email protected]/ >> http://inbox.dpdk.org/stable/[email protected]/ >> >> 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. > > Agreed with above, if it will be fixed in DPDK for 18.11 and a future > 19.11 then we can use them. > > In this case I will propose that we should move to recommend 19.11.2 > instead of 19.11.3 at the moment for OVS 2.13 and OVS master. 19.11.2 > has the vhost CVE security fixes and has been validated on our side > already. Once 19.11.4 is released we could then move to that if the zero > copy patch was resolved? > Sounds good to me. Luca has accepted the patch I sent yesterday for 19.11 branch, so it is due to be in 19.11.4. > Would this be acceptable? Or are there any critical patches in 19.11.3 > that we would be missing? > There are no CVE fixes added in 19.11.3 at least > BR > Ian > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
