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

[Jason Wang]

On 2020/9/22 上午11:01, Cindy Lu wrote:
> 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


Yes, and it's done by the following code.

But the question is there's no much value for the error here consider 
you've already had a solution.

Thanks


> > > +            }
> > >            }
> > >        }
> > >    }
> > > @@ -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;