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

[Cindy Lu]

On Thu, Sep 24, 2020 at 3:18 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 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
>
Sure, Thanks Jason I will remove this
>
> >>> +            }
> >>>            }
> >>>        }
> >>>    }
> >>> @@ -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;
> >>>
> >
>