[RFC 0/2] support vduse feature provisioning in vdpa netlink command

[RFC 0/2] support vduse feature provisioning in vdpa netlink command

[Eugenio Pérez]

This series implements features provisioning for vduse devices.  This allows
the device provisioner to clear the features exposed by the userland device, so
the driver never see them.  The intended use case is to provision more than one
different device with the same feature set, allowing live migration between
them.

The device addition validates the provisioned features to be a subset of the
parent features, as the rest of the backends.

Eugenio Pérez (2):
  vduse: support feature provisioning
  vduse: allow to specify device-specific features if it's multiclass

 drivers/vdpa/vdpa.c                |  9 ---------
 drivers/vdpa/vdpa_user/vduse_dev.c | 17 ++++++++++++++---
 2 files changed, 14 insertions(+), 12 deletions(-)

-- 
2.51.0

[RFC 1/2] vduse: support feature provisioning

[Eugenio Pérez]

This patch implements features provisioning for vduse devices.  This
allows the device provisioner to clear the features exposed by the
userland device, so the driver never see them.  The intended use case is
to provision more than one different device with the same feature set,
allowing live migration between them.

The device addition validates the provisioned features to be a subset of
the parent features, as the rest of the backends.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 6c74282d5721..ef8fc795cfeb 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -121,6 +121,7 @@ struct vduse_dev {
 	bool connected;
 	u64 api_version;
 	u64 device_features;
+	u64 supported_features;
 	u64 driver_features;
 	u32 device_id;
 	u32 vendor_id;
@@ -698,7 +699,7 @@ static u64 vduse_vdpa_get_device_features(struct vdpa_device *vdpa)
 {
 	struct vduse_dev *dev = vdpa_to_vduse(vdpa);
 
-	return dev->device_features;
+	return dev->supported_features;
 }
 
 static int vduse_vdpa_set_driver_features(struct vdpa_device *vdpa, u64 features)
@@ -2256,13 +2257,22 @@ struct vduse_mgmt_dev {
 
 static struct vduse_mgmt_dev *vduse_mgmt;
 
-static int vduse_dev_init_vdpa(struct vduse_dev *dev, const char *name)
+static int vduse_dev_init_vdpa(struct vduse_dev *dev, const char *name,
+			       const struct vdpa_dev_set_config *config)
 {
 	struct vduse_vdpa *vdev;
 
 	if (dev->vdev)
 		return -EEXIST;
 
+	if (config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
+		if (config->device_features & ~dev->device_features)
+			return -EINVAL;
+		dev->supported_features = config->device_features;
+	} else {
+		dev->supported_features = dev->device_features;
+	}
+
 	vdev = vdpa_alloc_device(struct vduse_vdpa, vdpa, dev->dev,
 				 &vduse_vdpa_config_ops, &vduse_map_ops,
 				 dev->ngroups, dev->nas, name, true);
@@ -2289,7 +2299,7 @@ static int vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
 		mutex_unlock(&vduse_lock);
 		return -EINVAL;
 	}
-	ret = vduse_dev_init_vdpa(dev, name);
+	ret = vduse_dev_init_vdpa(dev, name, config);
 	mutex_unlock(&vduse_lock);
 	if (ret)
 		return ret;
@@ -2376,6 +2386,7 @@ static int vduse_mgmtdev_init(void)
 	vduse_mgmt->mgmt_dev.id_table = id_table;
 	vduse_mgmt->mgmt_dev.ops = &vdpa_dev_mgmtdev_ops;
 	vduse_mgmt->mgmt_dev.device = &vduse_mgmt->dev;
+	vduse_mgmt->mgmt_dev.config_attr_mask = BIT_ULL(VDPA_ATTR_DEV_FEATURES);
 	ret = vdpa_mgmtdev_register(&vduse_mgmt->mgmt_dev);
 	if (ret)
 		device_unregister(&vduse_mgmt->dev);
-- 
2.51.0

[RFC 2/2] vduse: allow to specify device-specific features if it's multiclass

[Eugenio Pérez]

Even if the device supports more than one class, there are ways to solve
the ambiguity of which device are we creating.  In the VDUSE case it is
the name, for example.

RFC: I fon't understand 100% the motivation of this limitation, as the
backend should be the one filtering out the invalid features if any.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 drivers/vdpa/vdpa.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 34874beb0152..0fc32f3bd66f 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -688,15 +688,6 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct genl_info *i
 		err = -EINVAL;
 		goto err;
 	}
-	if (!(config.mask & VDPA_DEV_NET_ATTRS_MASK) &&
-	    config.mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES) &&
-	    classes & BIT_ULL(VIRTIO_ID_NET) && ncls > 1 &&
-	    config.device_features & VIRTIO_DEVICE_F_MASK) {
-		NL_SET_ERR_MSG_MOD(info->extack,
-				   "Management device supports multi-class while device features specified are ambiguous");
-		err = -EINVAL;
-		goto err;
-	}

 	err = mdev->ops->dev_add(mdev, name, &config);
 err:
