On Tue, 16 Jul 2024 at 13:37, Jason Wang <jasow...@redhat.com> wrote: > > On Tue, Jul 16, 2024 at 9:14 AM Cindy Lu <l...@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 <l...@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 >