When using a VDPA device, it is important to ensure that the MAC address
in the hardware matches the MAC address from the QEMU command line.

There are only two acceptable situations:
1. The hardware MAC address is the same as the MAC address specified in the QEMU
command line, and both MAC addresses are not 0.
2. The hardware MAC address is not 0, and the MAC address in the QEMU command line is 0.
In this situation, the hardware MAC address will overwrite the QEMU command line address.

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

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 9c7e85caea..8f79785f59 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -178,8 +178,8 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
          * correctly elsewhere - just not reported by the device.
          */
         if (memcmp(&netcfg.mac, &zero, sizeof(zero)) == 0) {
-            info_report("Zero hardware mac address detected. Ignoring.");
-            memcpy(netcfg.mac, n->mac, ETH_ALEN);
+          error_report("Zero hardware mac address detected in vdpa device. "
+                       "please check the vdpa device!");
         }
 
         netcfg.status |= virtio_tswap16(vdev,
@@ -3579,12 +3579,42 @@ static bool failover_hide_primary_device(DeviceListener *listener,
     /* failover_primary_hidden is set during feature negotiation */
     return qatomic_read(&n->failover_primary_hidden);
 }
+static bool virtio_net_check_vdpa_mac(NetClientState *nc, VirtIONet *n,
+                                      MACAddr *cmdline_mac, Error **errp) {
+  struct virtio_net_config hwcfg = {};
+  static const MACAddr zero = {.a = {0, 0, 0, 0, 0, 0}};
 
+  vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&hwcfg, ETH_ALEN);
+
+  /* For VDPA device: Only two situations are acceptable:
+   * 1.The hardware MAC address is the same as the QEMU command line MAC
+   *   address, and both of them are not 0.
+   * 2.The hardware MAC address is NOT 0, and the QEMU command line MAC address
+   *   is 0. In this situation, the hardware MAC address will overwrite the QEMU
+   *   command line address.
+   */
+
+  if (memcmp(&hwcfg.mac, &zero, sizeof(MACAddr)) != 0) {
+    if ((memcmp(&hwcfg.mac, cmdline_mac, sizeof(MACAddr)) == 0) ||
+        (memcmp(cmdline_mac, &zero, sizeof(MACAddr)) == 0)) {
+      /* overwrite the mac address with hardware address*/
+      memcpy(&n->mac[0], &hwcfg.mac, sizeof(n->mac));
+      memcpy(&n->nic_conf.macaddr, &hwcfg.mac, sizeof(n->mac));
+
+      return true;
+    }
+  }
+  error_setg(errp, "vdpa hardware mac != the mac address from "
+                   "qemu cmdline, please check the the vdpa device's setting.");
+
+  return false;
+}
 static void virtio_net_device_realize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIONet *n = VIRTIO_NET(dev);
     NetClientState *nc;
+    MACAddr macaddr_cmdline;
     int i;
 
     if (n->net_conf.mtu) {
@@ -3692,6 +3722,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
     virtio_net_add_queue(n, 0);
 
     n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);
+    memcpy(&macaddr_cmdline, &n->nic_conf.macaddr, sizeof(n->mac));
     qemu_macaddr_default_if_unset(&n->nic_conf.macaddr);
     memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac));
     n->status = VIRTIO_NET_S_LINK_UP;
@@ -3739,10 +3770,10 @@ 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);
+     if (!virtio_net_check_vdpa_mac(nc, n, &macaddr_cmdline, errp)) {
+       virtio_cleanup(vdev);
+       return;
+     }
     }
     QTAILQ_INIT(&n->rsc_chains);
     n->qdev = dev;
-- 
2.45.0

On Tue, Jul 16, 2024 at 9:14 AM Cindy Lu <lulu@redhat.com> wrote:
>
> When using a VDPA device, it is important to ensure that the MAC address
> in the hardware matches the MAC address from the QEMU command line.
>
> There are only two acceptable situations:
> 1. The hardware MAC address is the same as the MAC address specified in the QEMU
> command line, and both MAC addresses are not 0.
> 2. The hardware MAC address is not 0, and the MAC address in the QEMU command line is 0.
> In this situation, the hardware MAC address will overwrite the QEMU command line address.

If this patch tries to do the above two, I'd suggest splitting it into
two patches.

