2022-10-22 00:04:32

by Si-Wei Liu

[permalink] [raw]
Subject: [PATCH v3 0/4] vDPA: initial config export via "vdpa dev show"

Live migration of vdpa would typically require re-instate vdpa
device with an idential set of configs on the destination node,
same way as how source node created the device in the first place.

In order to allow live migration orchestration software to export the
initial set of vdpa attributes with which the device was created, it
will be useful if the vdpa tool can report the config on demand with
simple query. This will ease the orchestration software implementation
so that it doesn't have to keep track of vdpa config change, or have
to persist vdpa attributes across failure and recovery, in fear of
being killed due to accidental software error.

In this series, the initial device config for vdpa creation will be
exported via the "vdpa dev show" command. This is unlike the "vdpa
dev config show" command that usually goes with the live value in
the device config space, and is not reliable subject to the dynamics
of feature negotiation or possible change by driver to the config
space.

Examples:

1) Create vDPA by default without any config attribute

$ vdpa dev add mgmtdev pci/0000:41:04.2 name vdpa0
$ vdpa dev show vdpa0
vdpa0: type network mgmtdev pci/0000:41:04.2 vendor_id 5555 max_vqs 9 max_vq_size 256
$ vdpa dev -jp show vdpa0
{
"dev": {
"vdpa0": {
"type": "network",
"mgmtdev": "pci/0000:41:04.2",
"vendor_id": 5555,
"max_vqs": 9,
"max_vq_size": 256,
}
}
}

2) Create vDPA with config attribute(s) specified

$ vdpa dev add mgmtdev pci/0000:41:04.2 name vdpa0 \
mac e4:11:c6:d3:45:f0 max_vq_pairs 4
$ vdpa dev show
vdpa0: type network mgmtdev pci/0000:41:04.2 vendor_id 5555 max_vqs 9 max_vq_size 256
initial_config: mac e4:11:c6:d3:45:f0 max_vq_pairs 4
$ vdpa dev -jp show
{
"dev": {
"vdpa0": {
"type": "network",
"mgmtdev": "pci/0000:41:04.2",
"vendor_id": 5555,
"max_vqs": 9,
"max_vq_size": 256,
"initial_config": {
"mac": "e4:11:c6:d3:45:f0",
"max_vq_pairs": 4
}
}
}
}

---
v2 -> v3:
- Rename vdev_cfg to init_cfg and also related function (Jason)
- Change "virtio_config" to "initial_config" in command example
output (Parav)

v1 -> v2:
- Revised example output to export all config attributes under a
json object (Jason)

---

Si-Wei Liu (4):
vdpa: save vdpa_dev_set_config in struct vdpa_device
vdpa: pass initial config to _vdpa_register_device()
vdpa: show dev config as-is in "vdpa dev show" output
vdpa: fix improper error message when adding vdpa dev

drivers/vdpa/ifcvf/ifcvf_main.c | 2 +-
drivers/vdpa/mlx5/net/mlx5_vnet.c | 2 +-
drivers/vdpa/vdpa.c | 63 +++++++++++++++++++++++++++++++++---
drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 2 +-
drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 2 +-
drivers/vdpa/vdpa_user/vduse_dev.c | 2 +-
drivers/vdpa/virtio_pci/vp_vdpa.c | 3 +-
include/linux/vdpa.h | 26 ++++++++-------
8 files changed, 80 insertions(+), 22 deletions(-)

--
1.8.3.1


2022-10-22 00:33:30

by Si-Wei Liu

[permalink] [raw]
Subject: [PATCH v3 4/4] vdpa: fix improper error message when adding vdpa dev

In below example, before the fix, mtu attribute is supported
by the parent mgmtdev, but the error message showing "All
provided are not supported" is just misleading.

$ vdpa mgmtdev show
vdpasim_net:
supported_classes net
max_supported_vqs 3
dev_features MTU MAC CTRL_VQ CTRL_MAC_ADDR ANY_LAYOUT VERSION_1 ACCESS_PLATFORM

$ vdpa dev add mgmtdev vdpasim_net name vdpasim0 mtu 5000 max_vqp 2
Error: vdpa: All provided attributes are not supported.
kernel answers: Operation not supported

After fix, the relevant error message will be like:

$ vdpa dev add mgmtdev vdpasim_net name vdpasim0 mtu 5000 max_vqp 2
Error: vdpa: Some provided attributes are not supported.
kernel answers: Operation not supported

$ vdpa dev add mgmtdev vdpasim_net name vdpasim0 max_vqp 2
Error: vdpa: All provided attributes are not supported.
kernel answers: Operation not supported

