Re: [PATCH 1/3] virtio-net: Set mac address to hardware if the peer is vdpa

[Cindy Lu]

On Tue, Sep 22, 2020 at 9:55 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2020/9/17 下午11:58, Cindy Lu wrote:
> > If the peer's type is vdpa,set the mac address to NIC in 
> > virtio_net_device_realize,
> > Also sometime vdpa get an all 0 macaddress from the hardware, this will 
> > cause the traffic down
> > So we add the check for this part.
> > if we get an 0 mac address we will use the default mac address instead
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> >   hw/net/virtio-net.c | 12 +++++++++++-
> >   1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index cb0d27084c..7db9da1482 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -126,6 +126,7 @@ static void virtio_net_get_config(VirtIODevice *vdev, 
> > uint8_t *config)
> >       VirtIONet *n = VIRTIO_NET(vdev);
> >       struct virtio_net_config netcfg;
> >       NetClientState *nc = qemu_get_queue(n->nic);
> > +    static const MACAddr zero = { .a = { 0, 0, 0, 0, 0, 0 } };
> >
> >       int ret = 0;
> >       memset(&netcfg, 0 , sizeof(struct virtio_net_config));
> > @@ -151,7 +152,11 @@ static void virtio_net_get_config(VirtIODevice *vdev, 
> > uint8_t *config)
> >           ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t 
> > *)&netcfg,
> >                                      n->config_size);
> >           if (ret != -1) {
> > -            memcpy(config, &netcfg, n->config_size);
> > +            if (memcmp(&netcfg.mac, &zero, sizeof(zero)) != 0) {
> > +                memcpy(config, &netcfg, n->config_size);
> > +        } else {
> > +                error_report("Get an all zero mac address from hardware");
>
>
> This is probably a hint that MAC is not properly provisioned.
>
> So I guess we can leave this as is, or simply warn until the management
> interface is finalized.
>
Hi Jason, For sure this is NIC card's problem, They cannot provide an
correct MAC address,
But if we continue use this 0 mac address will cause this traffic
down, maybe will cost a lot of effort in debugging
So I think maybe Just an warn is not enough, We can use the default
mac address  and let the traffic working
>
> > +            }
> >           }
> >       }
> >   }
> > @@ -3399,6 +3404,11 @@ 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) {
> > +        struct virtio_net_config netcfg = {};
> > +        memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
> > +        virtio_net_set_config(vdev, (uint8_t *)&netcfg);
>
>
> Won't this overwrite all other fields in the netcfg? I think we should
> only touch mac part.
>
> Thanks
>
>
Sure, will fix this
> > +    }
> >       QTAILQ_INIT(&n->rsc_chains);
> >       n->qdev = dev;
> >
>