>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>  hw/net/virtio-net.c | 43 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 37 insertions(+), 6 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 9c7e85caea..8f79785f59 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -178,8 +178,8 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
>           * correctly elsewhere - just not reported by the device.
>           */
>          if (memcmp(&netcfg.mac, &zero, sizeof(zero)) == 0) {
> -            info_report("Zero hardware mac address detected. Ignoring.");
> -            memcpy(netcfg.mac, n->mac, ETH_ALEN);
> +          error_report("Zero hardware mac address detected in vdpa device. "
> +                       "please check the vdpa device!");

I had two questions:

1) any reason to do this check while the guest is running?
2) I think we need a workaround for this, unless I miss something.

>          }
>
>          netcfg.status |= virtio_tswap16(vdev,
> @@ -3579,12 +3579,42 @@ static bool failover_hide_primary_device(DeviceListener *listener,
>      /* failover_primary_hidden is set during feature negotiation */
>      return qatomic_read(&n->failover_primary_hidden);
>  }
> +static bool virtio_net_check_vdpa_mac(NetClientState *nc, VirtIONet *n,
> +                                      MACAddr *cmdline_mac, Error **errp) {
> +  struct virtio_net_config hwcfg = {};
> +  static const MACAddr zero = {.a = {0, 0, 0, 0, 0, 0}};
>
> +  vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&hwcfg, ETH_ALEN);
> +
> +  /* For VDPA device: Only two situations are acceptable:
> +   * 1.The hardware MAC address is the same as the QEMU command line MAC
> +   *   address, and both of them are not 0.
> +   * 2.The hardware MAC address is NOT 0, and the QEMU command line MAC address
> +   *   is 0.

Did you mean -device virtio-net-pci,macaddr=0 ? Or you mean mac
address is not specified in the qemu command line?

> In this situation, the hardware MAC address will overwrite the QEMU
> +   *   command line address.
> +   */
> +
> +  if (memcmp(&hwcfg.mac, &zero, sizeof(MACAddr)) != 0) {
> +    if ((memcmp(&hwcfg.mac, cmdline_mac, sizeof(MACAddr)) == 0) ||
> +        (memcmp(cmdline_mac, &zero, sizeof(MACAddr)) == 0)) {
> +      /* overwrite the mac address with hardware address*/
> +      memcpy(&n->mac[0], &hwcfg.mac, sizeof(n->mac));
> +      memcpy(&n->nic_conf.macaddr, &hwcfg.mac, sizeof(n->mac));
> +
> +      return true;
> +    }
> +  }
> +  error_setg(errp, "vdpa hardware mac != the mac address from "
> +                   "qemu cmdline, please check the the vdpa device's setting.");
> +
> +  return false;
> +}
>  static void virtio_net_device_realize(DeviceState *dev, Error **errp)
>  {
>      VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>      VirtIONet *n = VIRTIO_NET(dev);
>      NetClientState *nc;
> +    MACAddr macaddr_cmdline;
>      int i;
>
>      if (n->net_conf.mtu) {
> @@ -3692,6 +3722,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
>      virtio_net_add_queue(n, 0);
>
>      n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);
> +    memcpy(&macaddr_cmdline, &n->nic_conf.macaddr, sizeof(n->mac));
>      qemu_macaddr_default_if_unset(&n->nic_conf.macaddr);
>      memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac));
>      n->status = VIRTIO_NET_S_LINK_UP;
> @@ -3739,10 +3770,10 @@ 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);
> +     if (!virtio_net_check_vdpa_mac(nc, n, &macaddr_cmdline, errp)) {
> +       virtio_cleanup(vdev);
> +       return;
> +     }
>      }
>      QTAILQ_INIT(&n->rsc_chains);
>      n->qdev = dev;
> --
> 2.45.0
>

Thanks

