All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iproute2/vdpa: Add support for reading device features
@ 2022-10-14  9:41 Zhu Lingshan
  2022-10-17  7:13   ` Jason Wang
  0 siblings, 1 reply; 17+ messages in thread
From: Zhu Lingshan @ 2022-10-14  9:41 UTC (permalink / raw)
  To: jasowang, mst, stephen, dsahern
  Cc: virtualization, netdev, hang.yuan, Zhu Lingshan

This commit implements support for reading vdpa device
features in iproute2.

Example:
$ vdpa dev config show vdpa0
vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 4 mtu 1500
  negotiated_features MRG_RXBUF CTRL_VQ MQ VERSION_1 ACCESS_PLATFORM
  dev_features MTU MAC MRG_RXBUF CTRL_VQ MQ ANY_LAYOUT VERSION_1 ACCESS_PLATFORM

Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
---
 vdpa/vdpa.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
index b73e40b4..89844e92 100644
--- a/vdpa/vdpa.c
+++ b/vdpa/vdpa.c
@@ -87,6 +87,8 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
 	[VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
 	[VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32,
 	[VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
+	[VDPA_ATTR_DEV_FEATURES] = MNL_TYPE_U64,
+	[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
 };
 
 static int attr_cb(const struct nlattr *attr, void *data)
@@ -482,7 +484,7 @@ static const char * const *dev_to_feature_str[] = {
 
 #define NUM_FEATURE_BITS 64
 
-static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
+static void print_features(struct vdpa *vdpa, uint64_t features, bool devf,
 			   uint16_t dev_id)
 {
 	const char * const *feature_strs = NULL;
@@ -492,7 +494,7 @@ static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
 	if (dev_id < ARRAY_SIZE(dev_to_feature_str))
 		feature_strs = dev_to_feature_str[dev_id];
 
-	if (mgmtdevf)
+	if (devf)
 		pr_out_array_start(vdpa, "dev_features");
 	else
 		pr_out_array_start(vdpa, "negotiated_features");
@@ -771,6 +773,15 @@ static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
 		val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
 		print_features(vdpa, val_u64, false, dev_id);
 	}
+	if (tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]) {
+		uint16_t dev_id = 0;
+
+		if (tb[VDPA_ATTR_DEV_ID])
+			dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
+
+		val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]);
+		print_features(vdpa, val_u64, true, dev_id);
+	}
 }
 
 static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] iproute2/vdpa: Add support for reading device features
  2022-10-14  9:41 [PATCH] iproute2/vdpa: Add support for reading device features Zhu Lingshan
@ 2022-10-17  7:13   ` Jason Wang
  0 siblings, 0 replies; 17+ messages in thread
From: Jason Wang @ 2022-10-17  7:13 UTC (permalink / raw)
  To: Zhu Lingshan; +Cc: mst, stephen, dsahern, virtualization, netdev, hang.yuan

On Fri, Oct 14, 2022 at 5:50 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
>
> This commit implements support for reading vdpa device
> features in iproute2.
>
> Example:
> $ vdpa dev config show vdpa0
> vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 4 mtu 1500
>   negotiated_features MRG_RXBUF CTRL_VQ MQ VERSION_1 ACCESS_PLATFORM
>   dev_features MTU MAC MRG_RXBUF CTRL_VQ MQ ANY_LAYOUT VERSION_1 ACCESS_PLATFORM
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>

Note that Si Wei proposed to unify the two new attributes:


> ---
>  vdpa/vdpa.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
> index b73e40b4..89844e92 100644
> --- a/vdpa/vdpa.c
> +++ b/vdpa/vdpa.c
> @@ -87,6 +87,8 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
>         [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
>         [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32,
>         [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> +       [VDPA_ATTR_DEV_FEATURES] = MNL_TYPE_U64,
> +       [VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
>  };
>
>  static int attr_cb(const struct nlattr *attr, void *data)
> @@ -482,7 +484,7 @@ static const char * const *dev_to_feature_str[] = {
>
>  #define NUM_FEATURE_BITS 64
>
> -static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> +static void print_features(struct vdpa *vdpa, uint64_t features, bool devf,
>                            uint16_t dev_id)
>  {
>         const char * const *feature_strs = NULL;
> @@ -492,7 +494,7 @@ static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
>         if (dev_id < ARRAY_SIZE(dev_to_feature_str))
>                 feature_strs = dev_to_feature_str[dev_id];
>
> -       if (mgmtdevf)
> +       if (devf)
>                 pr_out_array_start(vdpa, "dev_features");
>         else
>                 pr_out_array_start(vdpa, "negotiated_features");
> @@ -771,6 +773,15 @@ static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
>                 val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
>                 print_features(vdpa, val_u64, false, dev_id);
>         }
> +       if (tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]) {
> +               uint16_t dev_id = 0;
> +
> +               if (tb[VDPA_ATTR_DEV_ID])
> +                       dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
> +
> +               val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]);
> +               print_features(vdpa, val_u64, true, dev_id);
> +       }
>  }
>
>  static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
> --
> 2.31.1
>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] iproute2/vdpa: Add support for reading device features
@ 2022-10-17  7:13   ` Jason Wang
  0 siblings, 0 replies; 17+ messages in thread
From: Jason Wang @ 2022-10-17  7:13 UTC (permalink / raw)
  To: Zhu Lingshan; +Cc: mst, netdev, dsahern, virtualization, stephen, hang.yuan

On Fri, Oct 14, 2022 at 5:50 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
>
> This commit implements support for reading vdpa device
> features in iproute2.
>
> Example:
> $ vdpa dev config show vdpa0
> vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 4 mtu 1500
>   negotiated_features MRG_RXBUF CTRL_VQ MQ VERSION_1 ACCESS_PLATFORM
>   dev_features MTU MAC MRG_RXBUF CTRL_VQ MQ ANY_LAYOUT VERSION_1 ACCESS_PLATFORM
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>

Note that Si Wei proposed to unify the two new attributes:


> ---
>  vdpa/vdpa.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
> index b73e40b4..89844e92 100644
> --- a/vdpa/vdpa.c
> +++ b/vdpa/vdpa.c
> @@ -87,6 +87,8 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
>         [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
>         [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32,
>         [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> +       [VDPA_ATTR_DEV_FEATURES] = MNL_TYPE_U64,
> +       [VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
>  };
>
>  static int attr_cb(const struct nlattr *attr, void *data)
> @@ -482,7 +484,7 @@ static const char * const *dev_to_feature_str[] = {
>
>  #define NUM_FEATURE_BITS 64
>
> -static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> +static void print_features(struct vdpa *vdpa, uint64_t features, bool devf,
>                            uint16_t dev_id)
>  {
>         const char * const *feature_strs = NULL;
> @@ -492,7 +494,7 @@ static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
>         if (dev_id < ARRAY_SIZE(dev_to_feature_str))
>                 feature_strs = dev_to_feature_str[dev_id];
>
> -       if (mgmtdevf)
> +       if (devf)
>                 pr_out_array_start(vdpa, "dev_features");
>         else
>                 pr_out_array_start(vdpa, "negotiated_features");
> @@ -771,6 +773,15 @@ static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
>                 val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
>                 print_features(vdpa, val_u64, false, dev_id);
>         }
> +       if (tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]) {
> +               uint16_t dev_id = 0;
> +
> +               if (tb[VDPA_ATTR_DEV_ID])
> +                       dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
> +
> +               val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]);
> +               print_features(vdpa, val_u64, true, dev_id);
> +       }
>  }
>
>  static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
> --
> 2.31.1
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] iproute2/vdpa: Add support for reading device features
  2022-10-17  7:13   ` Jason Wang
@ 2022-10-17  7:13     ` Jason Wang
  -1 siblings, 0 replies; 17+ messages in thread
From: Jason Wang @ 2022-10-17  7:13 UTC (permalink / raw)
  To: Zhu Lingshan; +Cc: mst, stephen, dsahern, virtualization, netdev, hang.yuan

On Mon, Oct 17, 2022 at 3:13 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Fri, Oct 14, 2022 at 5:50 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
> >
> > This commit implements support for reading vdpa device
> > features in iproute2.
> >
> > Example:
> > $ vdpa dev config show vdpa0
> > vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 4 mtu 1500
> >   negotiated_features MRG_RXBUF CTRL_VQ MQ VERSION_1 ACCESS_PLATFORM
> >   dev_features MTU MAC MRG_RXBUF CTRL_VQ MQ ANY_LAYOUT VERSION_1 ACCESS_PLATFORM
> >
> > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>
> Note that Si Wei proposed to unify the two new attributes:

https://patchew.org/linux/1665422823-18364-1-git-send-email-si-wei.liu@oracle.com/

Thanks

>
>
> > ---
> >  vdpa/vdpa.c | 15 +++++++++++++--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
> > index b73e40b4..89844e92 100644
> > --- a/vdpa/vdpa.c
> > +++ b/vdpa/vdpa.c
> > @@ -87,6 +87,8 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
> >         [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
> >         [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32,
> >         [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> > +       [VDPA_ATTR_DEV_FEATURES] = MNL_TYPE_U64,
> > +       [VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> >  };
> >
> >  static int attr_cb(const struct nlattr *attr, void *data)
> > @@ -482,7 +484,7 @@ static const char * const *dev_to_feature_str[] = {
> >
> >  #define NUM_FEATURE_BITS 64
> >
> > -static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> > +static void print_features(struct vdpa *vdpa, uint64_t features, bool devf,
> >                            uint16_t dev_id)
> >  {
> >         const char * const *feature_strs = NULL;
> > @@ -492,7 +494,7 @@ static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> >         if (dev_id < ARRAY_SIZE(dev_to_feature_str))
> >                 feature_strs = dev_to_feature_str[dev_id];
> >
> > -       if (mgmtdevf)
> > +       if (devf)
> >                 pr_out_array_start(vdpa, "dev_features");
> >         else
> >                 pr_out_array_start(vdpa, "negotiated_features");
> > @@ -771,6 +773,15 @@ static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
> >                 val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
> >                 print_features(vdpa, val_u64, false, dev_id);
> >         }
> > +       if (tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]) {
> > +               uint16_t dev_id = 0;
> > +
> > +               if (tb[VDPA_ATTR_DEV_ID])
> > +                       dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
> > +
> > +               val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]);
> > +               print_features(vdpa, val_u64, true, dev_id);
> > +       }
> >  }
> >
> >  static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
> > --
> > 2.31.1
> >


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] iproute2/vdpa: Add support for reading device features
@ 2022-10-17  7:13     ` Jason Wang
  0 siblings, 0 replies; 17+ messages in thread
