Re: [RFC] virtio-net: check the mac address for vdpa device

[Jason Wang]

On Tue, Jul 9, 2024 at 10:56 AM Cindy Lu <lulu@redhat.com> wrote:
>
> On Tue, 9 Jul 2024 at 10:47, Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Tue, Jul 9, 2024 at 10:41 AM Cindy Lu <lulu@redhat.com> wrote:
> > >
> > > On Tue, 9 Jul 2024 at 10:33, Jason Wang <jasowang@redhat.com> wrote:
> > > >
> > > > On Tue, Jul 9, 2024 at 10:27 AM Cindy Lu <lulu@redhat.com> wrote:
> > > > >
> > > > > When using VDPA device, we should verify whether the MAC address in 
> > > > > the
> > > > > hardware matches the MAC address from the QEMU command line. If not,
> > > > > we will need to update the related information.
> > > > >
> > > > > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > > >
> > > > This seems to be a workaround, for example it means the mac address
> > > > set from the qemu command line has the higher priority compared to the
> > > > one that is provisioned by the host?
> > > >
> > > > At least we need to have a warning here?
> > > >
> > > In this design, the MAC address from the host will take higher
> > > priority over the MAC address from the command line. I'm not sure if
> > > this is acceptable?
> >
> > That's fine but it seems not what I read for this patch?
> >
> > As you try to set config, or anything I missed here?
> >
> > Thanks
> >
> yes I use   vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
>  ETH_ALEN); to get the mac  from hardware and copy it
> to n->nic_conf.macaddr and n->mac
>   memcpy(&n->nic_conf.macaddr, &netcfg.mac, sizeof(MACAddr));
>   memcpy(&n->mac[0], &netcfg.mac, sizeof(MACAddr));
> qemu will use this information for the following steps

I think we need to explain:

1) why do we need a get_config in realization, or what happens if we
don't do that. (AFAIK we will call get_config when a guest is trying
to read the config space).
2) why do we need a set_config, what happens if we don't do that.

Thanks

>
> Thanks
> cindy
>
> > > sure, will add some warning here
> > > > Thanks
> > > >
> > > > > ---
> > > > >  hw/net/virtio-net.c | 15 +++++++++++----
> > > > >  1 file changed, 11 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > > > > index 9c7e85caea..db04331b82 100644
> > > > > --- a/hw/net/virtio-net.c
> > > > > +++ b/hw/net/virtio-net.c
> > > > > @@ -3739,10 +3739,17 @@ 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);
> > > > > +     struct virtio_net_config netcfg = {};
> > > > > +     static const MACAddr zero = {.a = {0, 0, 0, 0, 0, 0}};
> > > > > +     vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t 
> > > > > *)&netcfg,
> > > > > +                          ETH_ALEN);
> > > > > +     if ((memcmp(&netcfg.mac, &n->nic_conf.macaddr, sizeof(MACAddr)) 
> > > > > != 0) &&
> > > > > +         memcmp(&netcfg.mac, &zero, sizeof(zero) != 0)) {
> > > > > +       memcpy(&n->nic_conf.macaddr, &netcfg.mac, sizeof(MACAddr));
> > > > > +       memcpy(&n->mac[0], &netcfg.mac, sizeof(MACAddr));
> > > > > +     }
> > > > > +     vhost_net_set_config(get_vhost_net(nc->peer), (uint8_t 
> > > > > *)&netcfg, 0,
> > > > > +                          ETH_ALEN, VHOST_SET_CONFIG_TYPE_FRONTEND);
> > > > >      }
> > > > >      QTAILQ_INIT(&n->rsc_chains);
> > > > >      n->qdev = dev;
> > > > > --
> > > > > 2.45.0
> > > > >
> > > >
> > >
> >
>