On Wed, Nov 4, 2020 at 1:13 PM Daniel P. Berrangé <berra...@redhat.com>
wrote:
> On Wed, Nov 04, 2020 at 01:07:41PM +0200, Yuri Benditovich wrote:
> > On Wed, Nov 4, 2020 at 5:09 AM Jason Wang <jasow...@redhat.com> wrote:
> >
> > >
> > > On 2020/11/3 上午2:51, Andrew Melnychenko wrote:
> > > > From: Andrew <and...@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.benditov...@daynix.com>
> > > > Signed-off-by: Andrew Melnychenko <and...@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.
>
>
We understand that. But the eBPF itself does not affect the guest ABI.
The 'RSS' feature of virtio-net device already exists (implemented in QEMU,
so it can work in case the vhost is off).
eBPF is able to do the same on TAP/TUN level (so it can do the job also
when vhost is on).
By default it is turned off and requires explicit command line switch
'rss=on'
> 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 :|
>
>
