diff mbox series

[RFC,v3,5/6] virtio-net: Added eBPF RSS to virtio-net.

Message ID 20210114211612.387052-6-andrew@daynix.com
State New
Headers show
Series eBPF RSS support for virtio-net | expand

Commit Message

Andrew Melnichenko Jan. 14, 2021, 9:16 p.m. UTC
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            | 125 +++++++++++++++++++++++++++++++--
 include/hw/virtio/virtio-net.h |   4 ++
 net/vhost-vdpa.c               |   2 +
 4 files changed, 129 insertions(+), 4 deletions(-)

Comments

Jason Wang Jan. 15, 2021, 7:20 a.m. UTC | #1
On 2021/1/15 上午5:16, 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            | 125 +++++++++++++++++++++++++++++++--
>   include/hw/virtio/virtio-net.h |   4 ++
>   net/vhost-vdpa.c               |   2 +
>   4 files changed, 129 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 09ceb02c9d..37016fc73a 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -691,6 +691,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 */


I still think we should not clear feature silently. This may break 
migraiton if the feature is cleared on destination.


> +    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);
> +
> +    return ret;
> +}
> +
>   static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
>                                           Error **errp)
>   {
> @@ -725,9 +738,9 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
>           return features;
>       }
>   
> -    virtio_clear_feature(&features, VIRTIO_NET_F_RSS);
> -    virtio_clear_feature(&features, VIRTIO_NET_F_HASH_REPORT);
> -    features = vhost_net_get_features(get_vhost_net(nc->peer), features);
> +    features = fix_ebpf_vhost_features(
> +            vhost_net_get_features(get_vhost_net(nc->peer), features));
> +
>       vdev->backend_features = features;
>   
>       if (n->mtu_bypass_backend &&
> @@ -1151,12 +1164,79 @@ static int virtio_net_handle_announce(VirtIONet *n, uint8_t cmd,
>       }
>   }
>   
> +static void virtio_net_detach_epbf_rss(VirtIONet *n);
> +
>   static void virtio_net_disable_rss(VirtIONet *n)
>   {
>       if (n->rss_data.enabled) {
>           trace_virtio_net_rss_disable();
>       }
>       n->rss_data.enabled = false;
> +
> +    virtio_net_detach_epbf_rss(n);
> +}
> +
> +static bool virtio_net_attach_ebpf_to_backend(NICState *nic, int prog_fd)
> +{
> +    NetClientState *nc = qemu_get_peer(qemu_get_queue(nic), 0);
> +    if (nc == NULL || nc->info->set_steering_ebpf == NULL) {
> +        return false;
> +    }
> +
> +    return nc->info->set_steering_ebpf(nc, prog_fd);
> +}
> +
> +static void rss_data_to_rss_config(struct VirtioNetRssData *data,
> +                                   struct EBPFRSSConfig *config)
> +{
> +    config->redirect = data->redirect;
> +    config->populate_hash = data->populate_hash;
> +    config->hash_types = data->hash_types;
> +    config->indirections_len = data->indirections_len;
> +    config->default_queue = data->default_queue;
> +}
> +
> +static bool virtio_net_attach_epbf_rss(VirtIONet *n)
> +{
> +    struct EBPFRSSConfig config = {};
> +
> +    if (!ebpf_rss_is_loaded(&n->ebpf_rss)) {
> +        return false;
> +    }
> +
> +    rss_data_to_rss_config(&n->rss_data, &config);
> +
> +    if (!ebpf_rss_set_all(&n->ebpf_rss, &config,
> +                          n->rss_data.indirections_table, n->rss_data.key)) {
> +        return false;
> +    }
> +
> +    if (!virtio_net_attach_ebpf_to_backend(n->nic, n->ebpf_rss.program_fd)) {
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
> +static void virtio_net_detach_epbf_rss(VirtIONet *n)
> +{
> +    virtio_net_attach_ebpf_to_backend(n->nic, -1);
> +}
> +
> +static bool virtio_net_load_ebpf(VirtIONet *n)
> +{
> +    if (!virtio_net_attach_ebpf_to_backend(n->nic, -1)) {
> +        /* backend does't support steering ebpf */
> +        return false;
> +    }
> +
> +    return ebpf_rss_load(&n->ebpf_rss);
> +}
> +
> +static void virtio_net_unload_ebpf(VirtIONet *n)
> +{
> +    virtio_net_attach_ebpf_to_backend(n->nic, -1);
> +    ebpf_rss_unload(&n->ebpf_rss);
>   }
>   
>   static uint16_t virtio_net_handle_rss(VirtIONet *n,
> @@ -1271,6 +1351,25 @@ static uint16_t virtio_net_handle_rss(VirtIONet *n,
>           goto error;
>       }
>       n->rss_data.enabled = true;
> +
> +    if (!n->rss_data.populate_hash) {
> +        if (!virtio_net_attach_epbf_rss(n)) {
> +            /* EBPF must be loaded for vhost */
> +            if (get_vhost_net(qemu_get_queue(n->nic)->peer)) {
> +                warn_report("Can't load eBPF RSS for vhost");
> +                goto error;


How about stop the vhost in this case?

Thanks
Yuri Benditovich Jan. 17, 2021, 9:04 a.m. UTC | #2
On Fri, Jan 15, 2021 at 9:20 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2021/1/15 上午5:16, 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            | 125 +++++++++++++++++++++++++++++++--
> >   include/hw/virtio/virtio-net.h |   4 ++
> >   net/vhost-vdpa.c               |   2 +
> >   4 files changed, 129 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 09ceb02c9d..37016fc73a 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -691,6 +691,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 */
>
>
> I still think we should not clear feature silently. This may break
> migraiton if the feature is cleared on destination.

Do I understand it correctly that if we do not clear features silently
and implement a graceful drop to vhost=off when we can't do what we
need with vhost - then we do not need to add any migration blocker?

>
>
> > +    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);
> > +
> > +    return ret;
> > +}
> > +
> >   static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
> >                                           Error **errp)
> >   {
> > @@ -725,9 +738,9 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
> >           return features;
> >       }
> >
> > -    virtio_clear_feature(&features, VIRTIO_NET_F_RSS);
> > -    virtio_clear_feature(&features, VIRTIO_NET_F_HASH_REPORT);
> > -    features = vhost_net_get_features(get_vhost_net(nc->peer), features);
> > +    features = fix_ebpf_vhost_features(
> > +            vhost_net_get_features(get_vhost_net(nc->peer), features));
> > +
> >       vdev->backend_features = features;
> >
> >       if (n->mtu_bypass_backend &&
> > @@ -1151,12 +1164,79 @@ static int virtio_net_handle_announce(VirtIONet *n, uint8_t cmd,
> >       }
> >   }
> >
> > +static void virtio_net_detach_epbf_rss(VirtIONet *n);
> > +
> >   static void virtio_net_disable_rss(VirtIONet *n)
> >   {
> >       if (n->rss_data.enabled) {
> >           trace_virtio_net_rss_disable();
> >       }
> >       n->rss_data.enabled = false;
> > +
> > +    virtio_net_detach_epbf_rss(n);
> > +}
> > +
> > +static bool virtio_net_attach_ebpf_to_backend(NICState *nic, int prog_fd)
> > +{
> > +    NetClientState *nc = qemu_get_peer(qemu_get_queue(nic), 0);
> > +    if (nc == NULL || nc->info->set_steering_ebpf == NULL) {
> > +        return false;
> > +    }
> > +
> > +    return nc->info->set_steering_ebpf(nc, prog_fd);
> > +}
> > +
> > +static void rss_data_to_rss_config(struct VirtioNetRssData *data,
> > +                                   struct EBPFRSSConfig *config)
> > +{
> > +    config->redirect = data->redirect;
> > +    config->populate_hash = data->populate_hash;
> > +    config->hash_types = data->hash_types;
> > +    config->indirections_len = data->indirections_len;
> > +    config->default_queue = data->default_queue;
> > +}
> > +
> > +static bool virtio_net_attach_epbf_rss(VirtIONet *n)
> > +{
> > +    struct EBPFRSSConfig config = {};
> > +
> > +    if (!ebpf_rss_is_loaded(&n->ebpf_rss)) {
> > +        return false;
> > +    }
> > +
> > +    rss_data_to_rss_config(&n->rss_data, &config);
> > +
> > +    if (!ebpf_rss_set_all(&n->ebpf_rss, &config,
> > +                          n->rss_data.indirections_table, n->rss_data.key)) {
> > +        return false;
> > +    }
> > +
> > +    if (!virtio_net_attach_ebpf_to_backend(n->nic, n->ebpf_rss.program_fd)) {
> > +        return false;
> > +    }
> > +
> > +    return true;
> > +}
> > +
> > +static void virtio_net_detach_epbf_rss(VirtIONet *n)
> > +{
> > +    virtio_net_attach_ebpf_to_backend(n->nic, -1);
> > +}
> > +
> > +static bool virtio_net_load_ebpf(VirtIONet *n)
> > +{
> > +    if (!virtio_net_attach_ebpf_to_backend(n->nic, -1)) {
> > +        /* backend does't support steering ebpf */
> > +        return false;
> > +    }
> > +
> > +    return ebpf_rss_load(&n->ebpf_rss);
> > +}
> > +
> > +static void virtio_net_unload_ebpf(VirtIONet *n)
> > +{
> > +    virtio_net_attach_ebpf_to_backend(n->nic, -1);
> > +    ebpf_rss_unload(&n->ebpf_rss);
> >   }
> >
> >   static uint16_t virtio_net_handle_rss(VirtIONet *n,
> > @@ -1271,6 +1351,25 @@ static uint16_t virtio_net_handle_rss(VirtIONet *n,
> >           goto error;
> >       }
> >       n->rss_data.enabled = true;
> > +
> > +    if (!n->rss_data.populate_hash) {
> > +        if (!virtio_net_attach_epbf_rss(n)) {
> > +            /* EBPF must be loaded for vhost */
> > +            if (get_vhost_net(qemu_get_queue(n->nic)->peer)) {
> > +                warn_report("Can't load eBPF RSS for vhost");
> > +                goto error;
>
>
> How about stop the vhost in this case?
>
> Thanks
>
>
Jason Wang Jan. 18, 2021, 3:16 a.m. UTC | #3
On 2021/1/17 下午5:04, Yuri Benditovich wrote:
> On Fri, Jan 15, 2021 at 9:20 AM Jason Wang<jasowang@redhat.com>  wrote:
>> On 2021/1/15 上午5:16, 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            | 125 +++++++++++++++++++++++++++++++--
>>>    include/hw/virtio/virtio-net.h |   4 ++
>>>    net/vhost-vdpa.c               |   2 +
>>>    4 files changed, 129 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 09ceb02c9d..37016fc73a 100644
>>> --- a/hw/net/virtio-net.c
>>> +++ b/hw/net/virtio-net.c
>>> @@ -691,6 +691,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 */
>> I still think we should not clear feature silently. This may break
>> migraiton if the feature is cleared on destination.
> Do I understand it correctly that if we do not clear features silently
> and implement a graceful drop to vhost=off when we can't do what we
> need with vhost - then we do not need to add any migration blocker?


Yes. I think we won't go with migration blocker since we need support 
migration in the end.

Thanks


>
Yuri Benditovich Jan. 24, 2021, 8:24 a.m. UTC | #4
Hi Jason,

I've prepared a POC of graceful switch to 'vhost off' if respective
features are acked by the guest.
Such a way we do not need to silently clear RSS and hash report
features in case of 'vhost on'.
Can you please review it and provide your feedback?

I think the only open question is what to do with cases of vhost-user
and vhost-vdpa.

https://github.com/qemu/qemu/pull/105
This pull request is for reviews only.

Thanks in advance






On Mon, Jan 18, 2021 at 5:16 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2021/1/17 下午5:04, Yuri Benditovich wrote:
> > On Fri, Jan 15, 2021 at 9:20 AM Jason Wang<jasowang@redhat.com>  wrote:
> >> On 2021/1/15 上午5:16, 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            | 125 +++++++++++++++++++++++++++++++--
> >>>    include/hw/virtio/virtio-net.h |   4 ++
> >>>    net/vhost-vdpa.c               |   2 +
> >>>    4 files changed, 129 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 09ceb02c9d..37016fc73a 100644
> >>> --- a/hw/net/virtio-net.c
> >>> +++ b/hw/net/virtio-net.c
> >>> @@ -691,6 +691,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 */
> >> I still think we should not clear feature silently. This may break
> >> migraiton if the feature is cleared on destination.
> > Do I understand it correctly that if we do not clear features silently
> > and implement a graceful drop to vhost=off when we can't do what we
> > need with vhost - then we do not need to add any migration blocker?
>
>
> Yes. I think we won't go with migration blocker since we need support
> migration in the end.
>
> Thanks
>
>
> >
>
Yuri Benditovich Jan. 24, 2021, 11:33 a.m. UTC | #5
On Sun, Jan 24, 2021 at 10:24 AM Yuri Benditovich
<yuri.benditovich@daynix.com> wrote:
>
> Hi Jason,
>
> I've prepared a POC of graceful switch to 'vhost off' if respective
> features are acked by the guest.
> Such a way we do not need to silently clear RSS and hash report
> features in case of 'vhost on'.
> Can you please review it and provide your feedback?
>
> I think the only open question is what to do with cases of vhost-user
> and vhost-vdpa.
>
> https://github.com/qemu/qemu/pull/105
> This pull request is for reviews only.

Unfortunately qemu github PR is closed for comments
This is the link to the same on Daynix repository

https://github.com/daynix/qemu/pull/1

>
> Thanks in advance
>
>
>
>
>
>
> On Mon, Jan 18, 2021 at 5:16 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> >
> > On 2021/1/17 下午5:04, Yuri Benditovich wrote:
> > > On Fri, Jan 15, 2021 at 9:20 AM Jason Wang<jasowang@redhat.com>  wrote:
> > >> On 2021/1/15 上午5:16, 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            | 125 +++++++++++++++++++++++++++++++--
> > >>>    include/hw/virtio/virtio-net.h |   4 ++
> > >>>    net/vhost-vdpa.c               |   2 +
> > >>>    4 files changed, 129 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 09ceb02c9d..37016fc73a 100644
> > >>> --- a/hw/net/virtio-net.c
> > >>> +++ b/hw/net/virtio-net.c
> > >>> @@ -691,6 +691,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 */
> > >> I still think we should not clear feature silently. This may break
> > >> migraiton if the feature is cleared on destination.
> > > Do I understand it correctly that if we do not clear features silently
> > > and implement a graceful drop to vhost=off when we can't do what we
> > > need with vhost - then we do not need to add any migration blocker?
> >
> >
> > Yes. I think we won't go with migration blocker since we need support
> > migration in the end.
> >
> > Thanks
> >
> >
> > >
> >
Jason Wang Jan. 25, 2021, 8:59 a.m. UTC | #6
On 2021/1/24 下午4:24, Yuri Benditovich wrote:
> Hi Jason,
>
> I've prepared a POC of graceful switch to 'vhost off' if respective
> features are acked by the guest.
> Such a way we do not need to silently clear RSS and hash report
> features in case of 'vhost on'.
> Can you please review it and provide your feedback?
>
> I think the only open question is what to do with cases of vhost-user
> and vhost-vdpa.
>
> https://github.com/qemu/qemu/pull/105
> This pull request is for reviews only.
>
> Thanks in advance


Will review it sometime this week.

Thanks


>
>
>
>
>
>
> On Mon, Jan 18, 2021 at 5:16 AM Jason Wang <jasowang@redhat.com> wrote:
>>
>> On 2021/1/17 下午5:04, Yuri Benditovich wrote:
>>> On Fri, Jan 15, 2021 at 9:20 AM Jason Wang<jasowang@redhat.com>  wrote:
>>>> On 2021/1/15 上午5:16, 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            | 125 +++++++++++++++++++++++++++++++--
>>>>>     include/hw/virtio/virtio-net.h |   4 ++
>>>>>     net/vhost-vdpa.c               |   2 +
>>>>>     4 files changed, 129 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 09ceb02c9d..37016fc73a 100644
>>>>> --- a/hw/net/virtio-net.c
>>>>> +++ b/hw/net/virtio-net.c
>>>>> @@ -691,6 +691,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 */
>>>> I still think we should not clear feature silently. This may break
>>>> migraiton if the feature is cleared on destination.
>>> Do I understand it correctly that if we do not clear features silently
>>> and implement a graceful drop to vhost=off when we can't do what we
>>> need with vhost - then we do not need to add any migration blocker?
>>
>> Yes. I think we won't go with migration blocker since we need support
>> migration in the end.
>>
>> Thanks
>>
>>
diff mbox series

Patch

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 09ceb02c9d..37016fc73a 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -691,6 +691,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);
+
+    return ret;
+}
+
 static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
                                         Error **errp)
 {
@@ -725,9 +738,9 @@  static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
         return features;
     }
 
-    virtio_clear_feature(&features, VIRTIO_NET_F_RSS);
-    virtio_clear_feature(&features, VIRTIO_NET_F_HASH_REPORT);
-    features = vhost_net_get_features(get_vhost_net(nc->peer), features);
+    features = fix_ebpf_vhost_features(
+            vhost_net_get_features(get_vhost_net(nc->peer), features));
+
     vdev->backend_features = features;
 
     if (n->mtu_bypass_backend &&
@@ -1151,12 +1164,79 @@  static int virtio_net_handle_announce(VirtIONet *n, uint8_t cmd,
     }
 }
 
+static void virtio_net_detach_epbf_rss(VirtIONet *n);
+
 static void virtio_net_disable_rss(VirtIONet *n)
 {
     if (n->rss_data.enabled) {
         trace_virtio_net_rss_disable();
     }
     n->rss_data.enabled = false;
+
+    virtio_net_detach_epbf_rss(n);
+}
+
+static bool virtio_net_attach_ebpf_to_backend(NICState *nic, int prog_fd)
+{
+    NetClientState *nc = qemu_get_peer(qemu_get_queue(nic), 0);
+    if (nc == NULL || nc->info->set_steering_ebpf == NULL) {
+        return false;
+    }
+
+    return nc->info->set_steering_ebpf(nc, prog_fd);
+}
+
+static void rss_data_to_rss_config(struct VirtioNetRssData *data,
+                                   struct EBPFRSSConfig *config)
+{
+    config->redirect = data->redirect;
+    config->populate_hash = data->populate_hash;
+    config->hash_types = data->hash_types;
+    config->indirections_len = data->indirections_len;
+    config->default_queue = data->default_queue;
+}
+
+static bool virtio_net_attach_epbf_rss(VirtIONet *n)
+{
+    struct EBPFRSSConfig config = {};
+
+    if (!ebpf_rss_is_loaded(&n->ebpf_rss)) {
+        return false;
+    }
+
+    rss_data_to_rss_config(&n->rss_data, &config);
+
+    if (!ebpf_rss_set_all(&n->ebpf_rss, &config,
+                          n->rss_data.indirections_table, n->rss_data.key)) {
+        return false;
+    }
+
+    if (!virtio_net_attach_ebpf_to_backend(n->nic, n->ebpf_rss.program_fd)) {
+        return false;
+    }
+
+    return true;
+}
+
+static void virtio_net_detach_epbf_rss(VirtIONet *n)
+{
+    virtio_net_attach_ebpf_to_backend(n->nic, -1);
+}
+
+static bool virtio_net_load_ebpf(VirtIONet *n)
+{
+    if (!virtio_net_attach_ebpf_to_backend(n->nic, -1)) {
+        /* backend does't support steering ebpf */
+        return false;
+    }
+
+    return ebpf_rss_load(&n->ebpf_rss);
+}
+
+static void virtio_net_unload_ebpf(VirtIONet *n)
+{
+    virtio_net_attach_ebpf_to_backend(n->nic, -1);
+    ebpf_rss_unload(&n->ebpf_rss);
 }
 
 static uint16_t virtio_net_handle_rss(VirtIONet *n,
@@ -1271,6 +1351,25 @@  static uint16_t virtio_net_handle_rss(VirtIONet *n,
         goto error;
     }
     n->rss_data.enabled = true;
+
+    if (!n->rss_data.populate_hash) {
+        if (!virtio_net_attach_epbf_rss(n)) {
+            /* EBPF must be loaded for vhost */
+            if (get_vhost_net(qemu_get_queue(n->nic)->peer)) {
+                warn_report("Can't load eBPF RSS for vhost");
+                goto error;
+            }
+            /* fallback to software RSS */
+            warn_report("Can't load eBPF RSS - fallback to software RSS");
+            n->rss_data.enabled_software_rss = true;
+        }
+    } else {
+        /* use software RSS for hash populating */
+        /* and detach eBPF if was loaded before */
+        virtio_net_detach_epbf_rss(n);
+        n->rss_data.enabled_software_rss = true;
+    }
+
     trace_virtio_net_rss_enable(n->rss_data.hash_types,
                                 n->rss_data.indirections_len,
                                 temp.b);
@@ -1656,7 +1755,7 @@  static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
         return -1;
     }
 
-    if (!no_rss && n->rss_data.enabled) {
+    if (!no_rss && n->rss_data.enabled && n->rss_data.enabled_software_rss) {
         int index = virtio_net_process_rss(nc, buf, size);
         if (index >= 0) {
             NetClientState *nc2 = qemu_get_subqueue(n->nic, index);
@@ -2760,6 +2859,18 @@  static int virtio_net_post_load_device(void *opaque, int version_id)
     }
 
     if (n->rss_data.enabled) {
+        n->rss_data.enabled_software_rss = n->rss_data.populate_hash;
+        if (!n->rss_data.populate_hash) {
+            if (!virtio_net_attach_epbf_rss(n)) {
+                if (get_vhost_net(qemu_get_queue(n->nic)->peer)) {
+                    error_report("Can't post-load eBPF RSS for vhost");
+                } else {
+                    warn_report("Can't post-load eBPF RSS - fallback to software RSS");
+                    n->rss_data.enabled_software_rss = true;
+                }
+            }
+        }
+
         trace_virtio_net_rss_enable(n->rss_data.hash_types,
                                     n->rss_data.indirections_len,
                                     sizeof(n->rss_data.key));
@@ -3336,6 +3447,8 @@  static void virtio_net_device_realize(DeviceState *dev, Error **errp)
     n->qdev = dev;
 
     net_rx_pkt_init(&n->rx_pkt, false);
+
+    virtio_net_load_ebpf(n);
 }
 
 static void virtio_net_device_unrealize(DeviceState *dev)
@@ -3344,6 +3457,8 @@  static void virtio_net_device_unrealize(DeviceState *dev)
     VirtIONet *n = VIRTIO_NET(dev);
     int i, max_queues;
 
+    virtio_net_unload_ebpf(n);
+
     /* This will stop vhost backend if appropriate. */
     virtio_net_set_status(vdev, 0);
 
@@ -3386,6 +3501,8 @@  static void virtio_net_instance_init(Object *obj)
     device_add_bootindex_property(obj, &n->nic_conf.bootindex,
                                   "bootindex", "/ethernet-phy@0",
                                   DEVICE(n));
+
+    ebpf_rss_init(&n->ebpf_rss);
 }
 
 static int virtio_net_pre_save(void *opaque)
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index 7e96d193aa..824a69c23f 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -21,6 +21,8 @@ 
 #include "qemu/option_int.h"
 #include "qom/object.h"
 
+#include "ebpf/ebpf_rss.h"
+
 #define TYPE_VIRTIO_NET "virtio-net-device"
 OBJECT_DECLARE_SIMPLE_TYPE(VirtIONet, VIRTIO_NET)
 
@@ -130,6 +132,7 @@  typedef struct VirtioNetRscChain {
 
 typedef struct VirtioNetRssData {
     bool    enabled;
+    bool    enabled_software_rss;
     bool    redirect;
     bool    populate_hash;
     uint32_t hash_types;
@@ -209,6 +212,7 @@  struct VirtIONet {
     Notifier migration_state;
     VirtioNetRssData rss_data;
     struct NetRxPkt *rx_pkt;
+    struct EBPFRSSContext ebpf_rss;
 };
 
 void virtio_net_set_netclient_name(VirtIONet *n, const char *name,
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index fe659ec9e2..8b14215549 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -54,6 +54,8 @@  const int vdpa_feature_bits[] = {
     VIRTIO_NET_F_MTU,
     VIRTIO_F_IOMMU_PLATFORM,
     VIRTIO_F_RING_PACKED,
+    VIRTIO_NET_F_RSS,
+    VIRTIO_NET_F_HASH_REPORT,
     VIRTIO_NET_F_GUEST_ANNOUNCE,
     VIRTIO_NET_F_STATUS,
     VHOST_INVALID_FEATURE_BIT