[PATCH v5 0/4] virtio_net: Add the check for vdpa's mac address

[PATCH v5 0/4] virtio_net: Add the check for vdpa's mac address

[Cindy Lu]

When using a VDPA device, it is important to ensure that the MAC address
is correctly set. In this patch series, we add a new parameter to
enable this check.
Only three MAC setup configurations are acceptable; any other will
fail to boot.

The usage is:
....
-netdev type=vhost-vdpa,vhostdev=/dev/vhost-vdpa-0,id=vhost-vdpa0,check-mac=true\
-device virtio-net-pci,netdev=vhost-vdpa0\
....

tested by ConnectX-6 Dx/vdpa_sim device

change in v3
1. add a new parameter to enable the check and keep the old behavior
2. adjust the comment and make it more clear

change in v4
1. change the new parameter's name to check-mac
2. change the comment and make it more clear

change in v5
1.These patches haven't been merged for a while, so I rebased
  them with the latest code and resubmitted

Cindy Lu (4):
  vhost_vdpa : Add a new parameter to enable check mac address
  virtio_net: Add the check for vdpa's mac address
  virtio_net: Add second acceptable configuration for MAC setup
  virtio_net: Add third acceptable configuration for MAC setup.

 hw/net/virtio-net.c | 67 ++++++++++++++++++++++++++++++++++++++++++++-
 include/net/net.h   |  1 +
 net/vhost-vdpa.c    |  4 +++
 qapi/net.json       |  5 ++++
 4 files changed, 76 insertions(+), 1 deletion(-)

-- 
2.45.0

[PATCH v5 1/4] vhost_vdpa : Add a new parameter to enable check mac address

[Cindy Lu]

When using a VDPA device, it's important to ensure that the MAC
address is correctly set.
Add a new parameter in qemu cmdline to enable this check, default value
is false

The usage is:
....
-netdev type=vhost-vdpa,vhostdev=/dev/vhost-vdpa-0,id=vhost-vdpa0,check-mac=true\
-device virtio-net-pci,netdev=vhost-vdpa0\
....

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 include/net/net.h | 1 +
 net/vhost-vdpa.c  | 4 ++++
 qapi/net.json     | 5 +++++
 3 files changed, 10 insertions(+)

diff --git a/include/net/net.h b/include/net/net.h
index cdd5b109b0..fac1951b6e 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -112,6 +112,7 @@ struct NetClientState {
     bool is_netdev;
     bool do_not_pad; /* do not pad to the minimum ethernet frame length */
     bool is_datapath;
+    bool check_mac;
     QTAILQ_HEAD(, NetFilterState) filters;
 };
 
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 7ca8b46eee..ba1da31741 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -1870,6 +1870,8 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char *name,
                                      iova_range, features, shared, errp);
         if (!ncs[i])
             goto err;
+
+        ncs[i]->check_mac = opts->check_mac;
     }
 
     if (has_cvq) {
@@ -1882,6 +1884,8 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char *name,
                                  errp);
         if (!nc)
             goto err;
+
+        nc->check_mac = opts->check_mac;
     }
 
     return 0;
diff --git a/qapi/net.json b/qapi/net.json
index 310cc4fd19..a5c70d1df8 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -510,6 +510,10 @@
 # @queues: number of queues to be created for multiqueue vhost-vdpa
 #     (default: 1)
 #
+# @check-mac: Enable the check for whether the device's MAC address
+#     and the MAC in QEMU command line are acceptable for booting.
+#     (default: false)
+#
 # @x-svq: Start device with (experimental) shadow virtqueue.  (Since
 #     7.1) (default: false)
 #
@@ -524,6 +528,7 @@
     '*vhostdev':     'str',
     '*vhostfd':      'str',
     '*queues':       'int',
+    '*check-mac':    'bool',
     '*x-svq':        {'type': 'bool', 'features' : [ 'unstable'] } } }
 
 ##
-- 
2.45.0

