> -----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> 

Reply via email to