[PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically

[PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically

[chenh]

From: Hao Chen <chenh@yusur.tech>

When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
start the virtual machine through libvirt or qemu, but now, the libvirt or
qemu can call dpdk vdpa vendor driver's ops .get_config through vhost_net_get_config
to get the mac address of the vdpa hardware without manual configuration.

v1->v2:
Only copy ETH_ALEN data of netcfg for some vdpa device such as
NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.
We only need the mac address and don't care about the status field.

Signed-off-by: Hao Chen <chenh@yusur.tech>
---
 hw/block/vhost-user-blk.c |  1 -
 hw/net/virtio-net.c       |  7 +++++++
 hw/virtio/vhost-user.c    | 19 -------------------
 3 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 9117222456..5dca4eab09 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
 
     vhost_dev_set_config_notifier(&s->dev, &blk_ops);
 
-    s->vhost_user.supports_config = true;
     ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0,
                          errp);
     if (ret < 0) {
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index dd0d056fde..90405083b1 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -166,6 +166,13 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
             }
             memcpy(config, &netcfg, n->config_size);
         }
+    } else if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
+        ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
+                                   n->config_size);
+        if (ret != -1) {
+		/* Automatically obtain the mac address of the vdpa device
+		 * when using the dpdk vdpa */
+                memcpy(config, &netcfg, ETH_ALEN);
     }
 }
 
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index bd24741be8..8b01078249 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
     }
 
     if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
-        bool supports_f_config = vus->supports_config ||
-            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
         uint64_t protocol_features;
 
         dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
@@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
          */
         protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
 
-        if (supports_f_config) {
-            if (!virtio_has_feature(protocol_features,
-                                    VHOST_USER_PROTOCOL_F_CONFIG)) {
-                error_setg(errp, "vhost-user device expecting "
-                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
-                           "not support it.");
-                return -EPROTO;
-            }
-        } else {
-            if (virtio_has_feature(protocol_features,
-                                   VHOST_USER_PROTOCOL_F_CONFIG)) {
-                warn_reportf_err(*errp, "vhost-user backend supports "
-                                 "VHOST_USER_PROTOCOL_F_CONFIG but QEMU does not.");
-                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
-            }
-        }
-
         /* final set of protocol features */
         dev->protocol_features = protocol_features;
         err = vhost_user_set_protocol_features(dev, dev->protocol_features);
-- 
2.27.0

Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically

[Raphael Norwitz]

[-- Attachment #1: Type: text/plain, Size: 4631 bytes --]

If I read your response on the other thread correctly, this change is intended

to prioritize the MAC address exposed by DPDK over the one provided by the

QEMU command line? Sounds reasonable in principle, but I would get confirmation

from vDPA/vhost-net maintainers.



That said the way you’re hacking the vhost-user code breaks a valuable check for

bad vhost-user-blk backends. I would suggest finding another implementation which

does not regress functionality for other device types.





>From: Hao Chen <chenh@yusur.tech>

>

>When use dpdk-vdpa tests vdpa device. You need to specify the mac address to

>start the virtual machine through libvirt or qemu, but now, the libvirt or

>qemu can call dpdk vdpa vendor driver's ops .get_config through vhost_net_get_config

>to get the mac address of the vdpa hardware without manual configuration.

>

>v1->v2:

>Only copy ETH_ALEN data of netcfg for some vdpa device such as

>NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.

>We only need the mac address and don't care about the status field.

>

>Signed-off-by: Hao Chen <chenh@yusur.tech>

>---

> hw/block/vhost-user-blk.c |  1 -

> hw/net/virtio-net.c       |  7 +++++++

> hw/virtio/vhost-user.c    | 19 -------------------

> 3 files changed, 7 insertions(+), 20 deletions(-)

>

>diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c

>index 9117222456..5dca4eab09 100644

>--- a/hw/block/vhost-user-blk.c

>+++ b/hw/block/vhost-user-blk.c

>@@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)

>

>     vhost_dev_set_config_notifier(&s->dev, &blk_ops);

>

>-    s->vhost_user.supports_config = true;

>     ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0,

>                          errp);

>     if (ret < 0) {

>diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c

>index dd0d056fde..90405083b1 100644

>--- a/hw/net/virtio-net.c

>+++ b/hw/net/virtio-net.c

>@@ -166,6 +166,13 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)

>             }

>             memcpy(config, &netcfg, n->config_size);

>         }

>+    } else if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {

>+        ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,

>+                                   n->config_size);

>+        if (ret != -1) {

>+               /* Automatically obtain the mac address of the vdpa device

>+                * when using the dpdk vdpa */

>+                memcpy(config, &netcfg, ETH_ALEN);

>     }

> }

>

>diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c

>index bd24741be8..8b01078249 100644

>--- a/hw/virtio/vhost-user.c

>+++ b/hw/virtio/vhost-user.c

>@@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,

>     }

>

>     if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {

>-        bool supports_f_config = vus->supports_config ||

>-            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);

>         uint64_t protocol_features;

>

>         dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;

>@@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,

>          */

>         protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;

>

>-        if (supports_f_config) {

>-            if (!virtio_has_feature(protocol_features,

>-                                    VHOST_USER_PROTOCOL_F_CONFIG)) {

>-                error_setg(errp, "vhost-user device expecting "

>-                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "

>-                           "not support it.");

>-                return -EPROTO;

>-            }

>-        } else {

>-            if (virtio_has_feature(protocol_features,

>-                                   VHOST_USER_PROTOCOL_F_CONFIG)) {

>-                warn_reportf_err(*errp, "vhost-user backend supports "

>-                                 "VHOST_USER_PROTOCOL_F_CONFIG but QEMU does not.");

>-                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);

>-            }

>-        }

>-

>         /* final set of protocol features */

>         dev->protocol_features = protocol_features;

>         err = vhost_user_set_protocol_features(dev, dev->protocol_features);

>--

>2.27.0

>


[-- Attachment #2: Type: text/html, Size: 16735 bytes --]

Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically

[Jason Wang]

On Thu, Sep 22, 2022 at 1:58 AM Raphael Norwitz
<raphael.norwitz@nutanix.com> wrote:
>
> If I read your response on the other thread correctly, this change is intended
>
> to prioritize the MAC address exposed by DPDK over the one provided by the
>
> QEMU command line? Sounds reasonable in principle, but I would get confirmation
>
> from vDPA/vhost-net maintainers.

I think the best way is to (and it seems easier)

1) have the management layer to make sure the mac came from cli
matches the underlayer vDPA
2) having a sanity check and fail the device initialization if they don't match

Thanks

>
>
>
> That said the way you’re hacking the vhost-user code breaks a valuable check for
>
> bad vhost-user-blk backends. I would suggest finding another implementation which
>
> does not regress functionality for other device types.
>
>
>
>
>
> >From: Hao Chen <chenh@yusur.tech>
>
> >
>
> >When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
>
> >start the virtual machine through libvirt or qemu, but now, the libvirt or
>
> >qemu can call dpdk vdpa vendor driver's ops .get_config through vhost_net_get_config
>
> >to get the mac address of the vdpa hardware without manual configuration.
>
> >
>
> >v1->v2:
>
> >Only copy ETH_ALEN data of netcfg for some vdpa device such as
>
> >NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.
>
> >We only need the mac address and don't care about the status field.
>
> >
>
> >Signed-off-by: Hao Chen <chenh@yusur.tech>
>
> >---
>
> > hw/block/vhost-user-blk.c |  1 -
>
> > hw/net/virtio-net.c       |  7 +++++++
>
> > hw/virtio/vhost-user.c    | 19 -------------------
>
> > 3 files changed, 7 insertions(+), 20 deletions(-)
>
> >
>
> >diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
>
> >index 9117222456..5dca4eab09 100644
>
> >--- a/hw/block/vhost-user-blk.c
>
> >+++ b/hw/block/vhost-user-blk.c
>
> >@@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
>
> >
>
> >     vhost_dev_set_config_notifier(&s->dev, &blk_ops);
>
> >
>
> >-    s->vhost_user.supports_config = true;
>
> >     ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0,
>
> >                          errp);
>
> >     if (ret < 0) {
>
> >diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>
> >index dd0d056fde..90405083b1 100644
>
> >--- a/hw/net/virtio-net.c
>
> >+++ b/hw/net/virtio-net.c
>
> >@@ -166,6 +166,13 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
>
> >             }
>
> >             memcpy(config, &netcfg, n->config_size);
>
> >         }
>
> >+    } else if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
>
> >+        ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
>
> >+                                   n->config_size);
>
> >+        if (ret != -1) {
>
> >+               /* Automatically obtain the mac address of the vdpa device
>
> >+                * when using the dpdk vdpa */
>
> >+                memcpy(config, &netcfg, ETH_ALEN);
>
> >     }
>
> > }
>
> >
>
> >diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>
> >index bd24741be8..8b01078249 100644
>
> >--- a/hw/virtio/vhost-user.c
>
> >+++ b/hw/virtio/vhost-user.c
>
> >@@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>
> >     }
>
> >
>
> >     if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
>
> >-        bool supports_f_config = vus->supports_config ||
>
> >-            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
>
> >         uint64_t protocol_features;
>
> >
>
> >         dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
>
> >@@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>
> >          */
>
> >         protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
>
> >
>
> >-        if (supports_f_config) {
>
> >-            if (!virtio_has_feature(protocol_features,
>
> >-                                    VHOST_USER_PROTOCOL_F_CONFIG)) {
>
> >-                error_setg(errp, "vhost-user device expecting "
>
> >-                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
>
> >-                           "not support it.");
>
> >-                return -EPROTO;
>
> >-            }
>
> >-        } else {
>
> >-            if (virtio_has_feature(protocol_features,
>
> >-                                   VHOST_USER_PROTOCOL_F_CONFIG)) {
>
> >-                warn_reportf_err(*errp, "vhost-user backend supports "
>
> >-                                 "VHOST_USER_PROTOCOL_F_CONFIG but QEMU does not.");
>
> >-                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
>
> >-            }
>
> >-        }
>
> >-
>
> >         /* final set of protocol features */
>
> >         dev->protocol_features = protocol_features;
>
> >         err = vhost_user_set_protocol_features(dev, dev->protocol_features);
>
> >--
>
> >2.27.0
>
> >
>
>

Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically

[Michael S. Tsirkin]

On Thu, Sep 22, 2022 at 09:34:41AM +0800, Jason Wang wrote:
> On Thu, Sep 22, 2022 at 1:58 AM Raphael Norwitz
> <raphael.norwitz@nutanix.com> wrote:
> >
> > If I read your response on the other thread correctly, this change is intended
> >
> > to prioritize the MAC address exposed by DPDK over the one provided by the
> >
> > QEMU command line? Sounds reasonable in principle, but I would get confirmation
> >
> > from vDPA/vhost-net maintainers.
> 
> I think the best way is to (and it seems easier)
> 
> 1) have the management layer to make sure the mac came from cli
> matches the underlayer vDPA
> 2) having a sanity check and fail the device initialization if they don't match
> 
> Thanks

I think I agree, we should not overwrite user supplied arguments.
I think the case where we might want to get the mac from VDPA
and use it is when it's been assigned randomly as opposed to coming from cli.


> >
> >
> >
> > That said the way you’re hacking the vhost-user code breaks a valuable check for
> >
> > bad vhost-user-blk backends. I would suggest finding another implementation which
> >
> > does not regress functionality for other device types.
> >
> >
> >
> >
> >
> > >From: Hao Chen <chenh@yusur.tech>
> >
> > >
> >
> > >When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
> >
> > >start the virtual machine through libvirt or qemu, but now, the libvirt or
> >
> > >qemu can call dpdk vdpa vendor driver's ops .get_config through vhost_net_get_config
> >
> > >to get the mac address of the vdpa hardware without manual configuration.
> >
> > >
> >
> > >v1->v2:
> >
> > >Only copy ETH_ALEN data of netcfg for some vdpa device such as
> >
> > >NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.
> >
> > >We only need the mac address and don't care about the status field.
> >
> > >
> >
> > >Signed-off-by: Hao Chen <chenh@yusur.tech>
> >
> > >---
> >
> > > hw/block/vhost-user-blk.c |  1 -
> >
> > > hw/net/virtio-net.c       |  7 +++++++
> >
> > > hw/virtio/vhost-user.c    | 19 -------------------
> >
> > > 3 files changed, 7 insertions(+), 20 deletions(-)
> >
> > >
> >
> > >diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> >
> > >index 9117222456..5dca4eab09 100644
> >
> > >--- a/hw/block/vhost-user-blk.c
> >
> > >+++ b/hw/block/vhost-user-blk.c
> >
> > >@@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
> >
> > >
> >
> > >     vhost_dev_set_config_notifier(&s->dev, &blk_ops);
> >
> > >
> >
> > >-    s->vhost_user.supports_config = true;
> >
> > >     ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0,
> >
> > >                          errp);
> >
> > >     if (ret < 0) {
> >
> > >diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >
> > >index dd0d056fde..90405083b1 100644
> >
> > >--- a/hw/net/virtio-net.c
> >
> > >+++ b/hw/net/virtio-net.c
> >
> > >@@ -166,6 +166,13 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
> >
> > >             }
> >
> > >             memcpy(config, &netcfg, n->config_size);
> >
> > >         }
> >
> > >+    } else if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> >
> > >+        ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
> >
> > >+                                   n->config_size);
> >
> > >+        if (ret != -1) {
> >
> > >+               /* Automatically obtain the mac address of the vdpa device
> >
> > >+                * when using the dpdk vdpa */
> >
> > >+                memcpy(config, &netcfg, ETH_ALEN);
> >
> > >     }
> >
> > > }
> >
> > >
> >
> > >diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> >
> > >index bd24741be8..8b01078249 100644
> >
> > >--- a/hw/virtio/vhost-user.c
> >
> > >+++ b/hw/virtio/vhost-user.c
> >
> > >@@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> >
> > >     }
> >
> > >
> >
> > >     if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
> >
> > >-        bool supports_f_config = vus->supports_config ||
> >
> > >-            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
> >
> > >         uint64_t protocol_features;
> >
> > >
> >
> > >         dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
> >
> > >@@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> >
> > >          */
> >
> > >         protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
> >
> > >
> >
> > >-        if (supports_f_config) {
> >
> > >-            if (!virtio_has_feature(protocol_features,
> >
> > >-                                    VHOST_USER_PROTOCOL_F_CONFIG)) {
> >
> > >-                error_setg(errp, "vhost-user device expecting "
> >
> > >-                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
> >
> > >-                           "not support it.");
> >
> > >-                return -EPROTO;
> >
> > >-            }
> >
> > >-        } else {
> >
> > >-            if (virtio_has_feature(protocol_features,
> >
> > >-                                   VHOST_USER_PROTOCOL_F_CONFIG)) {
> >
> > >-                warn_reportf_err(*errp, "vhost-user backend supports "
> >
> > >-                                 "VHOST_USER_PROTOCOL_F_CONFIG but QEMU does not.");
> >
> > >-                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
> >
> > >-            }
> >
> > >-        }
> >
> > >-
> >
> > >         /* final set of protocol features */
> >
> > >         dev->protocol_features = protocol_features;
> >
> > >         err = vhost_user_set_protocol_features(dev, dev->protocol_features);
> >
> > >--
> >
> > >2.27.0
> >
> > >
> >
> >

Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically

[陈浩]

On 2022/9/22 17:56, Michael S. Tsirkin wrote:
> On Thu, Sep 22, 2022 at 09:34:41AM +0800, Jason Wang wrote:
>> On Thu, Sep 22, 2022 at 1:58 AM Raphael Norwitz
>> <raphael.norwitz@nutanix.com> wrote:
>>> If I read your response on the other thread correctly, this change is intended
>>>
>>> to prioritize the MAC address exposed by DPDK over the one provided by the
>>>
>>> QEMU command line? Sounds reasonable in principle, but I would get confirmation
>>>
>>> from vDPA/vhost-net maintainers.
>> I think the best way is to (and it seems easier)
>>
>> 1) have the management layer to make sure the mac came from cli
>> matches the underlayer vDPA
>> 2) having a sanity check and fail the device initialization if they don't match
>>
>> Thanks
> I think I agree, we should not overwrite user supplied arguments.
> I think the case where we might want to get the mac from VDPA
> and use it is when it's been assigned randomly as opposed to coming from cli.
Yes, when the cli passes in a random mac address that is inconsistent 
with the vdpa device, qemu still starts the virtual machine as usual, 
but in this case, the qemu and vdpa environments do not work correctly. 
So I want to get mac address directly from vdpa device instead of cli.

Having a sanity check and fail the device initialization if they don't 
match is a good idea, but it seems more convenient to directly overwrite 
the cli mac address.
>
>
>>>
>>>
>>> That said the way you’re hacking the vhost-user code breaks a valuable check for
>>>
>>> bad vhost-user-blk backends. I would suggest finding another implementation which
>>>
>>> does not regress functionality for other device types.
>>>
>>>
>>>
>>>
>>>
>>>> From: Hao Chen <chenh@yusur.tech>
>>>> When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
>>>> start the virtual machine through libvirt or qemu, but now, the libvirt or
>>>> qemu can call dpdk vdpa vendor driver's ops .get_config through vhost_net_get_config
>>>> to get the mac address of the vdpa hardware without manual configuration.
>>>> v1->v2:
>>>> Only copy ETH_ALEN data of netcfg for some vdpa device such as
>>>> NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.
>>>> We only need the mac address and don't care about the status field.
>>>> Signed-off-by: Hao Chen <chenh@yusur.tech>
>>>> ---
>>>> hw/block/vhost-user-blk.c |  1 -
>>>> hw/net/virtio-net.c       |  7 +++++++
>>>> hw/virtio/vhost-user.c    | 19 -------------------
>>>> 3 files changed, 7 insertions(+), 20 deletions(-)
>>>> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
>>>> index 9117222456..5dca4eab09 100644
>>>> --- a/hw/block/vhost-user-blk.c
>>>> +++ b/hw/block/vhost-user-blk.c
>>>> @@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
>>>>      vhost_dev_set_config_notifier(&s->dev, &blk_ops);
>>>> -    s->vhost_user.supports_config = true;
>>>>      ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0,
>>>>                           errp);
>>>>      if (ret < 0) {
>>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>>>> index dd0d056fde..90405083b1 100644
>>>> --- a/hw/net/virtio-net.c
>>>> +++ b/hw/net/virtio-net.c
>>>> @@ -166,6 +166,13 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
>>>>              }
>>>>              memcpy(config, &netcfg, n->config_size);
>>>>          }
>>>> +    } else if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
>>>> +        ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
>>>> +                                   n->config_size);
>>>> +        if (ret != -1) {
>>>> +               /* Automatically obtain the mac address of the vdpa device
>>>> +                * when using the dpdk vdpa */
>>>> +                memcpy(config, &netcfg, ETH_ALEN);
>>>>      }
>>>> }
>>>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>>>> index bd24741be8..8b01078249 100644
>>>> --- a/hw/virtio/vhost-user.c
>>>> +++ b/hw/virtio/vhost-user.c
>>>> @@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>>>>      }
>>>>      if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
>>>> -        bool supports_f_config = vus->supports_config ||
>>>> -            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
>>>>          uint64_t protocol_features;
>>>>          dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
>>>> @@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>>>>           */
>>>>          protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
>>>> -        if (supports_f_config) {
>>>> -            if (!virtio_has_feature(protocol_features,
>>>> -                                    VHOST_USER_PROTOCOL_F_CONFIG)) {
>>>> -                error_setg(errp, "vhost-user device expecting "
>>>> -                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
>>>> -                           "not support it.");
>>>> -                return -EPROTO;
>>>> -            }
>>>> -        } else {
>>>> -            if (virtio_has_feature(protocol_features,
>>>> -                                   VHOST_USER_PROTOCOL_F_CONFIG)) {
>>>> -                warn_reportf_err(*errp, "vhost-user backend supports "
>>>> -                                 "VHOST_USER_PROTOCOL_F_CONFIG but QEMU does not.");
>>>> -                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
>>>> -            }
>>>> -        }
>>>> -
>>>>          /* final set of protocol features */
>>>>          dev->protocol_features = protocol_features;
>>>>          err = vhost_user_set_protocol_features(dev, dev->protocol_features);
>>>> --
>>>> 2.27.0
>>>
>

Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically

[Jason Wang]

On Fri, Sep 23, 2022 at 11:33 AM 陈浩 <chenh@yusur.tech> wrote:
>
>
> On 2022/9/22 17:56, Michael S. Tsirkin wrote:
> > On Thu, Sep 22, 2022 at 09:34:41AM +0800, Jason Wang wrote:
> >> On Thu, Sep 22, 2022 at 1:58 AM Raphael Norwitz
> >> <raphael.norwitz@nutanix.com> wrote:
> >>> If I read your response on the other thread correctly, this change is intended
> >>>
> >>> to prioritize the MAC address exposed by DPDK over the one provided by the
> >>>
> >>> QEMU command line? Sounds reasonable in principle, but I would get confirmation
> >>>
> >>> from vDPA/vhost-net maintainers.
> >> I think the best way is to (and it seems easier)
> >>
> >> 1) have the management layer to make sure the mac came from cli
> >> matches the underlayer vDPA
> >> 2) having a sanity check and fail the device initialization if they don't match
> >>
> >> Thanks
> > I think I agree, we should not overwrite user supplied arguments.
> > I think the case where we might want to get the mac from VDPA
> > and use it is when it's been assigned randomly as opposed to coming from cli.
> Yes, when the cli passes in a random mac address that is inconsistent
> with the vdpa device, qemu still starts the virtual machine as usual,
> but in this case, the qemu and vdpa environments do not work correctly.
> So I want to get mac address directly from vdpa device instead of cli.

Let's teach the management to do that. It has a lot of advantages, more below.

Not sure for the DPDK case, but kernel vDPA allows the mgmt to
provision the vDPA with a mac address which could be used in this
case.

>
> Having a sanity check and fail the device initialization if they don't
> match is a good idea, but it seems more convenient to directly overwrite
> the cli mac address.

This will confuse the management where it may do a lot of mac related setups.

Thanks

> >
> >
> >>>
> >>>
> >>> That said the way you’re hacking the vhost-user code breaks a valuable check for
> >>>
> >>> bad vhost-user-blk backends. I would suggest finding another implementation which
> >>>
> >>> does not regress functionality for other device types.
> >>>
> >>>
> >>>
> >>>
> >>>
> >>>> From: Hao Chen <chenh@yusur.tech>
> >>>> When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
> >>>> start the virtual machine through libvirt or qemu, but now, the libvirt or
> >>>> qemu can call dpdk vdpa vendor driver's ops .get_config through vhost_net_get_config
> >>>> to get the mac address of the vdpa hardware without manual configuration.
> >>>> v1->v2:
> >>>> Only copy ETH_ALEN data of netcfg for some vdpa device such as
> >>>> NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.
> >>>> We only need the mac address and don't care about the status field.
> >>>> Signed-off-by: Hao Chen <chenh@yusur.tech>
> >>>> ---
> >>>> hw/block/vhost-user-blk.c |  1 -
> >>>> hw/net/virtio-net.c       |  7 +++++++
> >>>> hw/virtio/vhost-user.c    | 19 -------------------
> >>>> 3 files changed, 7 insertions(+), 20 deletions(-)
> >>>> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> >>>> index 9117222456..5dca4eab09 100644
> >>>> --- a/hw/block/vhost-user-blk.c
> >>>> +++ b/hw/block/vhost-user-blk.c
> >>>> @@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
> >>>>      vhost_dev_set_config_notifier(&s->dev, &blk_ops);
> >>>> -    s->vhost_user.supports_config = true;
> >>>>      ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0,
> >>>>                           errp);
> >>>>      if (ret < 0) {
> >>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >>>> index dd0d056fde..90405083b1 100644
> >>>> --- a/hw/net/virtio-net.c
> >>>> +++ b/hw/net/virtio-net.c
> >>>> @@ -166,6 +166,13 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
> >>>>              }
> >>>>              memcpy(config, &netcfg, n->config_size);
> >>>>          }
> >>>> +    } else if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> >>>> +        ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
> >>>> +                                   n->config_size);
> >>>> +        if (ret != -1) {
> >>>> +               /* Automatically obtain the mac address of the vdpa device
> >>>> +                * when using the dpdk vdpa */
> >>>> +                memcpy(config, &netcfg, ETH_ALEN);
> >>>>      }
> >>>> }
> >>>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> >>>> index bd24741be8..8b01078249 100644
> >>>> --- a/hw/virtio/vhost-user.c
> >>>> +++ b/hw/virtio/vhost-user.c
> >>>> @@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> >>>>      }
> >>>>      if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
> >>>> -        bool supports_f_config = vus->supports_config ||
> >>>> -            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
> >>>>          uint64_t protocol_features;
> >>>>          dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
> >>>> @@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> >>>>           */
> >>>>          protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
> >>>> -        if (supports_f_config) {
> >>>> -            if (!virtio_has_feature(protocol_features,
> >>>> -                                    VHOST_USER_PROTOCOL_F_CONFIG)) {
> >>>> -                error_setg(errp, "vhost-user device expecting "
> >>>> -                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
> >>>> -                           "not support it.");
> >>>> -                return -EPROTO;
> >>>> -            }
> >>>> -        } else {
> >>>> -            if (virtio_has_feature(protocol_features,
> >>>> -                                   VHOST_USER_PROTOCOL_F_CONFIG)) {
> >>>> -                warn_reportf_err(*errp, "vhost-user backend supports "
> >>>> -                                 "VHOST_USER_PROTOCOL_F_CONFIG but QEMU does not.");
> >>>> -                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
> >>>> -            }
> >>>> -        }
> >>>> -
> >>>>          /* final set of protocol features */
> >>>>          dev->protocol_features = protocol_features;
> >>>>          err = vhost_user_set_protocol_features(dev, dev->protocol_features);
> >>>> --
> >>>> 2.27.0
> >>>
> >
>

Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically

[Michael S. Tsirkin]

On Fri, Sep 23, 2022 at 11:36:38AM +0800, Jason Wang wrote:
> On Fri, Sep 23, 2022 at 11:33 AM 陈浩 <chenh@yusur.tech> wrote:
> >
> >
> > On 2022/9/22 17:56, Michael S. Tsirkin wrote:
> > > On Thu, Sep 22, 2022 at 09:34:41AM +0800, Jason Wang wrote:
> > >> On Thu, Sep 22, 2022 at 1:58 AM Raphael Norwitz
> > >> <raphael.norwitz@nutanix.com> wrote:
> > >>> If I read your response on the other thread correctly, this change is intended
> > >>>
> > >>> to prioritize the MAC address exposed by DPDK over the one provided by the
> > >>>
> > >>> QEMU command line? Sounds reasonable in principle, but I would get confirmation
> > >>>
> > >>> from vDPA/vhost-net maintainers.
> > >> I think the best way is to (and it seems easier)
> > >>
> > >> 1) have the management layer to make sure the mac came from cli
> > >> matches the underlayer vDPA
> > >> 2) having a sanity check and fail the device initialization if they don't match
> > >>
> > >> Thanks
> > > I think I agree, we should not overwrite user supplied arguments.
> > > I think the case where we might want to get the mac from VDPA
> > > and use it is when it's been assigned randomly as opposed to coming from cli.
> > Yes, when the cli passes in a random mac address that is inconsistent
> > with the vdpa device, qemu still starts the virtual machine as usual,
> > but in this case, the qemu and vdpa environments do not work correctly.
> > So I want to get mac address directly from vdpa device instead of cli.
> 
> Let's teach the management to do that. It has a lot of advantages, more below.


No, making command line friendlier is a good idea.
the random address from cli is just that, random,
so why not take it from backend?


> Not sure for the DPDK case, but kernel vDPA allows the mgmt to
> provision the vDPA with a mac address which could be used in this
> case.
> 
> >
> > Having a sanity check and fail the device initialization if they don't
> > match is a good idea, but it seems more convenient to directly overwrite
> > the cli mac address.
> 
> This will confuse the management where it may do a lot of mac related setups.
> 
> Thanks

i do not think any management relies on the random mac hack.

