> -----Original Message-----
> From: David Marchand <david.march...@redhat.com>
> Sent: Tuesday, February 7, 2023 6:45 PM
> To: dev@dpdk.org
> Cc: maxime.coque...@redhat.com; step...@networkplumber.org; Xia, Chenbo
> <chenbo....@intel.com>; Hu, Jiayu <jiayu...@intel.com>; Wang, YuanX
> <yuanx.w...@intel.com>; Ding, Xuan <xuan.d...@intel.com>;
> m...@smartsharesystems.com
> Subject: [PATCH v6 2/9] vhost: simplify need reply handling
>
> Dedicate send_vhost_slave_message() helper to the case when no reply is
> needed.
>
> Add a send_vhost_slave_message_process_reply() helper for the opposite.
> This new helper merges both send_vhost_slave_message() and the code
> previously in process_slave_message_reply().
> The slave_req_lock lock is then only handled in this helper which will
> make lock checks trivial.
>
> Signed-off-by: David Marchand <david.march...@redhat.com>
> Acked-by: Morten Brørup <m...@smartsharesystems.com>
> Reviewed-by: Maxime Coquelin <maxime.coque...@redhat.com>
> ---
> lib/vhost/vhost_user.c | 119 ++++++++++++++++++-----------------------
> 1 file changed, 51 insertions(+), 68 deletions(-)
>
> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> index 8f33d5f4d9..60ec1bf5f6 100644
> --- a/lib/vhost/vhost_user.c
> +++ b/lib/vhost/vhost_user.c
> @@ -2878,18 +2878,46 @@ send_vhost_reply(struct virtio_net *dev, int
> sockfd, struct vhu_msg_context *ctx
> }
>
> static int
> -send_vhost_slave_message(struct virtio_net *dev,
> - struct vhu_msg_context *ctx)
> +send_vhost_slave_message(struct virtio_net *dev, struct vhu_msg_context
> *ctx)
> +{
> + return send_vhost_message(dev, dev->slave_req_fd, ctx);
> +}
> +
> +static int
> +send_vhost_slave_message_process_reply(struct virtio_net *dev, struct
> vhu_msg_context *ctx)
> {
> + struct vhu_msg_context msg_reply;
> int ret;
>
> - if (ctx->msg.flags & VHOST_USER_NEED_REPLY)
> - rte_spinlock_lock(&dev->slave_req_lock);
> + rte_spinlock_lock(&dev->slave_req_lock);
> + ret = send_vhost_slave_message(dev, ctx);
> + if (ret < 0) {
> + VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to send config
> change (%d)\n", ret);
> + goto out;
> + }
>
> - ret = send_vhost_message(dev, dev->slave_req_fd, ctx);
> - if (ret < 0 && (ctx->msg.flags & VHOST_USER_NEED_REPLY))
> - rte_spinlock_unlock(&dev->slave_req_lock);
> + ret = read_vhost_message(dev, dev->slave_req_fd, &msg_reply);
> + if (ret <= 0) {
> + if (ret < 0)
> + VHOST_LOG_CONFIG(dev->ifname, ERR,
> + "vhost read slave message reply failed\n");
> + else
> + VHOST_LOG_CONFIG(dev->ifname, INFO, "vhost peer
> closed\n");
> + ret = -1;
> + goto out;
> + }
> +
> + if (msg_reply.msg.request.slave != ctx->msg.request.slave) {
> + VHOST_LOG_CONFIG(dev->ifname, ERR,
> + "received unexpected msg type (%u), expected %u\n",
> + msg_reply.msg.request.slave, ctx->msg.request.slave);
> + ret = -1;
> + goto out;
> + }
>
> + ret = msg_reply.msg.payload.u64 ? -1 : 0;
> +out:
> + rte_spinlock_unlock(&dev->slave_req_lock);
> return ret;
> }
>
> @@ -3213,42 +3241,6 @@ vhost_user_msg_handler(int vid, int fd)
> return ret;
> }
>
> -static int process_slave_message_reply(struct virtio_net *dev,
> - const struct vhu_msg_context *ctx)
> -{
> - struct vhu_msg_context msg_reply;
> - int ret;
> -
> - if ((ctx->msg.flags & VHOST_USER_NEED_REPLY) == 0)
> - return 0;
> -
> - ret = read_vhost_message(dev, dev->slave_req_fd, &msg_reply);
> - if (ret <= 0) {
> - if (ret < 0)
> - VHOST_LOG_CONFIG(dev->ifname, ERR,
> - "vhost read slave message reply failed\n");
> - else
> - VHOST_LOG_CONFIG(dev->ifname, INFO, "vhost peer
> closed\n");
> - ret = -1;
> - goto out;
> - }
> -
> - ret = 0;
> - if (msg_reply.msg.request.slave != ctx->msg.request.slave) {
> - VHOST_LOG_CONFIG(dev->ifname, ERR,
> - "received unexpected msg type (%u), expected %u\n",
> - msg_reply.msg.request.slave, ctx->msg.request.slave);
> - ret = -1;
> - goto out;
> - }
> -
> - ret = msg_reply.msg.payload.u64 ? -1 : 0;
> -
> -out:
> - rte_spinlock_unlock(&dev->slave_req_lock);
> - return ret;
> -}
> -
> int
> vhost_user_iotlb_miss(struct virtio_net *dev, uint64_t iova, uint8_t perm)
> {
> @@ -3277,10 +3269,9 @@ vhost_user_iotlb_miss(struct virtio_net *dev,
> uint64_t iova, uint8_t perm)
> return 0;
> }
>
> -static int
> -vhost_user_slave_config_change(struct virtio_net *dev, bool need_reply)
> +int
> +rte_vhost_slave_config_change(int vid, bool need_reply)
> {
> - int ret;
> struct vhu_msg_context ctx = {
> .msg = {
> .request.slave = VHOST_USER_SLAVE_CONFIG_CHANGE_MSG,
> @@ -3288,29 +3279,23 @@ vhost_user_slave_config_change(struct virtio_net
> *dev, bool need_reply)
> .size = 0,
> }
> };
> -
> - if (need_reply)
> - ctx.msg.flags |= VHOST_USER_NEED_REPLY;
> -
> - ret = send_vhost_slave_message(dev, &ctx);
> - if (ret < 0) {
> - VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to send config
> change (%d)\n", ret);
> - return ret;
> - }
> -
> - return process_slave_message_reply(dev, &ctx);
> -}
> -
> -int
> -rte_vhost_slave_config_change(int vid, bool need_reply)
> -{
> struct virtio_net *dev;
> + int ret;
>
> dev = get_device(vid);
> if (!dev)
> return -ENODEV;
>
> - return vhost_user_slave_config_change(dev, need_reply);
> + if (!need_reply) {
> + ret = send_vhost_slave_message(dev, &ctx);
> + } else {
> + ctx.msg.flags |= VHOST_USER_NEED_REPLY;
> + ret = send_vhost_slave_message_process_reply(dev, &ctx);
> + }
> +
> + if (ret < 0)
> + VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to send config
> change (%d)\n", ret);
> + return ret;
> }
>
> static int vhost_user_slave_set_vring_host_notifier(struct virtio_net
> *dev,
> @@ -3339,13 +3324,11 @@ static int
> vhost_user_slave_set_vring_host_notifier(struct virtio_net *dev,
> ctx.fd_num = 1;
> }
>
> - ret = send_vhost_slave_message(dev, &ctx);
> - if (ret < 0) {
> + ret = send_vhost_slave_message_process_reply(dev, &ctx);
> + if (ret < 0)
> VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to set host
> notifier (%d)\n", ret);
> - return ret;
> - }
>
> - return process_slave_message_reply(dev, &ctx);
> + return ret;
> }
>
> int rte_vhost_host_notifier_ctrl(int vid, uint16_t qid, bool enable)
> --
> 2.39.1
Reviewed-by: Chenbo Xia <chenbo....@intel.com>
