Re: [RFC PATCH v1] vhost-vsock: SOCK_SEQPACKET feature bit support

[Arseny Krasnov]

On 22.06.2021 11:24, Stefano Garzarella wrote:
> On Mon, Jun 21, 2021 at 05:07:41PM +0300, Arseny Krasnov wrote:
>> This adds processing of VIRTIO_VSOCK_F_SEQPACKET features bit. Guest
>> negotiates it with vhost, thus both will know that SOCK_SEQPACKET
>> supported by peer.
>>
>> Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com>
>> ---
>> hw/virtio/vhost-vsock.c                       | 16 +++++++++++++++-
>> include/standard-headers/linux/virtio_vsock.h |  3 +++
>> 2 files changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
>> index 8ddfb9abfe..fc8c143df9 100644
>> --- a/hw/virtio/vhost-vsock.c
>> +++ b/hw/virtio/vhost-vsock.c
>> @@ -108,10 +108,23 @@ static uint64_t vhost_vsock_get_features(VirtIODevice 
>> *vdev,
>>                                          uint64_t requested_features,
>>                                          Error **errp)
>> {
>> -    /* No feature bits used yet */
>> +    VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vdev);
>> +
>> +    if (virtio_has_feature(vvc->vhost_dev.features, 
>> VIRTIO_VSOCK_F_SEQPACKET)) {
>> +        virtio_add_feature(&requested_features, VIRTIO_VSOCK_F_SEQPACKET);
>> +    }
>> +
> I think we can use vhost_get_features(), take a look at 
> hw/virtio/vhost-user-vsock.c

Hm, i've implemented use of 'vhost_get_features()' in the

vhost-user-vsock.c manner...

>
>
>>     return requested_features;
>> }
>>
>> +static void vhost_vsock_set_features(VirtIODevice *vdev, uint64_t features)
>> +{
>> +    VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vdev);
>> +    const VhostOps *vhost_ops = vvc->vhost_dev.vhost_ops;
>> +
>> +    vhost_ops->vhost_set_features(&vvc->vhost_dev, features);
> Maybe we can avoid vhost_vsock_set_features(), because in 
> vhost_vsock_common_start() we have this code that should do the same, 
> sice vhost_dev_start() already call vhost_set_features() with the acked 
> features:
>
>      vvc->vhost_dev.acked_features = vdev->guest_features;
>      ret = vhost_dev_start(&vvc->vhost_dev, vdev);
>
> I'm not sure if it works. Can you try simply removing 
> vhost_vsock_set_features()?
>
> Thanks,
> Stefano
Thank You, that works.
>
>> +}
>> +
>> static const VMStateDescription vmstate_virtio_vhost_vsock = {
>>     .name = "virtio-vhost_vsock",
>>     .minimum_version_id = VHOST_VSOCK_SAVEVM_VERSION,
>> @@ -224,6 +237,7 @@ static void vhost_vsock_class_init(ObjectClass *klass, 
>> void *data)
>>     vdc->realize = vhost_vsock_device_realize;
>>     vdc->unrealize = vhost_vsock_device_unrealize;
>>     vdc->get_features = vhost_vsock_get_features;
>> +    vdc->set_features = vhost_vsock_set_features;
>>     vdc->get_config = vhost_vsock_get_config;
>>     vdc->set_status = vhost_vsock_set_status;
>> }
>> diff --git a/include/standard-headers/linux/virtio_vsock.h 
>> b/include/standard-headers/linux/virtio_vsock.h
>> index be443211ce..5eac522ee2 100644
>> --- a/include/standard-headers/linux/virtio_vsock.h
>> +++ b/include/standard-headers/linux/virtio_vsock.h
>> @@ -38,6 +38,9 @@
>> #include "standard-headers/linux/virtio_ids.h"
>> #include "standard-headers/linux/virtio_config.h"
>>
>> +/* The feature bitmap for virtio vsock */
>> +#define VIRTIO_VSOCK_F_SEQPACKET       1       /* SOCK_SEQPACKET 
>> supported */
>> +
>> struct virtio_vsock_config {
>>      uint64_t guest_cid;
>> } QEMU_PACKED;
>> -- 
>> 2.25.1
>>
>