On Wed, Dec 7, 2022 at 1:12 PM Si-Wei Liu <si-wei.liu at oracle.com>
wrote:>
>
>
> On 12/5/2022 7:14 PM, Jason Wang wrote:
> > On Tue, Dec 6, 2022 at 9:43 AM Si-Wei Liu <si-wei.liu at
oracle.com> wrote:
> >>
> >>
> >> On 12/4/2022 10:46 PM, Jason Wang wrote:
> >>> On Thu, Dec 1, 2022 at 8:53 AM Si-Wei Liu <si-wei.liu at
oracle.com> wrote:
> >>>> Sorry for getting back late due to the snag of the
holidays.
> >>> No worries :)
> >>>
> >>>> On 11/23/2022 11:13 PM, Jason Wang wrote:
> >>>>> On Thu, Nov 24, 2022 at 6:53 AM Si-Wei Liu
<si-wei.liu at oracle.com> wrote:
> >>>>>> On 11/22/2022 7:35 PM, Jason Wang wrote:
> >>>>>>> On Wed, Nov 23, 2022 at 6:29 AM Si-Wei Liu
<si-wei.liu at oracle.com> wrote:
> >>>>>>>> On 11/16/2022 7:33 PM, Jason Wang wrote:
> >>>>>>>>> This patch allows device features to
be provisioned via vdpa. This
> >>>>>>>>> will be useful for preserving
migration compatibility between source
> >>>>>>>>> and destination:
> >>>>>>>>>
> >>>>>>>>> # vdpa dev add name dev1 mgmtdev
pci/0000:02:00.0 device_features 0x300020000
> >>>>>>>> Miss the actual "vdpa dev config
show" command below
> >>>>>>> Right, let me fix that.
> >>>>>>>
> >>>>>>>>> # dev1: mac 52:54:00:12:34:56 link up
link_announce false mtu 65535
> >>>>>>>>> negotiated_features CTRL_VQ
VERSION_1 ACCESS_PLATFORM
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Jason Wang <jasowang
at redhat.com>
> >>>>>>>>> ---
> >>>>>>>>> Changes since v1:
> >>>>>>>>> - Use uint64_t instead of __u64 for
device_features
> >>>>>>>>> - Fix typos and tweak the manpage
> >>>>>>>>> - Add device_features to the help text
> >>>>>>>>> ---
> >>>>>>>>> man/man8/vdpa-dev.8 |
15 +++++++++++++++
> >>>>>>>>> vdpa/include/uapi/linux/vdpa.h |
1 +
> >>>>>>>>> vdpa/vdpa.c |
32 +++++++++++++++++++++++++++++---
> >>>>>>>>> 3 files changed, 45
insertions(+), 3 deletions(-)
> >>>>>>>>>
> >>>>>>>>> diff --git a/man/man8/vdpa-dev.8
b/man/man8/vdpa-dev.8
> >>>>>>>>> index 9faf3838..43e5bf48 100644
> >>>>>>>>> --- a/man/man8/vdpa-dev.8
> >>>>>>>>> +++ b/man/man8/vdpa-dev.8
> >>>>>>>>> @@ -31,6 +31,7 @@ vdpa-dev \- vdpa
device configuration
> >>>>>>>>> .I NAME
> >>>>>>>>> .B mgmtdev
> >>>>>>>>> .I MGMTDEV
> >>>>>>>>> +.RI "[ device_features "
DEVICE_FEATURES " ]"
> >>>>>>>>> .RI "[ mac " MACADDR
" ]"
> >>>>>>>>> .RI "[ mtu " MTU
" ]"
> >>>>>>>>> .RI "[ max_vqp "
MAX_VQ_PAIRS " ]"
> >>>>>>>>> @@ -74,6 +75,15 @@ Name of the new
vdpa device to add.
> >>>>>>>>> Name of the management device to
use for device addition.
> >>>>>>>>>
> >>>>>>>>> .PP
> >>>>>>>>> +.BI device_features "
DEVICE_FEATURES"
> >>>>>>>>> +Specifies the virtio device features
bit-mask that is provisioned for the new vdpa device.
> >>>>>>>>> +
> >>>>>>>>> +The bits can be found under
include/uapi/linux/virtio*h.
> >>>>>>>>> +
> >>>>>>>>> +see macros such as VIRTIO_F_ and
VIRTIO_XXX(e.g NET)_F_ for specific bit values.
> >>>>>>>>> +
> >>>>>>>>> +This is optional.
> >>>>>>>> Document the behavior when this attribute
is missing? For e.g. inherit
> >>>>>>>> device features from parent device.
> >>>>>>> This is the current behaviour but unless
we've found a way to mandate
> >>>>>>> it, I'd like to not mention it. Maybe add
a description to say the
> >>>>>>> user needs to check the features after the add
if features are not
> >>>>>>> specified.
> >>>>>> Well, I think at least for live migration the mgmt
software should get
> >>>>>> to some consistent result between all vdpa parent
drivers regarding
> >>>>>> feature inheritance.
> >>>>> It would be hard. Especially for the device:
> >>>>>
> >>>>> 1) ask device_features from the device, in this case,
new features
> >>>>> could be advertised after e.g a firmware update
> >>>> The consistency I meant is to always inherit all device
features from
> >>>> the parent device for whatever it is capable of,
> >>> This looks fragile. How about the features that are mutually
> >>> exclusive? E.g FEATURE_X and FEATURE_Y that are both supported
by the
> >>> mgmt?
> >> Hmmm, in theory, yes, it's a bit cumbersome. Is this for
future proof,
> >> since so far as I see the virtio spec doesn't seem to define
features
> >> that are mutually exclusive, and the way how driver should respond
to
> >> mutually exclusive features in feature negotiation is completely
undefined?
> > My understanding is that if a driver accepts two mutually exclusive
> > features it should be a bug.
> It depends on the nature of the specific feature I guess. For e.g. there
> could be two versions of implementation for some device feature, which
> are mutually exclusive. The driver can well selectively ack one of the
> version it supports if seeing both present.
>
> >
> > But anyhow it's an example that it is not easy to have forward
> > compatibility if we mandating to inherit all features from the
> > management device.
>
> Yep, that I agree.
> >
> >>>> since that was the only
> >>>> reasonable behavior pre-dated the device_features
attribute, even though
> >>>> there's no mandatory check by the vdpa core. This way
it's
> >>>> self-descriptive and consistent for the mgmt software to
infer, as users
> >>>> can check into dev_features at the parent mgmtdev level to
know what
> >>>> features will be ended up with after 'vdpa dev
add'. I thought even
> >>>> though inheritance is not mandated as part of uAPI, it
should at least
> >>>> be mentioned as a recommended guide line (for drivers in
particular),
> >>>> especially this is the only reasonable behavior with
nowhere to check
> >>>> what features are ended up after add (i.e. for now we can
only set but
> >>>> not possible to read the exact device_features at vdpa dev
level, as yet).
> >>> I fully agree, but what I want to say is. Consider:
> >>>
> >>> 1) We've already had feature provisioning
> >>> 2) It would be hard or even impossible to mandate the semantic
> >>> (consistency) of the features inheritance.
> >>>
> >>> I'm fine with the doc, but the mgmt layer should not
depend on this
> >>> and they should use feature provisioning instead.
> >> OK, if it's for future proof to not mandate feature
inheritance I think
> >> I see the point.
> >>
> >>>>> 2) or have hierarchy architecture where several layers
were placed
> >>>>> between vDPA and the real hardware
> >>>> Not sure what it means but I don't get why extra
layers are needed. Do
> >>>> you mean extra layer to validate resulting features during
add? Why vdpa
> >>>> core is not the right place to do that?
> >>> Just want to go wild because we can't expect how many
layers are below vDPA.
> >>>
> >>> vDPA core is the right place but the validating should be done
during
> >>> feature provisioning since it's much more easier than
trying to
> >>> mandating code defined behaviour like inheritance.
> >> OK, thanks for the clarifications.
> >>
> >>>>>> This inheritance predates the exposure of device
> >>>>>> features, until which user can check into specific
features after
> >>>>>> creation. Imagine the case mgmt software of live
migration needs to work
> >>>>>> with older vdpa tool stack with no device_features
exposure, how does it
> >>>>>> know what device features are provisioned - it can
only tell it from
> >>>>>> dev_features shown at the parent mgmtdev level.
> >>>>> The behavior is totally defined by the code, it would
be not safe for
> >>>>> the mgmt layer to depend on. Instead, the mgmt layer
should use a
> >>>>> recent vdpa tool with feature provisioning interface
to guarantee the
> >>>>> device_features if it wants since it has a clear
semantic instead of
> >>>>> an implicit kernel behaviour which doesn't belong
to an uAPI.
> >>>> That is going to be a slightly harsh requirement. If
there's an existing
> >>>> vDPA setup already provisioned before the device_features
work, there is
> >>>> no way for it to live migrate even if the QEMU userspace
stack is made
> >>>> live migrate-able. It'd be the best to find some mild
alternative before
> >>>> claiming certain setup unmigrate-able.
> >>> It can still work in a passive way, mgmt layer check the
device
> >>> features and only allow the migration among the vDPA devices
that have
> >>> the same device_feature.
> >> Right, that is the scenario in concern which I'd like to get
support
> >> for, even though it's passive due to incompleteness in
previous CLI
> >> design (lack of individual device feature provisioning). Once the
tool
> >> is upgraded, vdpa features can be provisioned selectively on the
> >> destination node, matching those on the source.
> > This should work, but it probably requires the mgmt layer to collect
> > and compare features among the nodes.
> Yes. I know libvirt probably won't support this. But it would benefit
> other mgmt software implementation, where each node would have to record
> the initial config attributes in the first place. :)
>
> >
> >>> Less flexible than feature provisioning.
> >>>
> >>>>> If we can mandate the inheriting behaviour, users may
be surprised at
> >>>>> the features in the production environment which are
very hard to
> >>>>> debug.
> >>>> I'm not against an explicit uAPI to define and guard
device_features
> >>>> inheritance, but on the other hand, wouldn't it be
necessary to show the
> >>>> actual device_features at vdpa dev level if it's not
guaranteed to be
> >>>> the same with that of the parent mgmtdev?
> >>> I think this is already been done ,or anything I miss?
> >> The kernel patch is not merged yet, preventing the userspace patch
from
> >> being posted.
> > I may miss something, any potiner here?
> First the following rename patch has to get in to the kernel:
>
https://lore.kernel.org/virtualization/1665422823-18364-1-git-send-email-si-wei.liu
at oracle.com/
>
Michael, do you plan to merge this?
> then I can post the related iproute patch to include dev_features to the
> output of 'vdpa dev show'.
>
> This initial config series run independently, though the eventual goal
> is to get all of migration compatibility attributes packed in the same
> "initial_config" map.
>
>
https://lore.kernel.org/virtualization/1666392237-4042-1-git-send-email-si-wei.liu
at oracle.com/
Ok.
> >
> >> While the ideal situation is to allow query of
> >> device_features after adding a vdpa dev (for e.g. if not 100%
inherited
> >> from the parent mgmtdev), followed by allowing selectively
provision
> >> features individually.
> > Yes.
> >
> >>>> That is even needed before
> >>>> users are allowed to provision specific device_features
IMO...
> >>>>
> >>>> (that is the reason why I urged Michael to merge this
patch soon before
> >>>> 6.1 GA:
> >>>>
https://lore.kernel.org/virtualization/1665422823-18364-1-git-send-email-si-wei.liu
at oracle.com/,
> >>>> for which I have a pending iproute patch to expose
device_features at
> >>>> 'vdpa dev show' output).
> >>> Right.
> >>>
> >>>>>> IMHO it's not about whether vdpa core can or
should mandate it in a
> >>>>>> common place or not, it's that (the man page
of) the CLI tool should set
> >>>>>> user's expectation upfront for consumers (for
e.g. mgmt software). I.e.
> >>>>>> in case the parent driver doesn't follow the
man page doc, it should be
> >>>>>> considered as an implementation bug in the
individual driver rather than
> >>>>>> flexibility of its own.
> >>>>> So for the inheriting, it might be too late to do
that:
> >>>>>
> >>>>> 1) no facility to mandate the inheriting and even if
we had we can't
> >>>>> fix old kernels
> >>>> We don't need to fix any old kernel as all drivers
there had obeyed the
> >>>> inheriting rule since day 1. Or is there exception you did
see? If so we
> >>>> should treat it as a bug to fix in driver.
> >>> I'm not sure it's a bug consider a vDPA device have
only a subset
> >>> feature of what mgmt has.
> >> For example, F_MQ requires F_CTRL_VQ, but today this validation is
only
> >> done in individual driver. We should consider consolidating it to
the
> >> vdpa core.
> > This needs some balances, the core actually tries to be devince
> > agnostic (though it has some net specific code).
> Yes, this is already the case today. There has been various
> VIRTIO_ID_NET case switch'es in the vdpa.c code. I think if type
> specific validation code just limits itself to the netlink API
> interfacing layer rather than down to the driver API, it might just be
> okay (as that's already the case).
Yes.
>
> > One side effect is that it would be very hard for the core to catch up
> > with the spec development. With the current code, new features could
> > be added without the notice of the core.
> I thought at least the vdpa core can capture those validations already
> defined in the spec. For new development out of spec, driver can be a
> safe place to start.
That's fine, patches are more than welcomed.
Thanks
>
>
> Regards,
> -Siwei
>
> >
> >> But before that happens, if such validation is missing from
> >> driver, we should fix those in vendor drivers first.
> > Yes, that's the way. (E.g virtio-net driver has such validation)
> >
> >>>>> 2) no uAPI so there no entity to carry on the semantic
> >>>> Not against of introducing an explicit uAPI, but what it
may end up with
> >>>> is only some validation in a central place, right?
> >>> Well, this is what has been already done right now before the
feature
> >>> provisioning, the kernel for anyway needs to validate the
illegal
> >>> input from userspace.
> >> Right. What I meant is the kernel validation in vdpa_core should
be done
> >> anyway regardless of any new uAPI (for feature inheritance for
e.g). I
> >> guess we are in the same page here.
> > Great, I think so.
> >
> > Thanks
> >
> >> Thanks,
> >> -Siwei
> >>
> >>>> Why not do it now
> >>>> before adding device features provisioning to userspace.
Such that it's
> >>>> functionality complete and correct no matter if
device_features is
> >>>> specified or not.
> >>> So as discussed before, the kernel has already tried to do
validation,
> >>> if there's any bug, we can fix that. If you meant
userspace
> >>> validation, I'm not sure it is necessary:
> >>>
> >>> 1) kernel should do the validation
> >>> 2) hard to keep forward compatibility, e.g features supported
by the
> >>> mgmt device might not be even known by the userspace.
> >>>
> >>> Thanks
> >>>
> >>>> Thanks,
> >>>> -Siwei
> >>>>
> >>>>> And this is one of the goals that feature provisioning
tries to solve
> >>>>> so mgmt layer should use feature provisioning instead.
> >>>>>
> >>>>>>>> And what is the expected behavior when
feature bit mask is off but the
> >>>>>>>> corresponding config attr (for e.g. mac,
mtu, and max_vqp) is set?
> >>>>>>> It depends totally on the parent. And this
"issue" is not introduced
> >>>>>>> by this feature. Parents can decide to
provision MQ by itself even if
> >>>>>>> max_vqp is not specified.
> >>>>>> Sorry, maybe I wasn't clear enough. The case I
referred to was that the
> >>>>>> parent is capable of certain feature (for e.g.
_F_MQ), the associated
> >>>>>> config attr (for e.g. max_vqp) is already present
in the CLI, but the
> >>>>>> device_features bit mask doesn't have the
corresponding bit set (e.g.
> >>>>>> the _F_MQ bit). Are you saying that the failure of
this apparently
> >>>>>> invalid/ambiguous/conflicting command can't be
predicated and the
> >>>>>> resulting behavior is totally ruled by the parent
driver?
> >>>>> Ok, I get you. My understanding is that the kernel
should do the
> >>>>> validation at least, it should not trust any
configuration that is
> >>>>> sent from the userspace. This is how it works before
the device
> >>>>> provisioning. I think we can add some validation in
the kernel.
> >>>>>
> >>>>> Thanks
> >>>>>
> >>>>>> Thanks,
> >>>>>> -Siwei
> >>>>>>
> >>>>>>>> I think the previous behavior without
device_features is that any config
> >>>>>>>> attr implies the presence of the specific
corresponding feature (_F_MAC,
> >>>>>>>> _F_MTU, and _F_MQ). Should device_features
override the other config
> >>>>>>>> attribute, or such combination is
considered invalid thus should fail?
> >>>>>>> It follows the current policy, e.g if the
parent doesn't support
> >>>>>>> _F_MQ, we can neither provision _F_MQ nor
max_vqp.
> >>>>>>>
> >>>>>>> Thanks
> >>>>>>>
> >>>>>>>> Thanks,
> >>>>>>>> -Siwei
> >>>>>>>>
> >>>>>>>>> +
> >>>>>>>>> .BI mac " MACADDR"
> >>>>>>>>> - specifies the mac address for
the new vdpa device.
> >>>>>>>>> This is applicable only for the
network type of vdpa device. This is optional.
> >>>>>>>>> @@ -127,6 +137,11 @@ vdpa dev add name
foo mgmtdev vdpa_sim_net
> >>>>>>>>> Add the vdpa device named foo on
the management device vdpa_sim_net.
> >>>>>>>>> .RE
> >>>>>>>>> .PP
> >>>>>>>>> +vdpa dev add name foo mgmtdev
vdpa_sim_net device_features 0x300020000
> >>>>>>>>> +.RS 4
> >>>>>>>>> +Add the vdpa device named foo on the
management device vdpa_sim_net with device_features of 0x300020000
> >>>>>>>>> +.RE
> >>>>>>>>> +.PP
> >>>>>>>>> vdpa dev add name foo mgmtdev
vdpa_sim_net mac 00:11:22:33:44:55
> >>>>>>>>> .RS 4
> >>>>>>>>> Add the vdpa device named foo on
the management device vdpa_sim_net with mac address of 00:11:22:33:44:55.
> >>>>>>>>> diff --git
a/vdpa/include/uapi/linux/vdpa.h b/vdpa/include/uapi/linux/vdpa.h
> >>>>>>>>> index 94e4dad1..7c961991 100644
> >>>>>>>>> --- a/vdpa/include/uapi/linux/vdpa.h
> >>>>>>>>> +++ b/vdpa/include/uapi/linux/vdpa.h
> >>>>>>>>> @@ -51,6 +51,7 @@ enum vdpa_attr {
> >>>>>>>>> VDPA_ATTR_DEV_QUEUE_INDEX,
/* u32 */
> >>>>>>>>>
VDPA_ATTR_DEV_VENDOR_ATTR_NAME, /* string */
> >>>>>>>>>
VDPA_ATTR_DEV_VENDOR_ATTR_VALUE, /* u64 */
> >>>>>>>>> + VDPA_ATTR_DEV_FEATURES,
/* u64 */
> >>>>>>>>>
> >>>>>>>>> /* new attributes must be
added above here */
> >>>>>>>>> VDPA_ATTR_MAX,
> >>>>>>>>> diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
> >>>>>>>>> index b73e40b4..d0ce5e22 100644
> >>>>>>>>> --- a/vdpa/vdpa.c
> >>>>>>>>> +++ b/vdpa/vdpa.c
> >>>>>>>>> @@ -27,6 +27,7 @@
> >>>>>>>>> #define VDPA_OPT_VDEV_MTU
BIT(5)
> >>>>>>>>> #define VDPA_OPT_MAX_VQP
BIT(6)
> >>>>>>>>> #define VDPA_OPT_QUEUE_INDEX
BIT(7)
> >>>>>>>>> +#define VDPA_OPT_VDEV_FEATURES
BIT(8)
> >>>>>>>>>
> >>>>>>>>> struct vdpa_opts {
> >>>>>>>>> uint64_t present; /* flags
of present items */
> >>>>>>>>> @@ -38,6 +39,7 @@ struct vdpa_opts {
> >>>>>>>>> uint16_t mtu;
> >>>>>>>>> uint16_t max_vqp;
> >>>>>>>>> uint32_t queue_idx;
> >>>>>>>>> + uint64_t device_features;
> >>>>>>>>> };
> >>>>>>>>>
> >>>>>>>>> struct vdpa {
> >>>>>>>>> @@ -187,6 +189,17 @@ static int
vdpa_argv_u32(struct vdpa *vdpa, int argc, char **argv,
> >>>>>>>>> return get_u32(result,
*argv, 10);
> >>>>>>>>> }
> >>>>>>>>>
> >>>>>>>>> +static int vdpa_argv_u64_hex(struct
vdpa *vdpa, int argc, char **argv,
> >>>>>>>>> + uint64_t
*result)
> >>>>>>>>> +{
> >>>>>>>>> + if (argc <= 0 || !*argv) {
> >>>>>>>>> + fprintf(stderr,
"number expected\n");
> >>>>>>>>> + return -EINVAL;
> >>>>>>>>> + }
> >>>>>>>>> +
> >>>>>>>>> + return get_u64(result, *argv,
16);
> >>>>>>>>> +}
> >>>>>>>>> +
> >>>>>>>>> struct vdpa_args_metadata {
> >>>>>>>>> uint64_t o_flag;
> >>>>>>>>> const char *err_msg;
> >>>>>>>>> @@ -244,6 +257,10 @@ static void
vdpa_opts_put(struct nlmsghdr *nlh, struct vdpa *vdpa)
> >>>>>>>>>
mnl_attr_put_u16(nlh, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, opts->max_vqp);
> >>>>>>>>> if (opts->present &
VDPA_OPT_QUEUE_INDEX)
> >>>>>>>>>
mnl_attr_put_u32(nlh, VDPA_ATTR_DEV_QUEUE_INDEX, opts->queue_idx);
> >>>>>>>>> + if (opts->present &
VDPA_OPT_VDEV_FEATURES) {
> >>>>>>>>> + mnl_attr_put_u64(nlh,
VDPA_ATTR_DEV_FEATURES,
> >>>>>>>>> +
opts->device_features);
> >>>>>>>>> + }
> >>>>>>>>> }
> >>>>>>>>>
> >>>>>>>>> static int
vdpa_argv_parse(struct vdpa *vdpa, int argc, char **argv,
> >>>>>>>>> @@ -329,6 +346,14 @@ static int
vdpa_argv_parse(struct vdpa *vdpa, int argc, char **argv,
> >>>>>>>>>
> >>>>>>>>>
NEXT_ARG_FWD();
> >>>>>>>>> o_found |=
VDPA_OPT_QUEUE_INDEX;
> >>>>>>>>> + } else if
(!strcmp(*argv, "device_features") &&
> >>>>>>>>> + (o_optional
& VDPA_OPT_VDEV_FEATURES)) {
> >>>>>>>>> + NEXT_ARG_FWD();
> >>>>>>>>> + err =
vdpa_argv_u64_hex(vdpa, argc, argv,
> >>>>>>>>> +
&opts->device_features);
> >>>>>>>>> + if (err)
> >>>>>>>>> + return
err;
> >>>>>>>>> + o_found |=
VDPA_OPT_VDEV_FEATURES;
> >>>>>>>>> } else {
> >>>>>>>>>
fprintf(stderr, "Unknown option \"%s\"\n", *argv);
> >>>>>>>>> return
-EINVAL;
> >>>>>>>>> @@ -615,8 +640,9 @@ static int
cmd_mgmtdev(struct vdpa *vdpa, int argc, char **argv)
> >>>>>>>>> static void cmd_dev_help(void)
> >>>>>>>>> {
> >>>>>>>>> fprintf(stderr, "Usage:
vdpa dev show [ DEV ]\n");
> >>>>>>>>> - fprintf(stderr, "
vdpa dev add name NAME mgmtdev MANAGEMENTDEV [ mac MACADDR ] [ mtu MTU
]\n");
> >>>>>>>>> - fprintf(stderr, "
[ max_vqp MAX_VQ_PAIRS ]\n");
> >>>>>>>>> + fprintf(stderr, "
vdpa dev add name NAME mgmtdevMANAGEMENTDEV [ device_features
DEVICE_FEATURES]\n");
> >>>>>>>>> + fprintf(stderr, "
[ mac MACADDR ] [ mtu MTU ]\n");
> >>>>>>>>> + fprintf(stderr, "
[ max_vqp MAX_VQ_PAIRS ]\n");
> >>>>>>>>> fprintf(stderr, "
vdpa dev del DEV\n");
> >>>>>>>>> fprintf(stderr, "Usage:
vdpa dev config COMMAND [ OPTIONS ]\n");
> >>>>>>>>> fprintf(stderr, "Usage:
vdpa dev vstats COMMAND\n");
> >>>>>>>>> @@ -708,7 +734,7 @@ static int
cmd_dev_add(struct vdpa *vdpa, int argc, char **argv)
> >>>>>>>>> err =
vdpa_argv_parse_put(nlh, vdpa, argc, argv,
> >>>>>>>>>
VDPA_OPT_VDEV_MGMTDEV_HANDLE | VDPA_OPT_VDEV_NAME,
> >>>>>>>>>
VDPA_OPT_VDEV_MAC | VDPA_OPT_VDEV_MTU |
> >>>>>>>>> -
VDPA_OPT_MAX_VQP);
> >>>>>>>>> +
VDPA_OPT_MAX_VQP | VDPA_OPT_VDEV_FEATURES);
> >>>>>>>>> if (err)
> >>>>>>>>> return err;
> >>>>>>>>>
>