<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=gb2312">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);" class="elementToProof ContentPasted1">
> > > Since backend and frontend message are synchronous in the same thread,
<div class="ContentPasted1">> > > there will be a probability of message deadlock.</div>
<div class="ContentPasted1">> > > Consider each driver to determine whether to wait for response.</div>
<div class="ContentPasted1">> > ></div>
<div class="ContentPasted1">> > > Fixes: d90cf7d111ac ("vhost: support host notifier")</div>
<div class="ContentPasted1">> > > Cc: maxime.coquelin@redhat.com</div>
<div class="ContentPasted1">> > > Signed-off-by: Rma Ma <rma.ma@jaguarmicro.com></div>
<div class="ContentPasted1">> > > ---</div>
<div class="ContentPasted1">> > > v2 - fix format error in commit message</div>
<div class="ContentPasted1">> > > v3 - add --in-reply-to</div>
<div class="ContentPasted1">> > > ---</div>
<div class="ContentPasted1">> ></div>
<div class="ContentPasted1">> > Hi Maxime,</div>
<div class="ContentPasted1">> ></div>
<div class="ContentPasted1">> > This patch helps to fix vhost-user message deadlock, could you help</div>
<div class="ContentPasted1">> > review it?</div>
<div class="ContentPasted1">> </div>
<div class="ContentPasted1">> The patch introduces a new device op, but it is used nowhere in vDPA</div>
<div class="ContentPasted1">> drivers.</div>
<div class="ContentPasted1">> </div>
<div class="ContentPasted1">> What vDPA driver is it going to be used with?</div>
<div class="ContentPasted1">> </div>
<div class="ContentPasted1">> Regards,</div>
> Maxime<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);" class="elementToProof">
<br>
</div>
<div class="elementToProof">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);" class="ContentPasted2 ContentPasted3">
Hi,
<div><br class="ContentPasted3">
</div>
<div class="ContentPasted3">Our company's jmnd vdpa driver, which requires the rte_vhost_host_notifier_ctrl interface,</div>
<div class="ContentPasted3">replicates the problem with the following scenario:</div>
<div><br class="ContentPasted3">
</div>
<div class="ContentPasted3">QEMU start vhost-user with modern net and blk, backend use dpdk-vdpa process,</div>
<div class="ContentPasted3">after live migration, dest QEMU deadlock with dpdk-vdpa.</div>
<div><br class="ContentPasted3">
</div>
<div class="ContentPasted3">- QEMU sends VHOST_USER_SET_VRING_KICK to dpdk-vdpa net</div>
<div class="ContentPasted3">- QEMU does not need to wait for a response to this message</div>
<div class="ContentPasted3">- QEMU then sends VHOST_USER_SET_MEM_TABLE to dpdk-vdpa blk</div>
<div class="ContentPasted3">- QEMU needs to wait reply in this message</div>
<div class="ContentPasted3">- when dpdk-vdpa recv VHOST_USER_SET_VRING_KICK,</div>
<div class="ContentPasted3">- it will send VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG to QEMU</div>
<div class="ContentPasted3">- dpdk-vdpa needs to wait for a response to this message</div>
<div class="ContentPasted3">- QEMU will deadlock with dpdk-vdpa</div>
<div><br class="ContentPasted3">
</div>
<div class="ContentPasted3">I tried to add a patch to the qemu community that uses a new thread to loop backend channel,</div>
<div class="ContentPasted3">But there will be some multi-threaded synchronization issues</div>
<div><br class="ContentPasted3">
</div>
<div class="ContentPasted3">I think this is a public issue, and other backend messages take this into account,</div>
<div class="ContentPasted3">so I think this message also needs a flag to fix it.</div>
<div><br class="ContentPasted3">
</div>
and jmnd vdpa driver will subsequently be open-sourced to the community.<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div id="Signature">
<div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<p style="font-size: 10.5pt; font-family: 等线; text-align: justify; margin: 0px; color: rgb(36, 36, 36); background-color: rgb(255, 255, 255);">
<span lang="en-US" style="margin:0px" class="ContentPasted0">Best wishes,</span></p>
<p style="font-size: 10.5pt; font-family: 等线; text-align: justify; margin: 0px; color: rgb(36, 36, 36); background-color: rgb(255, 255, 255);">
Rma</p>
</div>
</div>
</div>
</div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>发件人:</b> Maxime Coquelin <maxime.coquelin@redhat.com><br>
<b>发送时间:</b> 2023年7月11日 21:25<br>
<b>收件人:</b> Rma Ma <rma.ma@jaguarmicro.com>; dpdk-dev <dev@dpdk.org><br>
<b>抄送:</b> Chenbo Xia <chenbo.xia@intel.com><br>
<b>主题:</b> Re: 回复: [PATCH v3] vhost: add notify reply ops to fix message deadlock</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">Hi,<br>
<br>
On 7/11/23 11:25, Rma Ma wrote:<br>
> > Since backend and frontend message are synchronous in the same thread,<br>
> > there will be a probability of message deadlock.<br>
> > Consider each driver to determine whether to wait for response.<br>
> ><br>
> > Fixes: d90cf7d111ac ("vhost: support host notifier")<br>
> > Cc: maxime.coquelin@redhat.com<br>
> > Signed-off-by: Rma Ma <rma.ma@jaguarmicro.com><br>
> > ---<br>
> > v2 - fix format error in commit message<br>
> > v3 - add --in-reply-to<br>
> > ---<br>
> <br>
> Hi Maxime,<br>
> <br>
> This patch helps to fix vhost-user message deadlock, could you help <br>
> review it?<br>
<br>
The patch introduces a new device op, but it is used nowhere in vDPA<br>
drivers.<br>
<br>
What vDPA driver is it going to be used with?<br>
<br>
Regards,<br>
Maxime<br>
<br>
> Thanks.<br>
> <br>
> Best wishes,<br>
> <br>
> Rma<br>
> <br>
> ------------------------------------------------------------------------<br>
> *发件人:* Rma Ma<br>
> *发送时间:* 2023年7月4日 10:52<br>
> *收件人:* dpdk-dev <dev@dpdk.org><br>
> *抄送:* Maxime Coquelin <maxime.coquelin@redhat.com>; Chenbo Xia <br>
> <chenbo.xia@intel.com>; Rma Ma <rma.ma@jaguarmicro.com><br>
> *主题:* [PATCH v3] vhost: add notify reply ops to fix message deadlock<br>
> Since backend and frontend message are synchronous in the same thread,<br>
> there will be a probability of message deadlock.<br>
> Consider each driver to determine whether to wait for response.<br>
> <br>
> Fixes: d90cf7d111ac ("vhost: support host notifier")<br>
> Cc: maxime.coquelin@redhat.com<br>
> Signed-off-by: Rma Ma <rma.ma@jaguarmicro.com><br>
> ---<br>
> v2 - fix format error in commit message<br>
> v3 - add --in-reply-to<br>
> ---<br>
> lib/vhost/vdpa_driver.h | 3 +++<br>
> lib/vhost/vhost_user.c | 23 ++++++++++++++++++-----<br>
> 2 files changed, 21 insertions(+), 5 deletions(-)<br>
> <br>
> diff --git a/lib/vhost/vdpa_driver.h b/lib/vhost/vdpa_driver.h<br>
> index 8db4ab9f4d..3d2ea3c90e 100644<br>
> --- a/lib/vhost/vdpa_driver.h<br>
> +++ b/lib/vhost/vdpa_driver.h<br>
> @@ -81,6 +81,9 @@ struct rte_vdpa_dev_ops {<br>
> <br>
> /** get device type: net device, blk device... */<br>
> int (*get_dev_type)(struct rte_vdpa_device *dev, uint32_t *type);<br>
> +<br>
> + /** Get the notify reply flag */<br>
> + int (*get_notify_reply_flag)(int vid, bool *need_reply);<br>
> };<br>
> <br>
> /**<br>
> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c<br>
> index 901a80bbaa..aa61992939 100644<br>
> --- a/lib/vhost/vhost_user.c<br>
> +++ b/lib/vhost/vhost_user.c<br>
> @@ -3365,13 +3365,14 @@ rte_vhost_backend_config_change(int vid, bool <br>
> need_reply)<br>
> static int vhost_user_backend_set_vring_host_notifier(struct <br>
> virtio_net *dev,<br>
> int index, int fd,<br>
> uint64_t offset,<br>
> - uint64_t size)<br>
> + uint64_t size,<br>
> + bool need_reply)<br>
> {<br>
> int ret;<br>
> struct vhu_msg_context ctx = {<br>
> .msg = {<br>
> .request.backend = <br>
> VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG,<br>
> - .flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY,<br>
> + .flags = VHOST_USER_VERSION,<br>
> .size = sizeof(ctx.msg.payload.area),<br>
> .payload.area = {<br>
> .u64 = index & VHOST_USER_VRING_IDX_MASK,<br>
> @@ -3388,7 +3389,13 @@ static int <br>
> vhost_user_backend_set_vring_host_notifier(struct virtio_net *dev,<br>
> ctx.fd_num = 1;<br>
> }<br>
> <br>
> - ret = send_vhost_backend_message_process_reply(dev, &ctx);<br>
> + if (!need_reply)<br>
> + ret = send_vhost_backend_message(dev, &ctx);<br>
> + else {<br>
> + ctx.msg.flags |= VHOST_USER_NEED_REPLY;<br>
> + ret = send_vhost_backend_message_process_reply(dev, &ctx);<br>
> + }<br>
> +<br>
> if (ret < 0)<br>
> VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to set host <br>
> notifier (%d)\n", ret);<br>
> <br>
> @@ -3402,6 +3409,7 @@ int rte_vhost_host_notifier_ctrl(int vid, uint16_t <br>
> qid, bool enable)<br>
> int vfio_device_fd, ret = 0;<br>
> uint64_t offset, size;<br>
> unsigned int i, q_start, q_last;<br>
> + bool need_reply;<br>
> <br>
> dev = get_device(vid);<br>
> if (!dev)<br>
> @@ -3440,6 +3448,11 @@ int rte_vhost_host_notifier_ctrl(int vid, <br>
> uint16_t qid, bool enable)<br>
> if (vfio_device_fd < 0)<br>
> return -ENOTSUP;<br>
> <br>
> + if (vdpa_dev->ops->get_notify_reply_flag == NULL)<br>
> + need_reply = true;<br>
> + else<br>
> + vdpa_dev->ops->get_notify_reply_flag(vid, &need_reply);<br>
> +<br>
> if (enable) {<br>
> for (i = q_start; i <= q_last; i++) {<br>
> if (vdpa_dev->ops->get_notify_area(vid, i, <br>
> &offset,<br>
> @@ -3449,7 +3462,7 @@ int rte_vhost_host_notifier_ctrl(int vid, uint16_t <br>
> qid, bool enable)<br>
> }<br>
> <br>
> if <br>
> (vhost_user_backend_set_vring_host_notifier(dev, i,<br>
> - vfio_device_fd, offset, size) < 0) {<br>
> + vfio_device_fd, offset, size, <br>
> need_reply) < 0) {<br>
> ret = -EFAULT;<br>
> goto disable;<br>
> }<br>
> @@ -3458,7 +3471,7 @@ int rte_vhost_host_notifier_ctrl(int vid, uint16_t <br>
> qid, bool enable)<br>
> disable:<br>
> for (i = q_start; i <= q_last; i++) {<br>
> <br>
> vhost_user_backend_set_vring_host_notifier(dev, i, -1,<br>
> - 0, 0);<br>
> + 0, 0, need_reply);<br>
> }<br>
> }<br>
> <br>
> -- <br>
> 2.17.1<br>
> <br>
<br>
</div>
</span></font></div>
</body>
</html>