[ovs-dev] [PATCH V4 2/2] netdev-dpdk: vHost IOMMU support

Kavanagh, Mark B mark.b.kavanagh at intel.com
Thu Dec 7 11:32:08 UTC 2017


Yuanhan's old email address was used in the previous mail - updated correctly here.
-Mark

>-----Original Message-----
>From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-bounces at openvswitch.org]
>On Behalf Of Kavanagh, Mark B
>Sent: Thursday, December 7, 2017 11:24 AM
>To: Stokes, Ian <ian.stokes at intel.com>; dev at openvswitch.org
>Cc: Liu, Yuanhan <yuanhan.liu at intel.com>; Bie, Tiwei <tiwei.bie at intel.com>;
>Mcnamara, John <john.mcnamara at intel.com>; maxime.coquelin at redhat.com;
>i.maximets at samsung.com
>Subject: Re: [ovs-dev] [PATCH V4 2/2] netdev-dpdk: vHost IOMMU support
>
>Hi folks,
>
>Thanks for all of your respective reviews and testing of this patchset.
>
>It seems, however, that an issue in DPDK v17.11 has come to light that affects
>guests which use testpmd.
>Specifically, traffic does not reach a guest when traffic is live prior to
>kicking off the testpmd app within said guest, and at least two forwarding
>cores are used. [1]
>
>This is explained in additional detail is [2], an extract of which is
>reproduced below:
>
>	"the vector Rx could be broken if backend has consumed all the avail
>descs before the
>	 device is started. Because in current implementation, the vector Rx will
>return immediately
> 	 without refilling the avail ring if the used ring is empty. So we have
>to refill
>	 the avail ring after flushing the elements in the used ring."
>
>This issue was initially uncovered by Antonio Fischetti, as part of the 17.11
>patchset validation, and has since been localized to DPDK, rather than OvS.
>As a result, it seems now that we should not move to 17.11, but seek an out-
>of-tree 17.11.1 stable/bugfix release. I'm interested in opinions on this -
>should we:
>	a) move to 17.11 now, note the issue above in the 'errata' section of the
>documentation, and move to 17.11.1 when it becomes available in February of
>next year
>	b) request the early release of the 17.11.1 stable branch, which
>incorporates a fix for this issue (the possibility, and availability, of which
>are both TBD).
>
>Thanks in advance,
>Mark
>
>[1] http://dpdk.org/ml/archives/dev/2017-December/082801.html
>[2] http://dpdk.org/ml/archives/dev/2017-December/082874.html
>
>
>>-----Original Message-----
>>From: Stokes, Ian
>>Sent: Thursday, December 7, 2017 8:43 AM
>>To: Kavanagh, Mark B <mark.b.kavanagh at intel.com>; dev at openvswitch.org
>>Cc: ktraynor at redhat.com; maxime.coquelin at redhat.com; i.maximets at samsung.com;
>>jan.scheurich at ericsson.com; Mooney, Sean K <sean.k.mooney at intel.com>
>>Subject: RE: [ovs-dev][PATCH V4 2/2] netdev-dpdk: vHost IOMMU support
>>
>>> DPDK v17.11 introduces support for the vHost IOMMU feature.
>>> This is a security feature, which restricts the vhost memory that a virtio
>>> device may access.
>>
>>Hi all,
>>
>>There were a few requests for changes in the v4 of this patch and it's an
>>important aspect of the DPDK 17.11 support for OVS.
>>
>>I'd like to get this series in to the pull request this week. If people have
>>flagged issues for the latest revision I'd appreciate if they could review
>the
>>latest revision and flag any new issues that need to be raised.
>>
>>Ian
>>
>>>
>>> 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 global config option, vhost-iommu-support, that
>>> controls enablement of the vhost IOMMU feature:
>>>
>>>     ovs-vsctl set Open_vSwitch . other_config:vhost-iommu-support=true
>>>
>>> This value defaults to false; to enable IOMMU support, this field should
>>> be set to true when setting other global parameters on init (such as
>>> "dpdk-socket-mem", for example). Changing the value at runtime is not
>>> supported, and requires restarting the vswitch daemon.
>>>
>>> Signed-off-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
>>>
>>> ---
>>>
>>> v4:  - rebase to HEAD of master
>>>      - clarify that IOMMU support applies exclusively to vhost user
>>>        client ports.
>>>      - reword caveats regarding modifying vhost-iommu-support at runtime
>>>      - use smap_get_bool() to parse vhost-iommu-support, instead of
>>>        process_vhost_flags().
>>>      - add stub implementation of dpdk_vhost_iommu_enabled().
>>>      - move stdbool.h #include outside of DPDK compiler guards.
>>>      - remove mention of vhost IOMMU mode from
>>>        netdev_dpdk_vhost_client_configure() log.
>>>
>>> v3:  - erroneous; disregard.
>>>
>>> v2:  - rebase to HEAD of master
>>>      - refactor vHost IOMMU enablement mechanism (use a global
>>>        config option, instead of the previous per-port approach).
>>> ---
>>>  Documentation/topics/dpdk/vhost-user.rst | 27 +++++++++++++++++++++++++++
>>>  NEWS                                     |  1 +
>>>  lib/dpdk-stub.c                          |  6 ++++++
>>>  lib/dpdk.c                               | 12 ++++++++++++
>>>  lib/dpdk.h                               |  3 +++
>>>  lib/netdev-dpdk.c                        | 14 ++++++++++----
>>>  vswitchd/vswitch.xml                     | 15 +++++++++++++++
>>>  7 files changed, 74 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/topics/dpdk/vhost-user.rst
>>> b/Documentation/topics/dpdk/vhost-user.rst
>>> index 5347995..ffef653 100644
>>> --- a/Documentation/topics/dpdk/vhost-user.rst
>>> +++ b/Documentation/topics/dpdk/vhost-user.rst
>>> @@ -273,6 +273,33 @@ One benefit of using this mode is the ability for
>>> vHost ports to 'reconnect' in  event of the switch crashing or being
>>> brought down. Once it is brought back up,  the vHost ports will reconnect
>>> automatically and normal service will resume.
>>>
>>> +vhost-user-client IOMMU Support
>>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> +
>>> +vhost IOMMU 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 support may be enabled via a global config value, ```vhost-iommu-
>>> support```.
>>> +Setting this to true enables vhost IOMMU support for all vhost ports
>>> +when/where
>>> +available::
>>> +
>>> +    $ ovs-vsctl set Open_vSwitch.other_config:vhost-iommu-support=true
>>> +
>>> +The default value is false.
>>> +
>>> +.. important::
>>> +
>>> +    Changing this value requires restarting the daemon.
>>> +
>>> +.. 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.
>>> +
>>>  .. _dpdk-testpmd:
>>>
>>>  DPDK in the Guest
>>> diff --git a/NEWS b/NEWS
>>> index d4a1c9a..99c47d8 100644
>>> --- a/NEWS
>>> +++ b/NEWS
>>> @@ -15,6 +15,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
>>>
>>>  v2.8.0 - 31 Aug 2017
>>>  --------------------
>>> diff --git a/lib/dpdk-stub.c b/lib/dpdk-stub.c index daef729..3602180
>>> 100644
>>> --- a/lib/dpdk-stub.c
>>> +++ b/lib/dpdk-stub.c
>>> @@ -48,3 +48,9 @@ dpdk_get_vhost_sock_dir(void)  {
>>>      return NULL;
>>>  }
>>> +
>>> +bool
>>> +dpdk_vhost_iommu_enabled(void)
>>> +{
>>> +    return false;
>>> +}
>>> diff --git a/lib/dpdk.c b/lib/dpdk.c
>>> index 8da6c32..6710d10 100644
>>> --- a/lib/dpdk.c
>>> +++ b/lib/dpdk.c
>>> @@ -41,6 +41,7 @@ VLOG_DEFINE_THIS_MODULE(dpdk);
>>>  static FILE *log_stream = NULL;       /* Stream for DPDK log redirection
>>> */
>>>
>>>  static char *vhost_sock_dir = NULL;   /* Location of vhost-user sockets
>>> */
>>> +static bool vhost_iommu_enabled = false; /* Status of vHost IOMMU
>>> +support */
>>>
>>>  static int
>>>  process_vhost_flags(char *flag, const char *default_val, int size, @@ -
>>> 345,6 +346,11 @@ dpdk_init__(const struct smap *ovs_other_config)
>>>          vhost_sock_dir = sock_dir_subcomponent;
>>>      }
>>>
>>> +    vhost_iommu_enabled = smap_get_bool(ovs_other_config,
>>> +                                        "vhost-iommu-support", false);
>>> +    VLOG_INFO("IOMMU support for vhost-user-client %s.",
>>> +               vhost_iommu_enabled ? "enabled" : "disabled");
>>> +
>>>      argv = grow_argv(&argv, 0, 1);
>>>      argc = 1;
>>>      argv[0] = xstrdup(ovs_get_program_name()); @@ -482,6 +488,12 @@
>>> dpdk_get_vhost_sock_dir(void)
>>>      return vhost_sock_dir;
>>>  }
>>>
>>> +bool
>>> +dpdk_vhost_iommu_enabled(void)
>>> +{
>>> +    return vhost_iommu_enabled;
>>> +}
>>> +
>>>  void
>>>  dpdk_set_lcore_id(unsigned cpu)
>>>  {
>>> diff --git a/lib/dpdk.h b/lib/dpdk.h
>>> index 673a1f1..dc58d96 100644
>>> --- a/lib/dpdk.h
>>> +++ b/lib/dpdk.h
>>> @@ -17,6 +17,8 @@
>>>  #ifndef DPDK_H
>>>  #define DPDK_H
>>>
>>> +#include <stdbool.h>
>>> +
>>>  #ifdef DPDK_NETDEV
>>>
>>>  #include <rte_config.h>
>>> @@ -35,5 +37,6 @@ struct smap;
>>>  void dpdk_init(const struct smap *ovs_other_config);  void
>>> dpdk_set_lcore_id(unsigned cpu);  const char
>>> *dpdk_get_vhost_sock_dir(void);
>>> +bool dpdk_vhost_iommu_enabled(void);
>>>
>>>  #endif /* dpdk.h */
>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index f552444..9715c39
>>> 100644
>>> --- a/lib/netdev-dpdk.c
>>> +++ b/lib/netdev-dpdk.c
>>> @@ -3253,6 +3253,7 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev
>>> *netdev)  {
>>>      struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>>>      int err;
>>> +    uint64_t vhost_flags = 0;
>>>
>>>      ovs_mutex_lock(&dev->mutex);
>>>
>>> @@ -3263,16 +3264,21 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev
>>> *netdev)
>>>       */
>>>      if (!(dev->vhost_driver_flags & RTE_VHOST_USER_CLIENT)
>>>              && strlen(dev->vhost_id)) {
>>> -        /* Register client-mode device */
>>> -        err = rte_vhost_driver_register(dev->vhost_id,
>>> -                                        RTE_VHOST_USER_CLIENT);
>>> +        /* Register client-mode device. */
>>> +        vhost_flags |= RTE_VHOST_USER_CLIENT;
>>> +
>>> +        /* Enable IOMMU support, if explicitly requested. */
>>> +        if (dpdk_vhost_iommu_enabled()) {
>>> +            vhost_flags |= RTE_VHOST_USER_IOMMU_SUPPORT;
>>> +        }
>>> +        err = rte_vhost_driver_register(dev->vhost_id, vhost_flags);
>>>          if (err) {
>>>              VLOG_ERR("vhost-user device setup failure for device %s\n",
>>>                       dev->vhost_id);
>>>              goto unlock;
>>>          } else {
>>>              /* Configuration successful */
>>> -            dev->vhost_driver_flags |= RTE_VHOST_USER_CLIENT;
>>> +            dev->vhost_driver_flags |= vhost_flags;
>>>              VLOG_INFO("vHost User device '%s' created in 'client' mode, "
>>>                        "using client socket '%s'",
>>>                        dev->up.name, dev->vhost_id); diff --git
>>> a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index c145e1a..6c6df50
>>> 100644
>>> --- a/vswitchd/vswitch.xml
>>> +++ b/vswitchd/vswitch.xml
>>> @@ -344,6 +344,21 @@
>>>          </p>
>>>        </column>
>>>
>>> +      <column name="other_config" key="vhost-iommu-support"
>>> +              type='{"type": "boolean"}'>
>>> +        <p>
>>> +          vHost IOMMU is a  security feature, which restricts the vhost
>>> memory
>>> +          that a virtio device may access. vHost IOMMU support is
>>> disabled by
>>> +          default, due to a bug in QEMU implementations of the vhost
>>> REPLY_ACK
>>> +          protocol, (on which vHost IOMMU relies) prior to v2.9.1.
>>> Setting this
>>> +          value to <code>true</code> enables vHost IOMMU support for
>>> vHost User
>>> +          Client ports in OvS-DPDK, starting from DPDK v17.11.
>>> +        </p>
>>> +        <p>
>>> +          Changing this value requires restarting the daemon.
>>> +        </p>
>>> +      </column>
>>> +
>>>        <column name="other_config" key="n-handler-threads"
>>>                type='{"type": "integer", "minInteger": 1}'>
>>>          <p>
>>> --
>>> 2.7.4
>
>_______________________________________________
>dev mailing list
>dev at openvswitch.org
>https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list