From: Jason Wang @ 2022-10-17  7:13 UTC (permalink / raw)
  To: Zhu Lingshan; +Cc: mst, netdev, dsahern, virtualization, stephen, hang.yuan

On Mon, Oct 17, 2022 at 3:13 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Fri, Oct 14, 2022 at 5:50 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
> >
> > This commit implements support for reading vdpa device
> > features in iproute2.
> >
> > Example:
> > $ vdpa dev config show vdpa0
> > vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 4 mtu 1500
> >   negotiated_features MRG_RXBUF CTRL_VQ MQ VERSION_1 ACCESS_PLATFORM
> >   dev_features MTU MAC MRG_RXBUF CTRL_VQ MQ ANY_LAYOUT VERSION_1 ACCESS_PLATFORM
> >
> > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>
> Note that Si Wei proposed to unify the two new attributes:

https://patchew.org/linux/1665422823-18364-1-git-send-email-si-wei.liu@oracle.com/

Thanks

>
>
> > ---
> >  vdpa/vdpa.c | 15 +++++++++++++--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
> > index b73e40b4..89844e92 100644
> > --- a/vdpa/vdpa.c
> > +++ b/vdpa/vdpa.c
> > @@ -87,6 +87,8 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
> >         [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
> >         [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32,
> >         [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> > +       [VDPA_ATTR_DEV_FEATURES] = MNL_TYPE_U64,
> > +       [VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> >  };
> >
> >  static int attr_cb(const struct nlattr *attr, void *data)
> > @@ -482,7 +484,7 @@ static const char * const *dev_to_feature_str[] = {
> >
> >  #define NUM_FEATURE_BITS 64
> >
> > -static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> > +static void print_features(struct vdpa *vdpa, uint64_t features, bool devf,
> >                            uint16_t dev_id)
> >  {
> >         const char * const *feature_strs = NULL;
> > @@ -492,7 +494,7 @@ static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> >         if (dev_id < ARRAY_SIZE(dev_to_feature_str))
> >                 feature_strs = dev_to_feature_str[dev_id];
> >
> > -       if (mgmtdevf)
> > +       if (devf)
> >                 pr_out_array_start(vdpa, "dev_features");
> >         else
> >                 pr_out_array_start(vdpa, "negotiated_features");
> > @@ -771,6 +773,15 @@ static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
> >                 val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
> >                 print_features(vdpa, val_u64, false, dev_id);
> >         }
> > +       if (tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]) {
> > +               uint16_t dev_id = 0;
> > +
> > +               if (tb[VDPA_ATTR_DEV_ID])
> > +                       dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
> > +
> > +               val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]);
> > +               print_features(vdpa, val_u64, true, dev_id);
> > +       }
> >  }
> >
> >  static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
> > --
> > 2.31.1
> >

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] iproute2/vdpa: Add support for reading device features
  2022-10-17  7:13     ` Jason Wang
  (?)
@ 2022-10-18  2:20     ` Zhu, Lingshan
  2022-10-18  6:44         ` Jason Wang
  -1 siblings, 1 reply; 17+ messages in thread
From: Zhu, Lingshan @ 2022-10-18  2:20 UTC (permalink / raw)
  To: Jason Wang; +Cc: mst, stephen, dsahern, virtualization, netdev, hang.yuan



On 10/17/2022 3:13 PM, Jason Wang wrote:
> On Mon, Oct 17, 2022 at 3:13 PM Jason Wang <jasowang@redhat.com> wrote:
>> On Fri, Oct 14, 2022 at 5:50 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
>>> This commit implements support for reading vdpa device
>>> features in iproute2.
>>>
>>> Example:
>>> $ vdpa dev config show vdpa0
>>> vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 4 mtu 1500
>>>    negotiated_features MRG_RXBUF CTRL_VQ MQ VERSION_1 ACCESS_PLATFORM
>>>    dev_features MTU MAC MRG_RXBUF CTRL_VQ MQ ANY_LAYOUT VERSION_1 ACCESS_PLATFORM
>>>
>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>> Note that Si Wei proposed to unify the two new attributes:
> https://patchew.org/linux/1665422823-18364-1-git-send-email-si-wei.liu@oracle.com/
I think we have discussed this before, there should be two netlink 
attributes to report management device features and vDPA device features,
they are different type of devices, this unification introduces 
unnecessary couplings

Thanks
>
> Thanks
>
>>
>>> ---
>>>   vdpa/vdpa.c | 15 +++++++++++++--
>>>   1 file changed, 13 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
>>> index b73e40b4..89844e92 100644
>>> --- a/vdpa/vdpa.c
>>> +++ b/vdpa/vdpa.c
>>> @@ -87,6 +87,8 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
>>>          [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
>>>          [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32,
>>>          [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
>>> +       [VDPA_ATTR_DEV_FEATURES] = MNL_TYPE_U64,
>>> +       [VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
>>>   };
>>>
>>>   static int attr_cb(const struct nlattr *attr, void *data)
>>> @@ -482,7 +484,7 @@ static const char * const *dev_to_feature_str[] = {
>>>
>>>   #define NUM_FEATURE_BITS 64
>>>
>>> -static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
>>> +static void print_features(struct vdpa *vdpa, uint64_t features, bool devf,
>>>                             uint16_t dev_id)
>>>   {
>>>          const char * const *feature_strs = NULL;
>>> @@ -492,7 +494,7 @@ static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
>>>          if (dev_id < ARRAY_SIZE(dev_to_feature_str))
>>>                  feature_strs = dev_to_feature_str[dev_id];
>>>
>>> -       if (mgmtdevf)
>>> +       if (devf)
>>>                  pr_out_array_start(vdpa, "dev_features");
>>>          else
>>>                  pr_out_array_start(vdpa, "negotiated_features");
>>> @@ -771,6 +773,15 @@ static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
>>>                  val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
>>>                  print_features(vdpa, val_u64, false, dev_id);
>>>          }
>>> +       if (tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]) {
>>> +               uint16_t dev_id = 0;
>>> +
>>> +               if (tb[VDPA_ATTR_DEV_ID])
>>> +                       dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
>>> +
>>> +               val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]);
>>> +               print_features(vdpa, val_u64, true, dev_id);
>>> +       }
>>>   }
>>>
>>>   static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
>>> --
>>> 2.31.1
>>>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] iproute2/vdpa: Add support for reading device features
  2022-10-18  2:20     ` Zhu, Lingshan
