qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Jason Wang
Subject: Re: [RFC PATCH v2 4/5] virtio-net: Added eBPF RSS to virtio-net.
Date: Wed, 2 Dec 2020 12:05:51 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0


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>> wrote:


    On 2020/11/19 下午7:13, Andrew Melnychenko wrote:
    > From: Andrew <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>>
    > Signed-off-by: Andrew Melnychenko <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 you can keep the current code as is and I will try to seek a way to solve the issue.

Thanks




reply via email to

[Prev in Thread] Current Thread [Next in Thread]