1. Patchew
  2. QEMU
  3. View series

[RFC] virtio-net: check the mac address for vdpa device

Cindy Lu posted 1 patch 4 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240709022707.579474-1-lulu@redhat.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>
There is a newer version of this series
hw/net/virtio-net.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
[RFC] virtio-net: check the mac address for vdpa device
Posted by Cindy Lu 4 months, 2 weeks ago 
When using VDPA device, we should verify whether the MAC address in the
hardware matches the MAC address from the QEMU command line. If not,
we will need to update the related information.

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 hw/net/virtio-net.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 9c7e85caea..db04331b82 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3739,10 +3739,17 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
     nc->rxfilter_notify_enabled = 1;
 
    if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
-        struct virtio_net_config netcfg = {};
-        memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
-        vhost_net_set_config(get_vhost_net(nc->peer),
-            (uint8_t *)&netcfg, 0, ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
+     struct virtio_net_config netcfg = {};
+     static const MACAddr zero = {.a = {0, 0, 0, 0, 0, 0}};
+     vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
+                          ETH_ALEN);
+     if ((memcmp(&netcfg.mac, &n->nic_conf.macaddr, sizeof(MACAddr)) != 0) &&
+         memcmp(&netcfg.mac, &zero, sizeof(zero) != 0)) {
+       memcpy(&n->nic_conf.macaddr, &netcfg.mac, sizeof(MACAddr));
+       memcpy(&n->mac[0], &netcfg.mac, sizeof(MACAddr));
+     }
+     vhost_net_set_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg, 0,
+                          ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
     }
     QTAILQ_INIT(&n->rsc_chains);
     n->qdev = dev;
-- 
2.45.0
Re: [RFC] virtio-net: check the mac address for vdpa device
Posted by Jason Wang 4 months, 2 weeks ago 
On Tue, Jul 9, 2024 at 10:27 AM Cindy Lu <lulu@redhat.com> wrote:
>
> When using VDPA device, we should verify whether the MAC address in the
> hardware matches the MAC address from the QEMU command line. If not,
> we will need to update the related information.
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>

This seems to be a workaround, for example it means the mac address
set from the qemu command line has the higher priority compared to the
one that is provisioned by the host?

At least we need to have a warning here?

Thanks