Signed-off-by: Si-Wei Liu <[email protected]>
---
drivers/vdpa/vdpa.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index bfb8f54..2638565 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -629,13 +629,20 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct genl_info *i
err = PTR_ERR(mdev);
goto err;
}
- if ((config.mask & mdev->config_attr_mask) != config.mask) {
+ if (config.mask && (config.mask & mdev->config_attr_mask) == 0) {
NL_SET_ERR_MSG_MOD(info->extack,
"All provided attributes are not supported");
err = -EOPNOTSUPP;
goto err;
}

+ if ((config.mask & mdev->config_attr_mask) != config.mask) {
+ NL_SET_ERR_MSG_MOD(info->extack,
+ "Some provided attributes are not supported");
+ err = -EOPNOTSUPP;
+ goto err;
+ }
+
err = mdev->ops->dev_add(mdev, name, &config);
err:
up_write(&vdpa_dev_lock);
--
1.8.3.1

2022-10-24 09:03:35

by Jason Wang

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] vdpa: fix improper error message when adding vdpa dev

On Sat, Oct 22, 2022 at 7:49 AM Si-Wei Liu <[email protected]> wrote:
>
> In below example, before the fix, mtu attribute is supported
> by the parent mgmtdev, but the error message showing "All
> provided are not supported" is just misleading.
>
> $ vdpa mgmtdev show
> vdpasim_net:
> supported_classes net
> max_supported_vqs 3
> dev_features MTU MAC CTRL_VQ CTRL_MAC_ADDR ANY_LAYOUT VERSION_1 ACCESS_PLATFORM
>
> $ vdpa dev add mgmtdev vdpasim_net name vdpasim0 mtu 5000 max_vqp 2
> Error: vdpa: All provided attributes are not supported.
> kernel answers: Operation not supported
>
> After fix, the relevant error message will be like:
>
> $ vdpa dev add mgmtdev vdpasim_net name vdpasim0 mtu 5000 max_vqp 2
> Error: vdpa: Some provided attributes are not supported.
> kernel answers: Operation not supported
>
> $ vdpa dev add mgmtdev vdpasim_net name vdpasim0 max_vqp 2
> Error: vdpa: All provided attributes are not supported.
> kernel answers: Operation not supported
>
> Signed-off-by: Si-Wei Liu <[email protected]>

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

> ---
> drivers/vdpa/vdpa.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index bfb8f54..2638565 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -629,13 +629,20 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct genl_info *i
> err = PTR_ERR(mdev);
> goto err;
> }
> - if ((config.mask & mdev->config_attr_mask) != config.mask) {
> + if (config.mask && (config.mask & mdev->config_attr_mask) == 0) {
> NL_SET_ERR_MSG_MOD(info->extack,
> "All provided attributes are not supported");
> err = -EOPNOTSUPP;
> goto err;
> }
>
> + if ((config.mask & mdev->config_attr_mask) != config.mask) {
> + NL_SET_ERR_MSG_MOD(info->extack,
> + "Some provided attributes are not supported");
> + err = -EOPNOTSUPP;
> + goto err;
> + }
> +
> err = mdev->ops->dev_add(mdev, name, &config);
> err:
> up_write(&vdpa_dev_lock);
> --
> 1.8.3.1
>

2023-01-27 08:17:01

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] vDPA: initial config export via "vdpa dev show"

Did you say you are going to post v4 of this?
I'm dropping this from review for now.

--
MST


2023-01-30 21:06:17

by Si-Wei Liu

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] vDPA: initial config export via "vdpa dev show"

Apologies, I was over booked for multiple things in parallel, and there
had been urgent internal priorities popped up at times for the past few
weeks. On the other hand, there were brokenness or incompleteness
identified around features provisioning while this series was being
developed, which makes it grow a bit larger than v3. If you are eager to
see patches posted I can split off the series. I will send out a
prerequisite patchset for this series shortly.

Thanks for your patience,
-Siwei

On 1/27/2023 12:16 AM, Michael S. Tsirkin wrote:
> Did you say you are going to post v4 of this?
> I'm dropping this from review for now.
>


2023-01-30 21:59:24

by Si-Wei Liu

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] vDPA: initial config export via "vdpa dev show"



On 1/30/2023 1:05 PM, Si-Wei Liu wrote:
> Apologies, I was over booked for multiple things in parallel, and
> there had been urgent internal priorities popped up at times for the
> past few weeks. On the other hand, there were brokenness or
> incompleteness identified around features provisioning while this
> series was being developed, which makes it grow a bit larger than v3.
> If you are eager to see patches posted I can split off the series. I
> will send out a prerequisite patchset for this series shortly.
Patches sent. Please review:

https://lore.kernel.org/virtualization/[email protected]/T/#t


>
> Thanks for your patience,
> -Siwei
>
> On 1/27/2023 12:16 AM, Michael S. Tsirkin wrote:
>> Did you say you are going to post v4 of this?
>> I'm dropping this from review for now.
>>
>
-Siwei