On Tue, 16 Jul 2024 at 13:37, Jason Wang <jasowang@redhat.com> wrote:
>
> On Tue, Jul 16, 2024 at 9:14 AM Cindy Lu <lulu@redhat.com> wrote:
> >
> > When using a VDPA device, it is important to ensure that the MAC address
> > in the hardware matches the MAC address from the QEMU command line.
> >
> > There are only two acceptable situations:
> > 1. The hardware MAC address is the same as the MAC address specified in the QEMU
> > command line, and both MAC addresses are not 0.
> > 2. The hardware MAC address is not 0, and the MAC address in the QEMU command line is 0.
> > In this situation, the hardware MAC address will overwrite the QEMU command line address.
>
> If this patch tries to do the above two, I'd suggest splitting it into
> two patches.
>
This code is very simple. So I have put these two into one function.
thanks
cindy
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> >  hw/net/virtio-net.c | 43 +++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 37 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index 9c7e85caea..8f79785f59 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -178,8 +178,8 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
> >           * correctly elsewhere - just not reported by the device.
> >           */
> >          if (memcmp(&netcfg.mac, &zero, sizeof(zero)) == 0) {
> > -            info_report("Zero hardware mac address detected. Ignoring.");
> > -            memcpy(netcfg.mac, n->mac, ETH_ALEN);
> > +          error_report("Zero hardware mac address detected in vdpa device. "
> > +                       "please check the vdpa device!");
>
> I had two questions:
>
> 1) any reason to do this check while the guest is running?
> 2) I think we need a workaround for this, unless I miss something.
>
this is a code change to adjust the new fix. If the mac address is 0
the guest should fail
to load. Maybe I can just assert fail here?
Thanks
cindy
> >          }
> >
> >          netcfg.status |= virtio_tswap16(vdev,
> > @@ -3579,12 +3579,42 @@ static bool failover_hide_primary_device(DeviceListener *listener,
> >      /* failover_primary_hidden is set during feature negotiation */
> >      return qatomic_read(&n->failover_primary_hidden);
> >  }
> > +static bool virtio_net_check_vdpa_mac(NetClientState *nc, VirtIONet *n,
> > +                                      MACAddr *cmdline_mac, Error **errp) {
> > +  struct virtio_net_config hwcfg = {};
> > +  static const MACAddr zero = {.a = {0, 0, 0, 0, 0, 0}};
> >
> > +  vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&hwcfg, ETH_ALEN);
> > +
> > +  /* For VDPA device: Only two situations are acceptable:
> > +   * 1.The hardware MAC address is the same as the QEMU command line MAC
> > +   *   address, and both of them are not 0.
> > +   * 2.The hardware MAC address is NOT 0, and the QEMU command line MAC address
> > +   *   is 0.
>
> Did you mean -device virtio-net-pci,macaddr=0 ? Or you mean mac
> address is not specified in the qemu command line?
>
yes, what I mean is mac address not specified, sorry for the confusion,
I will rewrite this part
Thanks
cindy

> > In this situation, the hardware MAC address will overwrite the QEMU
> > +   *   command line address.
> > +   */
> > +
> > +  if (memcmp(&hwcfg.mac, &zero, sizeof(MACAddr)) != 0) {
> > +    if ((memcmp(&hwcfg.mac, cmdline_mac, sizeof(MACAddr)) == 0) ||
> > +        (memcmp(cmdline_mac, &zero, sizeof(MACAddr)) == 0)) {
> > +      /* overwrite the mac address with hardware address*/
> > +      memcpy(&n->mac[0], &hwcfg.mac, sizeof(n->mac));
> > +      memcpy(&n->nic_conf.macaddr, &hwcfg.mac, sizeof(n->mac));
> > +
> > +      return true;
> > +    }
> > +  }
> > +  error_setg(errp, "vdpa hardware mac != the mac address from "
> > +                   "qemu cmdline, please check the the vdpa device's setting.");
> > +
> > +  return false;
> > +}
> >  static void virtio_net_device_realize(DeviceState *dev, Error **errp)
> >  {
> >      VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> >      VirtIONet *n = VIRTIO_NET(dev);
> >      NetClientState *nc;
> > +    MACAddr macaddr_cmdline;
> >      int i;
> >
> >      if (n->net_conf.mtu) {
> > @@ -3692,6 +3722,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
> >      virtio_net_add_queue(n, 0);
> >
> >      n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);
> > +    memcpy(&macaddr_cmdline, &n->nic_conf.macaddr, sizeof(n->mac));
> >      qemu_macaddr_default_if_unset(&n->nic_conf.macaddr);
> >      memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac));
> >      n->status = VIRTIO_NET_S_LINK_UP;
> > @@ -3739,10 +3770,10 @@ 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);
> > +     if (!virtio_net_check_vdpa_mac(nc, n, &macaddr_cmdline, errp)) {
> > +       virtio_cleanup(vdev);
> > +       return;
> > +     }
> >      }
> >      QTAILQ_INIT(&n->rsc_chains);
> >      n->qdev = dev;
> > --
> > 2.45.0
> >
>
> Thanks
>

