From: Zhu, Lingshan <redacted>
Sent: Friday, July 8, 2022 2:21 AM


On 7/2/2022 6:07 AM, Parav Pandit wrote:

quoted

quoted

From: Zhu Lingshan <redacted>
Sent: Friday, July 1, 2022 9:28 AM
If VIRTIO_NET_F_MQ == 0, the virtio device should have one queue
pair, so when userspace querying queue pair numbers, it should return
mq=1 than zero.

Function vdpa_dev_net_config_fill() fills the attributions of the
vDPA devices, so that it should call vdpa_dev_net_mq_config_fill() so
the parameter in vdpa_dev_net_mq_config_fill() should be
feature_device than feature_driver for the vDPA devices themselves

Before this change, when MQ = 0, iproute2 output:
$vdpa dev config show vdpa0
vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs
0 mtu 1500

The fix belongs to user space.
When a feature bit _MQ is not negotiated, vdpa kernel space will not add

attribute VDPA_ATTR_DEV_NET_CFG_MAX_VQP.

quoted

When such attribute is not returned by kernel, max_vq_pairs should not

be shown by the iproute2.
I think userspace tool does not need to care whether MQ is offered or
negotiated, it just needs to read the number of queues there, so if no MQ, it
is not "not any queues", there are still 1 queue pair to be a virtio-net device,
means two queues.

If not, how can you tell the user there are only 2 queues? The end users may
don't know this is default. They may misunderstand this as an error or
defects.

quoted

When max_vq_pairs is not shown, it means that device didn’t expose MAX_VQ_PAIRS attribute to its guest users.
(Because _MQ was not negotiated).
It is not error or defect. 
It precisely shows what is exposed.

User space will care when it wants to turn off/on _MQ feature bits and MAX_QP values.

Showing max_vq_pairs of 1 even when _MQ is not negotiated, incorrectly says that max_vq_pairs is exposed to the guest, but it is not offered.

So, please fix the iproute2 to not print max_vq_pairs when it is not returned by the kernel.

quoted

We have many config space fields that depend on the feature bits and

some of them do not have any defaults.

quoted

To keep consistency of existence of config space fields among all, we don't

want to show default like below.

quoted

Please fix the iproute2 to not print max_vq_pairs when it is not returned

by the kernel.

quoted

quoted

After applying this commit, when MQ = 0, iproute2 output:
$vdpa dev config show vdpa0
vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs
1 mtu 1500

Fixes: a64917bc2e9b (vdpa: Provide interface to read driver features)
Signed-off-by: Zhu Lingshan <redacted>
---
  drivers/vdpa/vdpa.c | 7 ++++---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index

d76b22b2f7ae..846dd37f3549 100644

--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c

@@ -806,9 +806,10 @@ static int vdpa_dev_net_mq_config_fill(struct

vdpa_device *vdev,
  	u16 val_u16;

  	if ((features & BIT_ULL(VIRTIO_NET_F_MQ)) == 0)
-		return 0;
+		val_u16 = 1;
+	else
+		val_u16 = __virtio16_to_cpu(true, config-

quoted

max_virtqueue_pairs);

-	val_u16 = le16_to_cpu(config->max_virtqueue_pairs);
  	return nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP,

val_u16);

quoted

quoted

}

@@ -842,7 +843,7 @@ static int vdpa_dev_net_config_fill(struct

vdpa_device *vdev, struct sk_buff *ms
  			      VDPA_ATTR_PAD))
  		return -EMSGSIZE;

-	return vdpa_dev_net_mq_config_fill(vdev, msg, features_driver,
&config);
+	return vdpa_dev_net_mq_config_fill(vdev, msg, features_device,
+&config);
  }

  static int
--
2.31.1