Message ID | 1510070368-242276-3-git-send-email-mark.b.kavanagh@intel.com |
---|---|
State | Superseded |
Headers | show |
Series | DPDK v17.11 Support | expand |
> > 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). > > 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 > > Note that support for this feature is only implemented for vhost > user client ports (since vhost user ports are considered deprecated). > > Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com> > --- > Documentation/topics/dpdk/vhost-user.rst | 19 +++++++++++++++++++ > NEWS | 1 + > lib/netdev-dpdk.c | 26 +++++++++++++++++++++----- > vswitchd/vswitch.xml | 10 ++++++++++ > 4 files changed, 51 insertions(+), 5 deletions(-) > > diff --git a/Documentation/topics/dpdk/vhost-user.rst > b/Documentation/topics/dpdk/vhost-user.rst > index fdf6ae1..ba70e45 100644 > --- a/Documentation/topics/dpdk/vhost-user.rst > +++ b/Documentation/topics/dpdk/vhost-user.rst > @@ -250,6 +250,25 @@ Once the vhost-user-client ports have been added > to the switch, they must be > added to the guest. Like vhost-user ports, there are two ways to do this: > using > QEMU directly, or using libvirt. Only the QEMU case is covered here. > > +vhost-user client IOMMU > +~~~~~~~~~~~~~~~~~~~~~~~ > +It is possible to enable IOMMU support for vHost User client ports. This is > +a feature which restricts the vhost memory that a virtio device can access, > and > +as such is useful in deployments in which security is a concern. IOMMU > mode may > +be enabled on the command line:: > + > + $ 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 > + > +.. important:: > + > + Enabling the IOMMU feature also enables the vhost user reply-ack > protocol; > + this is known to work on QEMU v2.10.0, but is buggy on older versions > + (2.7.0 - 2.9.0, inclusive). Consequently, the IOMMU feaure is disabled by > + default (and should remain so if using the aforementioned versions of > QEMU). > + > Adding vhost-user-client ports to the guest (QEMU) > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > diff --git a/NEWS b/NEWS > index 6acd8bd..de53d2f 100644 > --- a/NEWS > +++ b/NEWS > @@ -13,6 +13,7 @@ Post-v2.8.0 > * Add support for compiling OVS with the latest Linux 4.13 kernel > - DPDK: > * Add support for DPDK v17.11 > + * Add support for vHost IOMMU feature > > v2.8.0 - 31 Aug 2017 > -------------------- > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index ed5bf62..d214fac 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -1424,15 +1424,22 @@ netdev_dpdk_vhost_client_set_config(struct > netdev *netdev, > { > struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); > const char *path; > + bool iommu_enable; > > ovs_mutex_lock(&dev->mutex); > if (!(dev->vhost_driver_flags & RTE_VHOST_USER_CLIENT)) { > path = smap_get(args, "vhost-server-path"); > - if (path && strcmp(path, dev->vhost_id)) { > + if (path && strcmp(path, dev->vhost_id)) > strcpy(dev->vhost_id, path); > - netdev_request_reconfigure(netdev); > - } > } > + > + iommu_enable = smap_get_bool(args, "vhost-iommu-support", false); > + if (iommu_enable) > + dev->vhost_driver_flags |= RTE_VHOST_USER_IOMMU_SUPPORT; > + else > + dev->vhost_driver_flags &= ~RTE_VHOST_USER_IOMMU_SUPPORT; > + > + netdev_request_reconfigure(netdev); Here we will always request a reconfigure. Best to avoid unnecessary reconfigures. Suggest only reconfiguring if the iommu_enable has changed. The interface for setting the feature looks good to me & I think it's right to disable by default given the restrictions the feature places on QEMU versions. Thanks, Ciara > ovs_mutex_unlock(&dev->mutex); > > return 0; > @@ -3326,9 +3333,18 @@ netdev_dpdk_vhost_client_reconfigure(struct > netdev *netdev) > */ > if (!(dev->vhost_driver_flags & RTE_VHOST_USER_CLIENT) > && strlen(dev->vhost_id)) { > + /* Enable vhost IOMMU, if it was requested. > + * XXX: the 'flags' variable is required, as not all vhost backend > + * features are currently supported by OvS; in time, it should be > + * possible to invoke rte_vhost_driver_register(), passing > + * dev->vhost_driver_flags directly as a parameter to same. > + */ > + uint64_t flags = RTE_VHOST_USER_CLIENT; > + if (dev->vhost_driver_flags & RTE_VHOST_USER_IOMMU_SUPPORT) > + flags |= RTE_VHOST_USER_IOMMU_SUPPORT; > + > /* Register client-mode device */ > - err = rte_vhost_driver_register(dev->vhost_id, > - RTE_VHOST_USER_CLIENT); > + err = rte_vhost_driver_register(dev->vhost_id, flags); > if (err) { > VLOG_ERR("vhost-user device setup failure for device %s\n", > dev->vhost_id); > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml > index d7f6839..df5a42d 100644 > --- a/vswitchd/vswitch.xml > +++ b/vswitchd/vswitch.xml > @@ -2649,6 +2649,16 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 > type=patch options:peer=p1 \ > </p> > </column> > > + <column name="options" key="vhost-iommu-support" > + type='{"type": "boolean"}'> > + <p> > + The value specifies whether IOMMU support is enabled for a vHost > User > + client mode device that has been or will be created by QEMU. > + Only supported by dpdkvhostuserclient interfaces. If not specified or > + an incorrect value is specified, defaults to 'false'. > + </p> > + </column> > + > <column name="options" key="n_rxq_desc" > type='{"type": "integer", "minInteger": 1, "maxInteger": 4096}'> > <p> > -- > 1.9.3
>From: Loftus, Ciara >Sent: Tuesday, November 7, 2017 4:24 PM >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; dev@openvswitch.org >Cc: maxime.coquelin@redhat.com; ktraynor@redhat.com >Subject: RE: [ovs-dev][RFC PATCH 2/2] netdev-dpdk: add support for vhost IOMMU >feature > >> >> 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). >> >> 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 >> >> Note that support for this feature is only implemented for vhost >> user client ports (since vhost user ports are considered deprecated). >> >> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com> >> --- >> Documentation/topics/dpdk/vhost-user.rst | 19 +++++++++++++++++++ >> NEWS | 1 + >> lib/netdev-dpdk.c | 26 +++++++++++++++++++++----- >> vswitchd/vswitch.xml | 10 ++++++++++ >> 4 files changed, 51 insertions(+), 5 deletions(-) >> >> diff --git a/Documentation/topics/dpdk/vhost-user.rst >> b/Documentation/topics/dpdk/vhost-user.rst >> index fdf6ae1..ba70e45 100644 >> --- a/Documentation/topics/dpdk/vhost-user.rst >> +++ b/Documentation/topics/dpdk/vhost-user.rst >> @@ -250,6 +250,25 @@ Once the vhost-user-client ports have been added >> to the switch, they must be >> added to the guest. Like vhost-user ports, there are two ways to do this: >> using >> QEMU directly, or using libvirt. Only the QEMU case is covered here. >> >> +vhost-user client IOMMU >> +~~~~~~~~~~~~~~~~~~~~~~~ >> +It is possible to enable IOMMU support for vHost User client ports. This is >> +a feature which restricts the vhost memory that a virtio device can access, >> and >> +as such is useful in deployments in which security is a concern. IOMMU >> mode may >> +be enabled on the command line:: >> + >> + $ 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 >> + >> +.. important:: >> + >> + Enabling the IOMMU feature also enables the vhost user reply-ack >> protocol; >> + this is known to work on QEMU v2.10.0, but is buggy on older versions >> + (2.7.0 - 2.9.0, inclusive). Consequently, the IOMMU feaure is disabled >by >> + default (and should remain so if using the aforementioned versions of >> QEMU). >> + >> Adding vhost-user-client ports to the guest (QEMU) >> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> >> diff --git a/NEWS b/NEWS >> index 6acd8bd..de53d2f 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -13,6 +13,7 @@ Post-v2.8.0 >> * Add support for compiling OVS with the latest Linux 4.13 kernel >> - DPDK: >> * Add support for DPDK v17.11 >> + * Add support for vHost IOMMU feature >> >> v2.8.0 - 31 Aug 2017 >> -------------------- >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >> index ed5bf62..d214fac 100644 >> --- a/lib/netdev-dpdk.c >> +++ b/lib/netdev-dpdk.c >> @@ -1424,15 +1424,22 @@ netdev_dpdk_vhost_client_set_config(struct >> netdev *netdev, >> { >> struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); >> const char *path; >> + bool iommu_enable; >> >> ovs_mutex_lock(&dev->mutex); >> if (!(dev->vhost_driver_flags & RTE_VHOST_USER_CLIENT)) { >> path = smap_get(args, "vhost-server-path"); >> - if (path && strcmp(path, dev->vhost_id)) { >> + if (path && strcmp(path, dev->vhost_id)) >> strcpy(dev->vhost_id, path); >> - netdev_request_reconfigure(netdev); >> - } >> } >> + >> + iommu_enable = smap_get_bool(args, "vhost-iommu-support", false); >> + if (iommu_enable) >> + dev->vhost_driver_flags |= RTE_VHOST_USER_IOMMU_SUPPORT; >> + else >> + dev->vhost_driver_flags &= ~RTE_VHOST_USER_IOMMU_SUPPORT; >> + >> + netdev_request_reconfigure(netdev); > >Here we will always request a reconfigure. Best to avoid unnecessary >reconfigures. >Suggest only reconfiguring if the iommu_enable has changed. Ah yes, thanks for the catch. > >The interface for setting the feature looks good to me & I think it's right to >disable by default given the restrictions the feature places on QEMU versions. Great - thanks for the speedy feedback Ciara. -Mark > >Thanks, >Ciara > >> ovs_mutex_unlock(&dev->mutex); >> >> return 0; >> @@ -3326,9 +3333,18 @@ netdev_dpdk_vhost_client_reconfigure(struct >> netdev *netdev) >> */ >> if (!(dev->vhost_driver_flags & RTE_VHOST_USER_CLIENT) >> && strlen(dev->vhost_id)) { >> + /* Enable vhost IOMMU, if it was requested. >> + * XXX: the 'flags' variable is required, as not all vhost backend >> + * features are currently supported by OvS; in time, it should be >> + * possible to invoke rte_vhost_driver_register(), passing >> + * dev->vhost_driver_flags directly as a parameter to same. >> + */ >> + uint64_t flags = RTE_VHOST_USER_CLIENT; >> + if (dev->vhost_driver_flags & RTE_VHOST_USER_IOMMU_SUPPORT) >> + flags |= RTE_VHOST_USER_IOMMU_SUPPORT; >> + >> /* Register client-mode device */ >> - err = rte_vhost_driver_register(dev->vhost_id, >> - RTE_VHOST_USER_CLIENT); >> + err = rte_vhost_driver_register(dev->vhost_id, flags); >> if (err) { >> VLOG_ERR("vhost-user device setup failure for device %s\n", >> dev->vhost_id); >> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml >> index d7f6839..df5a42d 100644 >> --- a/vswitchd/vswitch.xml >> +++ b/vswitchd/vswitch.xml >> @@ -2649,6 +2649,16 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 >> type=patch options:peer=p1 \ >> </p> >> </column> >> >> + <column name="options" key="vhost-iommu-support" >> + type='{"type": "boolean"}'> >> + <p> >> + The value specifies whether IOMMU support is enabled for a vHost >> User >> + client mode device that has been or will be created by QEMU. >> + Only supported by dpdkvhostuserclient interfaces. If not >specified or >> + an incorrect value is specified, defaults to 'false'. >> + </p> >> + </column> >> + >> <column name="options" key="n_rxq_desc" >> type='{"type": "integer", "minInteger": 1, "maxInteger": >4096}'> >> <p> >> -- >> 1.9.3
Hi Mark, Thanks for the patch, it looks overall good to me once Ciara comments taken into account. I would just add to below note: On 11/07/2017 04:59 PM, Mark Kavanagh wrote: > +.. important:: > + > + Enabling the IOMMU feature also enables the vhost user reply-ack protocol; > + this is known to work on QEMU v2.10.0, but is buggy on older versions > + (2.7.0 - 2.9.0, inclusive). Consequently, the IOMMU feaure is disabled by > + default (and should remain so if using the aforementioned versions of QEMU) "Starting Qemu v2.9.1, vhost-iommu-support can safely be enabled, even without having an IOMMU device with no performance penalty." Cheers, Maxime
>From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com] >Sent: Tuesday, November 7, 2017 5:18 PM >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; dev@openvswitch.org >Cc: Loftus, Ciara <ciara.loftus@intel.com>; ktraynor@redhat.com >Subject: Re: [ovs-dev][RFC PATCH 2/2] netdev-dpdk: add support for vhost IOMMU >feature > >Hi Mark, > >Thanks for the patch, it looks overall good to me once Ciara comments >taken into account. > >I would just add to below note: > >On 11/07/2017 04:59 PM, Mark Kavanagh wrote: >> +.. important:: >> + >> + Enabling the IOMMU feature also enables the vhost user reply-ack >protocol; >> + this is known to work on QEMU v2.10.0, but is buggy on older versions >> + (2.7.0 - 2.9.0, inclusive). Consequently, the IOMMU feaure is disabled >by >> + default (and should remain so if using the aforementioned versions of >QEMU) > >"Starting Qemu v2.9.1, vhost-iommu-support can safely be enabled, even >without having an IOMMU device with no performance penalty." Hey Maxime, Thanks for the quick feedback - I'll roll the suggestions from both yourself and Ciara into the non-RFC version of the patch, once DPDK v17.11 is released (which should be in the next week or so). All the best, Mark > >Cheers, >Maxime
diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-user.rst index fdf6ae1..ba70e45 100644 --- a/Documentation/topics/dpdk/vhost-user.rst +++ b/Documentation/topics/dpdk/vhost-user.rst @@ -250,6 +250,25 @@ Once the vhost-user-client ports have been added to the switch, they must be added to the guest. Like vhost-user ports, there are two ways to do this: using QEMU directly, or using libvirt. Only the QEMU case is covered here. +vhost-user client IOMMU +~~~~~~~~~~~~~~~~~~~~~~~ +It is possible to enable IOMMU support for vHost User client ports. This is +a feature which restricts the vhost memory that a virtio device can access, and +as such is useful in deployments in which security is a concern. IOMMU mode may +be enabled on the command line:: + + $ 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 + +.. important:: + + Enabling the IOMMU feature also enables the vhost user reply-ack protocol; + this is known to work on QEMU v2.10.0, but is buggy on older versions + (2.7.0 - 2.9.0, inclusive). Consequently, the IOMMU feaure is disabled by + default (and should remain so if using the aforementioned versions of QEMU). + Adding vhost-user-client ports to the guest (QEMU) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/NEWS b/NEWS index 6acd8bd..de53d2f 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,7 @@ Post-v2.8.0 * Add support for compiling OVS with the latest Linux 4.13 kernel - DPDK: * Add support for DPDK v17.11 + * Add support for vHost IOMMU feature v2.8.0 - 31 Aug 2017 -------------------- diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index ed5bf62..d214fac 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -1424,15 +1424,22 @@ netdev_dpdk_vhost_client_set_config(struct netdev *netdev, { struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); const char *path; + bool iommu_enable; ovs_mutex_lock(&dev->mutex); if (!(dev->vhost_driver_flags & RTE_VHOST_USER_CLIENT)) { path = smap_get(args, "vhost-server-path"); - if (path && strcmp(path, dev->vhost_id)) { + if (path && strcmp(path, dev->vhost_id)) strcpy(dev->vhost_id, path); - netdev_request_reconfigure(netdev); - } } + + iommu_enable = smap_get_bool(args, "vhost-iommu-support", false); + if (iommu_enable) + dev->vhost_driver_flags |= RTE_VHOST_USER_IOMMU_SUPPORT; + else + dev->vhost_driver_flags &= ~RTE_VHOST_USER_IOMMU_SUPPORT; + + netdev_request_reconfigure(netdev); ovs_mutex_unlock(&dev->mutex); return 0; @@ -3326,9 +3333,18 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev *netdev) */ if (!(dev->vhost_driver_flags & RTE_VHOST_USER_CLIENT) && strlen(dev->vhost_id)) { + /* Enable vhost IOMMU, if it was requested. + * XXX: the 'flags' variable is required, as not all vhost backend + * features are currently supported by OvS; in time, it should be + * possible to invoke rte_vhost_driver_register(), passing + * dev->vhost_driver_flags directly as a parameter to same. + */ + uint64_t flags = RTE_VHOST_USER_CLIENT; + if (dev->vhost_driver_flags & RTE_VHOST_USER_IOMMU_SUPPORT) + flags |= RTE_VHOST_USER_IOMMU_SUPPORT; + /* Register client-mode device */ - err = rte_vhost_driver_register(dev->vhost_id, - RTE_VHOST_USER_CLIENT); + err = rte_vhost_driver_register(dev->vhost_id, flags); if (err) { VLOG_ERR("vhost-user device setup failure for device %s\n", dev->vhost_id); diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index d7f6839..df5a42d 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -2649,6 +2649,16 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \ </p> </column> + <column name="options" key="vhost-iommu-support" + type='{"type": "boolean"}'> + <p> + The value specifies whether IOMMU support is enabled for a vHost User + client mode device that has been or will be created by QEMU. + Only supported by dpdkvhostuserclient interfaces. If not specified or + an incorrect value is specified, defaults to 'false'. + </p> + </column> + <column name="options" key="n_rxq_desc" type='{"type": "integer", "minInteger": 1, "maxInteger": 4096}'> <p>
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). 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 Note that support for this feature is only implemented for vhost user client ports (since vhost user ports are considered deprecated). Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com> --- Documentation/topics/dpdk/vhost-user.rst | 19 +++++++++++++++++++ NEWS | 1 + lib/netdev-dpdk.c | 26 +++++++++++++++++++++----- vswitchd/vswitch.xml | 10 ++++++++++ 4 files changed, 51 insertions(+), 5 deletions(-)