[v6,2/9] vhost: simplify need reply handling
Checks
Commit Message
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.marchand@redhat.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
lib/vhost/vhost_user.c | 119 ++++++++++++++++++-----------------------
1 file changed, 51 insertions(+), 68 deletions(-)
Comments
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Tuesday, February 7, 2023 6:45 PM
> To: dev@dpdk.org
> Cc: maxime.coquelin@redhat.com; stephen@networkplumber.org; Xia, Chenbo
> <chenbo.xia@intel.com>; Hu, Jiayu <jiayu.hu@intel.com>; Wang, YuanX
> <yuanx.wang@intel.com>; Ding, Xuan <xuan.ding@intel.com>;
> mb@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.marchand@redhat.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@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.xia@intel.com>
@@ -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)