* [PATCH] vhost-user: Implement an empty VHOST_USER_SEND_RARP command
@ 2025-01-24 14:21 Laurent Vivier
2025-01-24 16:10 ` Stefano Brivio
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Vivier @ 2025-01-24 14:21 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier
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
+ * @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",
+ 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,
};
--
@@ -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
+ * @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",
+ 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,
};
--
2.48.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] vhost-user: Implement an empty VHOST_USER_SEND_RARP command
2025-01-24 14:21 [PATCH] vhost-user: Implement an empty VHOST_USER_SEND_RARP command Laurent Vivier
@ 2025-01-24 16:10 ` Stefano Brivio
2025-01-24 18:35 ` Laurent Vivier
0 siblings, 1 reply; 6+ messages in thread
From: Stefano Brivio @ 2025-01-24 16:10 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vhost-user: Implement an empty VHOST_USER_SEND_RARP command
2025-01-24 16:10 ` Stefano Brivio
@ 2025-01-24 18:35 ` Laurent Vivier
2025-01-24 18:40 ` Stefano Brivio
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Vivier @ 2025-01-24 18:35 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev
On 24/01/2025 17:10, Stefano Brivio wrote:
> 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.
I think it's interesting to have this comment as it comes from the vhost-user
specification as it describes the aim of the command, and we can add something like "but
as passt don't need to update any ARP table we do nothing only to silence QEMU bogus error
message".
>
>> + * @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/?
Yes, you're right.
The purpose of this debug message was to identify for :) which interface QEMU wants this
broadcast.
>
>> + 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,
>> };
>
Thanks,
Laurent
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vhost-user: Implement an empty VHOST_USER_SEND_RARP command
2025-01-24 18:35 ` Laurent Vivier
@ 2025-01-24 18:40 ` Stefano Brivio
2025-01-24 18:50 ` Laurent Vivier
0 siblings, 1 reply; 6+ messages in thread
From: Stefano Brivio @ 2025-01-24 18:40 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev
On Fri, 24 Jan 2025 19:35:33 +0100
Laurent Vivier <lvivier@redhat.com> wrote:
> On 24/01/2025 17:10, Stefano Brivio wrote:
> > 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.
>
> I think it's interesting to have this comment as it comes from the vhost-user
> specification as it describes the aim of the command, and we can add something like "but
> as passt don't need to update any ARP table we do nothing only to silence QEMU bogus error
> message".
Eh, if we're already making it verbose, maybe we can go with something
like:
vhost-user specification says: "Broadcast ...", but passt ...
> >> + * @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/?
>
> Yes, you're right.
> The purpose of this debug message was to identify for :) which interface QEMU wants this
> broadcast.
>
> >
> >> + 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vhost-user: Implement an empty VHOST_USER_SEND_RARP command
2025-01-24 18:40 ` Stefano Brivio
@ 2025-01-24 18:50 ` Laurent Vivier
2025-01-24 18:57 ` Stefano Brivio
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Vivier @ 2025-01-24 18:50 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev
On 24/01/2025 19:40, Stefano Brivio wrote:
> On Fri, 24 Jan 2025 19:35:33 +0100
> Laurent Vivier <lvivier@redhat.com> wrote:
>
>> On 24/01/2025 17:10, Stefano Brivio wrote:
>>> 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.
>>
>> I think it's interesting to have this comment as it comes from the vhost-user
>> specification as it describes the aim of the command, and we can add something like "but
>> as passt don't need to update any ARP table we do nothing only to silence QEMU bogus error
>> message".
>
> Eh, if we're already making it verbose, maybe we can go with something
> like:
>
> vhost-user specification says: "Broadcast ...", but passt ...
As you prefer...
If you want you can update the patch when you commit it.
>
>>>> + * @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/?
>>
>> Yes, you're right.
>> The purpose of this debug message was to identify for :) which interface QEMU wants this
>> broadcast.
>>
>>>
>>>> + 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,
>>>> };
>
Thanks,
Laurent
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vhost-user: Implement an empty VHOST_USER_SEND_RARP command
2025-01-24 18:50 ` Laurent Vivier
@ 2025-01-24 18:57 ` Stefano Brivio
0 siblings, 0 replies; 6+ messages in thread
From: Stefano Brivio @ 2025-01-24 18:57 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev
On Fri, 24 Jan 2025 19:50:57 +0100
Laurent Vivier <lvivier@redhat.com> wrote:
> On 24/01/2025 19:40, Stefano Brivio wrote:
> > On Fri, 24 Jan 2025 19:35:33 +0100
> > Laurent Vivier <lvivier@redhat.com> wrote:
> >
> >> On 24/01/2025 17:10, Stefano Brivio wrote:
> >>> 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.
> >>
> >> I think it's interesting to have this comment as it comes from the vhost-user
> >> specification as it describes the aim of the command, and we can add something like "but
> >> as passt don't need to update any ARP table we do nothing only to silence QEMU bogus error
> >> message".
> >
> > Eh, if we're already making it verbose, maybe we can go with something
> > like:
> >
> > vhost-user specification says: "Broadcast ...", but passt ...
>
> As you prefer...
>
> If you want you can update the patch when you commit it.
Given it's already two edits it would save me some time if you could
write/format it properly...
--
Stefano
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-01-24 18:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-01-24 14:21 [PATCH] vhost-user: Implement an empty VHOST_USER_SEND_RARP command Laurent Vivier
2025-01-24 16:10 ` Stefano Brivio
2025-01-24 18:35 ` Laurent Vivier
2025-01-24 18:40 ` Stefano Brivio
2025-01-24 18:50 ` Laurent Vivier
2025-01-24 18:57 ` Stefano Brivio
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).