@ 2022-10-18  6:44         ` Jason Wang
  0 siblings, 0 replies; 17+ messages in thread
From: Jason Wang @ 2022-10-18  6:44 UTC (permalink / raw)
  To: Zhu, Lingshan; +Cc: mst, stephen, dsahern, virtualization, netdev, hang.yuan

On Tue, Oct 18, 2022 at 10:20 AM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>
>
>
> On 10/17/2022 3:13 PM, Jason Wang wrote:
> > On Mon, Oct 17, 2022 at 3:13 PM Jason Wang <jasowang@redhat.com> wrote:
> >> On Fri, Oct 14, 2022 at 5:50 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
> >>> This commit implements support for reading vdpa device
> >>> features in iproute2.
> >>>
> >>> Example:
> >>> $ vdpa dev config show vdpa0
> >>> vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 4 mtu 1500
> >>>    negotiated_features MRG_RXBUF CTRL_VQ MQ VERSION_1 ACCESS_PLATFORM
> >>>    dev_features MTU MAC MRG_RXBUF CTRL_VQ MQ ANY_LAYOUT VERSION_1 ACCESS_PLATFORM
> >>>
> >>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> >> Note that Si Wei proposed to unify the two new attributes:
> > https://patchew.org/linux/1665422823-18364-1-git-send-email-si-wei.liu@oracle.com/
> I think we have discussed this before, there should be two netlink
> attributes to report management device features and vDPA device features,
> they are different type of devices, this unification introduces
> unnecessary couplings

I suggest going through the above patch, both attributes are for the
vDPA device only.

Thanks

>
> Thanks
> >
> > Thanks
> >
> >>
> >>> ---
> >>>   vdpa/vdpa.c | 15 +++++++++++++--
> >>>   1 file changed, 13 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
> >>> index b73e40b4..89844e92 100644
> >>> --- a/vdpa/vdpa.c
> >>> +++ b/vdpa/vdpa.c
> >>> @@ -87,6 +87,8 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
> >>>          [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
> >>>          [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32,
> >>>          [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> >>> +       [VDPA_ATTR_DEV_FEATURES] = MNL_TYPE_U64,
> >>> +       [VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> >>>   };
> >>>
> >>>   static int attr_cb(const struct nlattr *attr, void *data)
> >>> @@ -482,7 +484,7 @@ static const char * const *dev_to_feature_str[] = {
> >>>
> >>>   #define NUM_FEATURE_BITS 64
> >>>
> >>> -static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> >>> +static void print_features(struct vdpa *vdpa, uint64_t features, bool devf,
> >>>                             uint16_t dev_id)
> >>>   {
> >>>          const char * const *feature_strs = NULL;
> >>> @@ -492,7 +494,7 @@ static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> >>>          if (dev_id < ARRAY_SIZE(dev_to_feature_str))
> >>>                  feature_strs = dev_to_feature_str[dev_id];
> >>>
> >>> -       if (mgmtdevf)
> >>> +       if (devf)
> >>>                  pr_out_array_start(vdpa, "dev_features");
> >>>          else
> >>>                  pr_out_array_start(vdpa, "negotiated_features");
> >>> @@ -771,6 +773,15 @@ static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
> >>>                  val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
> >>>                  print_features(vdpa, val_u64, false, dev_id);
> >>>          }
> >>> +       if (tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]) {
> >>> +               uint16_t dev_id = 0;
> >>> +
> >>> +               if (tb[VDPA_ATTR_DEV_ID])
> >>> +                       dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
> >>> +
> >>> +               val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]);
> >>> +               print_features(vdpa, val_u64, true, dev_id);
> >>> +       }
> >>>   }
> >>>
> >>>   static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
> >>> --
> >>> 2.31.1
> >>>
>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] iproute2/vdpa: Add support for reading device features
@ 2022-10-18  6:44         ` Jason Wang
  0 siblings, 0 replies; 17+ messages in thread
From: Jason Wang @ 2022-10-18  6:44 UTC (permalink / raw)
  To: Zhu, Lingshan; +Cc: mst, netdev, dsahern, virtualization, stephen, hang.yuan

On Tue, Oct 18, 2022 at 10:20 AM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>
>
>
> On 10/17/2022 3:13 PM, Jason Wang wrote:
> > On Mon, Oct 17, 2022 at 3:13 PM Jason Wang <jasowang@redhat.com> wrote:
> >> On Fri, Oct 14, 2022 at 5:50 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
> >>> This commit implements support for reading vdpa device
> >>> features in iproute2.
> >>>
> >>> Example:
> >>> $ vdpa dev config show vdpa0
> >>> vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 4 mtu 1500
> >>>    negotiated_features MRG_RXBUF CTRL_VQ MQ VERSION_1 ACCESS_PLATFORM
> >>>    dev_features MTU MAC MRG_RXBUF CTRL_VQ MQ ANY_LAYOUT VERSION_1 ACCESS_PLATFORM
> >>>
> >>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> >> Note that Si Wei proposed to unify the two new attributes:
> > https://patchew.org/linux/1665422823-18364-1-git-send-email-si-wei.liu@oracle.com/
> I think we have discussed this before, there should be two netlink
> attributes to report management device features and vDPA device features,
> they are different type of devices, this unification introduces
> unnecessary couplings

I suggest going through the above patch, both attributes are for the
vDPA device only.

Thanks

>
> Thanks
> >
> > Thanks
> >
> >>
> >>> ---
> >>>   vdpa/vdpa.c | 15 +++++++++++++--
> >>>   1 file changed, 13 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
> >>> index b73e40b4..89844e92 100644
> >>> --- a/vdpa/vdpa.c
> >>> +++ b/vdpa/vdpa.c
> >>> @@ -87,6 +87,8 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
> >>>          [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
> >>>          [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32,
> >>>          [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> >>> +       [VDPA_ATTR_DEV_FEATURES] = MNL_TYPE_U64,
> >>> +       [VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> >>>   };
> >>>
> >>>   static int attr_cb(const struct nlattr *attr, void *data)
> >>> @@ -482,7 +484,7 @@ static const char * const *dev_to_feature_str[] = {
> >>>
> >>>   #define NUM_FEATURE_BITS 64
> >>>
> >>> -static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> >>> +static void print_features(struct vdpa *vdpa, uint64_t features, bool devf,
> >>>                             uint16_t dev_id)
> >>>   {
> >>>          const char * const *feature_strs = NULL;
> >>> @@ -492,7 +494,7 @@ static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> >>>          if (dev_id < ARRAY_SIZE(dev_to_feature_str))
> >>>                  feature_strs = dev_to_feature_str[dev_id];
> >>>
> >>> -       if (mgmtdevf)
> >>> +       if (devf)
> >>>                  pr_out_array_start(vdpa, "dev_features");
> >>>          else
> >>>                  pr_out_array_start(vdpa, "negotiated_features");
> >>> @@ -771,6 +773,15 @@ static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
> >>>                  val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
> >>>                  print_features(vdpa, val_u64, false, dev_id);
> >>>          }
> >>> +       if (tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]) {
> >>> +               uint16_t dev_id = 0;
> >>> +
> >>> +               if (tb[VDPA_ATTR_DEV_ID])
> >>> +                       dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
> >>> +
> >>> +               val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]);
> >>> +               print_features(vdpa, val_u64, true, dev_id);
> >>> +       }
> >>>   }
> >>>
> >>>   static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
> >>> --
> >>> 2.31.1
> >>>
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] iproute2/vdpa: Add support for reading device features
  2022-10-18  6:44         ` Jason Wang
  (?)
@ 2022-10-18  7:28         ` Zhu, Lingshan
  2022-10-18  7:30             ` Jason Wang
  -1 siblings, 1 reply; 17+ messages in thread
From: Zhu, Lingshan @ 2022-10-18  7:28 UTC (permalink / raw)
  To: Jason Wang; +Cc: mst, stephen, dsahern, virtualization, netdev, hang.yuan



On 10/18/2022 2:44 PM, Jason Wang wrote:
> On Tue, Oct 18, 2022 at 10:20 AM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>>
>>
>> On 10/17/2022 3:13 PM, Jason Wang wrote:
>>> On Mon, Oct 17, 2022 at 3:13 PM Jason Wang <jasowang@redhat.com> wrote:
>>>> On Fri, Oct 14, 2022 at 5:50 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
>>>>> This commit implements support for reading vdpa device
>>>>> features in iproute2.
>>>>>
>>>>> Example:
>>>>> $ vdpa dev config show vdpa0
>>>>> vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 4 mtu 1500
>>>>>     negotiated_features MRG_RXBUF CTRL_VQ MQ VERSION_1 ACCESS_PLATFORM
>>>>>     dev_features MTU MAC MRG_RXBUF CTRL_VQ MQ ANY_LAYOUT VERSION_1 ACCESS_PLATFORM
>>>>>
>>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>>>> Note that Si Wei proposed to unify the two new attributes:
>>> https://patchew.org/linux/1665422823-18364-1-git-send-email-si-wei.liu@oracle.com/
>> I think we have discussed this before, there should be two netlink
>> attributes to report management device features and vDPA device features,
>> they are different type of devices, this unification introduces
>> unnecessary couplings
> I suggest going through the above patch, both attributes are for the
> vDPA device only.
It seems not vDPA device only, from above patch, we see it re-uses
VDPA_ATTR_DEV_FEATURES for reporting vDPA device features

Thanks
>
> Thanks
>
>> Thanks
>>> Thanks
>>>
>>>>> ---
>>>>>    vdpa/vdpa.c | 15 +++++++++++++--
>>>>>    1 file changed, 13 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
>>>>> index b73e40b4..89844e92 100644
>>>>> --- a/vdpa/vdpa.c
>>>>> +++ b/vdpa/vdpa.c
>>>>> @@ -87,6 +87,8 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
>>>>>           [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
>>>>>           [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32,
>>>>>           [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
>>>>> +       [VDPA_ATTR_DEV_FEATURES] = MNL_TYPE_U64,
>>>>> +       [VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
>>>>>    };
>>>>>
>>>>>    static int attr_cb(const struct nlattr *attr, void *data)
>>>>> @@ -482,7 +484,7 @@ static const char * const *dev_to_feature_str[] = {
>>>>>
>>>>>    #define NUM_FEATURE_BITS 64
>>>>>
>>>>> -static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
>>>>> +static void print_features(struct vdpa *vdpa, uint64_t features, bool devf,
>>>>>                              uint16_t dev_id)
>>>>>    {
>>>>>           const char * const *feature_strs = NULL;
>>>>> @@ -492,7 +494,7 @@ static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
>>>>>           if (dev_id < ARRAY_SIZE(dev_to_feature_str))
>>>>>                   feature_strs = dev_to_feature_str[dev_id];
>>>>>
>>>>> -       if (mgmtdevf)
>>>>> +       if (devf)
>>>>>                   pr_out_array_start(vdpa, "dev_features");
>>>>>           else
>>>>>                   pr_out_array_start(vdpa, "negotiated_features");
>>>>> @@ -771,6 +773,15 @@ static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
>>>>>                   val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
>>>>>                   print_features(vdpa, val_u64, false, dev_id);
>>>>>           }
>>>>> +       if (tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]) {
>>>>> +               uint16_t dev_id = 0;
>>>>> +
>>>>> +               if (tb[VDPA_ATTR_DEV_ID])
>>>>> +                       dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
>>>>> +
>>>>> +               val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]);
>>>>> +               print_features(vdpa, val_u64, true, dev_id);
>>>>> +       }
>>>>>    }
>>>>>
>>>>>    static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
>>>>> --
>>>>> 2.31.1
>>>>>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] iproute2/vdpa: Add support for reading device features
  2022-10-18  7:28         ` Zhu, Lingshan
