vdpa: merge functionally duplicated dev_features attributes

[PATCH] vdpa: merge functionally duplicated dev_features attributes

Posted by Si-Wei Liu 1 year, 6 months ago

We can merge VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES with
VDPA_ATTR_DEV_FEATURES which is functionally equivalent.
While at it, tweak the comment in header file to make
user provioned device features distinguished from those
supported by the parent mgmtdev device: the former of
which can be inherited as a whole from the latter, or
can be a subset of the latter if explicitly specified.

Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
---
 drivers/vdpa/vdpa.c       | 2 +-
 include/uapi/linux/vdpa.h | 4 +---
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index febdc99..41ed563 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -855,7 +855,7 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms
 
 	features_device = vdev->config->get_device_features(vdev);
 
-	if (nla_put_u64_64bit(msg, VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES, features_device,
+	if (nla_put_u64_64bit(msg, VDPA_ATTR_DEV_FEATURES, features_device,
 			      VDPA_ATTR_PAD))
 		return -EMSGSIZE;
 
diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
index 9bd7923..54b649a 100644
--- a/include/uapi/linux/vdpa.h
+++ b/include/uapi/linux/vdpa.h
@@ -53,11 +53,9 @@ enum vdpa_attr {
 	VDPA_ATTR_DEV_VENDOR_ATTR_NAME,		/* string */
 	VDPA_ATTR_DEV_VENDOR_ATTR_VALUE,        /* u64 */
 
+	/* virtio features that are provisioned to the vDPA device */
 	VDPA_ATTR_DEV_FEATURES,                 /* u64 */
 
-	/* virtio features that are supported by the vDPA device */
-	VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES,	/* u64 */
-
 	/* new attributes must be added above here */
 	VDPA_ATTR_MAX,
 };
-- 
1.8.3.1

Re: [PATCH] vdpa: merge functionally duplicated dev_features attributes

Posted by Jason Wang 1 year, 6 months ago

On Tue, Oct 11, 2022 at 2:32 AM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>
> We can merge VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES with
> VDPA_ATTR_DEV_FEATURES which is functionally equivalent.
> While at it, tweak the comment in header file to make
> user provioned device features distinguished from those
> supported by the parent mgmtdev device: the former of
> which can be inherited as a whole from the latter, or
> can be a subset of the latter if explicitly specified.
>
> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>

Acked-by: Jason Wang <jasowang@redhat.com>

> ---
>  drivers/vdpa/vdpa.c       | 2 +-
>  include/uapi/linux/vdpa.h | 4 +---
>  2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index febdc99..41ed563 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -855,7 +855,7 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms
>
>         features_device = vdev->config->get_device_features(vdev);
>
> -       if (nla_put_u64_64bit(msg, VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES, features_device,
> +       if (nla_put_u64_64bit(msg, VDPA_ATTR_DEV_FEATURES, features_device,
>                               VDPA_ATTR_PAD))
>                 return -EMSGSIZE;
>
> diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
> index 9bd7923..54b649a 100644
> --- a/include/uapi/linux/vdpa.h
> +++ b/include/uapi/linux/vdpa.h
> @@ -53,11 +53,9 @@ enum vdpa_attr {
>         VDPA_ATTR_DEV_VENDOR_ATTR_NAME,         /* string */
>         VDPA_ATTR_DEV_VENDOR_ATTR_VALUE,        /* u64 */
>
> +       /* virtio features that are provisioned to the vDPA device */
>         VDPA_ATTR_DEV_FEATURES,                 /* u64 */
>
> -       /* virtio features that are supported by the vDPA device */
> -       VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES,  /* u64 */
> -
>         /* new attributes must be added above here */
>         VDPA_ATTR_MAX,
>  };
> --
> 1.8.3.1
>

Re: [PATCH] vdpa: merge functionally duplicated dev_features attributes

Posted by Si-Wei Liu 1 year, 5 months ago

Ping with friendly reminder...

Could this simple patch be pulled to 6.1 as a follow-up fix before the 
release? Looks like the VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES to be 
removed had been pulled by iproute prematurely, though no actual 
userspace code is referencing it as yet. Actually it already hinders 
further vdpa tool development and iproute integration around 
VDPA_ATTR_DEV_FEATURES...

Thanks,
-Siwei

On 10/11/2022 8:07 PM, Jason Wang wrote:
> On Tue, Oct 11, 2022 at 2:32 AM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>> We can merge VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES with
>> VDPA_ATTR_DEV_FEATURES which is functionally equivalent.
>> While at it, tweak the comment in header file to make
>> user provioned device features distinguished from those
>> supported by the parent mgmtdev device: the former of
>> which can be inherited as a whole from the latter, or
>> can be a subset of the latter if explicitly specified.
>>
>> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> Acked-by: Jason Wang <jasowang@redhat.com>
>
>> ---
>>   drivers/vdpa/vdpa.c       | 2 +-
>>   include/uapi/linux/vdpa.h | 4 +---
>>   2 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
>> index febdc99..41ed563 100644
>> --- a/drivers/vdpa/vdpa.c
>> +++ b/drivers/vdpa/vdpa.c
>> @@ -855,7 +855,7 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms
>>
>>          features_device = vdev->config->get_device_features(vdev);
>>
>> -       if (nla_put_u64_64bit(msg, VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES, features_device,
>> +       if (nla_put_u64_64bit(msg, VDPA_ATTR_DEV_FEATURES, features_device,
>>                                VDPA_ATTR_PAD))
>>                  return -EMSGSIZE;
>>
>> diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
>> index 9bd7923..54b649a 100644
>> --- a/include/uapi/linux/vdpa.h
>> +++ b/include/uapi/linux/vdpa.h
>> @@ -53,11 +53,9 @@ enum vdpa_attr {
>>          VDPA_ATTR_DEV_VENDOR_ATTR_NAME,         /* string */
>>          VDPA_ATTR_DEV_VENDOR_ATTR_VALUE,        /* u64 */
>>
>> +       /* virtio features that are provisioned to the vDPA device */
>>          VDPA_ATTR_DEV_FEATURES,                 /* u64 */
>>
>> -       /* virtio features that are supported by the vDPA device */
>> -       VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES,  /* u64 */
>> -
>>          /* new attributes must be added above here */
>>          VDPA_ATTR_MAX,
>>   };
>> --
>> 1.8.3.1
>>