diff mbox series

[v5,03/16] vhost: add support for VHOST_USER_GET_CONFIG and VHOST_USER_SET_CONFIG

Message ID 1648451862-123318-4-git-send-email-andy.pei@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series add virtio_blk device support to vdpa/ifc |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Pei, Andy March 28, 2022, 7:17 a.m. UTC 
  Add support for VHOST_USER_GET_CONFIG and VHOST_USER_SET_CONFIG.
VHOST_USER_GET_CONFIG and VHOST_USER_SET_CONFIG message is only
supported by virtio blk VDPA device.

Signed-off-by: Andy Pei <andy.pei@intel.com>
---
 lib/vhost/vhost_user.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/vhost/vhost_user.h | 15 +++++++++++++++
 2 files changed, 65 insertions(+)

Comments

Chenbo Xia April 20, 2022, 1:53 p.m. UTC | #1 
Hi Andy,

> -----Original Message-----
> From: Pei, Andy <andy.pei@intel.com>
> Sent: Monday, March 28, 2022 3:17 PM
> To: dev@dpdk.org
> Cc: Xia, Chenbo <chenbo.xia@intel.com>; maxime.coquelin@redhat.com; Cao,
> Gang <gang.cao@intel.com>; Liu, Changpeng <changpeng.liu@intel.com>
> Subject: [PATCH v5 03/16] vhost: add support for VHOST_USER_GET_CONFIG and
> VHOST_USER_SET_CONFIG

Let's make the title a bit short...

./devtools/check-git-log.sh will help you find other similar errors for other
patches.

> 
> Add support for VHOST_USER_GET_CONFIG and VHOST_USER_SET_CONFIG.
> VHOST_USER_GET_CONFIG and VHOST_USER_SET_CONFIG message is only
> supported by virtio blk VDPA device.
> 
> Signed-off-by: Andy Pei <andy.pei@intel.com>
> ---
>  lib/vhost/vhost_user.c | 50
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/vhost/vhost_user.h | 15 +++++++++++++++
>  2 files changed, 65 insertions(+)
> 
> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> index 1d39067..55e8bd0 100644
> --- a/lib/vhost/vhost_user.c
> +++ b/lib/vhost/vhost_user.c
> @@ -80,6 +80,8 @@
>  	[VHOST_USER_NET_SET_MTU]  = "VHOST_USER_NET_SET_MTU",
>  	[VHOST_USER_SET_SLAVE_REQ_FD]  = "VHOST_USER_SET_SLAVE_REQ_FD",
>  	[VHOST_USER_IOTLB_MSG]  = "VHOST_USER_IOTLB_MSG",
> +	[VHOST_USER_GET_CONFIG]  = "VHOST_USER_GET_CONFIG",
> +	[VHOST_USER_SET_CONFIG]  = "VHOST_USER_SET_CONFIG",
>  	[VHOST_USER_CRYPTO_CREATE_SESS] = "VHOST_USER_CRYPTO_CREATE_SESS",
>  	[VHOST_USER_CRYPTO_CLOSE_SESS] = "VHOST_USER_CRYPTO_CLOSE_SESS",
>  	[VHOST_USER_POSTCOPY_ADVISE]  = "VHOST_USER_POSTCOPY_ADVISE",
> @@ -2542,6 +2544,52 @@ static int is_vring_iotlb(struct virtio_net *dev,
>  }
> 
>  static int
> +vhost_user_get_config(struct virtio_net **pdev,
> +			struct vhu_msg_context *ctx,
> +			int main_fd __rte_unused)
> +{
> +	struct virtio_net *dev = *pdev;
> +	struct rte_vdpa_device *vdpa_dev = dev->vdpa_dev;
> +	int ret = 0;
> +
> +	if (vdpa_dev->ops->get_config) {
> +		ret = vdpa_dev->ops->get_config(dev->vid,
> +					   ctx->msg.payload.cfg.region,
> +					   ctx->msg.payload.cfg.size);
> +		if (ret != 0) {
> +			ctx->msg.size = 0;
> +			VHOST_LOG_CONFIG(ERR, "get_config() return error!\n");
> +		}
> +	} else {
> +		VHOST_LOG_CONFIG(ERR, "get_config() not supported!\n");
> +	}
> +
> +	return RTE_VHOST_MSG_RESULT_REPLY;
> +}
> +
> +static int
> +vhost_user_set_config(struct virtio_net **pdev,
> +			struct vhu_msg_context *ctx,
> +			int main_fd __rte_unused)
> +{
> +	struct virtio_net *dev = *pdev;
> +	struct rte_vdpa_device *vdpa_dev = dev->vdpa_dev;
> +	int ret = 0;
> +
> +	if (vdpa_dev->ops->set_config) {
> +		ret = vdpa_dev->ops->set_config(dev->vid,
> +			ctx->msg.payload.cfg.region,
> +			ctx->msg.payload.cfg.offset,
> +			ctx->msg.payload.cfg.size,
> +			ctx->msg.payload.cfg.flags);
> +	} else {
> +		VHOST_LOG_CONFIG(ERR, "set_config() not supported!\n");
> +	}
> +
> +	return ret == 0 ? RTE_VHOST_MSG_RESULT_OK : RTE_VHOST_MSG_RESULT_ERR;

I think when set_config fails in vdpa driver, it should not break message handler by
returning RESULT_ERR here.

All error log above, please print dev->ifname too, which will be user-friendly.

> +}
> +
> +static int
>  vhost_user_iotlb_msg(struct virtio_net **pdev,
>  			struct vhu_msg_context *ctx,
>  			int main_fd __rte_unused)
> @@ -2782,6 +2830,8 @@ typedef int (*vhost_message_handler_t)(struct
> virtio_net **pdev,
>  	[VHOST_USER_NET_SET_MTU] = vhost_user_net_set_mtu,
>  	[VHOST_USER_SET_SLAVE_REQ_FD] = vhost_user_set_req_fd,
>  	[VHOST_USER_IOTLB_MSG] = vhost_user_iotlb_msg,
> +	[VHOST_USER_GET_CONFIG] = vhost_user_get_config,
> +	[VHOST_USER_SET_CONFIG] = vhost_user_set_config,
>  	[VHOST_USER_POSTCOPY_ADVISE] = vhost_user_set_postcopy_advise,
>  	[VHOST_USER_POSTCOPY_LISTEN] = vhost_user_set_postcopy_listen,
>  	[VHOST_USER_POSTCOPY_END] = vhost_user_postcopy_end,
> diff --git a/lib/vhost/vhost_user.h b/lib/vhost/vhost_user.h
> index c946cc2..d3f014e 100644
> --- a/lib/vhost/vhost_user.h
> +++ b/lib/vhost/vhost_user.h
> @@ -50,6 +50,8 @@
>  	VHOST_USER_NET_SET_MTU = 20,
>  	VHOST_USER_SET_SLAVE_REQ_FD = 21,
>  	VHOST_USER_IOTLB_MSG = 22,
> +	VHOST_USER_GET_CONFIG = 24,
> +	VHOST_USER_SET_CONFIG = 25,
>  	VHOST_USER_CRYPTO_CREATE_SESS = 26,
>  	VHOST_USER_CRYPTO_CLOSE_SESS = 27,
>  	VHOST_USER_POSTCOPY_ADVISE = 28,
> @@ -125,6 +127,18 @@
>  	uint16_t queue_size;
>  } VhostUserInflight;
> 
> +#ifndef VHOST_USER_MAX_CONFIG_SIZE
> +#define VHOST_USER_MAX_CONFIG_SIZE		256
> +#endif

For this config size, maybe '+#define VHOST_USER_MAX_CONFIG_SIZE 256' is enough?

> +
> +/** Get/set config msg payload */
> +struct vhost_user_config {
> +	uint32_t offset;
> +	uint32_t size;
> +	uint32_t flags;
> +	uint8_t region[VHOST_USER_MAX_CONFIG_SIZE];
> +};

