Hi Chenbo,
Thanks for your reply.
> -----Original Message-----
> From: Xia, Chenbo <chenbo....@intel.com>
> Sent: Wednesday, May 11, 2022 10:24 PM
> To: Pei, Andy <andy....@intel.com>; dev@dpdk.org
> Cc: maxime.coque...@redhat.com; Cao, Gang <gang....@intel.com>; Liu,
> Changpeng <changpeng....@intel.com>
> Subject: RE: [PATCH v7 03/18] vhost: add vhost msg support
>
> > -----Original Message-----
> > From: Pei, Andy <andy....@intel.com>
> > Sent: Wednesday, April 27, 2022 4:30 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 v7 03/18] vhost: add vhost msg support
>
> Title better be: vhost: add vhost msg support for get/set config
>
Sure.
> >
> > 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 | 83
> > ++++++++++++++++++++++++++++++++++++++++++++++++++
> > lib/vhost/vhost_user.h | 13 ++++++++
> > 2 files changed, 96 insertions(+)
> >
> > diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index
> > 1d39067..e925428 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,85 @@ 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->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);
> > + }
> > +
> > + 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.size != sizeof(struct vhost_user_config)) {
> > + VHOST_LOG_CONFIG(ERR,
> > + "(%s) invalid set config msg size: %"PRIu32" != %d\n",
> > + dev->ifname, ctx->msg.size,
> > + (int)sizeof(struct vhost_user_config));
> > + goto out;
> > + }
>
> Sorry, I was wrong in v6, after double check, the size can be checked in
> read_vhost_message (although not that accurate check). So we can remove
> above. (btw, even we check, the logic should Be size <= sizeof(config))
>
> Rest of the patch seems good, thanks.
>
> Chenbo
>
OK. I will remove ctx->msg.size check here.
> > +
> > + 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->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)
> > @@ -2782,6 +2863,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
>