> > >
> > >
> > >>>
> > >>>
> > >>> That said the way you’re hacking the vhost-user code breaks a valuable check for
> > >>>
> > >>> bad vhost-user-blk backends. I would suggest finding another implementation which
> > >>>
> > >>> does not regress functionality for other device types.
> > >>>
> > >>>
> > >>>
> > >>>
> > >>>
> > >>>> From: Hao Chen <chenh@yusur.tech>
> > >>>> When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
> > >>>> start the virtual machine through libvirt or qemu, but now, the libvirt or
> > >>>> qemu can call dpdk vdpa vendor driver's ops .get_config through vhost_net_get_config
> > >>>> to get the mac address of the vdpa hardware without manual configuration.
> > >>>> v1->v2:
> > >>>> Only copy ETH_ALEN data of netcfg for some vdpa device such as
> > >>>> NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.
> > >>>> We only need the mac address and don't care about the status field.
> > >>>> Signed-off-by: Hao Chen <chenh@yusur.tech>
> > >>>> ---
> > >>>> hw/block/vhost-user-blk.c |  1 -
> > >>>> hw/net/virtio-net.c       |  7 +++++++
> > >>>> hw/virtio/vhost-user.c    | 19 -------------------
> > >>>> 3 files changed, 7 insertions(+), 20 deletions(-)
> > >>>> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > >>>> index 9117222456..5dca4eab09 100644
> > >>>> --- a/hw/block/vhost-user-blk.c
> > >>>> +++ b/hw/block/vhost-user-blk.c
> > >>>> @@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
> > >>>>      vhost_dev_set_config_notifier(&s->dev, &blk_ops);
> > >>>> -    s->vhost_user.supports_config = true;
> > >>>>      ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0,
> > >>>>                           errp);
> > >>>>      if (ret < 0) {
> > >>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > >>>> index dd0d056fde..90405083b1 100644
> > >>>> --- a/hw/net/virtio-net.c
> > >>>> +++ b/hw/net/virtio-net.c
> > >>>> @@ -166,6 +166,13 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
> > >>>>              }
> > >>>>              memcpy(config, &netcfg, n->config_size);
> > >>>>          }
> > >>>> +    } else if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> > >>>> +        ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
> > >>>> +                                   n->config_size);
> > >>>> +        if (ret != -1) {
> > >>>> +               /* Automatically obtain the mac address of the vdpa device
> > >>>> +                * when using the dpdk vdpa */
> > >>>> +                memcpy(config, &netcfg, ETH_ALEN);
> > >>>>      }
> > >>>> }
> > >>>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > >>>> index bd24741be8..8b01078249 100644
> > >>>> --- a/hw/virtio/vhost-user.c
> > >>>> +++ b/hw/virtio/vhost-user.c
> > >>>> @@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> > >>>>      }
> > >>>>      if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
> > >>>> -        bool supports_f_config = vus->supports_config ||
> > >>>> -            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
> > >>>>          uint64_t protocol_features;
> > >>>>          dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
> > >>>> @@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> > >>>>           */
> > >>>>          protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
> > >>>> -        if (supports_f_config) {
> > >>>> -            if (!virtio_has_feature(protocol_features,
> > >>>> -                                    VHOST_USER_PROTOCOL_F_CONFIG)) {
> > >>>> -                error_setg(errp, "vhost-user device expecting "
> > >>>> -                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
> > >>>> -                           "not support it.");
> > >>>> -                return -EPROTO;
> > >>>> -            }
> > >>>> -        } else {
> > >>>> -            if (virtio_has_feature(protocol_features,
> > >>>> -                                   VHOST_USER_PROTOCOL_F_CONFIG)) {
> > >>>> -                warn_reportf_err(*errp, "vhost-user backend supports "
> > >>>> -                                 "VHOST_USER_PROTOCOL_F_CONFIG but QEMU does not.");
> > >>>> -                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
> > >>>> -            }
> > >>>> -        }
> > >>>> -
> > >>>>          /* final set of protocol features */
> > >>>>          dev->protocol_features = protocol_features;
> > >>>>          err = vhost_user_set_protocol_features(dev, dev->protocol_features);
> > >>>> --
> > >>>> 2.27.0
> > >>>
> > >
> >

Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically

[Cindy Lu]

On Tue, Feb 3, 2026 at 8:48 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Fri, Sep 23, 2022 at 11:36:38AM +0800, Jason Wang wrote:
> > On Fri, Sep 23, 2022 at 11:33 AM 陈浩 <chenh@yusur.tech> wrote:
> > >
> > >
> > > On 2022/9/22 17:56, Michael S. Tsirkin wrote:
> > > > On Thu, Sep 22, 2022 at 09:34:41AM +0800, Jason Wang wrote:
> > > >> On Thu, Sep 22, 2022 at 1:58 AM Raphael Norwitz
> > > >> <raphael.norwitz@nutanix.com> wrote:
> > > >>> If I read your response on the other thread correctly, this change is intended
> > > >>>
> > > >>> to prioritize the MAC address exposed by DPDK over the one provided by the
> > > >>>
> > > >>> QEMU command line? Sounds reasonable in principle, but I would get confirmation
> > > >>>
> > > >>> from vDPA/vhost-net maintainers.
> > > >> I think the best way is to (and it seems easier)
> > > >>
> > > >> 1) have the management layer to make sure the mac came from cli
> > > >> matches the underlayer vDPA
> > > >> 2) having a sanity check and fail the device initialization if they don't match
> > > >>
> > > >> Thanks
> > > > I think I agree, we should not overwrite user supplied arguments.
> > > > I think the case where we might want to get the mac from VDPA
> > > > and use it is when it's been assigned randomly as opposed to coming from cli.
> > > Yes, when the cli passes in a random mac address that is inconsistent
> > > with the vdpa device, qemu still starts the virtual machine as usual,
> > > but in this case, the qemu and vdpa environments do not work correctly.
> > > So I want to get mac address directly from vdpa device instead of cli.
> >
> > Let's teach the management to do that. It has a lot of advantages, more below.
>
>
> No, making command line friendlier is a good idea.
> the random address from cli is just that, random,
> so why not take it from backend?
>
Yes, we have a vdpa fix recently that allows changing the MAC address
via the vdpa tool before userspace loads. I wonder maybe you can
leverage that.
https://patchew.org/linux/20260126094848.9601-1-lulu@redhat.com/
thanks
cindy
>
> > Not sure for the DPDK case, but kernel vDPA allows the mgmt to
> > provision the vDPA with a mac address which could be used in this
> > case.
> >
> > >
> > > Having a sanity check and fail the device initialization if they don't
> > > match is a good idea, but it seems more convenient to directly overwrite
> > > the cli mac address.
> >
> > This will confuse the management where it may do a lot of mac related setups.
> >
> > Thanks
>
> i do not think any management relies on the random mac hack.
>
> > > >
> > > >
> > > >>>
> > > >>>
> > > >>> That said the way you’re hacking the vhost-user code breaks a valuable check for
> > > >>>
> > > >>> bad vhost-user-blk backends. I would suggest finding another implementation which
> > > >>>
> > > >>> does not regress functionality for other device types.
> > > >>>
> > > >>>
> > > >>>
> > > >>>
> > > >>>
> > > >>>> From: Hao Chen <chenh@yusur.tech>
> > > >>>> When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
> > > >>>> start the virtual machine through libvirt or qemu, but now, the libvirt or
> > > >>>> qemu can call dpdk vdpa vendor driver's ops .get_config through vhost_net_get_config
> > > >>>> to get the mac address of the vdpa hardware without manual configuration.
> > > >>>> v1->v2:
> > > >>>> Only copy ETH_ALEN data of netcfg for some vdpa device such as
> > > >>>> NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.
> > > >>>> We only need the mac address and don't care about the status field.
> > > >>>> Signed-off-by: Hao Chen <chenh@yusur.tech>
> > > >>>> ---
> > > >>>> hw/block/vhost-user-blk.c |  1 -
> > > >>>> hw/net/virtio-net.c       |  7 +++++++
> > > >>>> hw/virtio/vhost-user.c    | 19 -------------------
> > > >>>> 3 files changed, 7 insertions(+), 20 deletions(-)
> > > >>>> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > > >>>> index 9117222456..5dca4eab09 100644
> > > >>>> --- a/hw/block/vhost-user-blk.c
> > > >>>> +++ b/hw/block/vhost-user-blk.c
> > > >>>> @@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
> > > >>>>      vhost_dev_set_config_notifier(&s->dev, &blk_ops);
> > > >>>> -    s->vhost_user.supports_config = true;
> > > >>>>      ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0,
> > > >>>>                           errp);
> > > >>>>      if (ret < 0) {
> > > >>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > > >>>> index dd0d056fde..90405083b1 100644
> > > >>>> --- a/hw/net/virtio-net.c
> > > >>>> +++ b/hw/net/virtio-net.c
> > > >>>> @@ -166,6 +166,13 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
> > > >>>>              }
> > > >>>>              memcpy(config, &netcfg, n->config_size);
> > > >>>>          }
> > > >>>> +    } else if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> > > >>>> +        ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
> > > >>>> +                                   n->config_size);
> > > >>>> +        if (ret != -1) {
> > > >>>> +               /* Automatically obtain the mac address of the vdpa device
> > > >>>> +                * when using the dpdk vdpa */
> > > >>>> +                memcpy(config, &netcfg, ETH_ALEN);
> > > >>>>      }
> > > >>>> }
> > > >>>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > > >>>> index bd24741be8..8b01078249 100644
> > > >>>> --- a/hw/virtio/vhost-user.c
> > > >>>> +++ b/hw/virtio/vhost-user.c
> > > >>>> @@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> > > >>>>      }
> > > >>>>      if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
> > > >>>> -        bool supports_f_config = vus->supports_config ||
> > > >>>> -            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
> > > >>>>          uint64_t protocol_features;
> > > >>>>          dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
> > > >>>> @@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> > > >>>>           */
> > > >>>>          protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
> > > >>>> -        if (supports_f_config) {
> > > >>>> -            if (!virtio_has_feature(protocol_features,
> > > >>>> -                                    VHOST_USER_PROTOCOL_F_CONFIG)) {
> > > >>>> -                error_setg(errp, "vhost-user device expecting "
> > > >>>> -                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
> > > >>>> -                           "not support it.");
> > > >>>> -                return -EPROTO;
> > > >>>> -            }
> > > >>>> -        } else {
> > > >>>> -            if (virtio_has_feature(protocol_features,
> > > >>>> -                                   VHOST_USER_PROTOCOL_F_CONFIG)) {
> > > >>>> -                warn_reportf_err(*errp, "vhost-user backend supports "
> > > >>>> -                                 "VHOST_USER_PROTOCOL_F_CONFIG but QEMU does not.");
> > > >>>> -                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
> > > >>>> -            }
> > > >>>> -        }
> > > >>>> -
> > > >>>>          /* final set of protocol features */
> > > >>>>          dev->protocol_features = protocol_features;
> > > >>>>          err = vhost_user_set_protocol_features(dev, dev->protocol_features);
> > > >>>> --
> > > >>>> 2.27.0
> > > >>>
> > > >
> > >
>

Re: Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically

[houyl]

[-- Attachment #1: Type: text/plain, Size: 5867 bytes --]

On Thu, 22 Sep 2022 09:34:41 +0800 Jason Wang<jasowang@redhat.com>  wrote:

>On Thu, Sep 22, 2022 at 1:58 AM Raphael Norwitz
><raphael.norwitz@nutanix.com> wrote:
>>
>> If I read your response on the other thread correctly, this change is intended
>>
>> to prioritize the MAC address exposed by DPDK over the one provided by the
>>
>> QEMU command line? Sounds reasonable in principle, but I would get confirmation
>>
>> from vDPA/vhost-net maintainers.
 
>I think the best way is to (and it seems easier)
 
>1) have the management layer to make sure the mac came from cli
>matches the underlayer vDPA

 Agreed, that's no problem.

>2) having a sanity check and fail the device initialization if they don't match

However, one MAC address for integrity check is from the cli, and the other MAC address is from the vDPA device, 
How to get it?

The current situation is if MAC came from cli don't match the underlayer vDPA, the virtual machine can still start without any hints.

Thanks


>Thanks
 
>>
>>
>>
>> That said the way you’re hacking the vhost-user code breaks a valuable check for
>>
>> bad vhost-user-blk backends. I would suggest finding another implementation which
>>
>> does not regress functionality for other device types.
>>
>>
>>
>>
>>
>> >From: Hao Chen <chenh@yusur.tech>
>>
>> >
>>
>> >When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
>>
>> >start the virtual machine through libvirt or qemu, but now, the libvirt or
>>
>> >qemu can call dpdk vdpa vendor driver's ops .get_config through vhost_net_get_config
>>
>> >to get the mac address of the vdpa hardware without manual configuration.
>>
>> >
>>
>> >v1->v2:
>>
>> >Only copy ETH_ALEN data of netcfg for some vdpa device such as
>>
>> >NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.
>>
>> >We only need the mac address and don't care about the status field.
>>
>> >
>>
>> >Signed-off-by: Hao Chen <chenh@yusur.tech>
>>
>> >---
>>
>> > hw/block/vhost-user-blk.c |  1 -
>>
>> > hw/net/virtio-net.c       |  7 +++++++
>>
>> > hw/virtio/vhost-user.c    | 19 -------------------
>>
>> > 3 files changed, 7 insertions(+), 20 deletions(-)
>>
>> >
>>
>> >diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
>>
>> >index 9117222456..5dca4eab09 100644
>>
>> >--- a/hw/block/vhost-user-blk.c
>>
>> >+++ b/hw/block/vhost-user-blk.c
>>
>> >@@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
>>
>> >
>>
>> >     vhost_dev_set_config_notifier(&s->dev, &blk_ops);
>>
>> >
>>
>> >-    s->vhost_user.supports_config = true;
>>
>> >     ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0,
>>
>> >                          errp);
>>
>> >     if (ret < 0) {
>>
>> >diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>>
>> >index dd0d056fde..90405083b1 100644
>>
>> >--- a/hw/net/virtio-net.c
>>
>> >+++ b/hw/net/virtio-net.c
>>
>> >@@ -166,6 +166,13 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
>>
>> >             }
>>
>> >             memcpy(config, &netcfg, n->config_size);
>>
>> >         }
>>
>> >+    } else if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
>>
>> >+        ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
>>
>> >+                                   n->config_size);
>>
>> >+        if (ret != -1) {
>>
>> >+               /* Automatically obtain the mac address of the vdpa device
>>
>> >+                * when using the dpdk vdpa */
>>
>> >+                memcpy(config, &netcfg, ETH_ALEN);
>>
>> >     }
>>
>> > }
>>
>> >
>>
>> >diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>>
>> >index bd24741be8..8b01078249 100644
>>
>> >--- a/hw/virtio/vhost-user.c
>>
>> >+++ b/hw/virtio/vhost-user.c
>>
>> >@@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>>
>> >     }
>>
>> >
>>
>> >     if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
>>
>> >-        bool supports_f_config = vus->supports_config ||
>>
>> >-            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
>>
>> >         uint64_t protocol_features;
>>
>> >
>>
>> >         dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
>>
>> >@@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>>
>> >          */
>>
>> >         protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
>>
>> >
>>
>> >-        if (supports_f_config) {
>>
>> >-            if (!virtio_has_feature(protocol_features,
>>
>> >-                                    VHOST_USER_PROTOCOL_F_CONFIG)) {
>>
>> >-                error_setg(errp, "vhost-user device expecting "
>>
>> >-                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
>>
>> >-                           "not support it.");
>>
>> >-                return -EPROTO;
>>
>> >-            }
>>
>> >-        } else {
>>
>> >-            if (virtio_has_feature(protocol_features,
>>
>> >-                                   VHOST_USER_PROTOCOL_F_CONFIG)) {
>>
>> >-                warn_reportf_err(*errp, "vhost-user backend supports "
>>
>> >-                                 "VHOST_USER_PROTOCOL_F_CONFIG but QEMU does not.");
>>
>> >-                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
>>
>> >-            }
>>
>> >-        }
>>
>> >-
>>
>> >         /* final set of protocol features */
>>
>> >         dev->protocol_features = protocol_features;
>>
>> >         err = vhost_user_set_protocol_features(dev, dev->protocol_features);
>>
>> >--
>>
>> >2.27.0
>>
>> >
>>
>>
 

[-- Attachment #2: Type: text/html, Size: 13326 bytes --]

Re: Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically

[Jason Wang]

On Fri, Sep 23, 2022 at 11:55 AM houyl@yusur.tech <houyl@yusur.tech> wrote:
>
> On Thu, 22 Sep 2022 09:34:41 +0800 Jason Wang<jasowang@redhat.com>  wrote:
>
>
> >On Thu, Sep 22, 2022 at 1:58 AM Raphael Norwitz
> ><raphael.norwitz@nutanix.com> wrote:
> >>
> >> If I read your response on the other thread correctly, this change is intended
> >>
> >> to prioritize the MAC address exposed by DPDK over the one provided by the
> >>
> >> QEMU command line? Sounds reasonable in principle, but I would get confirmation
> >>
> >> from vDPA/vhost-net maintainers.
>
> >I think the best way is to (and it seems easier)
>
> >1) have the management layer to make sure the mac came from cli
> >matches the underlayer vDPA
>
>  Agreed, that's no problem.
>
> >2) having a sanity check and fail the device initialization if they don't match
>
> However, one MAC address for integrity check is from the cli, and the other MAC address is from the vDPA device,
> How to get it?

VHOST_USER_GET_CONFIG?

Thanks

>
> The current situation is if MAC came from cli don't match the underlayer vDPA, the virtual machine can still start without any hints.
>
> Thanks
>
>
> >Thanks
>
> >>
> >>
> >>
> >> That said the way you’re hacking the vhost-user code breaks a valuable check for
> >>
> >> bad vhost-user-blk backends. I would suggest finding another implementation which
> >>
> >> does not regress functionality for other device types.
> >>
> >>
> >>
> >>
> >>
> >> >From: Hao Chen <chenh@yusur.tech>
> >>
> >> >
> >>
> >> >When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
> >>
> >> >start the virtual machine through libvirt or qemu, but now, the libvirt or
> >>
> >> >qemu can call dpdk vdpa vendor driver's ops .get_config through vhost_net_get_config
> >>
> >> >to get the mac address of the vdpa hardware without manual configuration.
> >>
> >> >
> >>
> >> >v1->v2:
> >>
> >> >Only copy ETH_ALEN data of netcfg for some vdpa device such as
> >>
> >> >NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.
> >>
> >> >We only need the mac address and don't care about the status field.
> >>
> >> >
> >>
> >> >Signed-off-by: Hao Chen <chenh@yusur.tech>
> >>
> >> >---
> >>
> >> > hw/block/vhost-user-blk.c |  1 -
> >>
> >> > hw/net/virtio-net.c       |  7 +++++++
> >>
> >> > hw/virtio/vhost-user.c    | 19 -------------------
> >>
> >> > 3 files changed, 7 insertions(+), 20 deletions(-)
> >>
> >> >
> >>
> >> >diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> >>
> >> >index 9117222456..5dca4eab09 100644
> >>
> >> >--- a/hw/block/vhost-user-blk.c
> >>
> >> >+++ b/hw/block/vhost-user-blk.c
> >>
> >> >@@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
> >>
> >> >
> >>
> >> >     vhost_dev_set_config_notifier(&s->dev, &blk_ops);
> >>
> >> >
> >>
> >> >-    s->vhost_user.supports_config = true;
> >>
> >> >     ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0,
> >>
> >> >                          errp);
> >>
> >> >     if (ret < 0) {
> >>
> >> >diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >>
> >> >index dd0d056fde..90405083b1 100644
> >>
> >> >--- a/hw/net/virtio-net.c
> >>
> >> >+++ b/hw/net/virtio-net.c
> >>
> >> >@@ -166,6 +166,13 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
> >>
> >> >             }
> >>
> >> >             memcpy(config, &netcfg, n->config_size);
> >>
> >> >         }
> >>
> >> >+    } else if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> >>
> >> >+        ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
> >>
> >> >+                                   n->config_size);
> >>
> >> >+        if (ret != -1) {
> >>
> >> >+               /* Automatically obtain the mac address of the vdpa device
> >>
> >> >+                * when using the dpdk vdpa */
> >>
> >> >+                memcpy(config, &netcfg, ETH_ALEN);
> >>
> >> >     }
> >>
> >> > }
> >>
> >> >
> >>
> >> >diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> >>
> >> >index bd24741be8..8b01078249 100644
> >>
> >> >--- a/hw/virtio/vhost-user.c
> >>
> >> >+++ b/hw/virtio/vhost-user.c
> >>
> >> >@@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> >>
> >> >     }
> >>
> >> >
> >>
> >> >     if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
> >>
> >> >-        bool supports_f_config = vus->supports_config ||
> >>
> >> >-            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
> >>
> >> >         uint64_t protocol_features;
> >>
> >> >
> >>
> >> >         dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
> >>
> >> >@@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> >>
> >> >          */
> >>
> >> >         protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
> >>
> >> >
> >>
> >> >-        if (supports_f_config) {
> >>
> >> >-            if (!virtio_has_feature(protocol_features,
> >>
> >> >-                                    VHOST_USER_PROTOCOL_F_CONFIG)) {
> >>
> >> >-                error_setg(errp, "vhost-user device expecting "
> >>
> >> >-                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
> >>
> >> >-                           "not support it.");
> >>
> >> >-                return -EPROTO;
> >>
> >> >-            }
> >>
> >> >-        } else {
> >>
> >> >-            if (virtio_has_feature(protocol_features,
> >>
> >> >-                                   VHOST_USER_PROTOCOL_F_CONFIG)) {
> >>
> >> >-                warn_reportf_err(*errp, "vhost-user backend supports "
> >>
> >> >-                                 "VHOST_USER_PROTOCOL_F_CONFIG but QEMU does not.");
> >>
> >> >-                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
> >>
> >> >-            }
> >>
> >> >-        }
> >>
> >> >-
> >>
> >> >         /* final set of protocol features */
> >>
> >> >         dev->protocol_features = protocol_features;
> >>
> >> >         err = vhost_user_set_protocol_features(dev, dev->protocol_features);
> >>
> >> >--
> >>
> >> >2.27.0
> >>
> >> >
> >>
> >>
>

Re: Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically

[Michael S. Tsirkin]

On Fri, Sep 23, 2022 at 12:05:40PM +0800, Jason Wang wrote:
> On Fri, Sep 23, 2022 at 11:55 AM houyl@yusur.tech <houyl@yusur.tech> wrote:
> >
> > On Thu, 22 Sep 2022 09:34:41 +0800 Jason Wang<jasowang@redhat.com>  wrote:
> >
> >
> > >On Thu, Sep 22, 2022 at 1:58 AM Raphael Norwitz
> > ><raphael.norwitz@nutanix.com> wrote:
> > >>
> > >> If I read your response on the other thread correctly, this change is intended
> > >>
> > >> to prioritize the MAC address exposed by DPDK over the one provided by the
> > >>
> > >> QEMU command line? Sounds reasonable in principle, but I would get confirmation
> > >>
> > >> from vDPA/vhost-net maintainers.
> >
> > >I think the best way is to (and it seems easier)
> >
> > >1) have the management layer to make sure the mac came from cli
> > >matches the underlayer vDPA
> >
> >  Agreed, that's no problem.

Actually, if we start failing here, won't it break working designs
in which vhost user does not really have a valid mac?

I am not sure we can start changing things so easily.
Let's add a protocol feature?


> > >2) having a sanity check and fail the device initialization if they don't match
> >
> > However, one MAC address for integrity check is from the cli, and the other MAC address is from the vDPA device,
> > How to get it?
> 
> VHOST_USER_GET_CONFIG?
> 
> Thanks
> 
> >
> > The current situation is if MAC came from cli don't match the underlayer vDPA, the virtual machine can still start without any hints.
> >
> > Thanks
> >
> >
> > >Thanks
> >
> > >>
> > >>
> > >>
> > >> That said the way you’re hacking the vhost-user code breaks a valuable check for
> > >>
> > >> bad vhost-user-blk backends. I would suggest finding another implementation which
> > >>
> > >> does not regress functionality for other device types.
> > >>
> > >>
> > >>
> > >>
> > >>
> > >> >From: Hao Chen <chenh@yusur.tech>
> > >>
> > >> >
> > >>
> > >> >When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
> > >>
> > >> >start the virtual machine through libvirt or qemu, but now, the libvirt or
> > >>
> > >> >qemu can call dpdk vdpa vendor driver's ops .get_config through vhost_net_get_config
> > >>
> > >> >to get the mac address of the vdpa hardware without manual configuration.
> > >>
> > >> >
> > >>
> > >> >v1->v2:
> > >>
> > >> >Only copy ETH_ALEN data of netcfg for some vdpa device such as
> > >>
> > >> >NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.
> > >>
> > >> >We only need the mac address and don't care about the status field.
> > >>
> > >> >
> > >>
> > >> >Signed-off-by: Hao Chen <chenh@yusur.tech>
> > >>
> > >> >---
> > >>
> > >> > hw/block/vhost-user-blk.c |  1 -
> > >>
> > >> > hw/net/virtio-net.c       |  7 +++++++
> > >>
> > >> > hw/virtio/vhost-user.c    | 19 -------------------
> > >>
> > >> > 3 files changed, 7 insertions(+), 20 deletions(-)
> > >>
> > >> >
> > >>
> > >> >diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > >>
> > >> >index 9117222456..5dca4eab09 100644
> > >>
> > >> >--- a/hw/block/vhost-user-blk.c
> > >>
> > >> >+++ b/hw/block/vhost-user-blk.c
> > >>
> > >> >@@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
> > >>
> > >> >
> > >>
> > >> >     vhost_dev_set_config_notifier(&s->dev, &blk_ops);
> > >>
> > >> >
> > >>
> > >> >-    s->vhost_user.supports_config = true;
> > >>
> > >> >     ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0,
> > >>
> > >> >                          errp);
> > >>
> > >> >     if (ret < 0) {
> > >>
> > >> >diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > >>
> > >> >index dd0d056fde..90405083b1 100644
> > >>
> > >> >--- a/hw/net/virtio-net.c
> > >>
> > >> >+++ b/hw/net/virtio-net.c
> > >>
> > >> >@@ -166,6 +166,13 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
> > >>
> > >> >             }
> > >>
> > >> >             memcpy(config, &netcfg, n->config_size);
> > >>
> > >> >         }
> > >>
> > >> >+    } else if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> > >>
> > >> >+        ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
> > >>
> > >> >+                                   n->config_size);
> > >>
> > >> >+        if (ret != -1) {
> > >>
> > >> >+               /* Automatically obtain the mac address of the vdpa device
> > >>
> > >> >+                * when using the dpdk vdpa */
> > >>
> > >> >+                memcpy(config, &netcfg, ETH_ALEN);
> > >>
> > >> >     }
> > >>
> > >> > }
> > >>
> > >> >
> > >>
> > >> >diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > >>
> > >> >index bd24741be8..8b01078249 100644
> > >>
> > >> >--- a/hw/virtio/vhost-user.c
> > >>
> > >> >+++ b/hw/virtio/vhost-user.c
> > >>
> > >> >@@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> > >>
> > >> >     }
> > >>
> > >> >
> > >>
> > >> >     if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
> > >>
> > >> >-        bool supports_f_config = vus->supports_config ||
> > >>
> > >> >-            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
> > >>
> > >> >         uint64_t protocol_features;
> > >>
> > >> >
> > >>
> > >> >         dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
> > >>
> > >> >@@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> > >>
> > >> >          */
> > >>
> > >> >         protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
> > >>
> > >> >
> > >>
> > >> >-        if (supports_f_config) {
> > >>
> > >> >-            if (!virtio_has_feature(protocol_features,
> > >>
> > >> >-                                    VHOST_USER_PROTOCOL_F_CONFIG)) {
> > >>
> > >> >-                error_setg(errp, "vhost-user device expecting "
> > >>
> > >> >-                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
> > >>
> > >> >-                           "not support it.");
> > >>
> > >> >-                return -EPROTO;
> > >>
> > >> >-            }
> > >>
> > >> >-        } else {
> > >>
> > >> >-            if (virtio_has_feature(protocol_features,
> > >>
> > >> >-                                   VHOST_USER_PROTOCOL_F_CONFIG)) {
> > >>
> > >> >-                warn_reportf_err(*errp, "vhost-user backend supports "
> > >>
> > >> >-                                 "VHOST_USER_PROTOCOL_F_CONFIG but QEMU does not.");
> > >>
> > >> >-                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
> > >>
> > >> >-            }
> > >>
> > >> >-        }
> > >>
> > >> >-
> > >>
> > >> >         /* final set of protocol features */
> > >>
> > >> >         dev->protocol_features = protocol_features;
> > >>
> > >> >         err = vhost_user_set_protocol_features(dev, dev->protocol_features);
> > >>
> > >> >--
> > >>
> > >> >2.27.0
> > >>
> > >> >
> > >>
> > >>
> >

