Re: [PATCH] vhost-user: Skip unnecessary duplicated VHOST_USER_SET_LOG_BASE requests

[Michael S. Tsirkin]

On Mon, Jul 01, 2024 at 04:14:35PM +0100, Alex Bennée wrote:
> "BillXiang" <xiangwencheng@dayudpu.com> writes:
> 
> >> From: "Alex Bennée"<alex.bennee@linaro.org>
> >> Date:  Mon, Jul 1, 2024, 16:49
> >> Subject:  Re: [PATCH] vhost-user: Skip unnecessary duplicated 
> >> VHOST_USER_SET_LOG_BASE requests
> >> To: "项文成"<xiangwencheng@dayudpu.com>
> >> Cc: <qemu-devel@nongnu.org>, <mst@redhat.com>
> >> 项文成 <xiangwencheng@dayudpu.com> writes:
> >> 
> >> > From: BillXiang <xiangwencheng@dayudpu.com>
> >> >
> >> > The VHOST_USER_SET_LOG_BASE requests should be categorized into
> >> > non-vring specific messages, and should be sent only once.
> >> > If send more than once, dpdk will munmap old log_addr which may has
> >> > been used and cause segmentation fault.
> >> 
> >> This looks fine to me but looking at the vhost-user.rst we don't seem to
> >> make any explicit statements about how many times given messages should
> >> be sent.
> >> 
> > There is indeed no explicit statements about how many times given messages
> >  should be sent in vhost-user.rst but already have some discussions such as 
> > https://lore.kernel.org/qemu-devel/20230127083027-mutt-send-email-mst@kernel.org/.
> 
> Right, but I think we should then update the specification if this is
> the way we want things to work. Otherwise we are putting a backend
> specific hack that another backend might be able to tolerate.

I think it's a dpdk bug, we *allow* resending same message many times.
However, less messages is better, I don't see a reason to
repeat the same message many times.

> >> >
> >> > Signed-off-by: BillXiang <xiangwencheng@dayudpu.com>
> >> > ---
> >> >  hw/virtio/vhost-user.c | 1 +
> >> >  1 file changed, 1 insertion(+)
> >> >
> >> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> >> > index cdf9af4a4b..41e34edd49 100644
> >> > --- a/hw/virtio/vhost-user.c
> >> > +++ b/hw/virtio/vhost-user.c
> >> > @@ -371,6 +371,7 @@ static bool 
> >> > vhost_user_per_device_request(VhostUserRequest request)
> >> >      case VHOST_USER_RESET_DEVICE:
> >> >      case VHOST_USER_ADD_MEM_REG:
> >> >      case VHOST_USER_REM_MEM_REG:
> >> > +    case VHOST_USER_SET_LOG_BASE:
> >> >          return true;
> >> >      default:
> >> >          return false;
> >> 
> >> -- 
> >> Alex Bennée
> >> Virtualisation Tech Lead @ Linaro
> 
> -- 
> Alex Bennée
> Virtualisation Tech Lead @ Linaro