[PATCH v5 2/4] virtio_net: Add the check for vdpa's mac address

[Cindy Lu]

When using a VDPA device, it is important to ensure that the MAC
address is correctly set. The MAC address in the hardware should
match the MAC address from the QEMU command line. This is a recommended
configuration and will allow the system to boot.

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

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index de87cfadff..a3b431e000 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3749,12 +3749,43 @@ 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 following situations are acceptable:*/
+
+    if (memcmp(&hwcfg.mac, &zero, sizeof(MACAddr)) != 0) {
+        /*
+         * 1.The hardware MAC address is the same as the QEMU command line MAC
+         *   address, and both of them are not 0.
+         */
+        if ((memcmp(&hwcfg.mac, cmdline_mac, sizeof(MACAddr)) == 0)) {
+            return true;
+        }
+    }
 
+    error_setg(errp,
+               "vDPA device's mac %02x:%02x:%02x:%02x:%02x:%02x"
+               "not same with the cmdline's mac %02x:%02x:%02x:%02x:%02x:%02x,"
+               "Please check.",
+               hwcfg.mac[0], hwcfg.mac[1], hwcfg.mac[2], hwcfg.mac[3],
+               hwcfg.mac[4], hwcfg.mac[5], cmdline_mac->a[0], cmdline_mac->a[1],
+               cmdline_mac->a[2], cmdline_mac->a[3], cmdline_mac->a[4],
+               cmdline_mac->a[5]);
+
+    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) {
@@ -3862,6 +3893,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;
@@ -3908,7 +3940,13 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
     nc = qemu_get_queue(n->nic);
     nc->rxfilter_notify_enabled = 1;
 
-   if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
+    if (nc->peer && (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA)) {
+        if (nc->peer->check_mac) {
+            if (!virtio_net_check_vdpa_mac(nc, n, &macaddr_cmdline, errp)) {
+                virtio_cleanup(vdev);
+                return;
+            }
+        }
         struct virtio_net_config netcfg = {};
         memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
         vhost_net_set_config(get_vhost_net(nc->peer),
-- 
2.45.0

[PATCH v5 3/4] virtio_net: Add second acceptable configuration for MAC setup

[Cindy Lu]

For VDPA devices, Allow configurations where the hardware MAC address
is non-zero while the MAC address in the QEMU command line is zero.

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

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index a3b431e000..1fd0403d5d 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3767,6 +3767,20 @@ static bool virtio_net_check_vdpa_mac(NetClientState *nc, VirtIONet *n,
         if ((memcmp(&hwcfg.mac, cmdline_mac, sizeof(MACAddr)) == 0)) {
             return true;
         }
+        /*
+         * 2.The hardware MAC address is NOT 0,
+         *  and the MAC address in the QEMU command line is 0.
+         *  In this situation, Here we use the hardware MAC address overwrite
+         *  the QEMU command line address(is 0) in VirtIONet->mac[0].
+         *  in the follwoing process, QEMU will use this mac in VirtIONet and
+         *  finish the bring up
+         */
+        if (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,
-- 
2.45.0

[PATCH v5 4/4] virtio_net: Add third acceptable configuration for MAC setup.

[Cindy Lu]

For VDPA devices, Allow configurations where both the hardware MAC address
and QEMU command line MAC address are zero.

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

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 1fd0403d5d..d1f44850d5 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3782,6 +3782,19 @@ static bool virtio_net_check_vdpa_mac(NetClientState *nc, VirtIONet *n,
             return true;
         }
     }
+    /*
+     * 3.The hardware MAC address is 0,
+     *  and the MAC address in the QEMU command line is also 0.
+     *  In this situation, qemu will generate a random mac address
+     *  QEMU will try to use CVQ/set_config to set this address to
+     *  device
+     */
+    if ((memcmp(&hwcfg.mac, &zero, sizeof(MACAddr)) == 0) &&
+        (memcmp(cmdline_mac, &zero, sizeof(MACAddr)) == 0)) {
+        memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac));
+
+        return true;
+    }
 
     error_setg(errp,
                "vDPA device's mac %02x:%02x:%02x:%02x:%02x:%02x"
-- 
2.45.0

Re: [PATCH v5 2/4] virtio_net: Add the check for vdpa's mac address

[Lei Yang]

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

QE tested this series of patches with virtio-net regression tests,
everything works fine.

Tested-by: Lei Yang <leiyang@redhat.com>

On Wed, Mar 26, 2025 at 9:21 PM Cindy Lu <lulu@redhat.com> wrote:

> When using a VDPA device, it is important to ensure that the MAC
> address is correctly set. The MAC address in the hardware should
> match the MAC address from the QEMU command line. This is a recommended
> configuration and will allow the system to boot.
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>  hw/net/virtio-net.c | 40 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index de87cfadff..a3b431e000 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -3749,12 +3749,43 @@ 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 following situations are acceptable:*/
> +
> +    if (memcmp(&hwcfg.mac, &zero, sizeof(MACAddr)) != 0) {
> +        /*
> +         * 1.The hardware MAC address is the same as the QEMU command
> line MAC
> +         *   address, and both of them are not 0.
> +         */
> +        if ((memcmp(&hwcfg.mac, cmdline_mac, sizeof(MACAddr)) == 0)) {
> +            return true;
> +        }
> +    }
>
> +    error_setg(errp,
> +               "vDPA device's mac %02x:%02x:%02x:%02x:%02x:%02x"
> +               "not same with the cmdline's mac
> %02x:%02x:%02x:%02x:%02x:%02x,"
> +               "Please check.",
> +               hwcfg.mac[0], hwcfg.mac[1], hwcfg.mac[2], hwcfg.mac[3],
> +               hwcfg.mac[4], hwcfg.mac[5], cmdline_mac->a[0],
> cmdline_mac->a[1],
> +               cmdline_mac->a[2], cmdline_mac->a[3], cmdline_mac->a[4],
> +               cmdline_mac->a[5]);
> +
> +    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) {
> @@ -3862,6 +3893,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;
> @@ -3908,7 +3940,13 @@ static void virtio_net_device_realize(DeviceState
> *dev, Error **errp)
>      nc = qemu_get_queue(n->nic);
>      nc->rxfilter_notify_enabled = 1;
>
> -   if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> +    if (nc->peer && (nc->peer->info->type ==
> NET_CLIENT_DRIVER_VHOST_VDPA)) {
> +        if (nc->peer->check_mac) {
> +            if (!virtio_net_check_vdpa_mac(nc, n, &macaddr_cmdline,
> errp)) {
> +                virtio_cleanup(vdev);
> +                return;
> +            }
> +        }
>          struct virtio_net_config netcfg = {};
>          memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
>          vhost_net_set_config(get_vhost_net(nc->peer),
> --
> 2.45.0
>
>
>

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

Re: [PATCH v5 2/4] virtio_net: Add the check for vdpa's mac address

[Michael S. Tsirkin]

On Wed, Mar 26, 2025 at 09:19:31PM +0800, Cindy Lu wrote:
> When using a VDPA device, it is important to ensure that the MAC
> address is correctly set. The MAC address in the hardware should
> match the MAC address from the QEMU command line. This is a recommended
> configuration and will allow the system to boot.
> 
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>  hw/net/virtio-net.c | 40 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index de87cfadff..a3b431e000 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -3749,12 +3749,43 @@ 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 following situations are acceptable:*/


/* This is how you format comments in QEMU */



/*Never like this*/

> +    if (memcmp(&hwcfg.mac, &zero, sizeof(MACAddr)) != 0) {
> +        /*
> +         * 1.The hardware MAC address is the same as the QEMU command line MAC

space after .

> +         *   address, and both of them are not 0.
> +         */
> +        if ((memcmp(&hwcfg.mac, cmdline_mac, sizeof(MACAddr)) == 0)) {
> +            return true;
> +        }
> +    }
>  
> +    error_setg(errp,
> +               "vDPA device's mac %02x:%02x:%02x:%02x:%02x:%02x"
> +               "not same with the cmdline's mac %02x:%02x:%02x:%02x:%02x:%02x,"

the same with the command line mac (avoid abbreviation)

> +               "Please check.",

space after , and no uppercase

> +               hwcfg.mac[0], hwcfg.mac[1], hwcfg.mac[2], hwcfg.mac[3],
> +               hwcfg.mac[4], hwcfg.mac[5], cmdline_mac->a[0], cmdline_mac->a[1],
> +               cmdline_mac->a[2], cmdline_mac->a[3], cmdline_mac->a[4],
> +               cmdline_mac->a[5]);


check what?  maybe "initialization failed"?

> +
> +    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) {
> @@ -3862,6 +3893,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;
> @@ -3908,7 +3940,13 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
>      nc = qemu_get_queue(n->nic);
>      nc->rxfilter_notify_enabled = 1;
>  
> -   if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> +    if (nc->peer && (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA)) {
> +        if (nc->peer->check_mac) {
> +            if (!virtio_net_check_vdpa_mac(nc, n, &macaddr_cmdline, errp)) {
> +                virtio_cleanup(vdev);
> +                return;
> +            }
> +        }
>          struct virtio_net_config netcfg = {};
>          memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
>          vhost_net_set_config(get_vhost_net(nc->peer),
> -- 
> 2.45.0

Re: [PATCH v5 3/4] virtio_net: Add second acceptable configuration for MAC setup

[Michael S. Tsirkin]

On Wed, Mar 26, 2025 at 09:19:32PM +0800, Cindy Lu wrote:
> For VDPA devices, Allow configurations where the hardware MAC address
> is non-zero while the MAC address in the QEMU command line is zero.
> 
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>  hw/net/virtio-net.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index a3b431e000..1fd0403d5d 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -3767,6 +3767,20 @@ static bool virtio_net_check_vdpa_mac(NetClientState *nc, VirtIONet *n,
>          if ((memcmp(&hwcfg.mac, cmdline_mac, sizeof(MACAddr)) == 0)) {
>              return true;
>          }
> +        /*
> +         * 2.The hardware MAC address is NOT 0,

space after .

> +         *  and the MAC address in the QEMU command line is 0.
> +         *  In this situation, Here we use the hardware MAC address overwrite
> +         *  the QEMU command line address(is 0) in VirtIONet->mac[0].

drop "here" and add punctiation. period before overwrite maybe?
what is (is 0)?

> +         *  in the follwoing process, QEMU will use this mac in VirtIONet and
> +         *  finish the bring up
> +         */
> +        if (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,
> -- 
> 2.45.0

Re: [PATCH v5 4/4] virtio_net: Add third acceptable configuration for MAC setup.

[Michael S. Tsirkin]

On Wed, Mar 26, 2025 at 09:19:33PM +0800, Cindy Lu wrote:
> For VDPA devices, Allow configurations where both the hardware MAC address
> and QEMU command line MAC address are zero.
> 
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>  hw/net/virtio-net.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 1fd0403d5d..d1f44850d5 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -3782,6 +3782,19 @@ static bool virtio_net_check_vdpa_mac(NetClientState *nc, VirtIONet *n,
>              return true;
>          }
>      }
> +    /*
> +     * 3.The hardware MAC address is 0,
> +     *  and the MAC address in the QEMU command line is also 0.
> +     *  In this situation, qemu will generate a random mac address
> +     *  QEMU will try to use CVQ/set_config to set this address to
> +     *  device

same comments. end sentences with a period.

> +     */
> +    if ((memcmp(&hwcfg.mac, &zero, sizeof(MACAddr)) == 0) &&
> +        (memcmp(cmdline_mac, &zero, sizeof(MACAddr)) == 0)) {
> +        memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac));
> +
> +        return true;
> +    }
>  
>      error_setg(errp,
>                 "vDPA device's mac %02x:%02x:%02x:%02x:%02x:%02x"
> -- 
> 2.45.0

Re: [PATCH v5 0/4] virtio_net: Add the check for vdpa's mac address

[Michael S. Tsirkin]

On Wed, Mar 26, 2025 at 09:19:29PM +0800, Cindy Lu wrote:
> When using a VDPA device, it is important to ensure that the MAC address
> is correctly set. In this patch series, we add a new parameter to
> enable this check.
> Only three MAC setup configurations are acceptable; any other will
> fail to boot.
> 
> The usage is:
> ....
> -netdev type=vhost-vdpa,vhostdev=/dev/vhost-vdpa-0,id=vhost-vdpa0,check-mac=true\
> -device virtio-net-pci,netdev=vhost-vdpa0\
> ....
> 
> tested by ConnectX-6 Dx/vdpa_sim device
> 
> change in v3
> 1. add a new parameter to enable the check and keep the old behavior
> 2. adjust the comment and make it more clear
> 
> change in v4
> 1. change the new parameter's name to check-mac
> 2. change the comment and make it more clear
> 
> change in v5
> 1.These patches haven't been merged for a while, so I rebased
>   them with the latest code and resubmitted
> 
> Cindy Lu (4):
>   vhost_vdpa : Add a new parameter to enable check mac address
>   virtio_net: Add the check for vdpa's mac address
>   virtio_net: Add second acceptable configuration for MAC setup
>   virtio_net: Add third acceptable configuration for MAC setup.
> 
>  hw/net/virtio-net.c | 67 ++++++++++++++++++++++++++++++++++++++++++++-
>  include/net/net.h   |  1 +
>  net/vhost-vdpa.c    |  4 +++
>  qapi/net.json       |  5 ++++
>  4 files changed, 76 insertions(+), 1 deletion(-)


some minor nits.

> -- 
> 2.45.0

Re: [PATCH v5 3/4] virtio_net: Add second acceptable configuration for MAC setup

[Cindy Lu]

On Thu, Apr 3, 2025 at 12:35 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Mar 26, 2025 at 09:19:32PM +0800, Cindy Lu wrote:
> > For VDPA devices, Allow configurations where the hardware MAC address
> > is non-zero while the MAC address in the QEMU command line is zero.
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> >  hw/net/virtio-net.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index a3b431e000..1fd0403d5d 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -3767,6 +3767,20 @@ static bool virtio_net_check_vdpa_mac(NetClientState *nc, VirtIONet *n,
> >          if ((memcmp(&hwcfg.mac, cmdline_mac, sizeof(MACAddr)) == 0)) {
> >              return true;
> >          }
> > +        /*
> > +         * 2.The hardware MAC address is NOT 0,
>
> space after .
>
will fix this
> > +         *  and the MAC address in the QEMU command line is 0.
> > +         *  In this situation, Here we use the hardware MAC address overwrite
> > +         *  the QEMU command line address(is 0) in VirtIONet->mac[0].
>
> drop "here" and add punctiation. period before overwrite maybe?
> what is (is 0)?
>
Sure, will change this
Thanks
cindy
> > +         *  in the follwoing process, QEMU will use this mac in VirtIONet and
> > +         *  finish the bring up
> > +         */
> > +        if (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,
> > --
> > 2.45.0
>

Re: [PATCH v5 4/4] virtio_net: Add third acceptable configuration for MAC setup.

[Cindy Lu]

On Thu, Apr 3, 2025 at 12:36 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Mar 26, 2025 at 09:19:33PM +0800, Cindy Lu wrote:
> > For VDPA devices, Allow configurations where both the hardware MAC address
> > and QEMU command line MAC address are zero.
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> >  hw/net/virtio-net.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index 1fd0403d5d..d1f44850d5 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -3782,6 +3782,19 @@ static bool virtio_net_check_vdpa_mac(NetClientState *nc, VirtIONet *n,
> >              return true;
> >          }
> >      }
> > +    /*
> > +     * 3.The hardware MAC address is 0,
> > +     *  and the MAC address in the QEMU command line is also 0.
> > +     *  In this situation, qemu will generate a random mac address
> > +     *  QEMU will try to use CVQ/set_config to set this address to
> > +     *  device
>
> same comments. end sentences with a period.
>
sure. will fix this
Thanks
cindy
> > +     */
> > +    if ((memcmp(&hwcfg.mac, &zero, sizeof(MACAddr)) == 0) &&
> > +        (memcmp(cmdline_mac, &zero, sizeof(MACAddr)) == 0)) {
> > +        memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac));
> > +
> > +        return true;
> > +    }
> >
> >      error_setg(errp,
> >                 "vDPA device's mac %02x:%02x:%02x:%02x:%02x:%02x"
> > --
> > 2.45.0
>

Re: [PATCH v5 2/4] virtio_net: Add the check for vdpa's mac address

[Cindy Lu]

On Thu, Apr 3, 2025 at 12:34 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Mar 26, 2025 at 09:19:31PM +0800, Cindy Lu wrote:
> > When using a VDPA device, it is important to ensure that the MAC
> > address is correctly set. The MAC address in the hardware should
> > match the MAC address from the QEMU command line. This is a recommended
> > configuration and will allow the system to boot.
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> >  hw/net/virtio-net.c | 40 +++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 39 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index de87cfadff..a3b431e000 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -3749,12 +3749,43 @@ 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 following situations are acceptable:*/
>
>
> /* This is how you format comments in QEMU */
>
>
>
> /*Never like this*/
>
Will fix this

> > +    if (memcmp(&hwcfg.mac, &zero, sizeof(MACAddr)) != 0) {
> > +        /*
> > +         * 1.The hardware MAC address is the same as the QEMU command line MAC
>
> space after .
>
will fix this
> > +         *   address, and both of them are not 0.
> > +         */
> > +        if ((memcmp(&hwcfg.mac, cmdline_mac, sizeof(MACAddr)) == 0)) {
> > +            return true;
> > +        }
> > +    }
> >
> > +    error_setg(errp,
> > +               "vDPA device's mac %02x:%02x:%02x:%02x:%02x:%02x"
> > +               "not same with the cmdline's mac %02x:%02x:%02x:%02x:%02x:%02x,"
>
> the same with the command line mac (avoid abbreviation)
>
> > +               "Please check.",
>
> space after , and no uppercase
>
> > +               hwcfg.mac[0], hwcfg.mac[1], hwcfg.mac[2], hwcfg.mac[3],
> > +               hwcfg.mac[4], hwcfg.mac[5], cmdline_mac->a[0], cmdline_mac->a[1],
> > +               cmdline_mac->a[2], cmdline_mac->a[3], cmdline_mac->a[4],
> > +               cmdline_mac->a[5]);
>
>
> check what?  maybe "initialization failed"?
>
sure, will change these
thanks
cindy
> > +
> > +    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) {
> > @@ -3862,6 +3893,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;
> > @@ -3908,7 +3940,13 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
> >      nc = qemu_get_queue(n->nic);
> >      nc->rxfilter_notify_enabled = 1;
> >
> > -   if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> > +    if (nc->peer && (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA)) {
> > +        if (nc->peer->check_mac) {
> > +            if (!virtio_net_check_vdpa_mac(nc, n, &macaddr_cmdline, errp)) {
> > +                virtio_cleanup(vdev);
> > +                return;
> > +            }
> > +        }
> >          struct virtio_net_config netcfg = {};
> >          memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
> >          vhost_net_set_config(get_vhost_net(nc->peer),
> > --
> > 2.45.0
>