Re: Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically

[Michael S. Tsirkin]

On Fri, Sep 23, 2022 at 11:53:22AM +0800, houyl@yusur.tech wrote:
> On Thu, 22 Sep 2022 09:34:41 +0800 Jason Wang<jasowang@redhat.com>  wrote:
> 
> 
>     >On Thu, Sep 22, 2022 at 1:58 AM Raphael Norwitz
>     ><raphael.norwitz@nutanix.com> wrote:
>     >>
>     >> If I read your response on the other thread correctly, this change is
>     intended
>     >>
>     >> to prioritize the MAC address exposed by DPDK over the one provided by
>     the
>     >>
>     >> QEMU command line? Sounds reasonable in principle, but I would get
>     confirmation
>     >>
>     >> from vDPA/vhost-net maintainers.
>      
>     >I think the best way is to (and it seems easier)
>      
>     >1) have the management layer to make sure the mac came from cli
>     >matches the underlayer vDPA
> 
>      Agreed, that's no problem.
> 
>     >2) having a sanity check and fail the device initialization if they don't
>     match
> 
>     However, one MAC address for integrity check is from the cli, and the other
>     MAC address is from the vDPA device, 
>     How to get it?
> 
>     The current situation is if MAC came from cli don't match the underlayer
>     vDPA, the virtual machine can still start without any hints.
> 
>     Thanks
> 
> 

Let me ask then, why are you supplying the incorrect MAC on the cli?

-- 
MST

Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically

[Alex Bennée]

chenh <chenh@yusur.tech> writes:

> From: Hao Chen <chenh@yusur.tech>
>
> When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
> start the virtual machine through libvirt or qemu, but now, the libvirt or
> qemu can call dpdk vdpa vendor driver's ops .get_config through vhost_net_get_config
> to get the mac address of the vdpa hardware without manual configuration.
>
> v1->v2:
> Only copy ETH_ALEN data of netcfg for some vdpa device such as
> NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.
> We only need the mac address and don't care about the status field.
>
> Signed-off-by: Hao Chen <chenh@yusur.tech>
> ---
>  hw/block/vhost-user-blk.c |  1 -
>  hw/net/virtio-net.c       |  7 +++++++
>  hw/virtio/vhost-user.c    | 19 -------------------
>  3 files changed, 7 insertions(+), 20 deletions(-)
>
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 9117222456..5dca4eab09 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
>  
>      vhost_dev_set_config_notifier(&s->dev, &blk_ops);
>  
> -    s->vhost_user.supports_config = true;

<snip>

NACK from me. The supports_config flag is there for a reason.

>  
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index bd24741be8..8b01078249 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>      }
>  
>      if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
> -        bool supports_f_config = vus->supports_config ||
> -            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
>          uint64_t protocol_features;
>  
>          dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
> @@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>           */
>          protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
>  
> -        if (supports_f_config) {
> -            if (!virtio_has_feature(protocol_features,
> -                                    VHOST_USER_PROTOCOL_F_CONFIG)) {
> -                error_setg(errp, "vhost-user device expecting "
> -                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
> -                           "not support it.");
> -                return -EPROTO;
> -            }
> -        } else {
> -            if (virtio_has_feature(protocol_features,
> -                                   VHOST_USER_PROTOCOL_F_CONFIG)) {
> -                warn_reportf_err(*errp, "vhost-user backend supports "
> -                                 "VHOST_USER_PROTOCOL_F_CONFIG but QEMU does not.");
> -                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
> -            }
> -        }
> -
>          /* final set of protocol features */
>          dev->protocol_features = protocol_features;
>          err = vhost_user_set_protocol_features(dev, dev->protocol_features);


-- 
Alex Bennée

Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically

[Michael S. Tsirkin]

On Wed, Sep 21, 2022 at 07:23:12PM +0100, Alex Bennée wrote:
> 
> chenh <chenh@yusur.tech> writes:
> 
> > From: Hao Chen <chenh@yusur.tech>
> >
> > When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
> > start the virtual machine through libvirt or qemu, but now, the libvirt or
> > qemu can call dpdk vdpa vendor driver's ops .get_config through vhost_net_get_config
> > to get the mac address of the vdpa hardware without manual configuration.
> >
> > v1->v2:
> > Only copy ETH_ALEN data of netcfg for some vdpa device such as
> > NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.
> > We only need the mac address and don't care about the status field.
> >
> > Signed-off-by: Hao Chen <chenh@yusur.tech>
> > ---
> >  hw/block/vhost-user-blk.c |  1 -
> >  hw/net/virtio-net.c       |  7 +++++++
> >  hw/virtio/vhost-user.c    | 19 -------------------
> >  3 files changed, 7 insertions(+), 20 deletions(-)
> >
> > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > index 9117222456..5dca4eab09 100644
> > --- a/hw/block/vhost-user-blk.c
> > +++ b/hw/block/vhost-user-blk.c
> > @@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
> >  
> >      vhost_dev_set_config_notifier(&s->dev, &blk_ops);
> >  
> > -    s->vhost_user.supports_config = true;
> 
> <snip>
> 
> NACK from me. The supports_config flag is there for a reason.


Alex please, do not send NACKs. If you feel compelled to stress
your point, provide extra justification instead. Thanks!

> >  
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index bd24741be8..8b01078249 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> >      }
> >  
> >      if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
> > -        bool supports_f_config = vus->supports_config ||
> > -            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
> >          uint64_t protocol_features;
> >  
> >          dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
> > @@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> >           */
> >          protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
> >  
> > -        if (supports_f_config) {
> > -            if (!virtio_has_feature(protocol_features,
> > -                                    VHOST_USER_PROTOCOL_F_CONFIG)) {
> > -                error_setg(errp, "vhost-user device expecting "
> > -                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
> > -                           "not support it.");
> > -                return -EPROTO;
> > -            }
> > -        } else {
> > -            if (virtio_has_feature(protocol_features,
> > -                                   VHOST_USER_PROTOCOL_F_CONFIG)) {
> > -                warn_reportf_err(*errp, "vhost-user backend supports "
> > -                                 "VHOST_USER_PROTOCOL_F_CONFIG but QEMU does not.");
> > -                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
> > -            }
> > -        }
> > -
> >          /* final set of protocol features */
> >          dev->protocol_features = protocol_features;
> >          err = vhost_user_set_protocol_features(dev, dev->protocol_features);
> 
> 
> -- 
> Alex Bennée

Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically

[Alex Bennée]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Wed, Sep 21, 2022 at 07:23:12PM +0100, Alex Bennée wrote:
>> 
>> chenh <chenh@yusur.tech> writes:
>> 
>> > From: Hao Chen <chenh@yusur.tech>
>> >
>> > When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
>> > start the virtual machine through libvirt or qemu, but now, the libvirt or
>> > qemu can call dpdk vdpa vendor driver's ops .get_config through vhost_net_get_config
>> > to get the mac address of the vdpa hardware without manual configuration.
>> >
>> > v1->v2:
>> > Only copy ETH_ALEN data of netcfg for some vdpa device such as
>> > NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.
>> > We only need the mac address and don't care about the status field.
>> >
>> > Signed-off-by: Hao Chen <chenh@yusur.tech>
>> > ---
>> >  hw/block/vhost-user-blk.c |  1 -
>> >  hw/net/virtio-net.c       |  7 +++++++
>> >  hw/virtio/vhost-user.c    | 19 -------------------
>> >  3 files changed, 7 insertions(+), 20 deletions(-)
>> >
>> > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
>> > index 9117222456..5dca4eab09 100644
>> > --- a/hw/block/vhost-user-blk.c
>> > +++ b/hw/block/vhost-user-blk.c
>> > @@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
>> >  
>> >      vhost_dev_set_config_notifier(&s->dev, &blk_ops);
>> >  
>> > -    s->vhost_user.supports_config = true;
>> 
>> <snip>
>> 
>> NACK from me. The supports_config flag is there for a reason.
>
>
> Alex please, do not send NACKs. If you feel compelled to stress
> your point, provide extra justification instead. Thanks!

OK I was objecting to ripping out the common vhost-user code which was
implemented as a fix for behaviour found while attempting to upstream:

  Subject: [PATCH  v4 for 7.2 00/22] virtio-gpio and various virtio cleanups
  Date: Tue,  2 Aug 2022 10:49:48 +0100
  Message-Id: <20220802095010.3330793-1-alex.bennee@linaro.org>

I vhost-user-blk wants to suppress its use of vhost-user config messages
I guess that should be a control option but it sounds like a buggy
back-end.

>
>> >  
>> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>> > index bd24741be8..8b01078249 100644
>> > --- a/hw/virtio/vhost-user.c
>> > +++ b/hw/virtio/vhost-user.c
>> > @@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>> >      }
>> >  
>> >      if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
>> > -        bool supports_f_config = vus->supports_config ||
>> > -            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
>> >          uint64_t protocol_features;
>> >  
>> >          dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
>> > @@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>> >           */
>> >          protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
>> >  
>> > -        if (supports_f_config) {
>> > -            if (!virtio_has_feature(protocol_features,
>> > -                                    VHOST_USER_PROTOCOL_F_CONFIG)) {
>> > -                error_setg(errp, "vhost-user device expecting "
>> > -                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
>> > -                           "not support it.");
>> > -                return -EPROTO;
>> > -            }
>> > -        } else {
>> > -            if (virtio_has_feature(protocol_features,
>> > -                                   VHOST_USER_PROTOCOL_F_CONFIG)) {
>> > -                warn_reportf_err(*errp, "vhost-user backend supports "
>> > -                                 "VHOST_USER_PROTOCOL_F_CONFIG but QEMU does not.");
>> > -                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
>> > -            }
>> > -        }
>> > -
>> >          /* final set of protocol features */
>> >          dev->protocol_features = protocol_features;
>> >          err = vhost_user_set_protocol_features(dev, dev->protocol_features);
>> 
>> 
>> -- 
>> Alex Bennée


