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

[陈浩 <chenh@yusur.tech>]

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


在 2022/9/21 13:28, Raphael Norwitz 写道:
>
> I have some concerns from the vhost-user-blk side.
>
> >On Tue, Sep 13, 2022 at 5:13 PM Hao Chen <chenh@yusur.tech> wrote:
>
> >>
>
> >> 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.
>
> >>
>
> >> Signed-off-by: Hao Chen <chenh@yusur.tech>
>
> >
>
> >Adding Cindy for comments.
>
> >
>
> >Thanks
>
> >
>
> >> ---
>
> >>hw/block/vhost-user-blk.c |1 -
>
> >>hw/net/virtio-net.c |3 ++-
>
> >>hw/virtio/vhost-user.c| 19 -------------------
>
> >>3 files changed, 2 insertions(+), 21 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;
>
> vhost-user-blk requires the backend to support 
> VHOST_USER_PROTOCOL_F_CONFIG
>
> and vhost_user.supports_config is used to enforce that.
>
> Why are you removing it here?
>
> >>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..274ea84644 100644
>
> >> --- a/hw/net/virtio-net.c
>
> >> +++ b/hw/net/virtio-net.c
>
> >> @@ -149,7 +149,8 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
>
> >> * Is this VDPA? No peer means not VDPA: there's no way to
>
> >> * disconnect/reconnect a VDPA peer.
>
> >> */
>
> >> -if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
>
> >> +if ((nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) ||
>
> >> +(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) {
>
> >> 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,
>
> >>}
>
> >>
>
> Why are you removing this? Can you expand on how it helps dpdk-vdpa.
>
I implemented a function by modifying these places in the patch, and 
then implementing vhost msg ops '.get_config' in the driver under 
'DPDK_SRC/drivers/vdpa/' , qemu can use 'vhost_user_get_config' to send 
vhost msg to calls the dpdk vdpa vendor driver's ops 'get_config' to 
obtain the mac address information of the vdpa device. In this way, even 
if the mac address of the network device parameters configured by the 
user for qemu vhost user is inconsistent with the mac address of the 
vdpa hardware, qemu can be corrected to be consistent with the mac 
address of the vdpa hardware.

When vhost-user interface is used by dpdk-vdpa & qemu, 
'vhost_user_get_config' will get the vdpa device's mac address if dpdk 
vdpa vendor driver implement 'get_config',otherwise, it won't do anything.

When vhost-user interface is used by dpdk vhost-user, it won't do anything.

I think the functions provided by the VHOST_USER_PROTOCOL_F_CONFIG 
feature can be available separately, but the check here caused 
VHOST_USER_PROTOCOL_F_CONFIG feature is cleared, which will cause 
vhost_user_get_config is not working properly in virtio-net at 
hw/net/virtio_net.c.

Maybe the check here should be moved other side?
The new patch v2 has been sent, please reply there.

> >>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: 19114 bytes --]