Hello,

On 8/26/24 9:07 PM, Jason Wang wrote:

On Tue, Aug 27, 2024 at 3:23 AM Carlos Bilbao [off-list ref] wrote:

quoted

Hello,

On 8/26/24 10:53 AM, Dragos Tatulea wrote:

quoted

On 26.08.24 16:26, Carlos Bilbao wrote:

quoted

Hello Dragos,

On 8/26/24 4:06 AM, Dragos Tatulea wrote:

quoted

On 23.08.24 18:54, Carlos Bilbao wrote:

quoted

Hello,

I'm debugging my vDPA setup, and when using ioctl to retrieve the
configuration, I noticed that it's running in half duplex mode:

Configuration data (24 bytes):
  MAC address: (Mac address)
  Status: 0x0001
  Max virtqueue pairs: 8
  MTU: 1500
  Speed: 0 Mb
  Duplex: Half Duplex
  RSS max key size: 0
  RSS max indirection table length: 0
  Supported hash types: 0x00000000

I believe this might be contributing to the underperformance of vDPA.

mlx5_vdpa vDPA devicess currently do not support the VIRTIO_NET_F_SPEED_DUPLEX
feature which reports speed and duplex. You can check the state on the
PF.

According to ethtool, all my devices are running at full duplex. I assume I
can disregard this configuration output from the module then.

Yep.

quoted

quoted

quoted

While looking into how to change this option for Mellanox, I read the following
kernel code in mlx5_vnet.c:

static void mlx5_vdpa_set_config(struct vdpa_device *vdev, unsigned int offset, const void *buf,
                 unsigned int len)
{
    /* not supported */
}

I was wondering why this is the case.

TBH, I don't know why it was not added. But in general, the control VQ is the
better way as it's dynamic.

quoted

Is there another way for me to change
these configuration settings?

The configuration is done using control VQ for most things (MTU, MAC, VQs,
etc). Make sure that you have the CTRL_VQ feature set (should be on by
default). It should appear in `vdpa mgmtdev show` and `vdpa dev config
show`.

I see that CTRL_VQ is indeed enabled. Is there any documentation on how to
use the control VQ to get/set vDPA configuration values?

You are most likely using it already through through qemu. You can check
if the CTR_VQ feature also shows up in the output of `vdpa dev config show`.

What values are you trying to configure btw?

Yes, CTRL_VQ also shows up in vdpa dev config show. There isn't a specific
value I want to configure ATM, but my vDPA isn't performing as expected, so
I'm investigating potential issues. Below is the code I used to retrieve
the configuration from the driver; I'd be happy to send it as a patch if
you or someone else reviews it.

quoted

Thanks,
Dragos

Thanks,
Carlos

---

From ab6ea66c926eaf1e95eb5d73bc23183e0021ee27 Mon Sep 17 00:00:00 2001
From: Carlos Bilbao <redacted>
Date: Sat, 24 Aug 2024 00:24:56 +0000
Subject: [PATCH] mlx5: Add support to update the vDPA configuration

This is needed for VHOST_VDPA_SET_CONFIG.

Signed-off-by: Carlos Bilbao <redacted>
---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index b56aae3f7be3..da31c743b2b9 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c

@@ -2909,14 +2909,32 @@ static void mlx5_vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
     struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
     struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);

-    if (offset + len <= sizeof(struct virtio_net_config))
+    if (offset + len <= sizeof(struct virtio_net_config)) {
         memcpy(buf, (u8 *)&ndev->config + offset, len);
+        }
+        else
+        {
+            printk(KERN_ERR "%s: Offset and length out of bounds\n",
+            __func__);
+        }
+
 }

 static void mlx5_vdpa_set_config(struct vdpa_device *vdev, unsigned int offset, const void *buf,
                  unsigned int len)
 {
-    /* not supported */
+    struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
+    struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
+
+    if (offset + len <= sizeof(struct virtio_net_config))
+    {
+        memcpy((u8 *)&ndev->config + offset, buf, len);
+    }
+    else
+    {
+        printk(KERN_ERR "%s: Offset and length out of bounds\n",
+        __func__);
+    }
 }

This should follow the virtio-spec, for modern virtio-net devices,
most of the fields are read only.

From mlx5_vnet.c function mlx5v_probe:

mgtdev->mgtdev.config_attr_mask = BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MACADDR) |
                    BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP) |
                    BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MTU) |
                    BIT_ULL(VDPA_ATTR_DEV_FEATURES);

Does this mean these are the fields that set_config can update? I'm a bit
confused because, according to the virtio spec, I thought only speed and
duplex were not read-only -- but I was also told updating them isn't
supported by vDPA devices.

Thanks

quoted

 static u32 mlx5_vdpa_get_generation(struct vdpa_device *vdev)
--
2.34.1

Thanks, Carlos