[dpdk-dev] vhost: disable reply-ack protocol feature if iommu feature disabled
Checks
Commit Message
If the application has disabled VIRTIO_F_IOMMU_PLATFORM, disable
VHOST_USER_PROTOCOL_F_REPLY_ACK protocol feature that is only
mandatory with IOMMU for now.
This is done to provide a way for the application to support
multiqueue with old Qemu versions (v2.7.0 to v2.9.0) that have
reply-ack feature broken.
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
This is an alternative to my proposition of adding a new flag at
vhost register time. Advantage of this solution is that it does
not bring API change.
lib/librte_vhost/vhost_user.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
Comments
>From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>Sent: Friday, November 3, 2017 5:58 PM
>To: dev@dpdk.org; yliu@fridaylinux.org; Kavanagh, Mark B
><mark.b.kavanagh@intel.com>; thomas@monjalon.net; ktraynor@redhat.com
>Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>Subject: [PATCH] vhost: disable reply-ack protocol feature if iommu feature
>disabled
>
>If the application has disabled VIRTIO_F_IOMMU_PLATFORM, disable
>VHOST_USER_PROTOCOL_F_REPLY_ACK protocol feature that is only
>mandatory with IOMMU for now.
>
>This is done to provide a way for the application to support
>multiqueue with old Qemu versions (v2.7.0 to v2.9.0) that have
>reply-ack feature broken.
>
>Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>---
>
>This is an alternative to my proposition of adding a new flag at
>vhost register time. Advantage of this solution is that it does
>not bring API change.
>
> lib/librte_vhost/vhost_user.c | 24 ++++++++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
>
>diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
>index 1f6cba4b9..e35218688 100644
>--- a/lib/librte_vhost/vhost_user.c
>+++ b/lib/librte_vhost/vhost_user.c
>@@ -878,6 +878,27 @@ vhost_user_set_vring_enable(struct virtio_net **pdev,
> }
>
> static void
>+vhost_user_get_protocol_features(struct virtio_net *dev,
>+ struct VhostUserMsg *msg)
>+{
>+ uint64_t features, protocol_features = VHOST_USER_PROTOCOL_FEATURES;
>+
>+ rte_vhost_driver_get_features(dev->ifname, &features);
>+
>+ /*
>+ * REPLY_ACK protocol feature is only mandatory for now
>+ * for IOMMU feature. If IOMMU is explicitly disabled by the
>+ * application, disable also REPLY_ACK feature for older buggy
>+ * Qemu versions (from v2.7.0 to v2.9.0).
>+ */
>+ if (!(features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)))
Hi Maxime,
Thanks for this patch - I like this approach, as it maintains API compatibility.
Now for the gotchas, unfortunately:
- VIRTIO_F_IOMMU_PLATFORM is defined in vhost.h, as opposed to rte_vhost.h, so it is not exposed to OvS :(
- Currently, we use other similar macros in OvS (VIRTIO_NET_F_CSUM, VIRTIO_NET_F_HOST_TSO[4|6]); however, we obtain the definition of these from a kernel header file, as opposed to a DPDK header (linux/virtio_net.h)
-- We could adopt the same approach for VIRTIO_F_IOMMU_PLATFORM, and include linux/virtio_config.h; unfortunately, the VIRTIO_F_IOMMU_PLATFORM macro is only defined from kernel 4.8, which creates another problem entirely.
One potential solution is to move the VIRTIO_* definitions to rte_vhost.h, but at this stage in the DPDK release cycle, that's probably a tall ask.
Any thoughts?
Thanks again,
Mark
>+ protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK);
>+
>+ msg->payload.u64 = protocol_features;
>+ msg->size = sizeof(msg->payload.u64);
>+}
>+
>+static void
> vhost_user_set_protocol_features(struct virtio_net *dev,
> uint64_t protocol_features)
> {
>@@ -1248,8 +1269,7 @@ vhost_user_msg_handler(int vid, int fd)
> break;
>
> case VHOST_USER_GET_PROTOCOL_FEATURES:
>- msg.payload.u64 = VHOST_USER_PROTOCOL_FEATURES;
>- msg.size = sizeof(msg.payload.u64);
>+ vhost_user_get_protocol_features(dev, &msg);
> send_vhost_reply(fd, &msg);
> break;
> case VHOST_USER_SET_PROTOCOL_FEATURES:
>--
>2.13.6
This series disables IOMMU feature by default, and introduce
a new flag passed at vhost device registration time to enable
it explicitly.
When disabled, patch 1 also disables reply-ack protocol feature
to avoid Qemu v2.7.0-v2.9.0 reply-ack bug with multiqueue.
Last patch adds a Vhost PMD "iommu-support" parameter to enable
the IOMMU feature.
Maxime Coquelin (3):
vhost: disable reply-ack protocol feature if iommu feature disabled
vhost: add flag to enable iommu support
net: vhost: add iommu-support parameter to enable IOMMU feature
doc/guides/nics/vhost.rst | 5 +++++
doc/guides/prog_guide/vhost_lib.rst | 14 ++++++++++++++
doc/guides/rel_notes/release_17_11.rst | 3 ++-
drivers/net/vhost/rte_eth_vhost.c | 13 +++++++++++++
lib/librte_vhost/rte_vhost.h | 1 +
lib/librte_vhost/socket.c | 6 ++++++
lib/librte_vhost/vhost_user.c | 24 ++++++++++++++++++++++--
7 files changed, 63 insertions(+), 3 deletions(-)
On Mon, Nov 06, 2017 at 09:38:09PM +0100, Maxime Coquelin wrote:
> This series disables IOMMU feature by default, and introduce
> a new flag passed at vhost device registration time to enable
> it explicitly.
>
> When disabled, patch 1 also disables reply-ack protocol feature
> to avoid Qemu v2.7.0-v2.9.0 reply-ack bug with multiqueue.
>
> Last patch adds a Vhost PMD "iommu-support" parameter to enable
> the IOMMU feature.
Series Acked-by: Yuanhan Liu <yliu@fridaylinux.org>
Thanks!
--yliu
>
> Maxime Coquelin (3):
> vhost: disable reply-ack protocol feature if iommu feature disabled
> vhost: add flag to enable iommu support
> net: vhost: add iommu-support parameter to enable IOMMU feature
>
> doc/guides/nics/vhost.rst | 5 +++++
> doc/guides/prog_guide/vhost_lib.rst | 14 ++++++++++++++
> doc/guides/rel_notes/release_17_11.rst | 3 ++-
> drivers/net/vhost/rte_eth_vhost.c | 13 +++++++++++++
> lib/librte_vhost/rte_vhost.h | 1 +
> lib/librte_vhost/socket.c | 6 ++++++
> lib/librte_vhost/vhost_user.c | 24 ++++++++++++++++++++++--
> 7 files changed, 63 insertions(+), 3 deletions(-)
>
> --
> 2.13.6
>From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>Sent: Monday, November 6, 2017 8:38 PM
>To: dev@dpdk.org; yliu@fridaylinux.org; Kavanagh, Mark B
><mark.b.kavanagh@intel.com>; thomas@monjalon.net; ktraynor@redhat.com
>Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>Subject: [PATCH v2 0/3] vhost: disable iommu support by default
>
>This series disables IOMMU feature by default, and introduce
>a new flag passed at vhost device registration time to enable
>it explicitly.
>
>When disabled, patch 1 also disables reply-ack protocol feature
>to avoid Qemu v2.7.0-v2.9.0 reply-ack bug with multiqueue.
>
>Last patch adds a Vhost PMD "iommu-support" parameter to enable
>the IOMMU feature.
Hi Maxime,
I'm happy to confirm that this patchset resolves the vhost user mutltiq issue for OvS-DPDK, with QEMU v2.7.1.
Additionally, all of the individual patches look good - thanks for all of your efforts on this!
Tested-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
Acked-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
Cheers,
Mark
>
>Maxime Coquelin (3):
> vhost: disable reply-ack protocol feature if iommu feature disabled
> vhost: add flag to enable iommu support
> net: vhost: add iommu-support parameter to enable IOMMU feature
>
> doc/guides/nics/vhost.rst | 5 +++++
> doc/guides/prog_guide/vhost_lib.rst | 14 ++++++++++++++
> doc/guides/rel_notes/release_17_11.rst | 3 ++-
> drivers/net/vhost/rte_eth_vhost.c | 13 +++++++++++++
> lib/librte_vhost/rte_vhost.h | 1 +
> lib/librte_vhost/socket.c | 6 ++++++
> lib/librte_vhost/vhost_user.c | 24 ++++++++++++++++++++++--
> 7 files changed, 63 insertions(+), 3 deletions(-)
>
>--
>2.13.6
Hi Mark,
On 11/07/2017 11:56 AM, Kavanagh, Mark B wrote:
>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>> Sent: Monday, November 6, 2017 8:38 PM
>> To: dev@dpdk.org; yliu@fridaylinux.org; Kavanagh, Mark B
>> <mark.b.kavanagh@intel.com>; thomas@monjalon.net; ktraynor@redhat.com
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Subject: [PATCH v2 0/3] vhost: disable iommu support by default
>>
>> This series disables IOMMU feature by default, and introduce
>> a new flag passed at vhost device registration time to enable
>> it explicitly.
>>
>> When disabled, patch 1 also disables reply-ack protocol feature
>> to avoid Qemu v2.7.0-v2.9.0 reply-ack bug with multiqueue.
>>
>> Last patch adds a Vhost PMD "iommu-support" parameter to enable
>> the IOMMU feature.
>
> Hi Maxime,
>
> I'm happy to confirm that this patchset resolves the vhost user mutltiq issue for OvS-DPDK, with QEMU v2.7.1.
Thanks for the testing.
> Additionally, all of the individual patches look good - thanks for all of your efforts on this!
Great.
Now, what is required on OVS side is the introduction of a new vhost
port option to enable IOMMU support, so that management layer has a way
to enable it when VM has an iommu placed in front of the virtio device.
Note that OVS can set the flag even if no IOMMU is present, as Virtio
feature negotiation will manage this.
> Tested-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
> Acked-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
Thanks,
Maxime
> Cheers,
> Mark
>
>>
>> Maxime Coquelin (3):
>> vhost: disable reply-ack protocol feature if iommu feature disabled
>> vhost: add flag to enable iommu support
>> net: vhost: add iommu-support parameter to enable IOMMU feature
>>
>> doc/guides/nics/vhost.rst | 5 +++++
>> doc/guides/prog_guide/vhost_lib.rst | 14 ++++++++++++++
>> doc/guides/rel_notes/release_17_11.rst | 3 ++-
>> drivers/net/vhost/rte_eth_vhost.c | 13 +++++++++++++
>> lib/librte_vhost/rte_vhost.h | 1 +
>> lib/librte_vhost/socket.c | 6 ++++++
>> lib/librte_vhost/vhost_user.c | 24 ++++++++++++++++++++++--
>> 7 files changed, 63 insertions(+), 3 deletions(-)
>>
>> --
>> 2.13.6
>
>From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>Sent: Tuesday, November 7, 2017 11:05 AM
>To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; dev@dpdk.org;
>yliu@fridaylinux.org; thomas@monjalon.net; ktraynor@redhat.com
>Subject: Re: [PATCH v2 0/3] vhost: disable iommu support by default
>
>Hi Mark,
>
>On 11/07/2017 11:56 AM, Kavanagh, Mark B wrote:
>>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>>> Sent: Monday, November 6, 2017 8:38 PM
>>> To: dev@dpdk.org; yliu@fridaylinux.org; Kavanagh, Mark B
>>> <mark.b.kavanagh@intel.com>; thomas@monjalon.net; ktraynor@redhat.com
>>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>>> Subject: [PATCH v2 0/3] vhost: disable iommu support by default
>>>
>>> This series disables IOMMU feature by default, and introduce
>>> a new flag passed at vhost device registration time to enable
>>> it explicitly.
>>>
>>> When disabled, patch 1 also disables reply-ack protocol feature
>>> to avoid Qemu v2.7.0-v2.9.0 reply-ack bug with multiqueue.
>>>
>>> Last patch adds a Vhost PMD "iommu-support" parameter to enable
>>> the IOMMU feature.
>>
>> Hi Maxime,
>>
>> I'm happy to confirm that this patchset resolves the vhost user mutltiq
>issue for OvS-DPDK, with QEMU v2.7.1.
>
>Thanks for the testing.
>
>> Additionally, all of the individual patches look good - thanks for all of
>your efforts on this!
>
>Great.
>Now, what is required on OVS side is the introduction of a new vhost
>port option to enable IOMMU support, so that management layer has a way
>to enable it when VM has an iommu placed in front of the virtio device.
Sounds good - I can add this as part of the DPDK v17.11 upgrade patch :)
Thanks again,
Mark
>
>Note that OVS can set the flag even if no IOMMU is present, as Virtio
>feature negotiation will manage this.
>
>> Tested-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
>> Acked-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
>
>Thanks,
>Maxime
>
>> Cheers,
>> Mark
>>
>>>
>>> Maxime Coquelin (3):
>>> vhost: disable reply-ack protocol feature if iommu feature disabled
>>> vhost: add flag to enable iommu support
>>> net: vhost: add iommu-support parameter to enable IOMMU feature
>>>
>>> doc/guides/nics/vhost.rst | 5 +++++
>>> doc/guides/prog_guide/vhost_lib.rst | 14 ++++++++++++++
>>> doc/guides/rel_notes/release_17_11.rst | 3 ++-
>>> drivers/net/vhost/rte_eth_vhost.c | 13 +++++++++++++
>>> lib/librte_vhost/rte_vhost.h | 1 +
>>> lib/librte_vhost/socket.c | 6 ++++++
>>> lib/librte_vhost/vhost_user.c | 24 ++++++++++++++++++++++--
>>> 7 files changed, 63 insertions(+), 3 deletions(-)
>>>
>>> --
>>> 2.13.6
>>
On 11/07/2017 10:56 AM, Kavanagh, Mark B wrote:
>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>> Sent: Monday, November 6, 2017 8:38 PM
>> To: dev@dpdk.org; yliu@fridaylinux.org; Kavanagh, Mark B
>> <mark.b.kavanagh@intel.com>; thomas@monjalon.net; ktraynor@redhat.com
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Subject: [PATCH v2 0/3] vhost: disable iommu support by default
>>
>> This series disables IOMMU feature by default, and introduce
>> a new flag passed at vhost device registration time to enable
>> it explicitly.
>>
>> When disabled, patch 1 also disables reply-ack protocol feature
>> to avoid Qemu v2.7.0-v2.9.0 reply-ack bug with multiqueue.
>>
>> Last patch adds a Vhost PMD "iommu-support" parameter to enable
>> the IOMMU feature.
>
Hi Maxime - OVS-DPDK does not use the vhost pmd, so that means that
iommu could not be used with OVS-DPDK at present. Did I get that right?
Ciara proposed patches for vhost pmd in OVS-DPDK but it is quite an
intrusive change and has been postponed multiple times due to various
issues.
thanks,
Kevin.
> Hi Maxime,
>
> I'm happy to confirm that this patchset resolves the vhost user mutltiq issue for OvS-DPDK, with QEMU v2.7.1.
>
> Additionally, all of the individual patches look good - thanks for all of your efforts on this!
>
> Tested-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
> Acked-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
>
> Cheers,
> Mark
>
>>
>> Maxime Coquelin (3):
>> vhost: disable reply-ack protocol feature if iommu feature disabled
>> vhost: add flag to enable iommu support
>> net: vhost: add iommu-support parameter to enable IOMMU feature
>>
>> doc/guides/nics/vhost.rst | 5 +++++
>> doc/guides/prog_guide/vhost_lib.rst | 14 ++++++++++++++
>> doc/guides/rel_notes/release_17_11.rst | 3 ++-
>> drivers/net/vhost/rte_eth_vhost.c | 13 +++++++++++++
>> lib/librte_vhost/rte_vhost.h | 1 +
>> lib/librte_vhost/socket.c | 6 ++++++
>> lib/librte_vhost/vhost_user.c | 24 ++++++++++++++++++++++--
>> 7 files changed, 63 insertions(+), 3 deletions(-)
>>
>> --
>> 2.13.6
>
On 11/07/2017 12:25 PM, Kevin Traynor wrote:
> On 11/07/2017 10:56 AM, Kavanagh, Mark B wrote:
>>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>>> Sent: Monday, November 6, 2017 8:38 PM
>>> To: dev@dpdk.org; yliu@fridaylinux.org; Kavanagh, Mark B
>>> <mark.b.kavanagh@intel.com>; thomas@monjalon.net; ktraynor@redhat.com
>>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>>> Subject: [PATCH v2 0/3] vhost: disable iommu support by default
>>>
>>> This series disables IOMMU feature by default, and introduce
>>> a new flag passed at vhost device registration time to enable
>>> it explicitly.
>>>
>>> When disabled, patch 1 also disables reply-ack protocol feature
>>> to avoid Qemu v2.7.0-v2.9.0 reply-ack bug with multiqueue.
>>>
>>> Last patch adds a Vhost PMD "iommu-support" parameter to enable
>>> the IOMMU feature.
>>
Hi Kevin,
> Hi Maxime - OVS-DPDK does not use the vhost pmd, so that means that
> iommu could not be used with OVS-DPDK at present. Did I get that right?
No :)
This is supported both with and without using Vhost PMD.
When using the Vhost lib directly, you just have to pass
RTE_VHOST_USER_IOMMU_SUPPORT flag to rte_vhost_driver_register(),
this is patch 1.
When using Vhost PMD, passing the iommu-support=1 option to the vdev
cmdline will set this flag, this is patch 2.
Thanks,
Maxime
> Ciara proposed patches for vhost pmd in OVS-DPDK but it is quite an
> intrusive change and has been postponed multiple times due to various
> issues.
>
> thanks,
> Kevin.
>
>> Hi Maxime,
>>
>> I'm happy to confirm that this patchset resolves the vhost user mutltiq issue for OvS-DPDK, with QEMU v2.7.1.
>>
>> Additionally, all of the individual patches look good - thanks for all of your efforts on this!
>>
>> Tested-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
>> Acked-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
>>
>> Cheers,
>> Mark
>>
>>>
>>> Maxime Coquelin (3):
>>> vhost: disable reply-ack protocol feature if iommu feature disabled
>>> vhost: add flag to enable iommu support
>>> net: vhost: add iommu-support parameter to enable IOMMU feature
>>>
>>> doc/guides/nics/vhost.rst | 5 +++++
>>> doc/guides/prog_guide/vhost_lib.rst | 14 ++++++++++++++
>>> doc/guides/rel_notes/release_17_11.rst | 3 ++-
>>> drivers/net/vhost/rte_eth_vhost.c | 13 +++++++++++++
>>> lib/librte_vhost/rte_vhost.h | 1 +
>>> lib/librte_vhost/socket.c | 6 ++++++
>>> lib/librte_vhost/vhost_user.c | 24 ++++++++++++++++++++++--
>>> 7 files changed, 63 insertions(+), 3 deletions(-)
>>>
>>> --
>>> 2.13.6
>>
>
On 11/07/2017 11:30 AM, Maxime Coquelin wrote:
>
>
> On 11/07/2017 12:25 PM, Kevin Traynor wrote:
>> On 11/07/2017 10:56 AM, Kavanagh, Mark B wrote:
>>>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>>>> Sent: Monday, November 6, 2017 8:38 PM
>>>> To: dev@dpdk.org; yliu@fridaylinux.org; Kavanagh, Mark B
>>>> <mark.b.kavanagh@intel.com>; thomas@monjalon.net; ktraynor@redhat.com
>>>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>> Subject: [PATCH v2 0/3] vhost: disable iommu support by default
>>>>
>>>> This series disables IOMMU feature by default, and introduce
>>>> a new flag passed at vhost device registration time to enable
>>>> it explicitly.
>>>>
>>>> When disabled, patch 1 also disables reply-ack protocol feature
>>>> to avoid Qemu v2.7.0-v2.9.0 reply-ack bug with multiqueue.
>>>>
>>>> Last patch adds a Vhost PMD "iommu-support" parameter to enable
>>>> the IOMMU feature.
>>>
>
> Hi Kevin,
>
>> Hi Maxime - OVS-DPDK does not use the vhost pmd, so that means that
>> iommu could not be used with OVS-DPDK at present. Did I get that right?
>
> No :)
>
Good :)
> This is supported both with and without using Vhost PMD.
>
> When using the Vhost lib directly, you just have to pass
> RTE_VHOST_USER_IOMMU_SUPPORT flag to rte_vhost_driver_register(),
> this is patch 1.
>
> When using Vhost PMD, passing the iommu-support=1 option to the vdev
> cmdline will set this flag, this is patch 2.
>
Ah ok, just catching up, thanks for the ABC. It seems like a good
compromise, sorry for the noise.
> Thanks,
> Maxime
>
>> Ciara proposed patches for vhost pmd in OVS-DPDK but it is quite an
>> intrusive change and has been postponed multiple times due to various
>> issues.
>>
>> thanks,
>> Kevin.
>>
>>> Hi Maxime,
>>>
>>> I'm happy to confirm that this patchset resolves the vhost user
>>> mutltiq issue for OvS-DPDK, with QEMU v2.7.1.
>>>
>>> Additionally, all of the individual patches look good - thanks for
>>> all of your efforts on this!
>>>
>>> Tested-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
>>> Acked-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
>>>
>>> Cheers,
>>> Mark
>>>
>>>>
>>>> Maxime Coquelin (3):
>>>> vhost: disable reply-ack protocol feature if iommu feature disabled
>>>> vhost: add flag to enable iommu support
>>>> net: vhost: add iommu-support parameter to enable IOMMU feature
>>>>
>>>> doc/guides/nics/vhost.rst | 5 +++++
>>>> doc/guides/prog_guide/vhost_lib.rst | 14 ++++++++++++++
>>>> doc/guides/rel_notes/release_17_11.rst | 3 ++-
>>>> drivers/net/vhost/rte_eth_vhost.c | 13 +++++++++++++
>>>> lib/librte_vhost/rte_vhost.h | 1 +
>>>> lib/librte_vhost/socket.c | 6 ++++++
>>>> lib/librte_vhost/vhost_user.c | 24 ++++++++++++++++++++++--
>>>> 7 files changed, 63 insertions(+), 3 deletions(-)
>>>>
>>>> --
>>>> 2.13.6
>>>
>>
On 11/07/2017 12:08 PM, Kavanagh, Mark B wrote:
>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>> Sent: Tuesday, November 7, 2017 11:05 AM
>> To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; dev@dpdk.org;
>> yliu@fridaylinux.org; thomas@monjalon.net; ktraynor@redhat.com
>> Subject: Re: [PATCH v2 0/3] vhost: disable iommu support by default
>>
>> Hi Mark,
>>
>> On 11/07/2017 11:56 AM, Kavanagh, Mark B wrote:
>>>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>>>> Sent: Monday, November 6, 2017 8:38 PM
>>>> To: dev@dpdk.org; yliu@fridaylinux.org; Kavanagh, Mark B
>>>> <mark.b.kavanagh@intel.com>; thomas@monjalon.net; ktraynor@redhat.com
>>>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>> Subject: [PATCH v2 0/3] vhost: disable iommu support by default
>>>>
>>>> This series disables IOMMU feature by default, and introduce
>>>> a new flag passed at vhost device registration time to enable
>>>> it explicitly.
>>>>
>>>> When disabled, patch 1 also disables reply-ack protocol feature
>>>> to avoid Qemu v2.7.0-v2.9.0 reply-ack bug with multiqueue.
>>>>
>>>> Last patch adds a Vhost PMD "iommu-support" parameter to enable
>>>> the IOMMU feature.
>>>
>>> Hi Maxime,
>>>
>>> I'm happy to confirm that this patchset resolves the vhost user mutltiq
>> issue for OvS-DPDK, with QEMU v2.7.1.
>>
>> Thanks for the testing.
>>
>>> Additionally, all of the individual patches look good - thanks for all of
>> your efforts on this!
>>
>> Great.
>> Now, what is required on OVS side is the introduction of a new vhost
>> port option to enable IOMMU support, so that management layer has a way
>> to enable it when VM has an iommu placed in front of the virtio device.
>
> Sounds good - I can add this as part of the DPDK v17.11 upgrade patch :)
Thanks :) Please add me in cc when you'll post the change, so that I can
keep track of it.
Regards,
Maxime
> Thanks again,
> Mark
>
>
>>
>> Note that OVS can set the flag even if no IOMMU is present, as Virtio
>> feature negotiation will manage this.
>>
>>> Tested-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
>>> Acked-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
>>
>> Thanks,
>> Maxime
>>
>>> Cheers,
>>> Mark
>>>
>>>>
>>>> Maxime Coquelin (3):
>>>> vhost: disable reply-ack protocol feature if iommu feature disabled
>>>> vhost: add flag to enable iommu support
>>>> net: vhost: add iommu-support parameter to enable IOMMU feature
>>>>
>>>> doc/guides/nics/vhost.rst | 5 +++++
>>>> doc/guides/prog_guide/vhost_lib.rst | 14 ++++++++++++++
>>>> doc/guides/rel_notes/release_17_11.rst | 3 ++-
>>>> drivers/net/vhost/rte_eth_vhost.c | 13 +++++++++++++
>>>> lib/librte_vhost/rte_vhost.h | 1 +
>>>> lib/librte_vhost/socket.c | 6 ++++++
>>>> lib/librte_vhost/vhost_user.c | 24 ++++++++++++++++++++++--
>>>> 7 files changed, 63 insertions(+), 3 deletions(-)
>>>>
>>>> --
>>>> 2.13.6
>>>
07/11/2017 04:32, Yuanhan Liu:
> On Mon, Nov 06, 2017 at 09:38:09PM +0100, Maxime Coquelin wrote:
> > This series disables IOMMU feature by default, and introduce
> > a new flag passed at vhost device registration time to enable
> > it explicitly.
> >
> > When disabled, patch 1 also disables reply-ack protocol feature
> > to avoid Qemu v2.7.0-v2.9.0 reply-ack bug with multiqueue.
> >
> > Last patch adds a Vhost PMD "iommu-support" parameter to enable
> > the IOMMU feature.
>
> Series Acked-by: Yuanhan Liu <yliu@fridaylinux.org>
Applied, thanks
@@ -878,6 +878,27 @@ vhost_user_set_vring_enable(struct virtio_net **pdev,
}
static void
+vhost_user_get_protocol_features(struct virtio_net *dev,
+ struct VhostUserMsg *msg)
+{
+ uint64_t features, protocol_features = VHOST_USER_PROTOCOL_FEATURES;
+
+ rte_vhost_driver_get_features(dev->ifname, &features);
+
+ /*
+ * REPLY_ACK protocol feature is only mandatory for now
+ * for IOMMU feature. If IOMMU is explicitly disabled by the
+ * application, disable also REPLY_ACK feature for older buggy
+ * Qemu versions (from v2.7.0 to v2.9.0).
+ */
+ if (!(features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)))
+ protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK);
+
+ msg->payload.u64 = protocol_features;
+ msg->size = sizeof(msg->payload.u64);
+}
+
+static void
vhost_user_set_protocol_features(struct virtio_net *dev,
uint64_t protocol_features)
{
@@ -1248,8 +1269,7 @@ vhost_user_msg_handler(int vid, int fd)
break;
case VHOST_USER_GET_PROTOCOL_FEATURES:
- msg.payload.u64 = VHOST_USER_PROTOCOL_FEATURES;
- msg.size = sizeof(msg.payload.u64);
+ vhost_user_get_protocol_features(dev, &msg);
send_vhost_reply(fd, &msg);
break;
case VHOST_USER_SET_PROTOCOL_FEATURES: