Re: [PATCH 1/6] vdpa: fix improper error message when adding vdpa dev

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxxx>
Acked-by: Jason Wang <jasowang@xxxxxxxxxx>
---
  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);

_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization




[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux