[PATCH v9 03/13] vhost: add vhost msg support for get/set config

[Xia, Chenbo]

> -----Original Message-----
> From: Pei, Andy <andy.pei at intel.com>
> Sent: Monday, May 23, 2022 5:35 PM
> To: dev at dpdk.org
> Cc: Xia, Chenbo <chenbo.xia at intel.com>; maxime.coquelin at redhat.com; Cao,
> Gang <gang.cao at intel.com>; Liu, Changpeng <changpeng.liu at intel.com>; Xu,
> Rosen <rosen.xu at intel.com>; Xiao, QimaiX <qimaix.xiao at intel.com>
> Subject: [PATCH v9 03/13] vhost: add vhost msg support for get/set config
> 
> 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 at intel.com>
> ---
>  lib/vhost/vhost_user.c | 88
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/vhost/vhost_user.h | 13 ++++++++
>  2 files changed, 101 insertions(+)
> 
> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> index 850848c..b37d814 100644
> --- a/lib/vhost/vhost_user.c
> +++ b/lib/vhost/vhost_user.c
> @@ -2468,6 +2468,92 @@ 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 (validate_msg_fds(dev, ctx, 0) != 0)
> +		return RTE_VHOST_MSG_RESULT_ERR;
> +
> +	if (!vdpa_dev) {
> +		VHOST_LOG_CONFIG(ERR,
> +				 "(%s) is not vDPA device!\n",
> +				 dev->ifname);
> +		goto out;
> +	}

In my understanding, this message is only targeted for vDPA device.
Traditional SW back-end will not use it. So if vdpa_dev is not attached,
you can just return RTE_VHOST_MSG_RESULT_ERR here. Another way is
setting msg size to 0 and reply, but I think in this case. We should just
break the msg handling.

Besides, two lines are enough for the log above:

+		VHOST_LOG_CONFIG(ERR, "(%s) is not vDPA device!\n",
+				 dev->ifname);

Thanks,
Chenbo

> +
> +	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,
> +					 "(%s) get_config() return error!\n",
> +					 dev->ifname);
> +		}
> +	} else {
> +		VHOST_LOG_CONFIG(ERR, "(%s) get_config() not supported!\n",
> +				 dev->ifname);
> +	}
> +
> +out:
> +	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 (validate_msg_fds(dev, ctx, 0) != 0)
> +		return RTE_VHOST_MSG_RESULT_ERR;
> +
> +	if (ctx->msg.payload.cfg.size > VHOST_USER_MAX_CONFIG_SIZE) {
> +		VHOST_LOG_CONFIG(ERR,
> +			"(%s) vhost_user_config size: %"PRIu32", should not be
> larger than %d\n",
> +			dev->ifname, ctx->msg.payload.cfg.size,
> +			VHOST_USER_MAX_CONFIG_SIZE);
> +		goto out;
> +	}
> +
> +	if (!vdpa_dev) {
> +		VHOST_LOG_CONFIG(ERR,
> +				 "(%s) is not vDPA device!\n",
> +				 dev->ifname);
> +		goto out;
> +	}
> +
> +	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);
> +		if (ret)
> +			VHOST_LOG_CONFIG(ERR,
> +					 "(%s) set_config() return error!\n",
> +					 dev->ifname);
> +	} else {
> +		VHOST_LOG_CONFIG(ERR, "(%s) set_config() not supported!\n",
> +				 dev->ifname);
> +	}
> +
> +	return RTE_VHOST_MSG_RESULT_OK;
> +
> +out:
> +	return 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)
> @@ -2686,6 +2772,8 @@ static int is_vring_iotlb(struct virtio_net *dev,
>  VHOST_MESSAGE_HANDLER(VHOST_USER_NET_SET_MTU, vhost_user_net_set_mtu,
> false) \
>  VHOST_MESSAGE_HANDLER(VHOST_USER_SET_SLAVE_REQ_FD, vhost_user_set_req_fd,
> true) \
>  VHOST_MESSAGE_HANDLER(VHOST_USER_IOTLB_MSG, vhost_user_iotlb_msg, false)
> \
> +VHOST_MESSAGE_HANDLER(VHOST_USER_GET_CONFIG, vhost_user_get_config, false)
> \
> +VHOST_MESSAGE_HANDLER(VHOST_USER_SET_CONFIG, vhost_user_set_config, false)
> \
>  VHOST_MESSAGE_HANDLER(VHOST_USER_POSTCOPY_ADVISE,
> vhost_user_set_postcopy_advise, false) \
>  VHOST_MESSAGE_HANDLER(VHOST_USER_POSTCOPY_LISTEN,
> vhost_user_set_postcopy_listen, false) \
>  VHOST_MESSAGE_HANDLER(VHOST_USER_POSTCOPY_END, vhost_user_postcopy_end,
> false) \
> diff --git a/lib/vhost/vhost_user.h b/lib/vhost/vhost_user.h
> index ba1c5c7..c4d091e 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,
> @@ -123,6 +125,16 @@
>  	uint16_t queue_size;
>  } VhostUserInflight;
> 
> +#define VHOST_USER_MAX_CONFIG_SIZE		256
> +
> +/** 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 */
> @@ -146,6 +158,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