[ovs-dev] [PATCH V4 2/2] netdev-dpdk: vHost IOMMU support
Kavanagh, Mark B
mark.b.kavanagh at intel.com
Thu Dec 7 11:33:00 UTC 2017
>From: Ilya Maximets [mailto:i.maximets at samsung.com]
>Sent: Thursday, December 7, 2017 9:49 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;
>jan.scheurich at ericsson.com; Mooney, Sean K <sean.k.mooney at intel.com>; Stokes,
>Ian <ian.stokes at intel.com>
>Subject: Re: [ovs-dev][PATCH V4 2/2] netdev-dpdk: vHost IOMMU support
>
>Looks good to me in general, but I prefer if you'll fix following
>checkpatch warnings:
Thanks for your comments Ilya - I'll address same in the next version of this patchset, depending on the outcome of this: https://mail.openvswitch.org/pipermail/ovs-dev/2017-December/341701.html
Best regards,
Mark
>
>== Checking 2e9587f27add ("netdev-dpdk: vHost IOMMU support") ==
>WARNING: Line length is >79-characters long
>#52 FILE: Documentation/topics/dpdk/vhost-user.rst:280:
>can access, and as such is useful in deployments in which security is a
>concern.
>
>WARNING: Line length is >79-characters long
>#54 FILE: Documentation/topics/dpdk/vhost-user.rst:282:
>IOMMU support may be enabled via a global config value, ```vhost-iommu-
>support```.
>
>WARNING: Line length is >79-characters long
>#71 FILE: Documentation/topics/dpdk/vhost-user.rst:299:
> default (and should remain so if using the aforementioned versions of
>QEMU).
>
>One more comment inline.
>
>Best regards, Ilya Maximets.
>
>On 04.12.2017 14:15, Mark Kavanagh wrote:
>> 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.
>>
>> 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
>
>Few spaces missing in above command.
>
>> +
>> +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>
>>
More information about the dev
mailing list