Since the max size is defined, in the handler, we should check the size in the msg
handler.

Thanks,
Chenbo

> +
>  typedef struct VhostUserMsg {
>  	union {
>  		uint32_t master; /* a VhostUserRequest value */
> @@ -148,6 +162,7 @@
>  		VhostUserCryptoSessionParam crypto_session;
>  		VhostUserVringArea area;
>  		VhostUserInflight inflight;
> +		struct vhost_user_config cfg;
>  	} payload;
>  	/* Nothing should be added after the payload */
>  } __rte_packed VhostUserMsg;
> --
> 1.8.3.1
Pei, Andy April 21, 2022, 8:05 a.m. UTC | #2 
HI  Chenbo,

Thanks for your reply.
My reply is inline.

> -----Original Message-----
> From: Xia, Chenbo <chenbo.xia@intel.com>
> Sent: Wednesday, April 20, 2022 9:53 PM
> To: Pei, Andy <andy.pei@intel.com>; dev@dpdk.org
> Cc: maxime.coquelin@redhat.com; Cao, Gang <gang.cao@intel.com>; Liu,
> Changpeng <changpeng.liu@intel.com>
> Subject: RE: [PATCH v5 03/16] vhost: add support for
> VHOST_USER_GET_CONFIG and VHOST_USER_SET_CONFIG
>
> Hi Andy,
>
> > -----Original Message-----
> > From: Pei, Andy <andy.pei@intel.com>
> > Sent: Monday, March 28, 2022 3:17 PM
> > To: dev@dpdk.org
> > Cc: Xia, Chenbo <chenbo.xia@intel.com>; maxime.coquelin@redhat.com;
> > Cao, Gang <gang.cao@intel.com>; Liu, Changpeng
> > <changpeng.liu@intel.com>
> > Subject: [PATCH v5 03/16] vhost: add support for
> VHOST_USER_GET_CONFIG
> > and VHOST_USER_SET_CONFIG
>
> Let's make the title a bit short...
>
> ./devtools/check-git-log.sh will help you find other similar errors for other
> patches.
>
OK, I will send out V6 to fix commit log title.
Thanks for your suggestion.
> >
> > Add support for VHOST_USER_GET_CONFIG and
> VHOST_USER_SET_CONFIG.
> > VHOST_USER_GET_CONFIG and VHOST_USER_SET_CONFIG message is only
> > supported by virtio blk VDPA device.
> >
> > Signed-off-by: Andy Pei <andy.pei@intel.com>
> > ---
> >  lib/vhost/vhost_user.c | 50
> > ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  lib/vhost/vhost_user.h | 15 +++++++++++++++
> >  2 files changed, 65 insertions(+)
> >
> > diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index
> > 1d39067..55e8bd0 100644
> > --- a/lib/vhost/vhost_user.c
> > +++ b/lib/vhost/vhost_user.c
> > @@ -80,6 +80,8 @@
> >     [VHOST_USER_NET_SET_MTU]  = "VHOST_USER_NET_SET_MTU",
> >     [VHOST_USER_SET_SLAVE_REQ_FD]  =
> "VHOST_USER_SET_SLAVE_REQ_FD",
> >     [VHOST_USER_IOTLB_MSG]  = "VHOST_USER_IOTLB_MSG",
> > +   [VHOST_USER_GET_CONFIG]  = "VHOST_USER_GET_CONFIG",
> > +   [VHOST_USER_SET_CONFIG]  = "VHOST_USER_SET_CONFIG",
> >     [VHOST_USER_CRYPTO_CREATE_SESS] =
> "VHOST_USER_CRYPTO_CREATE_SESS",
> >     [VHOST_USER_CRYPTO_CLOSE_SESS] =
> "VHOST_USER_CRYPTO_CLOSE_SESS",
> >     [VHOST_USER_POSTCOPY_ADVISE]  =
> "VHOST_USER_POSTCOPY_ADVISE", @@
> > -2542,6 +2544,52 @@ static int is_vring_iotlb(struct virtio_net *dev,
> > }
> >
> >  static int
> > +vhost_user_get_config(struct virtio_net **pdev,
> > +                   struct vhu_msg_context *ctx,
> > +                   int main_fd __rte_unused)
> > +{
> > +   struct virtio_net *dev = *pdev;
> > +   struct rte_vdpa_device *vdpa_dev = dev->vdpa_dev;
> > +   int ret = 0;
> > +
> > +   if (vdpa_dev->ops->get_config) {
> > +           ret = vdpa_dev->ops->get_config(dev->vid,
> > +                                      ctx->msg.payload.cfg.region,
> > +                                      ctx->msg.payload.cfg.size);
> > +           if (ret != 0) {
> > +                   ctx->msg.size = 0;
> > +                   VHOST_LOG_CONFIG(ERR, "get_config() return
> error!\n");
> > +           }
> > +   } else {
> > +           VHOST_LOG_CONFIG(ERR, "get_config() not supported!\n");
> > +   }
> > +
> > +   return RTE_VHOST_MSG_RESULT_REPLY;
> > +}
> > +
> > +static int
> > +vhost_user_set_config(struct virtio_net **pdev,
> > +                   struct vhu_msg_context *ctx,
> > +                   int main_fd __rte_unused)
> > +{
> > +   struct virtio_net *dev = *pdev;
> > +   struct rte_vdpa_device *vdpa_dev = dev->vdpa_dev;
> > +   int ret = 0;
> > +
> > +   if (vdpa_dev->ops->set_config) {
> > +           ret = vdpa_dev->ops->set_config(dev->vid,
> > +                   ctx->msg.payload.cfg.region,
> > +                   ctx->msg.payload.cfg.offset,
> > +                   ctx->msg.payload.cfg.size,
> > +                   ctx->msg.payload.cfg.flags);
> > +   } else {
> > +           VHOST_LOG_CONFIG(ERR, "set_config() not supported!\n");
> > +   }
> > +
> > +   return ret == 0 ? RTE_VHOST_MSG_RESULT_OK :
> > +RTE_VHOST_MSG_RESULT_ERR;
>
> I think when set_config fails in vdpa driver, it should not break message
> handler by returning RESULT_ERR here.
>
I will return RTE_VHOST_MSG_RESULT_OK and output some log.

> All error log above, please print dev->ifname too, which will be user-friendly.
>
Sure.Thansk.
> > +}
> > +
> > +static int
> >  vhost_user_iotlb_msg(struct virtio_net **pdev,
> >                     struct vhu_msg_context *ctx,
> >                     int main_fd __rte_unused)
> > @@ -2782,6 +2830,8 @@ typedef int (*vhost_message_handler_t)(struct
> > virtio_net **pdev,
> >     [VHOST_USER_NET_SET_MTU] = vhost_user_net_set_mtu,
> >     [VHOST_USER_SET_SLAVE_REQ_FD] = vhost_user_set_req_fd,
> >     [VHOST_USER_IOTLB_MSG] = vhost_user_iotlb_msg,
> > +   [VHOST_USER_GET_CONFIG] = vhost_user_get_config,
> > +   [VHOST_USER_SET_CONFIG] = vhost_user_set_config,
> >     [VHOST_USER_POSTCOPY_ADVISE] =
> vhost_user_set_postcopy_advise,
> >     [VHOST_USER_POSTCOPY_LISTEN] = vhost_user_set_postcopy_listen,
> >     [VHOST_USER_POSTCOPY_END] = vhost_user_postcopy_end, diff --
> git
> > a/lib/vhost/vhost_user.h b/lib/vhost/vhost_user.h index
> > c946cc2..d3f014e 100644
> > --- a/lib/vhost/vhost_user.h
> > +++ b/lib/vhost/vhost_user.h
> > @@ -50,6 +50,8 @@
> >     VHOST_USER_NET_SET_MTU = 20,
> >     VHOST_USER_SET_SLAVE_REQ_FD = 21,
> >     VHOST_USER_IOTLB_MSG = 22,
> > +   VHOST_USER_GET_CONFIG = 24,
> > +   VHOST_USER_SET_CONFIG = 25,
> >     VHOST_USER_CRYPTO_CREATE_SESS = 26,
> >     VHOST_USER_CRYPTO_CLOSE_SESS = 27,
> >     VHOST_USER_POSTCOPY_ADVISE = 28,
> > @@ -125,6 +127,18 @@
> >     uint16_t queue_size;
> >  } VhostUserInflight;
> >
> > +#ifndef VHOST_USER_MAX_CONFIG_SIZE
> > +#define VHOST_USER_MAX_CONFIG_SIZE         256
> > +#endif
>
> For this config size, maybe '+#define VHOST_USER_MAX_CONFIG_SIZE 256' is
> enough?
>
Sure.
> > +
> > +/** Get/set config msg payload */
> > +struct vhost_user_config {
> > +   uint32_t offset;
> > +   uint32_t size;
> > +   uint32_t flags;
> > +   uint8_t region[VHOST_USER_MAX_CONFIG_SIZE];
> > +};
>
> Since the max size is defined, in the handler, we should check the size in the
> msg handler.
Sure.
>
> Thanks,
> Chenbo
>
> > +
> >  typedef struct VhostUserMsg {
> >     union {
> >             uint32_t master; /* a VhostUserRequest value */ @@ -148,6
> +162,7 @@
> >             VhostUserCryptoSessionParam crypto_session;
> >             VhostUserVringArea area;
> >             VhostUserInflight inflight;
> > +           struct vhost_user_config cfg;
> >     } payload;
> >     /* Nothing should be added after the payload */  } __rte_packed
> > VhostUserMsg;
> > --
> > 1.8.3.1
>
diff mbox series

Patch

diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
index 1d39067..55e8bd0 100644
--- a/lib/vhost/vhost_user.c
+++ b/lib/vhost/vhost_user.c
@@ -80,6 +80,8 @@ 
 	[VHOST_USER_NET_SET_MTU]  = "VHOST_USER_NET_SET_MTU",
 	[VHOST_USER_SET_SLAVE_REQ_FD]  = "VHOST_USER_SET_SLAVE_REQ_FD",
 	[VHOST_USER_IOTLB_MSG]  = "VHOST_USER_IOTLB_MSG",
+	[VHOST_USER_GET_CONFIG]  = "VHOST_USER_GET_CONFIG",
+	[VHOST_USER_SET_CONFIG]  = "VHOST_USER_SET_CONFIG",
 	[VHOST_USER_CRYPTO_CREATE_SESS] = "VHOST_USER_CRYPTO_CREATE_SESS",
 	[VHOST_USER_CRYPTO_CLOSE_SESS] = "VHOST_USER_CRYPTO_CLOSE_SESS",
 	[VHOST_USER_POSTCOPY_ADVISE]  = "VHOST_USER_POSTCOPY_ADVISE",
@@ -2542,6 +2544,52 @@  static int is_vring_iotlb(struct virtio_net *dev,
 }
 
 static int
+vhost_user_get_config(struct virtio_net **pdev,
+			struct vhu_msg_context *ctx,
+			int main_fd __rte_unused)
+{
+	struct virtio_net *dev = *pdev;
+	struct rte_vdpa_device *vdpa_dev = dev->vdpa_dev;
+	int ret = 0;
+
+	if (vdpa_dev->ops->get_config) {
+		ret = vdpa_dev->ops->get_config(dev->vid,
+					   ctx->msg.payload.cfg.region,
+					   ctx->msg.payload.cfg.size);
+		if (ret != 0) {
+			ctx->msg.size = 0;
+			VHOST_LOG_CONFIG(ERR, "get_config() return error!\n");
+		}
+	} else {
+		VHOST_LOG_CONFIG(ERR, "get_config() not supported!\n");
+	}
+
+	return RTE_VHOST_MSG_RESULT_REPLY;
+}
+
+static int
+vhost_user_set_config(struct virtio_net **pdev,
+			struct vhu_msg_context *ctx,
+			int main_fd __rte_unused)
+{
+	struct virtio_net *dev = *pdev;
+	struct rte_vdpa_device *vdpa_dev = dev->vdpa_dev;
+	int ret = 0;
+
+	if (vdpa_dev->ops->set_config) {
+		ret = vdpa_dev->ops->set_config(dev->vid,
+			ctx->msg.payload.cfg.region,
+			ctx->msg.payload.cfg.offset,
+			ctx->msg.payload.cfg.size,
+			ctx->msg.payload.cfg.flags);
+	} else {
+		VHOST_LOG_CONFIG(ERR, "set_config() not supported!\n");
+	}
+
+	return ret == 0 ? RTE_VHOST_MSG_RESULT_OK : RTE_VHOST_MSG_RESULT_ERR;
+}
+
+static int
 vhost_user_iotlb_msg(struct virtio_net **pdev,
 			struct vhu_msg_context *ctx,
 			int main_fd __rte_unused)
@@ -2782,6 +2830,8 @@  typedef int (*vhost_message_handler_t)(struct virtio_net **pdev,
 	[VHOST_USER_NET_SET_MTU] = vhost_user_net_set_mtu,
 	[VHOST_USER_SET_SLAVE_REQ_FD] = vhost_user_set_req_fd,
 	[VHOST_USER_IOTLB_MSG] = vhost_user_iotlb_msg,
+	[VHOST_USER_GET_CONFIG] = vhost_user_get_config,
+	[VHOST_USER_SET_CONFIG] = vhost_user_set_config,
 	[VHOST_USER_POSTCOPY_ADVISE] = vhost_user_set_postcopy_advise,
 	[VHOST_USER_POSTCOPY_LISTEN] = vhost_user_set_postcopy_listen,
 	[VHOST_USER_POSTCOPY_END] = vhost_user_postcopy_end,
diff --git a/lib/vhost/vhost_user.h b/lib/vhost/vhost_user.h
index c946cc2..d3f014e 100644
--- a/lib/vhost/vhost_user.h
+++ b/lib/vhost/vhost_user.h
@@ -50,6 +50,8 @@ 
 	VHOST_USER_NET_SET_MTU = 20,
 	VHOST_USER_SET_SLAVE_REQ_FD = 21,
 	VHOST_USER_IOTLB_MSG = 22,
+	VHOST_USER_GET_CONFIG = 24,
+	VHOST_USER_SET_CONFIG = 25,
 	VHOST_USER_CRYPTO_CREATE_SESS = 26,
 	VHOST_USER_CRYPTO_CLOSE_SESS = 27,
 	VHOST_USER_POSTCOPY_ADVISE = 28,
@@ -125,6 +127,18 @@ 
 	uint16_t queue_size;
 } VhostUserInflight;
 
+#ifndef VHOST_USER_MAX_CONFIG_SIZE
+#define VHOST_USER_MAX_CONFIG_SIZE		256
+#endif
+
+/** Get/set config msg payload */
+struct vhost_user_config {
+	uint32_t offset;
+	uint32_t size;
+	uint32_t flags;
+	uint8_t region[VHOST_USER_MAX_CONFIG_SIZE];
+};
+
 typedef struct VhostUserMsg {
 	union {
 		uint32_t master; /* a VhostUserRequest value */
@@ -148,6 +162,7 @@ 
 		VhostUserCryptoSessionParam crypto_session;
 		VhostUserVringArea area;
 		VhostUserInflight inflight;
+		struct vhost_user_config cfg;
 	} payload;
 	/* Nothing should be added after the payload */
 } __rte_packed VhostUserMsg;