Re: [PATCH] vhost-user: Implement an empty VHOST_USER_SEND_RARP command

[Stefano Brivio <sbrivio@redhat.com>]

On Fri, 24 Jan 2025 15:21:37 +0100
Laurent Vivier <lvivier@redhat.com> wrote:

> Passt cannot manage and doesn't need to manage the broadcast of a fake RARP,
> but QEMU will report an error message if Passt doesn't implement it.
> 
> Implement an empty SEND_RARP command to silence QEMU error message.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  vhost_user.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/vhost_user.c b/vhost_user.c
> index f12dec5ddc58..e6633ae75ce8 100644
> --- a/vhost_user.c
> +++ b/vhost_user.c
> @@ -914,7 +914,8 @@ static bool vu_get_protocol_features_exec(struct vu_dev *vdev,
>  {
>  	uint64_t features = 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK |
>  			    1ULL << VHOST_USER_PROTOCOL_F_LOG_SHMFD |
> -			    1ULL << VHOST_USER_PROTOCOL_F_DEVICE_STATE;
> +			    1ULL << VHOST_USER_PROTOCOL_F_DEVICE_STATE |
> +			    1ULL << VHOST_USER_PROTOCOL_F_RARP;
>  
>  	(void)vdev;
>  	vmsg_set_reply_u64(msg, features);
> @@ -981,6 +982,30 @@ static bool vu_set_vring_enable_exec(struct vu_dev *vdev,
>  	return false;
>  }
>  
> +/**
> + * vu_set_send_rarp_exec() - Broadcast a fake RARP to notify the migration
> + *			     is terminated

Fine, so we need to add this.

But can we at least make it clear for our future benefit? That is,
there's no such thing as "fake RARP". The only thing that's actually
fake here is this callback. For others, see thread at:

  https://lore.kernel.org/qemu-devel/20250121100029.1106973-1-lvivier@redhat.com/

What about "Do nothing to silence QEMU bogus error message"? Claiming
we are broadcasting a RARP message and not doing it is... confusing.

> + * @vdev:	vhost-user device
> + * @vmsg:	vhost-user message
> + *
> + * Return: False as no reply is requested
> + */
> +static bool vu_send_rarp_exec(struct vu_dev *vdev,
> +			      struct vhost_user_msg *msg)
> +{
> +	char macstr[ETH_ADDRSTRLEN];
> +
> +	(void)vdev;
> +
> +	/* ignore the command */
> +
> +	debug("Ignore command VHOST_USER_SEND_RARP from %s",

This is also a bit confusing, but I'm not sure I got it right.

We don't actually get any message *from* the guest, correct? We get a
message from QEMU telling us we should send a RARP broadcast *for* a
given MAC address...? If I got it right, s/from/for/?

> +	      eth_ntop((unsigned char *)&msg->payload.u64, macstr,
> +		       sizeof(macstr)));
> +
> +	return false;
> +}
> +
>  /**
>   * vu_set_migration_watch() - Add the migration file descriptor to epoll
>   * @vdev:	vhost-user device
> @@ -1177,6 +1202,7 @@ static bool (*vu_handle[VHOST_USER_MAX])(struct vu_dev *vdev,
>  	[VHOST_USER_SET_VRING_CALL]	   = vu_set_vring_call_exec,
>  	[VHOST_USER_SET_VRING_ERR]	   = vu_set_vring_err_exec,
>  	[VHOST_USER_SET_VRING_ENABLE]	   = vu_set_vring_enable_exec,
> +	[VHOST_USER_SEND_RARP]		   = vu_send_rarp_exec,
>  	[VHOST_USER_SET_DEVICE_STATE_FD]   = vu_set_device_state_fd_exec,
>  	[VHOST_USER_CHECK_DEVICE_STATE]    = vu_check_device_state_exec,
>  };

-- 
Stefano