Re: [RFC PATCH v2 3/8] vhost_net: Emulate link state up if backend doesn't expose it

[Eugenio Perez Martin <eperezma@redhat.com>]

On Thu, Oct 20, 2022 at 6:30 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Wed, Oct 19, 2022 at 8:52 PM Eugenio Pérez <eperezma@redhat.com> wrote:
> >
> > At this moment this code path is not reached, but vdpa devices can offer
> > VIRTIO_NET_F_STATUS unconditionally.
>
> So I guess what you mean is that, for the parent that doesn't support
> VIRTIO_NET_F_STATUS, emulate one for making sure the ANNOUCNE to work.
> This is safe since the spec said the driver will assume the link is
> always up if without this feature.
>

Right.

> > While the guest must assume that
> > link is always up by the standard, qemu will set the status bit to 1
> > always in this case.
> >
> > This makes little use by itself, but VIRTIO_NET_F_STATUS is needed for
> > the guest to read status bit VIRTIO_NET_F_GUEST_ANNOUNCE, used by feature
> > VIRTIO_NET_F_GUEST_ANNOUNCE. So qemu must emulate status feature in case
> > it needs to emulate the guest announce feature.
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> >  hw/net/vhost_net.c | 27 ++++++++++++++++++++++++++-
> >  1 file changed, 26 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index d28f8b974b..5660606c1d 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -117,7 +117,32 @@ uint64_t vhost_net_get_features(struct vhost_net *net, uint64_t features)
> >  int vhost_net_get_config(struct vhost_net *net,  uint8_t *config,
> >                           uint32_t config_len)
> >  {
> > -    return vhost_dev_get_config(&net->dev, config, config_len, NULL);
> > +    VirtIODevice *vdev;
> > +    int r = vhost_dev_get_config(&net->dev, config, config_len, NULL);
> > +
> > +    if (unlikely(r != 0)) {
> > +        return r;
> > +    }
> > +
> > +    if (config_len < endof(struct virtio_net_config, status)) {
> > +        return 0;
> > +    }
> > +
> > +    /*
> > +     * TODO: Perform this only if vhost_vdpa.
> > +     */
>
> Cindy adds some mediation codes for vhost-vDPA in
> virtio_net_get_config(), so I believe it can be done there?
>

Sure, let me take a look.

Thanks!


> Thanks
>
> > +    vdev = net->dev.vdev;
> > +    if (!vdev) {
> > +        /* Device is starting */
> > +        return 0;
> > +    }
> > +
> > +    if ((net->dev.acked_features & BIT_ULL(VIRTIO_NET_F_STATUS)) &&
> > +        !(net->dev.features & BIT_ULL(VIRTIO_NET_F_STATUS))) {
> > +        ((struct virtio_net_config *)config)->status |= VIRTIO_NET_S_LINK_UP;
> > +    }
> > +
> > +    return 0;
> >  }
> >  int vhost_net_set_config(struct vhost_net *net, const uint8_t *data,
> >                           uint32_t offset, uint32_t size, uint32_t flags)
> > --
> > 2.31.1
> >
>