On Tue, Jul 16, 2024 at 2:09 PM Cindy Lu <lulu@redhat.com> wrote:
>
> On Tue, 16 Jul 2024 at 13:37, Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Tue, Jul 16, 2024 at 9:14 AM Cindy Lu <lulu@redhat.com> wrote:
> > >
> > > When using a VDPA device, it is important to ensure that the MAC address
> > > in the hardware matches the MAC address from the QEMU command line.
> > >
> > > There are only two acceptable situations:
> > > 1. The hardware MAC address is the same as the MAC address specified in the QEMU
> > > command line, and both MAC addresses are not 0.
> > > 2. The hardware MAC address is not 0, and the MAC address in the QEMU command line is 0.
> > > In this situation, the hardware MAC address will overwrite the QEMU command line address.
> >
> > If this patch tries to do the above two, I'd suggest splitting it into
> > two patches.
> >
> This code is very simple. So I have put these two into one function.
> thanks

Better to split no matter how simple it is if there are two issues.

Btw, it's better to tell what kind of setup has been tested by this,
and do we need a fix tag here as well? (Or is this needed by -stable?)

> cindy
> > >
> > > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > > ---
> > >  hw/net/virtio-net.c | 43 +++++++++++++++++++++++++++++++++++++------
> > >  1 file changed, 37 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > > index 9c7e85caea..8f79785f59 100644
> > > --- a/hw/net/virtio-net.c
> > > +++ b/hw/net/virtio-net.c
> > > @@ -178,8 +178,8 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
> > >           * correctly elsewhere - just not reported by the device.
> > >           */
> > >          if (memcmp(&netcfg.mac, &zero, sizeof(zero)) == 0) {
> > > -            info_report("Zero hardware mac address detected. Ignoring.");
> > > -            memcpy(netcfg.mac, n->mac, ETH_ALEN);
> > > +          error_report("Zero hardware mac address detected in vdpa device. "
> > > +                       "please check the vdpa device!");
> >
> > I had two questions:
> >
> > 1) any reason to do this check while the guest is running?
> > 2) I think we need a workaround for this, unless I miss something.
> >
> this is a code change to adjust the new fix. If the mac address is 0
> the guest should fail
> to load. Maybe I can just assert fail here?

I mean get_config is usually called during VM running. It's not good
to assert here and can we move the check to realize or other place
before the VM is running?