--
2.51.0

Re: [RFC 2/2] vduse: allow to specify device-specific features if it's multiclass

[Jason Wang]

On Thu, Oct 2, 2025 at 6:36 PM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> Even if the device supports more than one class, there are ways to solve
> the ambiguity of which device are we creating.  In the VDUSE case it is
> the name, for example.

This may only work for VDUSE but not other parents.

>
> RFC: I fon't understand 100% the motivation of this limitation, as the
> backend should be the one filtering out the invalid features if any.

I think we can avoid these tricks. Currently, there's a one single
mgmt device, this seems to be in-convenient. Could we have a per
device mgmt device? For example, if user space creates vduse0, vdpa
mgmtdev show may see vduse0 so we won't have this issue.

>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>  drivers/vdpa/vdpa.c | 9 ---------
>  1 file changed, 9 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index 34874beb0152..0fc32f3bd66f 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -688,15 +688,6 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct genl_info *i
>                 err = -EINVAL;
>                 goto err;
>         }
> -       if (!(config.mask & VDPA_DEV_NET_ATTRS_MASK) &&
> -           config.mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES) &&
> -           classes & BIT_ULL(VIRTIO_ID_NET) && ncls > 1 &&
> -           config.device_features & VIRTIO_DEVICE_F_MASK) {
> -               NL_SET_ERR_MSG_MOD(info->extack,
> -                                  "Management device supports multi-class while device features specified are ambiguous");
> -               err = -EINVAL;
> -               goto err;
> -       }
>
>         err = mdev->ops->dev_add(mdev, name, &config);
>  err:
> --
> 2.51.0
>

Thanks

Re: [RFC 2/2] vduse: allow to specify device-specific features if it's multiclass

[Jason Wang]

On Fri, Oct 10, 2025 at 9:39 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Thu, Oct 2, 2025 at 6:36 PM Eugenio Pérez <eperezma@redhat.com> wrote:
> >
> > Even if the device supports more than one class, there are ways to solve
> > the ambiguity of which device are we creating.  In the VDUSE case it is
> > the name, for example.
>
> This may only work for VDUSE but not other parents.
>
> >
> > RFC: I fon't understand 100% the motivation of this limitation, as the
> > backend should be the one filtering out the invalid features if any.
>
> I think we can avoid these tricks. Currently, there's a one single
> mgmt device, this seems to be in-convenient. Could we have a per
> device mgmt device? For example, if user space creates vduse0, vdpa
> mgmtdev show may see vduse0 so we won't have this issue.

Or if we specify the class, there won't be ambiguity.

Thanks