@ 2022-10-18  7:30             ` Jason Wang
  0 siblings, 0 replies; 17+ messages in thread
From: Jason Wang @ 2022-10-18  7:30 UTC (permalink / raw)
  To: Zhu, Lingshan; +Cc: mst, stephen, dsahern, virtualization, netdev, hang.yuan

On Tue, Oct 18, 2022 at 3:28 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>
>
>
> On 10/18/2022 2:44 PM, Jason Wang wrote:
> > On Tue, Oct 18, 2022 at 10:20 AM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
> >>
> >>
> >> On 10/17/2022 3:13 PM, Jason Wang wrote:
> >>> On Mon, Oct 17, 2022 at 3:13 PM Jason Wang <jasowang@redhat.com> wrote:
> >>>> On Fri, Oct 14, 2022 at 5:50 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
> >>>>> This commit implements support for reading vdpa device
> >>>>> features in iproute2.
> >>>>>
> >>>>> Example:
> >>>>> $ vdpa dev config show vdpa0
> >>>>> vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 4 mtu 1500
> >>>>>     negotiated_features MRG_RXBUF CTRL_VQ MQ VERSION_1 ACCESS_PLATFORM
> >>>>>     dev_features MTU MAC MRG_RXBUF CTRL_VQ MQ ANY_LAYOUT VERSION_1 ACCESS_PLATFORM
> >>>>>
> >>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> >>>> Note that Si Wei proposed to unify the two new attributes:
> >>> https://patchew.org/linux/1665422823-18364-1-git-send-email-si-wei.liu@oracle.com/
> >> I think we have discussed this before, there should be two netlink
> >> attributes to report management device features and vDPA device features,
> >> they are different type of devices, this unification introduces
> >> unnecessary couplings
> > I suggest going through the above patch, both attributes are for the
> > vDPA device only.
> It seems not vDPA device only, from above patch, we see it re-uses
> VDPA_ATTR_DEV_FEATURES for reporting vDPA device features

Yes, anything wrong with this? The device features could be
provisioned via netlink.

Thanks

>
> Thanks
> >
> > Thanks
> >
> >> Thanks
> >>> Thanks
> >>>
> >>>>> ---
> >>>>>    vdpa/vdpa.c | 15 +++++++++++++--
> >>>>>    1 file changed, 13 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
> >>>>> index b73e40b4..89844e92 100644
> >>>>> --- a/vdpa/vdpa.c
> >>>>> +++ b/vdpa/vdpa.c
> >>>>> @@ -87,6 +87,8 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
> >>>>>           [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
> >>>>>           [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32,
> >>>>>           [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> >>>>> +       [VDPA_ATTR_DEV_FEATURES] = MNL_TYPE_U64,
> >>>>> +       [VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> >>>>>    };
> >>>>>
> >>>>>    static int attr_cb(const struct nlattr *attr, void *data)
> >>>>> @@ -482,7 +484,7 @@ static const char * const *dev_to_feature_str[] = {
> >>>>>
> >>>>>    #define NUM_FEATURE_BITS 64
> >>>>>
> >>>>> -static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> >>>>> +static void print_features(struct vdpa *vdpa, uint64_t features, bool devf,
> >>>>>                              uint16_t dev_id)
> >>>>>    {
> >>>>>           const char * const *feature_strs = NULL;
> >>>>> @@ -492,7 +494,7 @@ static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> >>>>>           if (dev_id < ARRAY_SIZE(dev_to_feature_str))
> >>>>>                   feature_strs = dev_to_feature_str[dev_id];
> >>>>>
> >>>>> -       if (mgmtdevf)
> >>>>> +       if (devf)
> >>>>>                   pr_out_array_start(vdpa, "dev_features");
> >>>>>           else
> >>>>>                   pr_out_array_start(vdpa, "negotiated_features");
> >>>>> @@ -771,6 +773,15 @@ static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
> >>>>>                   val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
> >>>>>                   print_features(vdpa, val_u64, false, dev_id);
> >>>>>           }
> >>>>> +       if (tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]) {
> >>>>> +               uint16_t dev_id = 0;
> >>>>> +
> >>>>> +               if (tb[VDPA_ATTR_DEV_ID])
> >>>>> +                       dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
> >>>>> +
> >>>>> +               val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]);
> >>>>> +               print_features(vdpa, val_u64, true, dev_id);
> >>>>> +       }
> >>>>>    }
> >>>>>
> >>>>>    static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
> >>>>> --
> >>>>> 2.31.1
> >>>>>
>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] iproute2/vdpa: Add support for reading device features
@ 2022-10-18  7:30             ` Jason Wang
  0 siblings, 0 replies; 17+ messages in thread
From: Jason Wang @ 2022-10-18  7:30 UTC (permalink / raw)
  To: Zhu, Lingshan; +Cc: mst, netdev, dsahern, virtualization, stephen, hang.yuan

On Tue, Oct 18, 2022 at 3:28 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>
>
>
> On 10/18/2022 2:44 PM, Jason Wang wrote:
> > On Tue, Oct 18, 2022 at 10:20 AM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
> >>
> >>
> >> On 10/17/2022 3:13 PM, Jason Wang wrote:
> >>> On Mon, Oct 17, 2022 at 3:13 PM Jason Wang <jasowang@redhat.com> wrote:
> >>>> On Fri, Oct 14, 2022 at 5:50 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
> >>>>> This commit implements support for reading vdpa device
> >>>>> features in iproute2.
> >>>>>
> >>>>> Example:
> >>>>> $ vdpa dev config show vdpa0
> >>>>> vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 4 mtu 1500
> >>>>>     negotiated_features MRG_RXBUF CTRL_VQ MQ VERSION_1 ACCESS_PLATFORM
> >>>>>     dev_features MTU MAC MRG_RXBUF CTRL_VQ MQ ANY_LAYOUT VERSION_1 ACCESS_PLATFORM
> >>>>>
> >>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> >>>> Note that Si Wei proposed to unify the two new attributes:
> >>> https://patchew.org/linux/1665422823-18364-1-git-send-email-si-wei.liu@oracle.com/
> >> I think we have discussed this before, there should be two netlink
> >> attributes to report management device features and vDPA device features,
> >> they are different type of devices, this unification introduces
> >> unnecessary couplings
> > I suggest going through the above patch, both attributes are for the
> > vDPA device only.
> It seems not vDPA device only, from above patch, we see it re-uses
> VDPA_ATTR_DEV_FEATURES for reporting vDPA device features

Yes, anything wrong with this? The device features could be
provisioned via netlink.

Thanks