> Thanks
> cindy
> > >          }
> > >
> > >          netcfg.status |= virtio_tswap16(vdev,
> > > @@ -3579,12 +3579,42 @@ static bool failover_hide_primary_device(DeviceListener *listener,
> > >      /* failover_primary_hidden is set during feature negotiation */
> > >      return qatomic_read(&n->failover_primary_hidden);
> > >  }
> > > +static bool virtio_net_check_vdpa_mac(NetClientState *nc, VirtIONet *n,
> > > +                                      MACAddr *cmdline_mac, Error **errp) {
> > > +  struct virtio_net_config hwcfg = {};
> > > +  static const MACAddr zero = {.a = {0, 0, 0, 0, 0, 0}};
> > >
> > > +  vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&hwcfg, ETH_ALEN);
> > > +
> > > +  /* For VDPA device: Only two situations are acceptable:
> > > +   * 1.The hardware MAC address is the same as the QEMU command line MAC
> > > +   *   address, and both of them are not 0.
> > > +   * 2.The hardware MAC address is NOT 0, and the QEMU command line MAC address
> > > +   *   is 0.
> >
> > Did you mean -device virtio-net-pci,macaddr=0 ? Or you mean mac
> > address is not specified in the qemu command line?
> >
> yes, what I mean is mac address not specified, sorry for the confusion,
> I will rewrite this part
> Thanks
> cindy
>

Thanks

Hi Cindy

If needed, QE can help test this MR before merging into the master branch.

Best Regards
Lei


On Tue, Jul 16, 2024 at 9:14 AM Cindy Lu <lulu@redhat.com> wrote:
>
> When using a VDPA device, it is important to ensure that the MAC address
> in the hardware matches the MAC address from the QEMU command line.
>
> There are only two acceptable situations:
> 1. The hardware MAC address is the same as the MAC address specified in the QEMU
> command line, and both MAC addresses are not 0.
> 2. The hardware MAC address is not 0, and the MAC address in the QEMU command line is 0.
> In this situation, the hardware MAC address will overwrite the QEMU command line address.
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>  hw/net/virtio-net.c | 43 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 37 insertions(+), 6 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 9c7e85caea..8f79785f59 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -178,8 +178,8 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
>           * correctly elsewhere - just not reported by the device.
>           */
>          if (memcmp(&netcfg.mac, &zero, sizeof(zero)) == 0) {
> -            info_report("Zero hardware mac address detected. Ignoring.");
> -            memcpy(netcfg.mac, n->mac, ETH_ALEN);
> +          error_report("Zero hardware mac address detected in vdpa device. "
> +                       "please check the vdpa device!");
>          }
>
>          netcfg.status |= virtio_tswap16(vdev,
> @@ -3579,12 +3579,42 @@ static bool failover_hide_primary_device(DeviceListener *listener,
>      /* failover_primary_hidden is set during feature negotiation */
>      return qatomic_read(&n->failover_primary_hidden);
>  }
> +static bool virtio_net_check_vdpa_mac(NetClientState *nc, VirtIONet *n,
> +                                      MACAddr *cmdline_mac, Error **errp) {
> +  struct virtio_net_config hwcfg = {};
> +  static const MACAddr zero = {.a = {0, 0, 0, 0, 0, 0}};
>
> +  vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&hwcfg, ETH_ALEN);
> +
> +  /* For VDPA device: Only two situations are acceptable:
> +   * 1.The hardware MAC address is the same as the QEMU command line MAC
> +   *   address, and both of them are not 0.
> +   * 2.The hardware MAC address is NOT 0, and the QEMU command line MAC address
> +   *   is 0. In this situation, the hardware MAC address will overwrite the QEMU
> +   *   command line address.
> +   */
> +
> +  if (memcmp(&hwcfg.mac, &zero, sizeof(MACAddr)) != 0) {
> +    if ((memcmp(&hwcfg.mac, cmdline_mac, sizeof(MACAddr)) == 0) ||
> +        (memcmp(cmdline_mac, &zero, sizeof(MACAddr)) == 0)) {
> +      /* overwrite the mac address with hardware address*/
> +      memcpy(&n->mac[0], &hwcfg.mac, sizeof(n->mac));
> +      memcpy(&n->nic_conf.macaddr, &hwcfg.mac, sizeof(n->mac));
> +
> +      return true;
> +    }
> +  }
> +  error_setg(errp, "vdpa hardware mac != the mac address from "
> +                   "qemu cmdline, please check the the vdpa device's setting.");
> +
> +  return false;
> +}
>  static void virtio_net_device_realize(DeviceState *dev, Error **errp)
>  {
>      VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>      VirtIONet *n = VIRTIO_NET(dev);
>      NetClientState *nc;
> +    MACAddr macaddr_cmdline;
>      int i;
>
>      if (n->net_conf.mtu) {
> @@ -3692,6 +3722,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
>      virtio_net_add_queue(n, 0);
>
>      n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);
> +    memcpy(&macaddr_cmdline, &n->nic_conf.macaddr, sizeof(n->mac));
>      qemu_macaddr_default_if_unset(&n->nic_conf.macaddr);
>      memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac));
>      n->status = VIRTIO_NET_S_LINK_UP;
> @@ -3739,10 +3770,10 @@ 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);
> +     if (!virtio_net_check_vdpa_mac(nc, n, &macaddr_cmdline, errp)) {
> +       virtio_cleanup(vdev);
> +       return;
> +     }
>      }
>      QTAILQ_INIT(&n->rsc_chains);
>      n->qdev = dev;
> --
> 2.45.0
>
>

On Tue, 16 Jul 2024 at 09:56, Lei Yang <leiyang@redhat.com> wrote:
>
> Hi Cindy
>
> If needed, QE can help test this MR before merging into the master branch.
>
> Best Regards
> Lei
>
sure， Really thanks for your help
thanks
cindy
>
> On Tue, Jul 16, 2024 at 9:14 AM Cindy Lu <lulu@redhat.com> wrote:
> >
> > When using a VDPA device, it is important to ensure that the MAC address
> > in the hardware matches the MAC address from the QEMU command line.
> >
> > There are only two acceptable situations:
> > 1. The hardware MAC address is the same as the MAC address specified in the QEMU
> > command line, and both MAC addresses are not 0.
> > 2. The hardware MAC address is not 0, and the MAC address in the QEMU command line is 0.
> > In this situation, the hardware MAC address will overwrite the QEMU command line address.
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> >  hw/net/virtio-net.c | 43 +++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 37 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index 9c7e85caea..8f79785f59 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -178,8 +178,8 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
> >           * correctly elsewhere - just not reported by the device.
> >           */
> >          if (memcmp(&netcfg.mac, &zero, sizeof(zero)) == 0) {
> > -            info_report("Zero hardware mac address detected. Ignoring.");
> > -            memcpy(netcfg.mac, n->mac, ETH_ALEN);
> > +          error_report("Zero hardware mac address detected in vdpa device. "
> > +                       "please check the vdpa device!");
> >          }
> >
> >          netcfg.status |= virtio_tswap16(vdev,
> > @@ -3579,12 +3579,42 @@ static bool failover_hide_primary_device(DeviceListener *listener,
> >      /* failover_primary_hidden is set during feature negotiation */
> >      return qatomic_read(&n->failover_primary_hidden);
> >  }
> > +static bool virtio_net_check_vdpa_mac(NetClientState *nc, VirtIONet *n,
> > +                                      MACAddr *cmdline_mac, Error **errp) {
> > +  struct virtio_net_config hwcfg = {};
> > +  static const MACAddr zero = {.a = {0, 0, 0, 0, 0, 0}};
> >
> > +  vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&hwcfg, ETH_ALEN);
> > +
> > +  /* For VDPA device: Only two situations are acceptable:
> > +   * 1.The hardware MAC address is the same as the QEMU command line MAC
> > +   *   address, and both of them are not 0.
> > +   * 2.The hardware MAC address is NOT 0, and the QEMU command line MAC address
> > +   *   is 0. In this situation, the hardware MAC address will overwrite the QEMU
> > +   *   command line address.
> > +   */
> > +
> > +  if (memcmp(&hwcfg.mac, &zero, sizeof(MACAddr)) != 0) {
> > +    if ((memcmp(&hwcfg.mac, cmdline_mac, sizeof(MACAddr)) == 0) ||
> > +        (memcmp(cmdline_mac, &zero, sizeof(MACAddr)) == 0)) {
> > +      /* overwrite the mac address with hardware address*/
> > +      memcpy(&n->mac[0], &hwcfg.mac, sizeof(n->mac));
> > +      memcpy(&n->nic_conf.macaddr, &hwcfg.mac, sizeof(n->mac));
> > +
> > +      return true;
> > +    }
> > +  }
> > +  error_setg(errp, "vdpa hardware mac != the mac address from "
> > +                   "qemu cmdline, please check the the vdpa device's setting.");
> > +
> > +  return false;
> > +}
> >  static void virtio_net_device_realize(DeviceState *dev, Error **errp)
> >  {
> >      VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> >      VirtIONet *n = VIRTIO_NET(dev);
> >      NetClientState *nc;
> > +    MACAddr macaddr_cmdline;
> >      int i;
> >
> >      if (n->net_conf.mtu) {
> > @@ -3692,6 +3722,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
> >      virtio_net_add_queue(n, 0);
> >
> >      n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);
> > +    memcpy(&macaddr_cmdline, &n->nic_conf.macaddr, sizeof(n->mac));
> >      qemu_macaddr_default_if_unset(&n->nic_conf.macaddr);
> >      memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac));
> >      n->status = VIRTIO_NET_S_LINK_UP;
> > @@ -3739,10 +3770,10 @@ 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);
> > +     if (!virtio_net_check_vdpa_mac(nc, n, &macaddr_cmdline, errp)) {
> > +       virtio_cleanup(vdev);
> > +       return;
> > +     }
> >      }
> >      QTAILQ_INIT(&n->rsc_chains);
> >      n->qdev = dev;
> > --
> > 2.45.0
> >
> >
>