Re: [PATCH RFC v3 10/13] vhost-net: implement extended features support

[Paolo Abeni <pabeni@redhat.com>]

On 7/18/25 3:01 PM, Stefano Garzarella wrote:
> On Fri, Jul 18, 2025 at 10:52:36AM +0200, Paolo Abeni wrote:
>> @@ -234,7 +234,8 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
>>     int r;
>>     bool backend_kernel = options->backend_type == VHOST_BACKEND_TYPE_KERNEL;
>>     struct vhost_net *net = g_new0(struct vhost_net, 1);
>> -    uint64_t features = 0;
>> +    uint64_t missing_features[VIRTIO_FEATURES_DWORDS];
>> +    uint64_t features[VIRTIO_FEATURES_DWORDS];
> 
> Should we initialize `features` (IIUC calling virtio_features_clear)
> since it was set to 0 before this patch?
> 
>>     Error *local_err = NULL;
>>
>>     if (!options->net_backend) {
>> @@ -261,7 +262,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
>>         net->backend = r;
>>         net->dev.protocol_features = 0;
>>     } else {
>> -        net->dev.backend_features = 0;
>> +        virtio_features_clear(net->dev.backend_features_ex);
>>         net->dev.protocol_features = 0;
>>         net->backend = -1;
>>
>> @@ -279,28 +280,31 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
>>     if (backend_kernel) {
>>         if (!qemu_has_vnet_hdr_len(options->net_backend,
>>                                sizeof(struct virtio_net_hdr_mrg_rxbuf))) {
>> -            net->dev.features &= ~(1ULL << VIRTIO_NET_F_MRG_RXBUF);
>> +            net->dev.features &= ~VIRTIO_BIT(VIRTIO_NET_F_MRG_RXBUF);
>>         }
>> -        if (~net->dev.features & net->dev.backend_features) {
>> -            fprintf(stderr, "vhost lacks feature mask 0x%" PRIx64
>> -                   " for backend\n",
>> -                   (uint64_t)(~net->dev.features & net->dev.backend_features));
>> +
>> +        if (virtio_features_andnot(missing_features,
>> +                                   net->dev.backend_features_ex,
>> +                                   net->dev.features_ex)) {
>> +            fprintf(stderr, "vhost lacks feature mask 0x" VIRTIO_FEATURES_FMT
>> +                   " for backend\n", VIRTIO_FEATURES_PR(missing_features));
>>             goto fail;
>>         }
>>     }
>>
>>     /* Set sane init value. Override when guest acks. */
>>     if (options->get_acked_features) {
>> -        features = options->get_acked_features(net->nc);
>> -        if (~net->dev.features & features) {
>> -            fprintf(stderr, "vhost lacks feature mask 0x%" PRIx64
>> -                    " for backend\n",
>> -                    (uint64_t)(~net->dev.features & features));
>> +        virtio_features_from_u64(features,
>> +                                 options->get_acked_features(net->nc));
>> +        if (virtio_features_andnot(missing_features, features,
>> +                                   net->dev.features_ex)) {
>> +            fprintf(stderr, "vhost lacks feature mask 0x" VIRTIO_FEATURES_FMT
>> +                    " for backend\n", VIRTIO_FEATURES_PR(missing_features));
>>             goto fail;
>>         }
>>     }
>>
>> -    vhost_net_ack_features(net, features);
>> +    vhost_net_ack_features_ex(net, features);
> 
> If `options->get_acked_features` is false, `features` here is not
> initialized (it was set to 0 before this patch).

Indeed the initialization is needed. I will fix the next revision.

Thanks,

Paolo