>
> Thanks
> >
> > Thanks
> >
> >> Thanks
> >>> Thanks
> >>>
> >>>>> ---
> >>>>>    vdpa/vdpa.c | 15 +++++++++++++--
> >>>>>    1 file changed, 13 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
> >>>>> index b73e40b4..89844e92 100644
> >>>>> --- a/vdpa/vdpa.c
> >>>>> +++ b/vdpa/vdpa.c
> >>>>> @@ -87,6 +87,8 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
> >>>>>           [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
> >>>>>           [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32,
> >>>>>           [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> >>>>> +       [VDPA_ATTR_DEV_FEATURES] = MNL_TYPE_U64,
> >>>>> +       [VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> >>>>>    };
> >>>>>
> >>>>>    static int attr_cb(const struct nlattr *attr, void *data)
> >>>>> @@ -482,7 +484,7 @@ static const char * const *dev_to_feature_str[] = {
> >>>>>
> >>>>>    #define NUM_FEATURE_BITS 64
> >>>>>
> >>>>> -static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> >>>>> +static void print_features(struct vdpa *vdpa, uint64_t features, bool devf,
> >>>>>                              uint16_t dev_id)
> >>>>>    {
> >>>>>           const char * const *feature_strs = NULL;
> >>>>> @@ -492,7 +494,7 @@ static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> >>>>>           if (dev_id < ARRAY_SIZE(dev_to_feature_str))
> >>>>>                   feature_strs = dev_to_feature_str[dev_id];
> >>>>>
> >>>>> -       if (mgmtdevf)
> >>>>> +       if (devf)
> >>>>>                   pr_out_array_start(vdpa, "dev_features");
> >>>>>           else
> >>>>>                   pr_out_array_start(vdpa, "negotiated_features");
> >>>>> @@ -771,6 +773,15 @@ static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
> >>>>>                   val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
> >>>>>                   print_features(vdpa, val_u64, false, dev_id);
> >>>>>           }
> >>>>> +       if (tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]) {
> >>>>> +               uint16_t dev_id = 0;
> >>>>> +
> >>>>> +               if (tb[VDPA_ATTR_DEV_ID])
> >>>>> +                       dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
> >>>>> +
> >>>>> +               val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]);
> >>>>> +               print_features(vdpa, val_u64, true, dev_id);
> >>>>> +       }
> >>>>>    }
> >>>>>
> >>>>>    static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
> >>>>> --
> >>>>> 2.31.1
> >>>>>
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] iproute2/vdpa: Add support for reading device features
  2022-10-18  7:30             ` Jason Wang
  (?)
@ 2022-10-18  7:46             ` Zhu, Lingshan
  2022-10-18  7:49                 ` Jason Wang
  -1 siblings, 1 reply; 17+ messages in thread
From: Zhu, Lingshan @ 2022-10-18  7:46 UTC (permalink / raw)
  To: Jason Wang; +Cc: mst, stephen, dsahern, virtualization, netdev, hang.yuan



On 10/18/2022 3:30 PM, Jason Wang wrote:
> On Tue, Oct 18, 2022 at 3:28 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>>
>>
>> On 10/18/2022 2:44 PM, Jason Wang wrote:
>>> On Tue, Oct 18, 2022 at 10:20 AM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>>>>
>>>> On 10/17/2022 3:13 PM, Jason Wang wrote:
>>>>> On Mon, Oct 17, 2022 at 3:13 PM Jason Wang <jasowang@redhat.com> wrote:
>>>>>> On Fri, Oct 14, 2022 at 5:50 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
>>>>>>> This commit implements support for reading vdpa device
>>>>>>> features in iproute2.
>>>>>>>
>>>>>>> Example:
>>>>>>> $ vdpa dev config show vdpa0
>>>>>>> vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 4 mtu 1500
>>>>>>>      negotiated_features MRG_RXBUF CTRL_VQ MQ VERSION_1 ACCESS_PLATFORM
>>>>>>>      dev_features MTU MAC MRG_RXBUF CTRL_VQ MQ ANY_LAYOUT VERSION_1 ACCESS_PLATFORM
>>>>>>>
>>>>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>>>>>> Note that Si Wei proposed to unify the two new attributes:
>>>>> https://patchew.org/linux/1665422823-18364-1-git-send-email-si-wei.liu@oracle.com/
>>>> I think we have discussed this before, there should be two netlink
>>>> attributes to report management device features and vDPA device features,
>>>> they are different type of devices, this unification introduces
>>>> unnecessary couplings
>>> I suggest going through the above patch, both attributes are for the
>>> vDPA device only.
>> It seems not vDPA device only, from above patch, we see it re-uses
>> VDPA_ATTR_DEV_FEATURES for reporting vDPA device features
> Yes, anything wrong with this? The device features could be
> provisioned via netlink.
I think the best netlink practice is to let every attr has its own
and unique purpose, to prevent potential bugs. I think we have discussed 
this before that re-using
an attr does not save any resource.

And iprout2 has already updated the uapi header.

Thanks
>
> Thanks
>
>> Thanks
>>> Thanks
>>>
>>>> Thanks
>>>>> Thanks
>>>>>
>>>>>>> ---
>>>>>>>     vdpa/vdpa.c | 15 +++++++++++++--
>>>>>>>     1 file changed, 13 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
>>>>>>> index b73e40b4..89844e92 100644
>>>>>>> --- a/vdpa/vdpa.c
>>>>>>> +++ b/vdpa/vdpa.c
>>>>>>> @@ -87,6 +87,8 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
>>>>>>>            [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
>>>>>>>            [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32,
>>>>>>>            [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
>>>>>>> +       [VDPA_ATTR_DEV_FEATURES] = MNL_TYPE_U64,
>>>>>>> +       [VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
>>>>>>>     };
>>>>>>>
>>>>>>>     static int attr_cb(const struct nlattr *attr, void *data)
>>>>>>> @@ -482,7 +484,7 @@ static const char * const *dev_to_feature_str[] = {
>>>>>>>
>>>>>>>     #define NUM_FEATURE_BITS 64
>>>>>>>
>>>>>>> -static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
>>>>>>> +static void print_features(struct vdpa *vdpa, uint64_t features, bool devf,
>>>>>>>                               uint16_t dev_id)
>>>>>>>     {
>>>>>>>            const char * const *feature_strs = NULL;
>>>>>>> @@ -492,7 +494,7 @@ static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
>>>>>>>            if (dev_id < ARRAY_SIZE(dev_to_feature_str))
>>>>>>>                    feature_strs = dev_to_feature_str[dev_id];
>>>>>>>
>>>>>>> -       if (mgmtdevf)
>>>>>>> +       if (devf)
>>>>>>>                    pr_out_array_start(vdpa, "dev_features");
>>>>>>>            else
>>>>>>>                    pr_out_array_start(vdpa, "negotiated_features");
>>>>>>> @@ -771,6 +773,15 @@ static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
>>>>>>>                    val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
>>>>>>>                    print_features(vdpa, val_u64, false, dev_id);
>>>>>>>            }
>>>>>>> +       if (tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]) {
>>>>>>> +               uint16_t dev_id = 0;
>>>>>>> +
>>>>>>> +               if (tb[VDPA_ATTR_DEV_ID])
>>>>>>> +                       dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
>>>>>>> +
>>>>>>> +               val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]);
>>>>>>> +               print_features(vdpa, val_u64, true, dev_id);
>>>>>>> +       }
>>>>>>>     }
>>>>>>>
>>>>>>>     static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
>>>>>>> --
>>>>>>> 2.31.1
>>>>>>>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] iproute2/vdpa: Add support for reading device features
  2022-10-18  7:46             ` Zhu, Lingshan
@ 2022-10-18  7:49                 ` Jason Wang
  0 siblings, 0 replies; 17+ messages in thread
From: Jason Wang @ 2022-10-18  7:49 UTC (permalink / raw)
  To: Zhu, Lingshan
  Cc: mst, stephen, dsahern, virtualization, netdev, hang.yuan, Si-Wei Liu

On Tue, Oct 18, 2022 at 3:46 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>
>
>
> On 10/18/2022 3:30 PM, Jason Wang wrote:
> > On Tue, Oct 18, 2022 at 3:28 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
> >>
> >>
> >> On 10/18/2022 2:44 PM, Jason Wang wrote:
> >>> On Tue, Oct 18, 2022 at 10:20 AM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
> >>>>
> >>>> On 10/17/2022 3:13 PM, Jason Wang wrote:
> >>>>> On Mon, Oct 17, 2022 at 3:13 PM Jason Wang <jasowang@redhat.com> wrote:
> >>>>>> On Fri, Oct 14, 2022 at 5:50 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
> >>>>>>> This commit implements support for reading vdpa device
> >>>>>>> features in iproute2.
> >>>>>>>
> >>>>>>> Example:
> >>>>>>> $ vdpa dev config show vdpa0
> >>>>>>> vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 4 mtu 1500
> >>>>>>>      negotiated_features MRG_RXBUF CTRL_VQ MQ VERSION_1 ACCESS_PLATFORM
> >>>>>>>      dev_features MTU MAC MRG_RXBUF CTRL_VQ MQ ANY_LAYOUT VERSION_1 ACCESS_PLATFORM
> >>>>>>>
> >>>>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> >>>>>> Note that Si Wei proposed to unify the two new attributes:
> >>>>> https://patchew.org/linux/1665422823-18364-1-git-send-email-si-wei.liu@oracle.com/
> >>>> I think we have discussed this before, there should be two netlink
> >>>> attributes to report management device features and vDPA device features,
> >>>> they are different type of devices, this unification introduces
> >>>> unnecessary couplings
> >>> I suggest going through the above patch, both attributes are for the
> >>> vDPA device only.
> >> It seems not vDPA device only, from above patch, we see it re-uses
> >> VDPA_ATTR_DEV_FEATURES for reporting vDPA device features
> > Yes, anything wrong with this? The device features could be
> > provisioned via netlink.
> I think the best netlink practice is to let every attr has its own
> and unique purpose, to prevent potential bugs. I think we have discussed
> this before that re-using
> an attr does not save any resource.

They have exactly the same semantic which is the device features for vDPA.

VDPA_ATTR_DEV_FEATURES is introduced by my features provisioning
series, which is used for the userspace to "set" the device features.
VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES is introduced in one of your
previous patches, which is used for userspace to "get" the device
features.

>
> And iprout2 has already updated the uapi header.

Yes, but iproute2 has the same schedule as kernel release, so it's not
too late to fix.

Thanks

>
> Thanks
> >
> > Thanks
> >
> >> Thanks
> >>> Thanks
> >>>
> >>>> Thanks
> >>>>> Thanks
> >>>>>
> >>>>>>> ---
> >>>>>>>     vdpa/vdpa.c | 15 +++++++++++++--
> >>>>>>>     1 file changed, 13 insertions(+), 2 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
> >>>>>>> index b73e40b4..89844e92 100644
> >>>>>>> --- a/vdpa/vdpa.c
> >>>>>>> +++ b/vdpa/vdpa.c
> >>>>>>> @@ -87,6 +87,8 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
> >>>>>>>            [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
> >>>>>>>            [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32,
> >>>>>>>            [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> >>>>>>> +       [VDPA_ATTR_DEV_FEATURES] = MNL_TYPE_U64,
> >>>>>>> +       [VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> >>>>>>>     };
> >>>>>>>
> >>>>>>>     static int attr_cb(const struct nlattr *attr, void *data)
> >>>>>>> @@ -482,7 +484,7 @@ static const char * const *dev_to_feature_str[] = {
> >>>>>>>
> >>>>>>>     #define NUM_FEATURE_BITS 64
> >>>>>>>
> >>>>>>> -static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> >>>>>>> +static void print_features(struct vdpa *vdpa, uint64_t features, bool devf,
> >>>>>>>                               uint16_t dev_id)
> >>>>>>>     {
> >>>>>>>            const char * const *feature_strs = NULL;
> >>>>>>> @@ -492,7 +494,7 @@ static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> >>>>>>>            if (dev_id < ARRAY_SIZE(dev_to_feature_str))
> >>>>>>>                    feature_strs = dev_to_feature_str[dev_id];
> >>>>>>>
> >>>>>>> -       if (mgmtdevf)
> >>>>>>> +       if (devf)
> >>>>>>>                    pr_out_array_start(vdpa, "dev_features");
> >>>>>>>            else
> >>>>>>>                    pr_out_array_start(vdpa, "negotiated_features");
> >>>>>>> @@ -771,6 +773,15 @@ static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
> >>>>>>>                    val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
> >>>>>>>                    print_features(vdpa, val_u64, false, dev_id);
> >>>>>>>            }
> >>>>>>> +       if (tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]) {
> >>>>>>> +               uint16_t dev_id = 0;
> >>>>>>> +
> >>>>>>> +               if (tb[VDPA_ATTR_DEV_ID])
> >>>>>>> +                       dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
> >>>>>>> +
> >>>>>>> +               val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]);
> >>>>>>> +               print_features(vdpa, val_u64, true, dev_id);
> >>>>>>> +       }
> >>>>>>>     }
> >>>>>>>
> >>>>>>>     static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
> >>>>>>> --
> >>>>>>> 2.31.1
> >>>>>>>
>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] iproute2/vdpa: Add support for reading device features
@ 2022-10-18  7:49                 ` Jason Wang
  0 siblings, 0 replies; 17+ messages in thread
From: Jason Wang @ 2022-10-18  7:49 UTC (permalink / raw)
  To: Zhu, Lingshan; +Cc: mst, netdev, dsahern, virtualization, stephen, hang.yuan

On Tue, Oct 18, 2022 at 3:46 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>
>
>
> On 10/18/2022 3:30 PM, Jason Wang wrote:
> > On Tue, Oct 18, 2022 at 3:28 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
> >>
> >>
> >> On 10/18/2022 2:44 PM, Jason Wang wrote:
> >>> On Tue, Oct 18, 2022 at 10:20 AM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
> >>>>
> >>>> On 10/17/2022 3:13 PM, Jason Wang wrote:
> >>>>> On Mon, Oct 17, 2022 at 3:13 PM Jason Wang <jasowang@redhat.com> wrote:
> >>>>>> On Fri, Oct 14, 2022 at 5:50 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
> >>>>>>> This commit implements support for reading vdpa device
> >>>>>>> features in iproute2.
> >>>>>>>
> >>>>>>> Example:
> >>>>>>> $ vdpa dev config show vdpa0
> >>>>>>> vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 4 mtu 1500
> >>>>>>>      negotiated_features MRG_RXBUF CTRL_VQ MQ VERSION_1 ACCESS_PLATFORM
> >>>>>>>      dev_features MTU MAC MRG_RXBUF CTRL_VQ MQ ANY_LAYOUT VERSION_1 ACCESS_PLATFORM
> >>>>>>>
> >>>>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> >>>>>> Note that Si Wei proposed to unify the two new attributes:
> >>>>> https://patchew.org/linux/1665422823-18364-1-git-send-email-si-wei.liu@oracle.com/
> >>>> I think we have discussed this before, there should be two netlink
> >>>> attributes to report management device features and vDPA device features,
> >>>> they are different type of devices, this unification introduces
> >>>> unnecessary couplings
> >>> I suggest going through the above patch, both attributes are for the
> >>> vDPA device only.
> >> It seems not vDPA device only, from above patch, we see it re-uses
> >> VDPA_ATTR_DEV_FEATURES for reporting vDPA device features
> > Yes, anything wrong with this? The device features could be
> > provisioned via netlink.
> I think the best netlink practice is to let every attr has its own
> and unique purpose, to prevent potential bugs. I think we have discussed
> this before that re-using
> an attr does not save any resource.

They have exactly the same semantic which is the device features for vDPA.

VDPA_ATTR_DEV_FEATURES is introduced by my features provisioning
series, which is used for the userspace to "set" the device features.
VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES is introduced in one of your
previous patches, which is used for userspace to "get" the device
features.

>
> And iprout2 has already updated the uapi header.

Yes, but iproute2 has the same schedule as kernel release, so it's not
too late to fix.

Thanks

