On 7/2/20 1:56 PM, Kevin Traynor wrote: > 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?
I agree. 19.11.2 is definitely better than 19.11.0 that we're recommending now. 19.11.3 breaks the feature in OVS (experimental, but anyway), so it makes sense to wait for 19.11.4. As, IIUC, 19.11.4 might not be released in a nearest future, it's better to move to 19.11.2 now and wait for 19.11.4. >> > > 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
