Looks good to me in general, but I prefer if you'll fix following
checkpatch warnings:
== 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 <[email protected]>
>
> ---
>
> 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>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