>
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> >  drivers/vdpa/vdpa.c | 9 ---------
> >  1 file changed, 9 deletions(-)
> >
> > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> > index 34874beb0152..0fc32f3bd66f 100644
> > --- a/drivers/vdpa/vdpa.c
> > +++ b/drivers/vdpa/vdpa.c
> > @@ -688,15 +688,6 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct genl_info *i
> >                 err = -EINVAL;
> >                 goto err;
> >         }
> > -       if (!(config.mask & VDPA_DEV_NET_ATTRS_MASK) &&
> > -           config.mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES) &&
> > -           classes & BIT_ULL(VIRTIO_ID_NET) && ncls > 1 &&
> > -           config.device_features & VIRTIO_DEVICE_F_MASK) {
> > -               NL_SET_ERR_MSG_MOD(info->extack,
> > -                                  "Management device supports multi-class while device features specified are ambiguous");
> > -               err = -EINVAL;
> > -               goto err;
> > -       }
> >
> >         err = mdev->ops->dev_add(mdev, name, &config);
> >  err:
> > --
> > 2.51.0
> >
>
> Thanks

Re: [RFC 1/2] vduse: support feature provisioning

[Si-Wei Liu]

On 10/2/2025 3:35 AM, Eugenio Pérez wrote:
> This patch implements features provisioning for vduse devices.  This
> allows the device provisioner to clear the features exposed by the
> userland device, so the driver never see them.  The intended use case is
> to provision more than one different device with the same feature set,
> allowing live migration between them.
>
> The device addition validates the provisioned features to be a subset of
> the parent features, as the rest of the backends.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>   drivers/vdpa/vdpa_user/vduse_dev.c | 17 ++++++++++++++---
>   1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 6c74282d5721..ef8fc795cfeb 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -121,6 +121,7 @@ struct vduse_dev {
>   	bool connected;
>   	u64 api_version;
>   	u64 device_features;
> +	u64 supported_features;
>   	u64 driver_features;
>   	u32 device_id;
>   	u32 vendor_id;
> @@ -698,7 +699,7 @@ static u64 vduse_vdpa_get_device_features(struct vdpa_device *vdpa)
>   {
>   	struct vduse_dev *dev = vdpa_to_vduse(vdpa);
>   
> -	return dev->device_features;
> +	return dev->supported_features;
>   }
>   
>   static int vduse_vdpa_set_driver_features(struct vdpa_device *vdpa, u64 features)
> @@ -2256,13 +2257,22 @@ struct vduse_mgmt_dev {
>   
>   static struct vduse_mgmt_dev *vduse_mgmt;
>   
> -static int vduse_dev_init_vdpa(struct vduse_dev *dev, const char *name)
> +static int vduse_dev_init_vdpa(struct vduse_dev *dev, const char *name,
> +			       const struct vdpa_dev_set_config *config)
>   {
>   	struct vduse_vdpa *vdev;
>   
>   	if (dev->vdev)
>   		return -EEXIST;
>   
> +	if (config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
> +		if (config->device_features & ~dev->device_features)
> +			return -EINVAL;
> +		dev->supported_features = config->device_features;
> +	} else {
> +		dev->supported_features = dev->device_features;
> +	}
> +
Why this feature filter can't be done in the client (library) of vduse 
itself, similar to other device specific features of the existing vduse 
client? I thought those vduse clients are never managed by vdpa tool, 
while the device class can't be bound until client had registered with 
vduse. What is the point to define the feature bit in one place, but the 
value of the feature (for e.g. mac, mtu) is still or has to be provided 
by the client itself? Is it the right behavior to filter features in a 
separate layer rather than contain all relevant feature bits and 
attributes in one central location?

Regards,
-Siwei

>   	vdev = vdpa_alloc_device(struct vduse_vdpa, vdpa, dev->dev,
>   				 &vduse_vdpa_config_ops, &vduse_map_ops,
>   				 dev->ngroups, dev->nas, name, true);
> @@ -2289,7 +2299,7 @@ static int vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
>   		mutex_unlock(&vduse_lock);
>   		return -EINVAL;
>   	}
> -	ret = vduse_dev_init_vdpa(dev, name);
> +	ret = vduse_dev_init_vdpa(dev, name, config);
>   	mutex_unlock(&vduse_lock);
>   	if (ret)
>   		return ret;
> @@ -2376,6 +2386,7 @@ static int vduse_mgmtdev_init(void)
>   	vduse_mgmt->mgmt_dev.id_table = id_table;
>   	vduse_mgmt->mgmt_dev.ops = &vdpa_dev_mgmtdev_ops;
>   	vduse_mgmt->mgmt_dev.device = &vduse_mgmt->dev;
> +	vduse_mgmt->mgmt_dev.config_attr_mask = BIT_ULL(VDPA_ATTR_DEV_FEATURES);
>   	ret = vdpa_mgmtdev_register(&vduse_mgmt->mgmt_dev);
>   	if (ret)
>   		device_unregister(&vduse_mgmt->dev);

Re: [RFC 1/2] vduse: support feature provisioning

[Eugenio Perez Martin]

On Fri, Oct 10, 2025 at 6:40 AM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>
>
>
> On 10/2/2025 3:35 AM, Eugenio Pérez wrote:
> > This patch implements features provisioning for vduse devices.  This
> > allows the device provisioner to clear the features exposed by the
> > userland device, so the driver never see them.  The intended use case is
> > to provision more than one different device with the same feature set,
> > allowing live migration between them.
> >
> > The device addition validates the provisioned features to be a subset of
> > the parent features, as the rest of the backends.
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> >   drivers/vdpa/vdpa_user/vduse_dev.c | 17 ++++++++++++++---
> >   1 file changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > index 6c74282d5721..ef8fc795cfeb 100644
> > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > @@ -121,6 +121,7 @@ struct vduse_dev {
> >       bool connected;
> >       u64 api_version;
> >       u64 device_features;
> > +     u64 supported_features;
> >       u64 driver_features;
> >       u32 device_id;
> >       u32 vendor_id;
> > @@ -698,7 +699,7 @@ static u64 vduse_vdpa_get_device_features(struct vdpa_device *vdpa)
> >   {
> >       struct vduse_dev *dev = vdpa_to_vduse(vdpa);
> >
> > -     return dev->device_features;
> > +     return dev->supported_features;
> >   }
> >
> >   static int vduse_vdpa_set_driver_features(struct vdpa_device *vdpa, u64 features)
> > @@ -2256,13 +2257,22 @@ struct vduse_mgmt_dev {
> >
> >   static struct vduse_mgmt_dev *vduse_mgmt;
> >
> > -static int vduse_dev_init_vdpa(struct vduse_dev *dev, const char *name)
> > +static int vduse_dev_init_vdpa(struct vduse_dev *dev, const char *name,
> > +                            const struct vdpa_dev_set_config *config)
> >   {
> >       struct vduse_vdpa *vdev;
> >
> >       if (dev->vdev)
> >               return -EEXIST;
> >
> > +     if (config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
> > +             if (config->device_features & ~dev->device_features)
> > +                     return -EINVAL;
> > +             dev->supported_features = config->device_features;
> > +     } else {
> > +             dev->supported_features = dev->device_features;
> > +     }
> > +
> Why this feature filter can't be done in the client (library) of vduse
> itself, similar to other device specific features of the existing vduse
> client? I thought those vduse clients are never managed by vdpa tool,
> while the device class can't be bound until client had registered with
> vduse. What is the point to define the feature bit in one place, but the
> value of the feature (for e.g. mac, mtu) is still or has to be provided
> by the client itself? Is it the right behavior to filter features in a
> separate layer rather than contain all relevant feature bits and
> attributes in one central location?
>

Maxime can expand on this, but the user of the vdpa command should not
need to know if the backend device is VDUSE, HW, or other device: It
should just call the vdpa command the same way, and expect the same
results.

That also implies that it should handle the same way if a HW device
has its own MAC address and then you set a different one with the vdpa
command, but device_featues just came first in the pending list :).