Re: [RFC PATCH 5/6] virtio-net: Added eBPF RSS to virtio-net.

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Nov 04, 2020 at 01:07:41PM +0200, Yuri Benditovich wrote:
> On Wed, Nov 4, 2020 at 5:09 AM Jason Wang <jasowang@redhat.com> wrote:
> 
> >
> > On 2020/11/3 上午2:51, Andrew Melnychenko wrote:
> > > From: Andrew <andrew@daynix.com>
> > >
> > > When RSS is enabled the device tries to load the eBPF program
> > > to select RX virtqueue in the TUN. If eBPF can be loaded
> > > the RSS will function also with vhost (works with kernel 5.8 and later).
> > > Software RSS is used as a fallback with vhost=off when eBPF can't be
> > loaded
> > > or when hash population requested by the guest.
> > >
> > > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> > > Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> > > ---
> > >   hw/net/vhost_net.c             |   2 +
> > >   hw/net/virtio-net.c            | 120 +++++++++++++++++++++++++++++++--
> > >   include/hw/virtio/virtio-net.h |   4 ++
> > >   net/vhost-vdpa.c               |   2 +
> > >   4 files changed, 124 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > index 24d555e764..16124f99c3 100644
> > > --- a/hw/net/vhost_net.c
> > > +++ b/hw/net/vhost_net.c
> > > @@ -71,6 +71,8 @@ static const int user_feature_bits[] = {
> > >       VIRTIO_NET_F_MTU,
> > >       VIRTIO_F_IOMMU_PLATFORM,
> > >       VIRTIO_F_RING_PACKED,
> > > +    VIRTIO_NET_F_RSS,
> > > +    VIRTIO_NET_F_HASH_REPORT,
> > >
> > >       /* This bit implies RARP isn't sent by QEMU out of band */
> > >       VIRTIO_NET_F_GUEST_ANNOUNCE,
> > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > > index 277289d56e..afcc3032ec 100644
> > > --- a/hw/net/virtio-net.c
> > > +++ b/hw/net/virtio-net.c
> > > @@ -698,6 +698,19 @@ static void virtio_net_set_queues(VirtIONet *n)
> > >
> > >   static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue);
> > >
> > > +static uint64_t fix_ebpf_vhost_features(uint64_t features)
> > > +{
> > > +    /* If vhost=on & CONFIG_EBPF doesn't set - disable RSS feature */
> > > +    uint64_t ret = features;
> > > +#ifndef CONFIG_EBPF
> > > +    virtio_clear_feature(&ret, VIRTIO_NET_F_RSS);
> > > +#endif
> > > +    /* for now, there is no solution for populating the hash from eBPF
> > */
> > > +    virtio_clear_feature(&ret, VIRTIO_NET_F_HASH_REPORT);
> >
> >
> > I think we probably need to to something reverse since RSS is under the
> > control on qemu cli, disable features like this may break migration.
> >
> >
> How by design we add new features to qemu in light of possible migration to
> older qemu version when the destination
> qemu does not support these features?

If the feature affects guest ABI, then we don't want to silently/
automatically turn on features that have a dependancy on kernel
features existing. They need to be an opt-in by mgmt app/admin.

IOW there needs to be an explicit property that is set to turn on use
of eBPF. If this property is set, then QEMU must use eBPF or fail
with an error. If it is unset, then QEMU must never use eBPF.

The mgmt app controlling QEMU will decide whether to use eBPF and
turn on the property, and will then know not to migrate it to a
host without eBPF support.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|