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

[PATCH v3 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.Here we add Add a new parameter to 
enable this check.
There are only three acceptable situations for MAC setup; any other
configuration will fail to boot.

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

tested by ConnectX-6 Dx 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

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 the 2rd acceptable situation for Mac setup.
  virtio_net: Add the 3rd acceptable situation 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 v3 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,macstrickcheck=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 c8f679761b..e00651a97b 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 46b02c50be..071c3ff065 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -1860,6 +1860,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->macstrickcheck;
     }
 
     if (has_cvq) {
@@ -1872,6 +1874,8 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char *name,
                                  errp);
         if (!nc)
             goto err;
+
+        nc->check_mac = opts->macstrickcheck;
     }
 
     return 0;
diff --git a/qapi/net.json b/qapi/net.json
index 87fc0d0b28..7d75119858 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)
 #
+# @macstrickcheck: 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',
+    '*macstrickcheck':    'bool',
     '*x-svq':        {'type': 'bool', 'features' : [ 'unstable'] } } }
 
 ##
-- 
2.45.0

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

[Michael S. Tsirkin]

On Sat, Oct 26, 2024 at 03:59:59PM +0800, Cindy Lu wrote:
> 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,macstrickcheck=true\

typos in command line are not welcome.
you should also separate words e.g. by dashes.
Are there more options than strict?
Also if not strict, should we still warn?


> -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 c8f679761b..e00651a97b 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 46b02c50be..071c3ff065 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -1860,6 +1860,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->macstrickcheck;
>      }
>  
>      if (has_cvq) {
> @@ -1872,6 +1874,8 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char *name,
>                                   errp);
>          if (!nc)
>              goto err;
> +
> +        nc->check_mac = opts->macstrickcheck;
>      }
>  
>      return 0;
> diff --git a/qapi/net.json b/qapi/net.json
> index 87fc0d0b28..7d75119858 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)
>  #
> +# @macstrickcheck: 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',
> +    '*macstrickcheck':    'bool',
>      '*x-svq':        {'type': 'bool', 'features' : [ 'unstable'] } } }
>  
>  ##
> -- 
> 2.45.0

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

[Cindy Lu]

On Wed, Nov 6, 2024 at 5:25 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Sat, Oct 26, 2024 at 03:59:59PM +0800, Cindy Lu wrote:
> > 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,macstrickcheck=true\
>
> typos in command line are not welcome.
> you should also separate words e.g. by dashes.
> Are there more options than strict?
> Also if not strict, should we still warn?
>
There is no other check for this, Thank you, Michael. I will change
this to a new name.
Thanks
Cindy
>
> > -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 c8f679761b..e00651a97b 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 46b02c50be..071c3ff065 100644
> > --- a/net/vhost-vdpa.c
> > +++ b/net/vhost-vdpa.c
> > @@ -1860,6 +1860,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->macstrickcheck;
> >      }
> >
> >      if (has_cvq) {
> > @@ -1872,6 +1874,8 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char *name,
> >                                   errp);
> >          if (!nc)
> >              goto err;
> > +
> > +        nc->check_mac = opts->macstrickcheck;
> >      }
> >
> >      return 0;
> > diff --git a/qapi/net.json b/qapi/net.json
> > index 87fc0d0b28..7d75119858 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)
> >  #
> > +# @macstrickcheck: 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',
> > +    '*macstrickcheck':    'bool',
> >      '*x-svq':        {'type': 'bool', 'features' : [ 'unstable'] } } }
> >
> >  ##
> > --
> > 2.45.0
>

[PATCH v3 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 fb84d142ee..0b342d0cc2 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3589,12 +3589,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) {
@@ -3702,6 +3733,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;
@@ -3748,7 +3780,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 v3 3/4] virtio_net: Add the 2rd acceptable situation for Mac setup.

[Cindy Lu]

When using a VDPA device, the following situations are
also acceptable: the hardware MAC address is not 0,
and the MAC address in the QEMU command line is 0.

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 0b342d0cc2..6900e3c44b 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3607,6 +3607,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 v3 4/4] virtio_net: Add the 3rd acceptable situation for Mac setup.

[Cindy Lu]

While the hardware MAC address is 0 and the MAC address in
the QEMU command line is also 0, this configuration is
acceptable.

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 6900e3c44b..1e6a31baf8 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3622,6 +3622,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