> ---
>  hw/net/virtio-net.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 9c7e85caea..db04331b82 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -3739,10 +3739,17 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
>      nc->rxfilter_notify_enabled = 1;
>
>     if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> -        struct virtio_net_config netcfg = {};
> -        memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
> -        vhost_net_set_config(get_vhost_net(nc->peer),
> -            (uint8_t *)&netcfg, 0, ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> +     struct virtio_net_config netcfg = {};
> +     static const MACAddr zero = {.a = {0, 0, 0, 0, 0, 0}};
> +     vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
> +                          ETH_ALEN);
> +     if ((memcmp(&netcfg.mac, &n->nic_conf.macaddr, sizeof(MACAddr)) != 0) &&
> +         memcmp(&netcfg.mac, &zero, sizeof(zero) != 0)) {
> +       memcpy(&n->nic_conf.macaddr, &netcfg.mac, sizeof(MACAddr));
> +       memcpy(&n->mac[0], &netcfg.mac, sizeof(MACAddr));
> +     }
> +     vhost_net_set_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg, 0,
> +                          ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
>      }
>      QTAILQ_INIT(&n->rsc_chains);
>      n->qdev = dev;
> --
> 2.45.0
>
Re: [RFC] virtio-net: check the mac address for vdpa device
Posted by Cindy Lu 4 months, 2 weeks ago 
On Tue, 9 Jul 2024 at 10:33, Jason Wang <jasowang@redhat.com> wrote:
>
> On Tue, Jul 9, 2024 at 10:27 AM Cindy Lu <lulu@redhat.com> wrote:
> >
> > When using VDPA device, we should verify whether the MAC address in the
> > hardware matches the MAC address from the QEMU command line. If not,
> > we will need to update the related information.
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
>
> This seems to be a workaround, for example it means the mac address
> set from the qemu command line has the higher priority compared to the
> one that is provisioned by the host?
>
> At least we need to have a warning here?
>
In this design, the MAC address from the host will take higher
priority over the MAC address from the command line. I'm not sure if
this is acceptable?
sure, will add some warning here
> Thanks
>
> > ---
> >  hw/net/virtio-net.c | 15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index 9c7e85caea..db04331b82 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -3739,10 +3739,17 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
> >      nc->rxfilter_notify_enabled = 1;
> >
> >     if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> > -        struct virtio_net_config netcfg = {};
> > -        memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
> > -        vhost_net_set_config(get_vhost_net(nc->peer),
> > -            (uint8_t *)&netcfg, 0, ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> > +     struct virtio_net_config netcfg = {};
> > +     static const MACAddr zero = {.a = {0, 0, 0, 0, 0, 0}};
> > +     vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
> > +                          ETH_ALEN);
> > +     if ((memcmp(&netcfg.mac, &n->nic_conf.macaddr, sizeof(MACAddr)) != 0) &&
> > +         memcmp(&netcfg.mac, &zero, sizeof(zero) != 0)) {
> > +       memcpy(&n->nic_conf.macaddr, &netcfg.mac, sizeof(MACAddr));
> > +       memcpy(&n->mac[0], &netcfg.mac, sizeof(MACAddr));
> > +     }
> > +     vhost_net_set_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg, 0,
> > +                          ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> >      }
> >      QTAILQ_INIT(&n->rsc_chains);
> >      n->qdev = dev;
> > --
> > 2.45.0
> >
>
Re: [RFC] virtio-net: check the mac address for vdpa device
Posted by Jason Wang 4 months, 2 weeks ago 
On Tue, Jul 9, 2024 at 10:41 AM Cindy Lu <lulu@redhat.com> wrote:
>
> On Tue, 9 Jul 2024 at 10:33, Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Tue, Jul 9, 2024 at 10:27 AM Cindy Lu <lulu@redhat.com> wrote:
> > >
> > > When using VDPA device, we should verify whether the MAC address in the
> > > hardware matches the MAC address from the QEMU command line. If not,
> > > we will need to update the related information.
> > >
> > > Signed-off-by: Cindy Lu <lulu@redhat.com>
> >
> > This seems to be a workaround, for example it means the mac address
> > set from the qemu command line has the higher priority compared to the
> > one that is provisioned by the host?
> >
> > At least we need to have a warning here?
> >
> In this design, the MAC address from the host will take higher
> priority over the MAC address from the command line. I'm not sure if
> this is acceptable?

That's fine but it seems not what I read for this patch?

As you try to set config, or anything I missed here?

Thanks

> sure, will add some warning here
> > Thanks
> >
> > > ---
> > >  hw/net/virtio-net.c | 15 +++++++++++----
> > >  1 file changed, 11 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > > index 9c7e85caea..db04331b82 100644
> > > --- a/hw/net/virtio-net.c
> > > +++ b/hw/net/virtio-net.c
> > > @@ -3739,10 +3739,17 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
> > >      nc->rxfilter_notify_enabled = 1;
> > >
> > >     if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> > > -        struct virtio_net_config netcfg = {};
> > > -        memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
> > > -        vhost_net_set_config(get_vhost_net(nc->peer),
> > > -            (uint8_t *)&netcfg, 0, ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> > > +     struct virtio_net_config netcfg = {};
> > > +     static const MACAddr zero = {.a = {0, 0, 0, 0, 0, 0}};
> > > +     vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
> > > +                          ETH_ALEN);
> > > +     if ((memcmp(&netcfg.mac, &n->nic_conf.macaddr, sizeof(MACAddr)) != 0) &&
> > > +         memcmp(&netcfg.mac, &zero, sizeof(zero) != 0)) {
> > > +       memcpy(&n->nic_conf.macaddr, &netcfg.mac, sizeof(MACAddr));
> > > +       memcpy(&n->mac[0], &netcfg.mac, sizeof(MACAddr));
> > > +     }
> > > +     vhost_net_set_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg, 0,
> > > +                          ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> > >      }
> > >      QTAILQ_INIT(&n->rsc_chains);
> > >      n->qdev = dev;
> > > --
> > > 2.45.0
> > >
> >
>
Re: [RFC] virtio-net: check the mac address for vdpa device
Posted by Cindy Lu 4 months, 2 weeks ago 
On Tue, 9 Jul 2024 at 10:47, Jason Wang <jasowang@redhat.com> wrote:
>
> On Tue, Jul 9, 2024 at 10:41 AM Cindy Lu <lulu@redhat.com> wrote:
> >
> > On Tue, 9 Jul 2024 at 10:33, Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > On Tue, Jul 9, 2024 at 10:27 AM Cindy Lu <lulu@redhat.com> wrote:
> > > >
> > > > When using VDPA device, we should verify whether the MAC address in the
> > > > hardware matches the MAC address from the QEMU command line. If not,
> > > > we will need to update the related information.
> > > >
> > > > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > >
> > > This seems to be a workaround, for example it means the mac address
> > > set from the qemu command line has the higher priority compared to the
> > > one that is provisioned by the host?
> > >
> > > At least we need to have a warning here?
> > >
> > In this design, the MAC address from the host will take higher
> > priority over the MAC address from the command line. I'm not sure if
> > this is acceptable?
>
> That's fine but it seems not what I read for this patch?
>
> As you try to set config, or anything I missed here?
>
> Thanks
>
yes I use   vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
 ETH_ALEN); to get the mac  from hardware and copy it
to n->nic_conf.macaddr and n->mac
  memcpy(&n->nic_conf.macaddr, &netcfg.mac, sizeof(MACAddr));
  memcpy(&n->mac[0], &netcfg.mac, sizeof(MACAddr));
qemu will use this information for the following steps

Thanks
cindy

> > sure, will add some warning here
> > > Thanks
> > >
> > > > ---
> > > >  hw/net/virtio-net.c | 15 +++++++++++----
> > > >  1 file changed, 11 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > > > index 9c7e85caea..db04331b82 100644
> > > > --- a/hw/net/virtio-net.c
> > > > +++ b/hw/net/virtio-net.c
> > > > @@ -3739,10 +3739,17 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
> > > >      nc->rxfilter_notify_enabled = 1;
> > > >
> > > >     if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> > > > -        struct virtio_net_config netcfg = {};
> > > > -        memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
> > > > -        vhost_net_set_config(get_vhost_net(nc->peer),
> > > > -            (uint8_t *)&netcfg, 0, ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> > > > +     struct virtio_net_config netcfg = {};
> > > > +     static const MACAddr zero = {.a = {0, 0, 0, 0, 0, 0}};
> > > > +     vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
> > > > +                          ETH_ALEN);
> > > > +     if ((memcmp(&netcfg.mac, &n->nic_conf.macaddr, sizeof(MACAddr)) != 0) &&
> > > > +         memcmp(&netcfg.mac, &zero, sizeof(zero) != 0)) {
> > > > +       memcpy(&n->nic_conf.macaddr, &netcfg.mac, sizeof(MACAddr));
> > > > +       memcpy(&n->mac[0], &netcfg.mac, sizeof(MACAddr));
> > > > +     }
> > > > +     vhost_net_set_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg, 0,
> > > > +                          ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> > > >      }
> > > >      QTAILQ_INIT(&n->rsc_chains);
> > > >      n->qdev = dev;
> > > > --
> > > > 2.45.0
> > > >
> > >
> >
>
Re: [RFC] virtio-net: check the mac address for vdpa device
Posted by Jason Wang 4 months, 2 weeks ago 
On Tue, Jul 9, 2024 at 10:56 AM Cindy Lu <lulu@redhat.com> wrote:
>
> On Tue, 9 Jul 2024 at 10:47, Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Tue, Jul 9, 2024 at 10:41 AM Cindy Lu <lulu@redhat.com> wrote:
> > >
> > > On Tue, 9 Jul 2024 at 10:33, Jason Wang <jasowang@redhat.com> wrote:
> > > >
> > > > On Tue, Jul 9, 2024 at 10:27 AM Cindy Lu <lulu@redhat.com> wrote:
> > > > >
> > > > > When using VDPA device, we should verify whether the MAC address in the
> > > > > hardware matches the MAC address from the QEMU command line. If not,
> > > > > we will need to update the related information.
> > > > >
> > > > > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > > >
> > > > This seems to be a workaround, for example it means the mac address
> > > > set from the qemu command line has the higher priority compared to the
> > > > one that is provisioned by the host?
> > > >
> > > > At least we need to have a warning here?
> > > >
> > > In this design, the MAC address from the host will take higher
> > > priority over the MAC address from the command line. I'm not sure if
> > > this is acceptable?
> >
> > That's fine but it seems not what I read for this patch?
> >
> > As you try to set config, or anything I missed here?
> >
> > Thanks
> >
> yes I use   vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
>  ETH_ALEN); to get the mac  from hardware and copy it
> to n->nic_conf.macaddr and n->mac
>   memcpy(&n->nic_conf.macaddr, &netcfg.mac, sizeof(MACAddr));
>   memcpy(&n->mac[0], &netcfg.mac, sizeof(MACAddr));
> qemu will use this information for the following steps

I think we need to explain:

1) why do we need a get_config in realization, or what happens if we
don't do that. (AFAIK we will call get_config when a guest is trying
to read the config space).
2) why do we need a set_config, what happens if we don't do that.

Thanks

>
> Thanks
> cindy
>
> > > sure, will add some warning here
> > > > Thanks
> > > >
> > > > > ---
> > > > >  hw/net/virtio-net.c | 15 +++++++++++----
> > > > >  1 file changed, 11 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > > > > index 9c7e85caea..db04331b82 100644
> > > > > --- a/hw/net/virtio-net.c
> > > > > +++ b/hw/net/virtio-net.c
> > > > > @@ -3739,10 +3739,17 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
> > > > >      nc->rxfilter_notify_enabled = 1;
> > > > >
> > > > >     if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> > > > > -        struct virtio_net_config netcfg = {};
> > > > > -        memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
> > > > > -        vhost_net_set_config(get_vhost_net(nc->peer),
> > > > > -            (uint8_t *)&netcfg, 0, ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> > > > > +     struct virtio_net_config netcfg = {};
> > > > > +     static const MACAddr zero = {.a = {0, 0, 0, 0, 0, 0}};
> > > > > +     vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
> > > > > +                          ETH_ALEN);
> > > > > +     if ((memcmp(&netcfg.mac, &n->nic_conf.macaddr, sizeof(MACAddr)) != 0) &&
> > > > > +         memcmp(&netcfg.mac, &zero, sizeof(zero) != 0)) {
> > > > > +       memcpy(&n->nic_conf.macaddr, &netcfg.mac, sizeof(MACAddr));
> > > > > +       memcpy(&n->mac[0], &netcfg.mac, sizeof(MACAddr));
> > > > > +     }
> > > > > +     vhost_net_set_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg, 0,
> > > > > +                          ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> > > > >      }
> > > > >      QTAILQ_INIT(&n->rsc_chains);
> > > > >      n->qdev = dev;
> > > > > --
> > > > > 2.45.0
> > > > >
> > > >
> > >
> >
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.