-- 
Alex Bennée

Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically

[Michael S. Tsirkin]

On Thu, Sep 22, 2022 at 11:02:56AM +0100, Alex Bennée wrote:
> 
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Wed, Sep 21, 2022 at 07:23:12PM +0100, Alex Bennée wrote:
> >> 
> >> chenh <chenh@yusur.tech> writes:
> >> 
> >> > From: Hao Chen <chenh@yusur.tech>
> >> >
> >> > When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
> >> > start the virtual machine through libvirt or qemu, but now, the libvirt or
> >> > qemu can call dpdk vdpa vendor driver's ops .get_config through vhost_net_get_config
> >> > to get the mac address of the vdpa hardware without manual configuration.
> >> >
> >> > v1->v2:
> >> > Only copy ETH_ALEN data of netcfg for some vdpa device such as
> >> > NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.
> >> > We only need the mac address and don't care about the status field.
> >> >
> >> > Signed-off-by: Hao Chen <chenh@yusur.tech>
> >> > ---
> >> >  hw/block/vhost-user-blk.c |  1 -
> >> >  hw/net/virtio-net.c       |  7 +++++++
> >> >  hw/virtio/vhost-user.c    | 19 -------------------
> >> >  3 files changed, 7 insertions(+), 20 deletions(-)
> >> >
> >> > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> >> > index 9117222456..5dca4eab09 100644
> >> > --- a/hw/block/vhost-user-blk.c
> >> > +++ b/hw/block/vhost-user-blk.c
> >> > @@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
> >> >  
> >> >      vhost_dev_set_config_notifier(&s->dev, &blk_ops);
> >> >  
> >> > -    s->vhost_user.supports_config = true;
> >> 
> >> <snip>
> >> 
> >> NACK from me. The supports_config flag is there for a reason.
> >
> >
> > Alex please, do not send NACKs. If you feel compelled to stress
> > your point, provide extra justification instead. Thanks!
> 
> OK I was objecting to ripping out the common vhost-user code which was
> implemented as a fix for behaviour found while attempting to upstream:
> 
>   Subject: [PATCH  v4 for 7.2 00/22] virtio-gpio and various virtio cleanups
>   Date: Tue,  2 Aug 2022 10:49:48 +0100
>   Message-Id: <20220802095010.3330793-1-alex.bennee@linaro.org>
> 
> I vhost-user-blk wants to suppress its use of vhost-user config messages
> I guess that should be a control option but it sounds like a buggy
> back-end.

Thanks for the review!

> >
> >> >  
> >> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> >> > index bd24741be8..8b01078249 100644
> >> > --- a/hw/virtio/vhost-user.c
> >> > +++ b/hw/virtio/vhost-user.c
> >> > @@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> >> >      }
> >> >  
> >> >      if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
> >> > -        bool supports_f_config = vus->supports_config ||
> >> > -            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
> >> >          uint64_t protocol_features;
> >> >  
> >> >          dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
> >> > @@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> >> >           */
> >> >          protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
> >> >  
> >> > -        if (supports_f_config) {
> >> > -            if (!virtio_has_feature(protocol_features,
> >> > -                                    VHOST_USER_PROTOCOL_F_CONFIG)) {
> >> > -                error_setg(errp, "vhost-user device expecting "
> >> > -                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
> >> > -                           "not support it.");
> >> > -                return -EPROTO;
> >> > -            }
> >> > -        } else {
> >> > -            if (virtio_has_feature(protocol_features,
> >> > -                                   VHOST_USER_PROTOCOL_F_CONFIG)) {
> >> > -                warn_reportf_err(*errp, "vhost-user backend supports "
> >> > -                                 "VHOST_USER_PROTOCOL_F_CONFIG but QEMU does not.");
> >> > -                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
> >> > -            }
> >> > -        }
> >> > -
> >> >          /* final set of protocol features */
> >> >          dev->protocol_features = protocol_features;
> >> >          err = vhost_user_set_protocol_features(dev, dev->protocol_features);
> >> 
> >> 
> >> -- 
> >> Alex Bennée
> 
> 
> -- 
> Alex Bennée

Re: [PATCH v2] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically

[陈浩]

On 2022/9/22 18:19, Michael S. Tsirkin wrote:
> On Thu, Sep 22, 2022 at 11:02:56AM +0100, Alex Bennée wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>>
>>> On Wed, Sep 21, 2022 at 07:23:12PM +0100, Alex Bennée wrote:
>>>> chenh <chenh@yusur.tech> writes:
>>>>
>>>>> From: Hao Chen <chenh@yusur.tech>
>>>>>
>>>>> When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
>>>>> start the virtual machine through libvirt or qemu, but now, the libvirt or
>>>>> qemu can call dpdk vdpa vendor driver's ops .get_config through vhost_net_get_config
>>>>> to get the mac address of the vdpa hardware without manual configuration.
>>>>>
>>>>> v1->v2:
>>>>> Only copy ETH_ALEN data of netcfg for some vdpa device such as
>>>>> NVIDIA BLUEFIELD DPU(BF2)'s netcfg->status is not right.
>>>>> We only need the mac address and don't care about the status field.
>>>>>
>>>>> Signed-off-by: Hao Chen <chenh@yusur.tech>
>>>>> ---
>>>>>   hw/block/vhost-user-blk.c |  1 -
>>>>>   hw/net/virtio-net.c       |  7 +++++++
>>>>>   hw/virtio/vhost-user.c    | 19 -------------------
>>>>>   3 files changed, 7 insertions(+), 20 deletions(-)
>>>>>
>>>>> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
>>>>> index 9117222456..5dca4eab09 100644
>>>>> --- a/hw/block/vhost-user-blk.c
>>>>> +++ b/hw/block/vhost-user-blk.c
>>>>> @@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
>>>>>   
>>>>>       vhost_dev_set_config_notifier(&s->dev, &blk_ops);
>>>>>   
>>>>> -    s->vhost_user.supports_config = true;
>>>> <snip>
>>>>
>>>> NACK from me. The supports_config flag is there for a reason.
>>>
>>> Alex please, do not send NACKs. If you feel compelled to stress
>>> your point, provide extra justification instead. Thanks!
>> OK I was objecting to ripping out the common vhost-user code which was
>> implemented as a fix for behaviour found while attempting to upstream:
>>
>>    Subject: [PATCH  v4 for 7.2 00/22] virtio-gpio and various virtio cleanups
>>    Date: Tue,  2 Aug 2022 10:49:48 +0100
>>    Message-Id: <20220802095010.3330793-1-alex.bennee@linaro.org>
>>
>> I vhost-user-blk wants to suppress its use of vhost-user config messages
>> I guess that should be a control option but it sounds like a buggy
>> back-end.
> Thanks for the review!

QEMU needs to obtain the mac address of the underlying vdpa hardware 
through the 'vhost_user_get_config' function, but this part of the check 
of virtio_blk makes 'vhost_user_get_config' unusable in virtio-net, 
because 'vhost_user_get_config' depends on the 
VHOST_USER_PROTOCOL_F_CONFIG feature, which is the key point of the 
problem.

Currently I only need to use 'vhost_user_get_config' function, it seems 
not necessary to implement dev->config_ops && 
dev->config_ops->vhost_dev_config_notifier in virtio-net.

Can this part of the checks be moved elsewhere? I don't know how to skip 
this part of the virtio blk related checks to achieve my functionality, 
so I removed those checks. 🙁

>
>>>>>   
>>>>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>>>>> index bd24741be8..8b01078249 100644
>>>>> --- a/hw/virtio/vhost-user.c
>>>>> +++ b/hw/virtio/vhost-user.c
>>>>> @@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>>>>>       }
>>>>>   
>>>>>       if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
>>>>> -        bool supports_f_config = vus->supports_config ||
>>>>> -            (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
>>>>>           uint64_t protocol_features;
>>>>>   
>>>>>           dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
>>>>> @@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>>>>>            */
>>>>>           protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;
>>>>>   
>>>>> -        if (supports_f_config) {
>>>>> -            if (!virtio_has_feature(protocol_features,
>>>>> -                                    VHOST_USER_PROTOCOL_F_CONFIG)) {
>>>>> -                error_setg(errp, "vhost-user device expecting "
>>>>> -                           "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
>>>>> -                           "not support it.");
>>>>> -                return -EPROTO;
>>>>> -            }
>>>>> -        } else {
>>>>> -            if (virtio_has_feature(protocol_features,
>>>>> -                                   VHOST_USER_PROTOCOL_F_CONFIG)) {
>>>>> -                warn_reportf_err(*errp, "vhost-user backend supports "
>>>>> -                                 "VHOST_USER_PROTOCOL_F_CONFIG but QEMU does not.");
>>>>> -                protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
>>>>> -            }
>>>>> -        }
>>>>> -
>>>>>           /* final set of protocol features */
>>>>>           dev->protocol_features = protocol_features;
>>>>>           err = vhost_user_set_protocol_features(dev, dev->protocol_features);
>>>>
>>>> -- 
>>>> Alex Bennée
>>
>> -- 
>> Alex Bennée