On 11/27/2017 12:55 PM, Kavanagh, Mark B wrote: > > >> From: Mooney, Sean K >> Sent: Sunday, November 26, 2017 11:28 PM >> To: Kavanagh, Mark B <[email protected]>; Jan Scheurich >> <[email protected]>; Kevin Traynor <[email protected]>; >> [email protected] >> Cc: [email protected]; Flavio Leitner <[email protected]>; Franck >> Baudin >> <[email protected]>; Ilya Maximets <[email protected]>; Stokes, Ian >> <[email protected]>; Loftus, Ciara <[email protected]>; Darrell Ball >> <[email protected]>; Aaron Conole <[email protected]>; Mooney, Sean K >> <[email protected]> >> Subject: RE: [ovs-dev] [PATCH 2/2] netdev-dpdk: add support for vhost IOMMU >> feature >> >> >> >>> -----Original Message----- >>> From: Kavanagh, Mark B >>> Sent: Friday, November 24, 2017 4:45 PM >>> To: Jan Scheurich <[email protected]>; Kevin Traynor >>> <[email protected]>; [email protected] >>> Cc: [email protected]; Flavio Leitner <[email protected]>; Franck >>> Baudin <[email protected]>; Mooney, Sean K <[email protected]>; >>> Ilya Maximets <[email protected]>; Stokes, Ian >>> <[email protected]>; Loftus, Ciara <[email protected]>; Darrell >>> Ball <[email protected]>; Aaron Conole <[email protected]> >>> Subject: RE: [ovs-dev] [PATCH 2/2] netdev-dpdk: add support for vhost >>> IOMMU feature >>> >>> Sounds good guys - I'll get cracking on this on Monday. >>> >>> Cheers, >>> Mark >>> >>>> -----Original Message----- >>>> From: Jan Scheurich [mailto:[email protected]] >>>> Sent: Friday, November 24, 2017 4:21 PM >>>> To: Kevin Traynor <[email protected]>; Kavanagh, Mark B >>>> <[email protected]>; [email protected] >>>> Cc: [email protected]; Flavio Leitner <[email protected]>; >>> Franck >>>> Baudin <[email protected]>; Mooney, Sean K <[email protected]>; >>>> Ilya Maximets <[email protected]>; Stokes, Ian >>>> <[email protected]>; Loftus, Ciara <[email protected]>; >>> Darrell >>>> Ball <[email protected]>; Aaron Conole <[email protected]> >>>> Subject: RE: [ovs-dev] [PATCH 2/2] netdev-dpdk: add support for vhost >>>> IOMMU feature >>>> >>>> +1 >>>> Jan >>>> >>>>> -----Original Message----- >>>>> From: Kevin Traynor [mailto:[email protected]] >>>>> Sent: Friday, 24 November, 2017 17:11 >>>>> To: Mark Kavanagh <[email protected]>; [email protected] >>>>> Cc: [email protected]; Flavio Leitner <[email protected]>; >>>>> Franck >>>> Baudin <[email protected]>; Mooney, Sean K >>>>> <[email protected]>; Ilya Maximets <[email protected]>; >>>>> Ian >>>> Stokes <[email protected]>; Loftus, Ciara >>>>> <[email protected]>; Darrell Ball <[email protected]>; Aaron >>>>> Conole >>>> <[email protected]>; Jan Scheurich >>>>> <[email protected]> >>>>> Subject: Re: [ovs-dev] [PATCH 2/2] netdev-dpdk: add support for >>> vhost >>>>> IOMMU >>>> feature >>>>> >>>>> On 11/16/2017 11:01 AM, Mark Kavanagh wrote: >>>>>> DPDK v17.11 introduces support for the vHost IOMMU feature. >>>>>> This is a security feature, that restricts the vhost memory that a >>>>>> virtio device may access. >>>>>> >>>>>> This feature also enables the vhost REPLY_ACK protocol, the >>>>>> implementation of which is known to work in newer versions of QEMU >>>>>> (i.e. v2.10.0), but is buggy in older versions (v2.7.0 - v2.9.0, >>>>>> inclusive). As such, the feature is disabled by default in (and >>>>>> should remain so, for the aforementioned older QEMU verions). >>>>>> Starting with QEMU v2.9.1, vhost-iommu-support can safely be >>>>>> enabled, even without having an IOMMU device, with no performance >>>>>> penalty. >>>>>> >>>>>> This patch adds a new vhost port option, vhost-iommu-support, to >>>>>> allow enablement of the vhost IOMMU feature: >>>>>> >>>>>> $ ovs-vsctl add-port br0 vhost-client-1 \ >>>>>> -- set Interface vhost-client-1 type=dpdkvhostuserclient \ >>>>>> options:vhost-server-path=$VHOST_USER_SOCKET_PATH \ >>>>>> options:vhost-iommu-support=true >>>>>> >>>>> >>>>> Hi Mark, All, >>>>> >>>>> I'm thinking about this and whether the current approach provides >>>>> more than what is actually needed by users at the cost of making the >>>>> user interface more complex. >> [Mooney, Sean K] I am personally split on this. To enable iommu support in >> openstack with the above interface we would have to do two things. 1 extend >> the image metatdata >> or flavor extra specs to allow requesting a vIOMMU. Second we would have to >> modify os-vif to produce >> the add-port command above. Using this interfaces gives us a nice parity in >> our api >> as we only enable iommu support in ovs if we enable for qemu. E.g. if the >> falvor/image does not request >> a virtualized iommu we wont declare its support in the options. >>>>> >>>>> As an alternative, how about having a global other_config (to be set >>>>> like vhost-socket-dir can be) for this instead of having to set it >>>>> for each individual interface. It would mean that it would only have >>>>> to be set once, instead of having this (ugly?!) option every time a >>>>> vhost port is added, so it's a less intrusive change and I can't >>>>> really think that a user would require to do this per vhostclient >> [Mooney, Sean K] well one taught that instantly comes to mind is if I set >> The global other_config option what happens if I do not request a iommu in >> qemu >> Or I have an old qemu. If it would have any negative effect on network >> connectivity >> Or prevent the vm from functioning, that would require the nova scheduler to >> be >> Aware of which node had this option set and take that into account when >> placing vms. >> I assume if it had no negative effects then we would not need a option, >> global or per >> Interface. >>>>> interface??? It's pain to have to add this at all for a bug in QEMU >>>>> and I'm sure in 1/2/3 years time someone will say that users could >>>>> still be using QEMU < 2.9.1 and we can't remove it, so it would be >>>>> nice to keep it as discreet as possible as we're going to be stuck >>> with it for a while. >>>>> >>>>> I assume that a user would only use one version of QEMU at a time >>> and >>>>> would either want or not want this feature. In the worst case, if >>>>> that proved completely wrong in the future, then a per interface >>>>> override could easily be added. Once there's a way to maintain >>>>> backwards compatibility (which there would be) I'd rather err on the >>>>> side of introducing just enough enough functionality over increasing >>>>> complexity for the user. >>>>> >>>>> What do you think? >> [Mooney, Sean K] I'm not sure that a single qemu version is a valid >> assumption >> to make.
Hi Sean, the solution in the current patch is effectively to allow iommu/different QEMU versions on a *per port* basis. I am assuming that level of granularity is not needed and instead proposing to allow iommu/different QEMU versions on a *per OVS instance* basis. It's not making any assumption about QEMU versions across multiple nodes. >> At least in an OpenStack context where rolling upgrades are a thing. But you >> are right >> That it will be less common however if it was no existent we would not have >> the issue with >> Live migration between nodes with different feature sets that is also trying >> to be addressed this >> Cycle. If we add a global config option for iommu support that is yet another >> thing that needs >> To be accounted for during live migration. Yes that's true, but it's difficult problem anyway and I don't think an additional binary field would make it much more complex. The alternative is never allow the iommu feature to be enabled, or drop support for QEMU versions < 2.9.1. It's default off, so you should have backwards compatibility at least. >>>>> > > Hi Kevin, Jan, > > Any thoughts on Sean's concerns? > Sean can confirm, but my reading is that Sean's primary concern is with the proposal to add any config option to enable iommu - not with the proposal to change from a per port to per OVS basis. Kevin. > I'll hold off on implementation until we have consensus. > > Thanks, > Mark > > > > >>>>> thanks, >>>>> Kevin. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
