On 2/5/2023 12:26 AM, Eli Cohen wrote:
On 03/02/2023 7:01, Si-Wei Liu wrote:
The spec says:
status only exists if VIRTIO_NET_F_STATUS is set
Similar to MAC and MTU, vdpa_dev_net_config_fill() should read
STATUS conditionally depending on the feature bits.
Signed-off-by: Si-Wei Liu <si-wei.liu@xxxxxxxxxx>
Reviewed-by: Parav Pandit <parav@xxxxxxxxxx>
---
drivers/vdpa/vdpa.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 3a82ca78..21c8aa3 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -843,18 +843,25 @@ static int vdpa_dev_net_mac_config_fill(struct
sk_buff *msg, u64 features,
sizeof(config->mac), config->mac);
}
+static int vdpa_dev_net_status_config_fill(struct sk_buff *msg,
u64 features,
+ const struct virtio_net_config *config)
+{
+ u16 val_u16;
+
+ if ((features & BIT_ULL(VIRTIO_NET_F_STATUS)) == 0)
+ return 0;
Instead of returning 0 here, it would be better to explicitly put 0 in
the message field for
VDPA_ATTR_DEV_NET_STATUS
In light of commit 41a2ad927aa2 ("vDPA: conditionally read MTU and MAC
in dev cfg space"), the userspace must now show the config space field
presented by the device *as-is*. If the feature bit is not offered by
device, the relevant field will not be displayed in 'vdpa dev config'
output. For instance, MAC address won't be shown if the MAC feature is
not supported/offered by the device (note this has nothing to do with
negotiated features), even though the vdpa parent may have a non-zero
MAC address of its own. I think STATUS should not be different from MAC
and MTU here.
Regards,
-Siwei
+
+ val_u16 = __virtio16_to_cpu(true, config->status);
+ return nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16);
+}
+
static int vdpa_dev_net_config_fill(struct vdpa_device *vdev,
struct sk_buff *msg)
{
struct virtio_net_config config = {};
u64 features_device;
- u16 val_u16;
vdev->config->get_config(vdev, 0, &config, sizeof(config));
- val_u16 = __virtio16_to_cpu(true, config.status);
- if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16))
- return -EMSGSIZE;
-
features_device = vdev->config->get_device_features(vdev);
if (nla_put_u64_64bit(msg, VDPA_ATTR_DEV_FEATURES,
features_device,
@@ -867,6 +874,9 @@ static int vdpa_dev_net_config_fill(struct
vdpa_device *vdev, struct sk_buff *ms
if (vdpa_dev_net_mac_config_fill(msg, features_device, &config))
return -EMSGSIZE;
+ if (vdpa_dev_net_status_config_fill(msg, features_device,
&config))
+ return -EMSGSIZE;
+
return vdpa_dev_net_mq_config_fill(msg, features_device, &config);
}
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