>
> Thanks
> >
> > Thanks
> >
> >> Thanks
> >>> Thanks
> >>>
> >>>> Thanks
> >>>>> Thanks
> >>>>>
> >>>>>>> ---
> >>>>>>>     vdpa/vdpa.c | 15 +++++++++++++--
> >>>>>>>     1 file changed, 13 insertions(+), 2 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
> >>>>>>> index b73e40b4..89844e92 100644
> >>>>>>> --- a/vdpa/vdpa.c
> >>>>>>> +++ b/vdpa/vdpa.c
> >>>>>>> @@ -87,6 +87,8 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
> >>>>>>>            [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
> >>>>>>>            [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32,
> >>>>>>>            [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> >>>>>>> +       [VDPA_ATTR_DEV_FEATURES] = MNL_TYPE_U64,
> >>>>>>> +       [VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> >>>>>>>     };
> >>>>>>>
> >>>>>>>     static int attr_cb(const struct nlattr *attr, void *data)
> >>>>>>> @@ -482,7 +484,7 @@ static const char * const *dev_to_feature_str[] = {
> >>>>>>>
> >>>>>>>     #define NUM_FEATURE_BITS 64
> >>>>>>>
> >>>>>>> -static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> >>>>>>> +static void print_features(struct vdpa *vdpa, uint64_t features, bool devf,
> >>>>>>>                               uint16_t dev_id)
> >>>>>>>     {
> >>>>>>>            const char * const *feature_strs = NULL;
> >>>>>>> @@ -492,7 +494,7 @@ static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> >>>>>>>            if (dev_id < ARRAY_SIZE(dev_to_feature_str))
> >>>>>>>                    feature_strs = dev_to_feature_str[dev_id];
> >>>>>>>
> >>>>>>> -       if (mgmtdevf)
> >>>>>>> +       if (devf)
> >>>>>>>                    pr_out_array_start(vdpa, "dev_features");
> >>>>>>>            else
> >>>>>>>                    pr_out_array_start(vdpa, "negotiated_features");
> >>>>>>> @@ -771,6 +773,15 @@ static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
> >>>>>>>                    val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
> >>>>>>>                    print_features(vdpa, val_u64, false, dev_id);
> >>>>>>>            }
> >>>>>>> +       if (tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]) {
> >>>>>>> +               uint16_t dev_id = 0;
> >>>>>>> +
> >>>>>>> +               if (tb[VDPA_ATTR_DEV_ID])
> >>>>>>> +                       dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
> >>>>>>> +
> >>>>>>> +               val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]);
> >>>>>>> +               print_features(vdpa, val_u64, true, dev_id);
> >>>>>>> +       }
> >>>>>>>     }
> >>>>>>>
> >>>>>>>     static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
> >>>>>>> --
> >>>>>>> 2.31.1
> >>>>>>>
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] iproute2/vdpa: Add support for reading device features
  2022-10-18  7:49                 ` Jason Wang
  (?)
@ 2022-10-18  7:55                 ` Zhu, Lingshan
  2022-10-18  8:00                     ` Jason Wang
  -1 siblings, 1 reply; 17+ messages in thread
From: Zhu, Lingshan @ 2022-10-18  7:55 UTC (permalink / raw)
  To: Jason Wang
  Cc: mst, stephen, dsahern, virtualization, netdev, hang.yuan, Si-Wei Liu



On 10/18/2022 3:49 PM, Jason Wang wrote:
> On Tue, Oct 18, 2022 at 3:46 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>>
>>
>> On 10/18/2022 3:30 PM, Jason Wang wrote:
>>> On Tue, Oct 18, 2022 at 3:28 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>>>>
>>>> On 10/18/2022 2:44 PM, Jason Wang wrote:
>>>>> On Tue, Oct 18, 2022 at 10:20 AM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>>>>>> On 10/17/2022 3:13 PM, Jason Wang wrote:
>>>>>>> On Mon, Oct 17, 2022 at 3:13 PM Jason Wang <jasowang@redhat.com> wrote:
>>>>>>>> On Fri, Oct 14, 2022 at 5:50 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
>>>>>>>>> This commit implements support for reading vdpa device
>>>>>>>>> features in iproute2.
>>>>>>>>>
>>>>>>>>> Example:
>>>>>>>>> $ vdpa dev config show vdpa0
>>>>>>>>> vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 4 mtu 1500
>>>>>>>>>       negotiated_features MRG_RXBUF CTRL_VQ MQ VERSION_1 ACCESS_PLATFORM
>>>>>>>>>       dev_features MTU MAC MRG_RXBUF CTRL_VQ MQ ANY_LAYOUT VERSION_1 ACCESS_PLATFORM
>>>>>>>>>
>>>>>>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>>>>>>>> Note that Si Wei proposed to unify the two new attributes:
>>>>>>> https://patchew.org/linux/1665422823-18364-1-git-send-email-si-wei.liu@oracle.com/
>>>>>> I think we have discussed this before, there should be two netlink
>>>>>> attributes to report management device features and vDPA device features,
>>>>>> they are different type of devices, this unification introduces
>>>>>> unnecessary couplings
>>>>> I suggest going through the above patch, both attributes are for the
>>>>> vDPA device only.
>>>> It seems not vDPA device only, from above patch, we see it re-uses
>>>> VDPA_ATTR_DEV_FEATURES for reporting vDPA device features
>>> Yes, anything wrong with this? The device features could be
>>> provisioned via netlink.
>> I think the best netlink practice is to let every attr has its own
>> and unique purpose, to prevent potential bugs. I think we have discussed
>> this before that re-using
>> an attr does not save any resource.
> They have exactly the same semantic which is the device features for vDPA.
>
> VDPA_ATTR_DEV_FEATURES is introduced by my features provisioning
> series, which is used for the userspace to "set" the device features.
> VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES is introduced in one of your
> previous patches, which is used for userspace to "get" the device
> features.
so basically we are just combining and renaming the attr,
if so, fine with me, get and set may never conflict and in
totally different netlink contexts.

Thanks
>
>> And iprout2 has already updated the uapi header.
> Yes, but iproute2 has the same schedule as kernel release, so it's not
> too late to fix.
>
> Thanks
>
>> Thanks
>>> Thanks
>>>
>>>> Thanks
>>>>> Thanks
>>>>>
>>>>>> Thanks
>>>>>>> Thanks
>>>>>>>
>>>>>>>>> ---
>>>>>>>>>      vdpa/vdpa.c | 15 +++++++++++++--
>>>>>>>>>      1 file changed, 13 insertions(+), 2 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
>>>>>>>>> index b73e40b4..89844e92 100644
>>>>>>>>> --- a/vdpa/vdpa.c
>>>>>>>>> +++ b/vdpa/vdpa.c
>>>>>>>>> @@ -87,6 +87,8 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
>>>>>>>>>             [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
>>>>>>>>>             [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32,
>>>>>>>>>             [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
>>>>>>>>> +       [VDPA_ATTR_DEV_FEATURES] = MNL_TYPE_U64,
>>>>>>>>> +       [VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
>>>>>>>>>      };
>>>>>>>>>
>>>>>>>>>      static int attr_cb(const struct nlattr *attr, void *data)
>>>>>>>>> @@ -482,7 +484,7 @@ static const char * const *dev_to_feature_str[] = {
>>>>>>>>>
>>>>>>>>>      #define NUM_FEATURE_BITS 64
>>>>>>>>>
>>>>>>>>> -static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
>>>>>>>>> +static void print_features(struct vdpa *vdpa, uint64_t features, bool devf,
>>>>>>>>>                                uint16_t dev_id)
>>>>>>>>>      {
>>>>>>>>>             const char * const *feature_strs = NULL;
>>>>>>>>> @@ -492,7 +494,7 @@ static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
>>>>>>>>>             if (dev_id < ARRAY_SIZE(dev_to_feature_str))
>>>>>>>>>                     feature_strs = dev_to_feature_str[dev_id];
>>>>>>>>>
>>>>>>>>> -       if (mgmtdevf)
>>>>>>>>> +       if (devf)
>>>>>>>>>                     pr_out_array_start(vdpa, "dev_features");
>>>>>>>>>             else
>>>>>>>>>                     pr_out_array_start(vdpa, "negotiated_features");
>>>>>>>>> @@ -771,6 +773,15 @@ static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
>>>>>>>>>                     val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
>>>>>>>>>                     print_features(vdpa, val_u64, false, dev_id);
>>>>>>>>>             }
>>>>>>>>> +       if (tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]) {
>>>>>>>>> +               uint16_t dev_id = 0;
>>>>>>>>> +
>>>>>>>>> +               if (tb[VDPA_ATTR_DEV_ID])
>>>>>>>>> +                       dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
>>>>>>>>> +
>>>>>>>>> +               val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]);
>>>>>>>>> +               print_features(vdpa, val_u64, true, dev_id);
>>>>>>>>> +       }
>>>>>>>>>      }
>>>>>>>>>
>>>>>>>>>      static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
>>>>>>>>> --
>>>>>>>>> 2.31.1
>>>>>>>>>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] iproute2/vdpa: Add support for reading device features
  2022-10-18  7:55                 ` Zhu, Lingshan
@ 2022-10-18  8:00                     ` Jason Wang
  0 siblings, 0 replies; 17+ messages in thread
From: Jason Wang @ 2022-10-18  8:00 UTC (permalink / raw)
  To: Zhu, Lingshan
  Cc: mst, stephen, dsahern, virtualization, netdev, hang.yuan, Si-Wei Liu

On Tue, Oct 18, 2022 at 3:55 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>
>
>
> On 10/18/2022 3:49 PM, Jason Wang wrote:
> > On Tue, Oct 18, 2022 at 3:46 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
> >>
> >>
> >> On 10/18/2022 3:30 PM, Jason Wang wrote:
> >>> On Tue, Oct 18, 2022 at 3:28 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
> >>>>
> >>>> On 10/18/2022 2:44 PM, Jason Wang wrote:
> >>>>> On Tue, Oct 18, 2022 at 10:20 AM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
> >>>>>> On 10/17/2022 3:13 PM, Jason Wang wrote:
> >>>>>>> On Mon, Oct 17, 2022 at 3:13 PM Jason Wang <jasowang@redhat.com> wrote:
> >>>>>>>> On Fri, Oct 14, 2022 at 5:50 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
> >>>>>>>>> This commit implements support for reading vdpa device
> >>>>>>>>> features in iproute2.
> >>>>>>>>>
> >>>>>>>>> Example:
> >>>>>>>>> $ vdpa dev config show vdpa0
> >>>>>>>>> vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 4 mtu 1500
> >>>>>>>>>       negotiated_features MRG_RXBUF CTRL_VQ MQ VERSION_1 ACCESS_PLATFORM
> >>>>>>>>>       dev_features MTU MAC MRG_RXBUF CTRL_VQ MQ ANY_LAYOUT VERSION_1 ACCESS_PLATFORM
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> >>>>>>>> Note that Si Wei proposed to unify the two new attributes:
> >>>>>>> https://patchew.org/linux/1665422823-18364-1-git-send-email-si-wei.liu@oracle.com/
> >>>>>> I think we have discussed this before, there should be two netlink
> >>>>>> attributes to report management device features and vDPA device features,
> >>>>>> they are different type of devices, this unification introduces
> >>>>>> unnecessary couplings
> >>>>> I suggest going through the above patch, both attributes are for the
> >>>>> vDPA device only.
> >>>> It seems not vDPA device only, from above patch, we see it re-uses
> >>>> VDPA_ATTR_DEV_FEATURES for reporting vDPA device features
> >>> Yes, anything wrong with this? The device features could be
> >>> provisioned via netlink.
> >> I think the best netlink practice is to let every attr has its own
> >> and unique purpose, to prevent potential bugs. I think we have discussed
> >> this before that re-using
> >> an attr does not save any resource.
> > They have exactly the same semantic which is the device features for vDPA.
> >
> > VDPA_ATTR_DEV_FEATURES is introduced by my features provisioning
> > series, which is used for the userspace to "set" the device features.
> > VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES is introduced in one of your
> > previous patches, which is used for userspace to "get" the device
> > features.
> so basically we are just combining and renaming the attr,
> if so, fine with me, get and set may never conflict and in
> totally different netlink contexts.

Yes, I think so.

Thanks

>
> Thanks
> >
> >> And iprout2 has already updated the uapi header.
> > Yes, but iproute2 has the same schedule as kernel release, so it's not
> > too late to fix.
> >
> > Thanks
> >
> >> Thanks
> >>> Thanks
> >>>
> >>>> Thanks
> >>>>> Thanks
> >>>>>
> >>>>>> Thanks
> >>>>>>> Thanks
> >>>>>>>
> >>>>>>>>> ---
> >>>>>>>>>      vdpa/vdpa.c | 15 +++++++++++++--
> >>>>>>>>>      1 file changed, 13 insertions(+), 2 deletions(-)
> >>>>>>>>>
> >>>>>>>>> diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
> >>>>>>>>> index b73e40b4..89844e92 100644
> >>>>>>>>> --- a/vdpa/vdpa.c
> >>>>>>>>> +++ b/vdpa/vdpa.c
> >>>>>>>>> @@ -87,6 +87,8 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
> >>>>>>>>>             [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
> >>>>>>>>>             [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32,
> >>>>>>>>>             [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> >>>>>>>>> +       [VDPA_ATTR_DEV_FEATURES] = MNL_TYPE_U64,
> >>>>>>>>> +       [VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> >>>>>>>>>      };
> >>>>>>>>>
> >>>>>>>>>      static int attr_cb(const struct nlattr *attr, void *data)
> >>>>>>>>> @@ -482,7 +484,7 @@ static const char * const *dev_to_feature_str[] = {
> >>>>>>>>>
> >>>>>>>>>      #define NUM_FEATURE_BITS 64
> >>>>>>>>>
> >>>>>>>>> -static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> >>>>>>>>> +static void print_features(struct vdpa *vdpa, uint64_t features, bool devf,
> >>>>>>>>>                                uint16_t dev_id)
> >>>>>>>>>      {
> >>>>>>>>>             const char * const *feature_strs = NULL;
> >>>>>>>>> @@ -492,7 +494,7 @@ static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> >>>>>>>>>             if (dev_id < ARRAY_SIZE(dev_to_feature_str))
> >>>>>>>>>                     feature_strs = dev_to_feature_str[dev_id];
> >>>>>>>>>
> >>>>>>>>> -       if (mgmtdevf)
> >>>>>>>>> +       if (devf)
> >>>>>>>>>                     pr_out_array_start(vdpa, "dev_features");
> >>>>>>>>>             else
> >>>>>>>>>                     pr_out_array_start(vdpa, "negotiated_features");
> >>>>>>>>> @@ -771,6 +773,15 @@ static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
> >>>>>>>>>                     val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
> >>>>>>>>>                     print_features(vdpa, val_u64, false, dev_id);
> >>>>>>>>>             }
> >>>>>>>>> +       if (tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]) {
> >>>>>>>>> +               uint16_t dev_id = 0;
> >>>>>>>>> +
> >>>>>>>>> +               if (tb[VDPA_ATTR_DEV_ID])
> >>>>>>>>> +                       dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
> >>>>>>>>> +
> >>>>>>>>> +               val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]);
> >>>>>>>>> +               print_features(vdpa, val_u64, true, dev_id);
> >>>>>>>>> +       }
> >>>>>>>>>      }
> >>>>>>>>>
> >>>>>>>>>      static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
> >>>>>>>>> --
> >>>>>>>>> 2.31.1
> >>>>>>>>>
>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] iproute2/vdpa: Add support for reading device features
@ 2022-10-18  8:00                     ` Jason Wang
  0 siblings, 0 replies; 17+ messages in thread
