Hi Andy,
> -----Original Message-----
> From: Pei, Andy <andy....@intel.com>
> Sent: Thursday, April 21, 2022 4:34 PM
> To: dev@dpdk.org
> Cc: Xia, Chenbo <chenbo....@intel.com>; maxime.coque...@redhat.com; Cao,
> Gang <gang....@intel.com>; Liu, Changpeng <changpeng....@intel.com>
> Subject: [PATCH v6 03/16] vhost: add vhost msg support
>
> 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....@intel.com>
> ---
> lib/vhost/vhost_user.c | 69
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> lib/vhost/vhost_user.h | 13 ++++++++++
> 2 files changed, 82 insertions(+)
>
> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> index 1d39067..3780804 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,71 @@ 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,
> + "(%s) get_config() return error!\n",
> + dev->ifname);
> + }
> + } else {
> + VHOST_LOG_CONFIG(ERR, "(%s) get_config() not supportted!\n",
Supported
> + dev->ifname);
> + }
> +
> + 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 (ctx->msg.size != sizeof(struct vhost_user_config)) {
I think you should do sanity check on payload.cfg.size and make sure it's
smaller
than VHOST_USER_MAX_CONFIG_SIZE
and same check for offset
> + VHOST_LOG_CONFIG(ERR,
> + "(%s) invalid set config msg size: %"PRId32" != %d\n",
> + dev->ifname, ctx->msg.size,
Based on you will change the log too, payload.cfg.size is uint32_t,
so PRId32 -> PRIu32
> + (int)sizeof(struct vhost_user_config));
So this can be %u
> + 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 supportted!\n",
Supported
> + dev->ifname);
> + }
> +
> + return RTE_VHOST_MSG_RESULT_OK;
> +
> +OUT:
Lower case looks better
> + return RTE_VHOST_MSG_RESULT_ERR;
> +}
Almost all handlers need check on expected fd num (this case is 0), so the
above new
handlers should also do that. Please refer to validate_msg_fds in other
handlers.
BTW, you can wait for review for other patches and send new versions later.
Thanks,
Chenbo
> +
> +static int
> vhost_user_iotlb_msg(struct virtio_net **pdev,
> struct vhu_msg_context *ctx,
> int main_fd __rte_unused)
> @@ -2782,6 +2849,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..97cfb2f 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,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 */
> @@ -148,6 +160,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
