2022-10-10 18:43:20

by Si-Wei Liu

[permalink] [raw]
Subject: [PATCH] vdpa: merge functionally duplicated dev_features attributes

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 <[email protected]>
---
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


2022-10-12 03:14:38

by Jason Wang

[permalink] [raw]
Subject: Re: [PATCH] vdpa: merge functionally duplicated dev_features attributes

On Tue, Oct 11, 2022 at 2:32 AM Si-Wei Liu <[email protected]> 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 <[email protected]>

Acked-by: Jason Wang <[email protected]>

> ---
> 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
>

2022-11-24 01:58:20

by Si-Wei Liu

[permalink] [raw]
Subject: Re: [PATCH] vdpa: merge functionally duplicated dev_features attributes

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 <[email protected]> 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 <[email protected]>
> Acked-by: Jason Wang <[email protected]>
>
>> ---
>> 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
>>