| Message ID | 20221014094152.5570-1-lingshan.zhu@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | David Ahern |
| Headers | show |
| Series | iproute2/vdpa: Add support for reading device features | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
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 >
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 > >
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 >>>
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 > >>> >
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 >>>>>
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 > >>>>> >
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 >>>>>>>
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 > >>>>>>> >
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 >>>>>>>>>
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 > >>>>>>>>> >
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)
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(-)