Messages in this thread | | | Date | Tue, 31 Jan 2023 14:51:03 -0800 | Subject | Re: [PATCH 1/6] vdpa: fix improper error message when adding vdpa dev | From | Si-Wei Liu <> |
| |
On 1/31/2023 3:42 AM, Eli Cohen wrote: > > On 30/01/2023 22:30, Si-Wei Liu 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 <si-wei.liu@oracle.com> >> Acked-by: Jason Wang <jasowang@redhat.com> >> --- >> 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 8ef7aa1..5e57935 100644 >> --- a/drivers/vdpa/vdpa.c >> +++ b/drivers/vdpa/vdpa.c >> @@ -622,13 +622,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) { > If config.mask is zero than the condition is false even without the > change so I don't see why is this change needed. This way it spews error message when none of the attributes provided by user is supported by device. I can remove the check if improving error message for the other check right below, but that is the expected conditional if the same error message has to be kept.
>> 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"); > Changing the message is needed but maybe list the attributes that were > specified but are not supported? If you can live with numeric representing the attributes instead of straight attribute string, certainly I can do that. Or else I anticipate userspace to query and fail the command rather than present attribute strings in kernel.
-Siwei
>> + err = -EOPNOTSUPP; >> + goto err; >> + } >> + >> err = mdev->ops->dev_add(mdev, name, &config); >> err: >> up_write(&vdpa_dev_lock);
| |