From: Jason Wang @ 2022-10-18  8:00 UTC (permalink / raw)
  To: Zhu, Lingshan; +Cc: mst, netdev, dsahern, virtualization, stephen, hang.yuan

On Tue, Oct 18, 2022 at 3:55 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>
>
>
> On 10/18/2022 3:49 PM, Jason Wang wrote:
> > On Tue, Oct 18, 2022 at 3:46 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
> >>
> >>
> >> On 10/18/2022 3:30 PM, Jason Wang wrote:
> >>> On Tue, Oct 18, 2022 at 3:28 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
> >>>>
> >>>> On 10/18/2022 2:44 PM, Jason Wang wrote:
> >>>>> On Tue, Oct 18, 2022 at 10:20 AM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
> >>>>>> On 10/17/2022 3:13 PM, Jason Wang wrote:
> >>>>>>> On Mon, Oct 17, 2022 at 3:13 PM Jason Wang <jasowang@redhat.com> wrote:
> >>>>>>>> On Fri, Oct 14, 2022 at 5:50 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
> >>>>>>>>> This commit implements support for reading vdpa device
> >>>>>>>>> features in iproute2.
> >>>>>>>>>
> >>>>>>>>> Example:
> >>>>>>>>> $ vdpa dev config show vdpa0
> >>>>>>>>> vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 4 mtu 1500
> >>>>>>>>>       negotiated_features MRG_RXBUF CTRL_VQ MQ VERSION_1 ACCESS_PLATFORM
> >>>>>>>>>       dev_features MTU MAC MRG_RXBUF CTRL_VQ MQ ANY_LAYOUT VERSION_1 ACCESS_PLATFORM
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> >>>>>>>> Note that Si Wei proposed to unify the two new attributes:
> >>>>>>> https://patchew.org/linux/1665422823-18364-1-git-send-email-si-wei.liu@oracle.com/
> >>>>>> I think we have discussed this before, there should be two netlink
> >>>>>> attributes to report management device features and vDPA device features,
> >>>>>> they are different type of devices, this unification introduces
> >>>>>> unnecessary couplings
> >>>>> I suggest going through the above patch, both attributes are for the
> >>>>> vDPA device only.
> >>>> It seems not vDPA device only, from above patch, we see it re-uses
> >>>> VDPA_ATTR_DEV_FEATURES for reporting vDPA device features
> >>> Yes, anything wrong with this? The device features could be
> >>> provisioned via netlink.
> >> I think the best netlink practice is to let every attr has its own
> >> and unique purpose, to prevent potential bugs. I think we have discussed
> >> this before that re-using
> >> an attr does not save any resource.
> > They have exactly the same semantic which is the device features for vDPA.
> >
> > VDPA_ATTR_DEV_FEATURES is introduced by my features provisioning
> > series, which is used for the userspace to "set" the device features.
> > VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES is introduced in one of your
> > previous patches, which is used for userspace to "get" the device
> > features.
> so basically we are just combining and renaming the attr,
> if so, fine with me, get and set may never conflict and in
> totally different netlink contexts.

Yes, I think so.

Thanks

>
> Thanks
> >
> >> And iprout2 has already updated the uapi header.
> > Yes, but iproute2 has the same schedule as kernel release, so it's not
> > too late to fix.
> >
> > Thanks
> >
> >> Thanks
> >>> Thanks
> >>>
> >>>> Thanks
> >>>>> Thanks
> >>>>>
> >>>>>> Thanks
> >>>>>>> Thanks
> >>>>>>>
> >>>>>>>>> ---
> >>>>>>>>>      vdpa/vdpa.c | 15 +++++++++++++--
> >>>>>>>>>      1 file changed, 13 insertions(+), 2 deletions(-)
> >>>>>>>>>
> >>>>>>>>> diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
> >>>>>>>>> index b73e40b4..89844e92 100644
> >>>>>>>>> --- a/vdpa/vdpa.c
> >>>>>>>>> +++ b/vdpa/vdpa.c
> >>>>>>>>> @@ -87,6 +87,8 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
> >>>>>>>>>             [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
> >>>>>>>>>             [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32,
> >>>>>>>>>             [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> >>>>>>>>> +       [VDPA_ATTR_DEV_FEATURES] = MNL_TYPE_U64,
> >>>>>>>>> +       [VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64,
> >>>>>>>>>      };
> >>>>>>>>>
> >>>>>>>>>      static int attr_cb(const struct nlattr *attr, void *data)
> >>>>>>>>> @@ -482,7 +484,7 @@ static const char * const *dev_to_feature_str[] = {
> >>>>>>>>>
> >>>>>>>>>      #define NUM_FEATURE_BITS 64
> >>>>>>>>>
> >>>>>>>>> -static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> >>>>>>>>> +static void print_features(struct vdpa *vdpa, uint64_t features, bool devf,
> >>>>>>>>>                                uint16_t dev_id)
> >>>>>>>>>      {
> >>>>>>>>>             const char * const *feature_strs = NULL;
> >>>>>>>>> @@ -492,7 +494,7 @@ static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
> >>>>>>>>>             if (dev_id < ARRAY_SIZE(dev_to_feature_str))
> >>>>>>>>>                     feature_strs = dev_to_feature_str[dev_id];
> >>>>>>>>>
> >>>>>>>>> -       if (mgmtdevf)
> >>>>>>>>> +       if (devf)
> >>>>>>>>>                     pr_out_array_start(vdpa, "dev_features");
> >>>>>>>>>             else
> >>>>>>>>>                     pr_out_array_start(vdpa, "negotiated_features");
> >>>>>>>>> @@ -771,6 +773,15 @@ static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
> >>>>>>>>>                     val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
> >>>>>>>>>                     print_features(vdpa, val_u64, false, dev_id);
> >>>>>>>>>             }
> >>>>>>>>> +       if (tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]) {
> >>>>>>>>> +               uint16_t dev_id = 0;
> >>>>>>>>> +
> >>>>>>>>> +               if (tb[VDPA_ATTR_DEV_ID])
> >>>>>>>>> +                       dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
> >>>>>>>>> +
> >>>>>>>>> +               val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES]);
> >>>>>>>>> +               print_features(vdpa, val_u64, true, dev_id);
> >>>>>>>>> +       }
> >>>>>>>>>      }
> >>>>>>>>>
> >>>>>>>>>      static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
> >>>>>>>>> --
> >>>>>>>>> 2.31.1
> >>>>>>>>>
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2022-10-18  8:01 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-14  9:41 [PATCH] iproute2/vdpa: Add support for reading device features Zhu Lingshan
2022-10-17  7:13 ` Jason Wang
2022-10-17  7:13   ` Jason Wang
2022-10-17  7:13   ` Jason Wang
2022-10-17  7:13     ` Jason Wang
2022-10-18  2:20     ` Zhu, Lingshan
2022-10-18  6:44       ` Jason Wang
2022-10-18  6:44         ` Jason Wang
2022-10-18  7:28         ` Zhu, Lingshan
2022-10-18  7:30           ` Jason Wang
2022-10-18  7:30             ` Jason Wang
2022-10-18  7:46             ` Zhu, Lingshan
2022-10-18  7:49               ` Jason Wang
2022-10-18  7:49                 ` Jason Wang
2022-10-18  7:55                 ` Zhu, Lingshan
2022-10-18  8:00                   ` Jason Wang
2022-10-18  8:00                     ` Jason Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.