On Wed, Dec 2, 2020 at 6:06 AM Jason Wang <jasowang@redhat.com
<mailto:jasowang@redhat.com>> wrote:
On 2020/12/1 下午3:40, Yuri Benditovich wrote:
>
>
> On Tue, Nov 24, 2020 at 10:49 AM Jason Wang <jasowang@redhat.com
<mailto:jasowang@redhat.com>
> <mailto:jasowang@redhat.com <mailto:jasowang@redhat.com>>> wrote:
>
>
> On 2020/11/19 下午7:13, Andrew Melnychenko wrote:
> > From: Andrew <andrew@daynix.com <mailto:andrew@daynix.com>
<mailto:andrew@daynix.com <mailto: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 <mailto:yuri.benditovich@daynix.com>
> <mailto:yuri.benditovich@daynix.com
<mailto:yuri.benditovich@daynix.com>>>
> > Signed-off-by: Andrew Melnychenko <andrew@daynix.com
<mailto:andrew@daynix.com>
> <mailto:andrew@daynix.com <mailto: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 there's still some misunderstanding here.
>
> When "rss" is enabled via command line, qemu can't not turn
it off
> silently, otherwise it may break migration. Instead, qemu should
> disable
> vhost-net if eBPF can't be loaded.
>
> When "hash_report" is enabled via command line, qemu should
disable
> vhost-net unconditionally.
>
>
> I agree in general with this requirement and I'm preparing an
> implementation of such fallback.
>
> The problem is that qemu already uses the mechanism of turning off
> host features
> silently if they are not supported by the current vhost in kernel:
>
https://github.com/qemu/qemu/blob/b0f8c22d6d4d07f3bd2307bcc62e1660ef965472/hw/virtio/vhost.c#L1526
>
> Can you please comment on it and let me know how it should be
modified
> in future?
> I've planned to use it in next work (implementing hash report in
kernel)
This looks like a bug that needs to be solved. Otherwise we break
migration from rss=on, vhost=off to rss=on,vhost=on.
I think I need to fill the gap in my understanding of migration's
prerequisites.
According to
https://github.com/qemu/qemu/blob/b0f8c22d6d4d07f3bd2307bcc62e1660ef965472/docs/devel/migration.rst
"... QEMU has to be launched with the same arguments the two times
..." and we test the migration during development
according to this statement.