Yuanhan's old email address was used in the previous mail - updated correctly here. -Mark
>-----Original Message----- >From: [email protected] [mailto:[email protected]] >On Behalf Of Kavanagh, Mark B >Sent: Thursday, December 7, 2017 11:24 AM >To: Stokes, Ian <[email protected]>; [email protected] >Cc: Liu, Yuanhan <[email protected]>; Bie, Tiwei <[email protected]>; >Mcnamara, John <[email protected]>; [email protected]; >[email protected] >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 <[email protected]>; [email protected] >>Cc: [email protected]; [email protected]; [email protected]; >>[email protected]; Mooney, Sean K <[email protected]> >>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 <[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 >>> + >>> +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 >[email protected] >https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
