[ovs-dev,RFC,V2,2/2] netdev-dpdk: add support for vhost IOMMU feature

Message ID 1510142410-169357-3-git-send-email-mark.b.kavanagh@intel.com
State Superseded
Headers show
Series
  • DPDK v17.11 Support
Related show

Commit Message

Mark Kavanagh Nov. 8, 2017, noon
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

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 | 21 +++++++++++++++++++++
 NEWS                                     |  1 +
 lib/netdev-dpdk.c                        | 29 ++++++++++++++++++++++++++---
 vswitchd/vswitch.xml                     | 10 ++++++++++
 4 files changed, 58 insertions(+), 3 deletions(-)

Comments

Loftus, Ciara Nov. 8, 2017, 1:25 p.m. | #1
> 
> 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
> 
> 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 | 21 +++++++++++++++++++++
>  NEWS                                     |  1 +
>  lib/netdev-dpdk.c                        | 29 ++++++++++++++++++++++++++---
>  vswitchd/vswitch.xml                     | 10 ++++++++++
>  4 files changed, 58 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/topics/dpdk/vhost-user.rst
> b/Documentation/topics/dpdk/vhost-user.rst
> index fdf6ae1..6d65971 100644
> --- a/Documentation/topics/dpdk/vhost-user.rst
> +++ b/Documentation/topics/dpdk/vhost-user.rst
> @@ -250,6 +250,27 @@ 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).
> +    Starting with QEMU v2.9.1, vhost-iommu-support can safely be enabled,
> even
> +    without having an IOMMU device, with no performance penalty.
> +
>  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..2e9633a 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1424,15 +1424,29 @@ netdev_dpdk_vhost_client_set_config(struct
> netdev *netdev,
>  {
>      struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>      const char *path;
> +    bool iommu_enable;
> +    bool request_reconfigure = false;
> +    uint64_t vhost_driver_flags_prev = dev->vhost_driver_flags;
> 
>      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)) {
>              strcpy(dev->vhost_id, path);
> -            netdev_request_reconfigure(netdev);
> +            request_reconfigure = true;
>          }
>      }
> +
> +    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;
> +    if (vhost_driver_flags_prev != dev->vhost_driver_flags)
> +        request_reconfigure = true;
> +
> +    if (request_reconfigure)
> +        netdev_request_reconfigure(netdev);
>      ovs_mutex_unlock(&dev->mutex);

Thanks for making the change - LGTM

Acked-by: Ciara Loftus <ciara.loftus@intel.com>

> 
>      return 0;
> @@ -3326,9 +3340,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 c145e1a..a633226 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -2654,6 +2654,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
Mark Kavanagh Nov. 8, 2017, 2:07 p.m. | #2
>From: Loftus, Ciara
>Sent: Wednesday, November 8, 2017 1:25 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 V2 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). 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
>>
>> 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 | 21 +++++++++++++++++++++
>>  NEWS                                     |  1 +
>>  lib/netdev-dpdk.c                        | 29 ++++++++++++++++++++++++++---
>>  vswitchd/vswitch.xml                     | 10 ++++++++++
>>  4 files changed, 58 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/topics/dpdk/vhost-user.rst
>> b/Documentation/topics/dpdk/vhost-user.rst
>> index fdf6ae1..6d65971 100644
>> --- a/Documentation/topics/dpdk/vhost-user.rst
>> +++ b/Documentation/topics/dpdk/vhost-user.rst
>> @@ -250,6 +250,27 @@ 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).
>> +    Starting with QEMU v2.9.1, vhost-iommu-support can safely be enabled,
>> even
>> +    without having an IOMMU device, with no performance penalty.
>> +
>>  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..2e9633a 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -1424,15 +1424,29 @@ netdev_dpdk_vhost_client_set_config(struct
>> netdev *netdev,
>>  {
>>      struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>>      const char *path;
>> +    bool iommu_enable;
>> +    bool request_reconfigure = false;
>> +    uint64_t vhost_driver_flags_prev = dev->vhost_driver_flags;
>>
>>      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)) {
>>              strcpy(dev->vhost_id, path);
>> -            netdev_request_reconfigure(netdev);
>> +            request_reconfigure = true;
>>          }
>>      }
>> +
>> +    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;
>> +    if (vhost_driver_flags_prev != dev->vhost_driver_flags)
>> +        request_reconfigure = true;
>> +
>> +    if (request_reconfigure)
>> +        netdev_request_reconfigure(netdev);
>>      ovs_mutex_unlock(&dev->mutex);
>
>Thanks for making the change - LGTM

No problem - thank you for the quick review, and the catch!
-Mark

>
>Acked-by: Ciara Loftus <ciara.loftus@intel.com>
>
>>
>>      return 0;
>> @@ -3326,9 +3340,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 c145e1a..a633226 100644
>> --- a/vswitchd/vswitch.xml
>> +++ b/vswitchd/vswitch.xml
>> @@ -2654,6 +2654,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

Patch

diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-user.rst
index fdf6ae1..6d65971 100644
--- a/Documentation/topics/dpdk/vhost-user.rst
+++ b/Documentation/topics/dpdk/vhost-user.rst
@@ -250,6 +250,27 @@  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).
+    Starting with QEMU v2.9.1, vhost-iommu-support can safely be enabled, even
+    without having an IOMMU device, with no performance penalty.
+
 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..2e9633a 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1424,15 +1424,29 @@  netdev_dpdk_vhost_client_set_config(struct netdev *netdev,
 {
     struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
     const char *path;
+    bool iommu_enable;
+    bool request_reconfigure = false;
+    uint64_t vhost_driver_flags_prev = dev->vhost_driver_flags;
 
     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)) {
             strcpy(dev->vhost_id, path);
-            netdev_request_reconfigure(netdev);
+            request_reconfigure = true;
         }
     }
+
+    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;
+    if (vhost_driver_flags_prev != dev->vhost_driver_flags)
+        request_reconfigure = true;
+
+    if (request_reconfigure)
+        netdev_request_reconfigure(netdev);
     ovs_mutex_unlock(&dev->mutex);
 
     return 0;
@@ -3326,9 +3340,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 c145e1a..a633226 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -2654,6 +2654,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>