All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] vhost: fix multiple queue not being enabled for older kernels
@ 2017-02-23  8:45 Yuanhan Liu
  2017-03-01  8:16 ` Maxime Coquelin
  2017-03-01 10:41 ` [PATCH v2 1/2] " Yuanhan Liu
  0 siblings, 2 replies; 5+ messages in thread
From: Yuanhan Liu @ 2017-02-23  8:45 UTC (permalink / raw)
  To: dev; +Cc: Maxime Coquelin, Yuanhan Liu, stable

Some macros (say VIRTIO_NET_F_MQ) are needed for enabling multiple queue,
however they are introduced since kernel v3.8, meaning build error happens
if we build DPDK vhost on those platforms.

71dfdbe66a66 ("vhost: fix build with kernel < 3.8") meant to fix it, but
in a wrong way: it completely disables the MQ features for those kernels.
However, the MQ feature doesn't depend on the kernel at all (except the
macros dependency stated above), that we could still enable the MQ feature
even the host kernel has no such support.

The right fix is to define the macro if it's not defined.

Fixes: 71dfdbe66a66 ("vhost: fix build with kernel < 3.8")

Cc: stable@dpdk.org
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 lib/librte_vhost/vhost.c      |  2 +-
 lib/librte_vhost/vhost.h      | 11 ++++-------
 lib/librte_vhost/vhost_user.c |  2 +-
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index e415093..3c3f6a4 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -56,7 +56,7 @@
 				(1ULL << VIRTIO_NET_F_CTRL_VQ) | \
 				(1ULL << VIRTIO_NET_F_CTRL_RX) | \
 				(1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE) | \
-				(VHOST_SUPPORTS_MQ)            | \
+				(1ULL << VIRTIO_NET_F_MQ)      | \
 				(1ULL << VIRTIO_F_VERSION_1)   | \
 				(1ULL << VHOST_F_LOG_ALL)      | \
 				(1ULL << VHOST_USER_F_PROTOCOL_FEATURES) | \
diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 22564f1..34af209 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -122,12 +122,9 @@ struct vhost_virtqueue {
  * introduced since kernel v3.8. This makes our
  * code buildable for older kernel.
  */
-#ifdef VIRTIO_NET_F_MQ
- #define VHOST_MAX_QUEUE_PAIRS	VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX
- #define VHOST_SUPPORTS_MQ	(1ULL << VIRTIO_NET_F_MQ)
-#else
- #define VHOST_MAX_QUEUE_PAIRS	1
- #define VHOST_SUPPORTS_MQ	0
+#ifndef VIRTIO_NET_F_MQ
+ #define VIRTIO_NET_F_MQ			22
+ #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX	0x8000
 #endif
 
 /*
@@ -159,7 +156,7 @@ struct virtio_net {
 	rte_atomic16_t		broadcast_rarp;
 	uint32_t		virt_qp_nb;
 	int			dequeue_zero_copy;
-	struct vhost_virtqueue	*virtqueue[VHOST_MAX_QUEUE_PAIRS * 2];
+	struct vhost_virtqueue	*virtqueue[VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX * 2];
 #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)
 	char			ifname[IF_NAME_SZ];
 	uint64_t		log_size;
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index cb2156a..9825e01 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -1015,7 +1015,7 @@
 		break;
 
 	case VHOST_USER_GET_QUEUE_NUM:
-		msg.payload.u64 = VHOST_MAX_QUEUE_PAIRS;
+		msg.payload.u64 = VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX;
 		msg.size = sizeof(msg.payload.u64);
 		send_vhost_message(fd, &msg);
 		break;
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] vhost: fix multiple queue not being enabled for older kernels
  2017-02-23  8:45 [PATCH] vhost: fix multiple queue not being enabled for older kernels Yuanhan Liu
@ 2017-03-01  8:16 ` Maxime Coquelin
  2017-03-01 10:41 ` [PATCH v2 1/2] " Yuanhan Liu
  1 sibling, 0 replies; 5+ messages in thread
From: Maxime Coquelin @ 2017-03-01  8:16 UTC (permalink / raw)
  To: Yuanhan Liu, dev; +Cc: stable



On 02/23/2017 09:45 AM, Yuanhan Liu wrote:
> Some macros (say VIRTIO_NET_F_MQ) are needed for enabling multiple queue,
> however they are introduced since kernel v3.8, meaning build error happens
> if we build DPDK vhost on those platforms.
>
> 71dfdbe66a66 ("vhost: fix build with kernel < 3.8") meant to fix it, but
> in a wrong way: it completely disables the MQ features for those kernels.
> However, the MQ feature doesn't depend on the kernel at all (except the
> macros dependency stated above), that we could still enable the MQ feature
> even the host kernel has no such support.
>
> The right fix is to define the macro if it's not defined.
>
> Fixes: 71dfdbe66a66 ("vhost: fix build with kernel < 3.8")
>
> Cc: stable@dpdk.org
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> ---
>  lib/librte_vhost/vhost.c      |  2 +-
>  lib/librte_vhost/vhost.h      | 11 ++++-------
>  lib/librte_vhost/vhost_user.c |  2 +-
>  3 files changed, 6 insertions(+), 9 deletions(-)
>
Agree this is the right fix:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks!
Maxime

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH v2 1/2] vhost: fix multiple queue not being enabled for older kernels
  2017-02-23  8:45 [PATCH] vhost: fix multiple queue not being enabled for older kernels Yuanhan Liu
  2017-03-01  8:16 ` Maxime Coquelin
@ 2017-03-01 10:41 ` Yuanhan Liu
  2017-03-01 10:41   ` [PATCH v2 2/2] vhost: fix max queues Yuanhan Liu
  1 sibling, 1 reply; 5+ messages in thread
From: Yuanhan Liu @ 2017-03-01 10:41 UTC (permalink / raw)
  To: dev; +Cc: Maxime Coquelin, Yuanhan Liu, stable

Some macros (say VIRTIO_NET_F_MQ) are needed for enabling multiple queue,
however they are introduced since kernel v3.8, meaning build error happens
if we build DPDK vhost on those platforms.

71dfdbe66a66 ("vhost: fix build with kernel < 3.8") meant to fix it, but
in a wrong way: it completely disables the MQ features for those kernels.
However, the MQ feature doesn't depend on the kernel at all (except the
macros dependency stated above), that we could still enable the MQ feature
even the host kernel has no such support.

The right fix is to define the macro if it's not defined.

Fixes: 71dfdbe66a66 ("vhost: fix build with kernel < 3.8")

Cc: stable@dpdk.org
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---

- v2: do not remove the VHOST_MAX_QUEUE_PAIRS macro. The value which
      in turn will be fixed in next patch.
---
 lib/librte_vhost/vhost.c | 2 +-
 lib/librte_vhost/vhost.h | 7 +++----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index e415093..3c3f6a4 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -56,7 +56,7 @@
 				(1ULL << VIRTIO_NET_F_CTRL_VQ) | \
 				(1ULL << VIRTIO_NET_F_CTRL_RX) | \
 				(1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE) | \
-				(VHOST_SUPPORTS_MQ)            | \
+				(1ULL << VIRTIO_NET_F_MQ)      | \
 				(1ULL << VIRTIO_F_VERSION_1)   | \
 				(1ULL << VHOST_F_LOG_ALL)      | \
 				(1ULL << VHOST_USER_F_PROTOCOL_FEATURES) | \
diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 22564f1..4391e62 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -123,11 +123,10 @@ struct vhost_virtqueue {
  * code buildable for older kernel.
  */
 #ifdef VIRTIO_NET_F_MQ
- #define VHOST_MAX_QUEUE_PAIRS	VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX
- #define VHOST_SUPPORTS_MQ	(1ULL << VIRTIO_NET_F_MQ)
+ #define VHOST_MAX_QUEUE_PAIRS		VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX
 #else
- #define VHOST_MAX_QUEUE_PAIRS	1
- #define VHOST_SUPPORTS_MQ	0
+ #define VIRTIO_NET_F_MQ		22
+ #define VHOST_MAX_QUEUE_PAIRS		0x8000
 #endif
 
 /*
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH v2 2/2] vhost: fix max queues
  2017-03-01 10:41 ` [PATCH v2 1/2] " Yuanhan Liu
@ 2017-03-01 10:41   ` Yuanhan Liu
  2017-03-22  8:36     ` Yuanhan Liu
  0 siblings, 1 reply; 5+ messages in thread
From: Yuanhan Liu @ 2017-03-01 10:41 UTC (permalink / raw)
  To: dev; +Cc: Maxime Coquelin, Yuanhan Liu, stable

0x8000 is the max virito-net queue pairs the virtio 1.0 spec claims to
support. While for vhost-user, it's a different story: the max vring
index could be passed by the vhost-user spec is 0xff, masked by the
VHOST_USER_VRING_IDX_MASK.

That said, the max queue pairs could vhost-user could supported is 0x80.
If user are asking more, I think the vhost-user need be extended.

Fixes: b09b198bfb5c ("vhost-user: announce queue number in message")

Cc: stable@dpdk.org
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 lib/librte_vhost/vhost.h | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 4391e62..d97df1d 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -110,24 +110,15 @@ struct vhost_virtqueue {
 	uint16_t                shadow_used_idx;
 } __rte_cache_aligned;
 
-/* Old kernels have no such macro defined */
+/* Old kernels have no such macros defined */
 #ifndef VIRTIO_NET_F_GUEST_ANNOUNCE
  #define VIRTIO_NET_F_GUEST_ANNOUNCE 21
 #endif
 
-
-/*
- * Make an extra wrapper for VIRTIO_NET_F_MQ and
- * VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX as they are
- * introduced since kernel v3.8. This makes our
- * code buildable for older kernel.
- */
-#ifdef VIRTIO_NET_F_MQ
- #define VHOST_MAX_QUEUE_PAIRS		VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX
-#else
+#ifndef VIRTIO_NET_F_MQ
  #define VIRTIO_NET_F_MQ		22
- #define VHOST_MAX_QUEUE_PAIRS		0x8000
 #endif
+#define VHOST_MAX_QUEUE_PAIRS		0x80
 
 /*
  * Define virtio 1.0 for older kernels
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] vhost: fix max queues
  2017-03-01 10:41   ` [PATCH v2 2/2] vhost: fix max queues Yuanhan Liu
@ 2017-03-22  8:36     ` Yuanhan Liu
  0 siblings, 0 replies; 5+ messages in thread
From: Yuanhan Liu @ 2017-03-22  8:36 UTC (permalink / raw)
  To: dev; +Cc: Maxime Coquelin, stable

On Wed, Mar 01, 2017 at 06:41:59PM +0800, Yuanhan Liu wrote:
> 0x8000 is the max virito-net queue pairs the virtio 1.0 spec claims to
> support. While for vhost-user, it's a different story: the max vring
> index could be passed by the vhost-user spec is 0xff, masked by the
> VHOST_USER_VRING_IDX_MASK.
> 
> That said, the max queue pairs could vhost-user could supported is 0x80.
> If user are asking more, I think the vhost-user need be extended.
> 
> Fixes: b09b198bfb5c ("vhost-user: announce queue number in message")
> 
> Cc: stable@dpdk.org
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

Both applied to dpdk-next-virtio.

	--yliu

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-22  8:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-23  8:45 [PATCH] vhost: fix multiple queue not being enabled for older kernels Yuanhan Liu
2017-03-01  8:16 ` Maxime Coquelin
2017-03-01 10:41 ` [PATCH v2 1/2] " Yuanhan Liu
2017-03-01 10:41   ` [PATCH v2 2/2] vhost: fix max queues Yuanhan Liu
2017-03-22  8:36     ` Yuanhan Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.