Re: vhost reply_ack negotiation (a.k.a differences in vhost-user behaviour with libvhost-user and vhost-user-backend.rs)

[Alex Bennée]

Dr. David Alan Gilbert <dgilbert@redhat.com> writes:

> * Alex Bennée (alex.bennee@linaro.org) wrote:
>> Hi,
>> 
>> I finally got a chance to get down into the guts of vhost-user while
>> attempting to port my original C RPMB daemon to Rust using the
>> vhost-user-backend and related crates. I ended up with this hang during
>> negotiation:
>> 
>>   startup
>> 
>>   vhost_user_write req:1 flags:0x1
>>   vhost_user_read_start
>>   vhost_user_read req:1 flags:0x5
>>   vhost_user_backend_init: we got 170000000

GET_FEATURES

>>   vhost_user_write req:15 flags:0x1
>>   vhost_user_read_start
>>   vhost_user_read req:15 flags:0x5
>>   vhost_user_set_protocol_features: 2008
>>   vhost_user_write req:16 flags:0x1
>>   vhost_user_write req:3 flags:0x1
>>   vhost_user_write req:1 flags:0x1
>>   vhost_user_read_start
>>   vhost_user_read req:1 flags:0x5
>>   vhost_user_write req:13 flags:0x1
>> 
>>   kernel initialises device
>> 
>>   virtio_rpmb virtio1: init done!
>>   vhost_user_write req:13 flags:0x1
>>   vhost_dev_set_features: 130000000
>>   vhost_user_set_features: 130000000

SET_FEATURES

>>   vhost_user_write req:2 flags:0x1
>>   vhost_user_write req:5 flags:0x9
>>   vhost_user_read_start
>> 
<snip>
>> 
>>  - Should QEMU have preserved VhostUserVirtioFeatures::PROTOCOL_FEATURES
>>    when doing the eventual VHOST_USER_SET_FEATURES reply?
>> 
>>  - Is vhost.rs being to strict or libvhost-user too lax in interpreting
>>    the negotiated features before processing the ``need_reply`` [Bit 3]
>>    field of the messages?
>
> I think vhost.rs is being correctly strict - but there would be no harm
> in it flagging that you'd hit an inconsistency if it finds a need_reply
> without the feature.

But the feature should have been negotiated. So unless the slave can
assume it is enabled because it asked I think QEMU is in the wrong by
not preserving the feature bits in it's SET_FEATURES reply. We just gets
away with it with libvhostuser being willing to reply anyway.

>
>>  - are VHOST_USER_SET_MEM_TABLE to VHOST_USER_SET_INFLIGHT_FD included
>>    in the "list of the ones that do" require replies or do they only
>>    reply when REPLY_ACK has been negotiated as the ambiguous "seealso::"
>>    box out seems to imply?
>
> set_mem_table gives a reply when postcopy is enabled (and then qemu
> replies to the reply!) but otherwise doesn't.
> (Note there's an issue opened for .rs to support ADD_MEM_REGION
> since it's a lot better than SET_MEM_TABLE which has a fixed size table
> that's small).

Thanks for the heads up.

>
> Dave
>
>> Currently I have some hacks in:
>> 
>>   https://github.com/stsquad/vhost/tree/my-hacks
>> 
>> which gets my daemon booting up to the point we actually need to do a
>> transaction. However I won't submit a PR until I've worked out exactly
>> where the problems are.
>> 
>> -- 
>> Alex Bennée
>> 


-- 
Alex Bennée