diff mbox series

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

Message ID 20201102185115.7425-6-andrew@daynix.com
State New
Headers show
Series [RFC,1/6] net: Added SetSteeringEBPF method for NetClientState. | expand

Commit Message

Andrew Melnichenko Nov. 2, 2020, 6:51 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            | 120 +++++++++++++++++++++++++++++++--
 include/hw/virtio/virtio-net.h |   4 ++
 net/vhost-vdpa.c               |   2 +
 4 files changed, 124 insertions(+), 4 deletions(-)

Comments

Jason Wang Nov. 4, 2020, 3:09 a.m. UTC | #1 
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.

We need disable vhost instead when:

1) eBPF is not supported but RSS is required from command line

or

2) HASH_REPORT is required from command line


> +
> +    return ret;
> +}
> +
>   static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
>                                           Error **errp)
>   {
> @@ -732,9 +745,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 &&
> @@ -1169,12 +1182,75 @@ static int virtio_net_handle_announce(VirtIONet *n, uint8_t cmd,
>       }
>   }
>   
> +static void virtio_net_unload_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;
> +
> +    if (!n->rss_data.enabled_software_rss && ebpf_rss_is_loaded(&n->ebpf_rss)) {
> +        virtio_net_unload_epbf_rss(n);
> +    }
> +}
> +
> +static bool virtio_net_attach_steering_ebpf(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_load_epbf_rss(VirtIONet *n)
> +{
> +    struct EBPFRSSConfig config = {};
> +
> +    if (!n->rss_data.enabled) {
> +        if (ebpf_rss_is_loaded(&n->ebpf_rss)) {
> +            ebpf_rss_unload(&n->ebpf_rss);
> +        }
> +        return true;
> +    }
> +
> +    if (!ebpf_rss_is_loaded(&n->ebpf_rss) && !ebpf_rss_load(&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)) {
> +        ebpf_rss_unload(&n->ebpf_rss);
> +        return false;
> +    }
> +
> +    if (!virtio_net_attach_steering_ebpf(n->nic, n->ebpf_rss.program_fd)) {
> +        ebpf_rss_unload(&n->ebpf_rss);
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
> +static void virtio_net_unload_epbf_rss(VirtIONet *n)
> +{
> +    virtio_net_attach_steering_ebpf(n->nic, -1);
> +    ebpf_rss_unload(&n->ebpf_rss);
>   }
>   
>   static uint16_t virtio_net_handle_rss(VirtIONet *n,
> @@ -1208,6 +1284,7 @@ static uint16_t virtio_net_handle_rss(VirtIONet *n,
>           err_value = (uint32_t)s;
>           goto error;
>       }
> +    n->rss_data.enabled_software_rss = false;
>       n->rss_data.hash_types = virtio_ldl_p(vdev, &cfg.hash_types);
>       n->rss_data.indirections_len =
>           virtio_lduw_p(vdev, &cfg.indirection_table_mask);
> @@ -1289,9 +1366,30 @@ static uint16_t virtio_net_handle_rss(VirtIONet *n,
>           goto error;
>       }
>       n->rss_data.enabled = true;
> +
> +    if (!n->rss_data.populate_hash) {
> +        /* load EBPF RSS */
> +        if (!virtio_net_load_epbf_rss(n)) {
> +            /* EBPF mast 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 unload eBPF if was loaded before */
> +        virtio_net_unload_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);
> +
>       return queues;
>   error:
>       trace_virtio_net_rss_error(err_msg, err_value);
> @@ -1674,7 +1772,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);
> @@ -2780,6 +2878,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_load_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));
> @@ -3453,6 +3563,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 f4852ac27b..4d29a577eb 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;


We probably need a better name of this since "software" is kind of 
confusing.


>       bool    redirect;
>       bool    populate_hash;
>       uint32_t hash_types;
> @@ -214,6 +217,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 99c476db8c..feb5fa8624 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,


This is fine but looks unrelated to eBPF RSS support.

Usually it means the hardware can support RSS/hash reporting.

Thanks


>       VIRTIO_NET_F_GUEST_ANNOUNCE,
>       VIRTIO_NET_F_STATUS,
>       VHOST_INVALID_FEATURE_BIT
Yuri Benditovich Nov. 4, 2020, 11:07 a.m. UTC | #2 
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?


> We need disable vhost instead when:
>
> 1) eBPF is not supported but RSS is required from command line
>
> or
>
> 2) HASH_REPORT is required from command line
>
> > +
> > +    return ret;
> > +}
> > +
> >   static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t
> features,
> >                                           Error **errp)
> >   {
> > @@ -732,9 +745,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 &&
> > @@ -1169,12 +1182,75 @@ static int virtio_net_handle_announce(VirtIONet
> *n, uint8_t cmd,
> >       }
> >   }
> >
> > +static void virtio_net_unload_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;
> > +
> > +    if (!n->rss_data.enabled_software_rss &&
> ebpf_rss_is_loaded(&n->ebpf_rss)) {
> > +        virtio_net_unload_epbf_rss(n);
> > +    }
> > +}
> > +
> > +static bool virtio_net_attach_steering_ebpf(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_load_epbf_rss(VirtIONet *n)
> > +{
> > +    struct EBPFRSSConfig config = {};
> > +
> > +    if (!n->rss_data.enabled) {
> > +        if (ebpf_rss_is_loaded(&n->ebpf_rss)) {
> > +            ebpf_rss_unload(&n->ebpf_rss);
> > +        }
> > +        return true;
> > +    }
> > +
> > +    if (!ebpf_rss_is_loaded(&n->ebpf_rss) &&
> !ebpf_rss_load(&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)) {
> > +        ebpf_rss_unload(&n->ebpf_rss);
> > +        return false;
> > +    }
> > +
> > +    if (!virtio_net_attach_steering_ebpf(n->nic,
> n->ebpf_rss.program_fd)) {
> > +        ebpf_rss_unload(&n->ebpf_rss);
> > +        return false;
> > +    }
> > +
> > +    return true;
> > +}
> > +
> > +static void virtio_net_unload_epbf_rss(VirtIONet *n)
> > +{
> > +    virtio_net_attach_steering_ebpf(n->nic, -1);
> > +    ebpf_rss_unload(&n->ebpf_rss);
> >   }
> >
> >   static uint16_t virtio_net_handle_rss(VirtIONet *n,
> > @@ -1208,6 +1284,7 @@ static uint16_t virtio_net_handle_rss(VirtIONet *n,
> >           err_value = (uint32_t)s;
> >           goto error;
> >       }
> > +    n->rss_data.enabled_software_rss = false;
> >       n->rss_data.hash_types = virtio_ldl_p(vdev, &cfg.hash_types);
> >       n->rss_data.indirections_len =
> >           virtio_lduw_p(vdev, &cfg.indirection_table_mask);
> > @@ -1289,9 +1366,30 @@ static uint16_t virtio_net_handle_rss(VirtIONet
> *n,
> >           goto error;
> >       }
> >       n->rss_data.enabled = true;
> > +
> > +    if (!n->rss_data.populate_hash) {
> > +        /* load EBPF RSS */
> > +        if (!virtio_net_load_epbf_rss(n)) {
> > +            /* EBPF mast 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 unload eBPF if was loaded before */
> > +        virtio_net_unload_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);
> > +
> >       return queues;
> >   error:
> >       trace_virtio_net_rss_error(err_msg, err_value);
> > @@ -1674,7 +1772,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);
> > @@ -2780,6 +2878,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_load_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));
> > @@ -3453,6 +3563,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 f4852ac27b..4d29a577eb 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;
>
>
> We probably need a better name of this since "software" is kind of
> confusing.
>
> No problem


>
> >       bool    redirect;
> >       bool    populate_hash;
> >       uint32_t hash_types;
> > @@ -214,6 +217,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 99c476db8c..feb5fa8624 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,
>
>
> This is fine but looks unrelated to eBPF RSS support.
>
> Usually it means the hardware can support RSS/hash reporting.
>
> As I see from
https://github.com/qemu/qemu/blob/master/hw/virtio/vhost.c#L1526:
If we do not add these features this means they are transparent to specific
vhost solution.
When in fact they are not. We need to ensure these features are not in use
in case of
vhost-vdpa or vhost-user.


> Thanks
>
>
> >       VIRTIO_NET_F_GUEST_ANNOUNCE,
> >       VIRTIO_NET_F_STATUS,
> >       VHOST_INVALID_FEATURE_BIT
>
>
Daniel P. Berrangé Nov. 4, 2020, 11:13 a.m. UTC | #3 
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
Yuri Benditovich Nov. 4, 2020, 3:51 p.m. UTC | #4 
On Wed, Nov 4, 2020 at 1:13 PM Daniel P. Berrangé <berrange@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 <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.
>
>
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 :|
>
>
Jason Wang Nov. 5, 2020, 3:29 a.m. UTC | #5 
On 2020/11/4 下午7:07, Yuri Benditovich wrote:
>
>
> On Wed, Nov 4, 2020 at 5:09 AM Jason Wang <jasowang@redhat.com 
> <mailto:jasowang@redhat.com>> wrote:
>
>
>     On 2020/11/3 上午2:51, 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 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?


There's a machine type definition in qemu, so we only guarantee the 
success of migration between the same machine type.

For new qemu with old machine type, new features will be disabled.

Thanks


>     We need disable vhost instead when:
>
>     1) eBPF is not supported but RSS is required from command line
>
>     or
>
>     2) HASH_REPORT is required from command line
>
>     > +
>     > +    return ret;
>     > +}
>     > +
>     >   static uint64_t virtio_net_get_features(VirtIODevice *vdev,
>     uint64_t features,
>     >                                           Error **errp)
>     >   {
>     > @@ -732,9 +745,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 &&
>     > @@ -1169,12 +1182,75 @@ static int
>     virtio_net_handle_announce(VirtIONet *n, uint8_t cmd,
>     >       }
>     >   }
>     >
>     > +static void virtio_net_unload_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;
>     > +
>     > +    if (!n->rss_data.enabled_software_rss &&
>     ebpf_rss_is_loaded(&n->ebpf_rss)) {
>     > +        virtio_net_unload_epbf_rss(n);
>     > +    }
>     > +}
>     > +
>     > +static bool virtio_net_attach_steering_ebpf(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_load_epbf_rss(VirtIONet *n)
>     > +{
>     > +    struct EBPFRSSConfig config = {};
>     > +
>     > +    if (!n->rss_data.enabled) {
>     > +        if (ebpf_rss_is_loaded(&n->ebpf_rss)) {
>     > +            ebpf_rss_unload(&n->ebpf_rss);
>     > +        }
>     > +        return true;
>     > +    }
>     > +
>     > +    if (!ebpf_rss_is_loaded(&n->ebpf_rss) &&
>     !ebpf_rss_load(&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)) {
>     > +        ebpf_rss_unload(&n->ebpf_rss);
>     > +        return false;
>     > +    }
>     > +
>     > +    if (!virtio_net_attach_steering_ebpf(n->nic,
>     n->ebpf_rss.program_fd)) {
>     > +        ebpf_rss_unload(&n->ebpf_rss);
>     > +        return false;
>     > +    }
>     > +
>     > +    return true;
>     > +}
>     > +
>     > +static void virtio_net_unload_epbf_rss(VirtIONet *n)
>     > +{
>     > +    virtio_net_attach_steering_ebpf(n->nic, -1);
>     > +    ebpf_rss_unload(&n->ebpf_rss);
>     >   }
>     >
>     >   static uint16_t virtio_net_handle_rss(VirtIONet *n,
>     > @@ -1208,6 +1284,7 @@ static uint16_t
>     virtio_net_handle_rss(VirtIONet *n,
>     >           err_value = (uint32_t)s;
>     >           goto error;
>     >       }
>     > +    n->rss_data.enabled_software_rss = false;
>     >       n->rss_data.hash_types = virtio_ldl_p(vdev, &cfg.hash_types);
>     >       n->rss_data.indirections_len =
>     >           virtio_lduw_p(vdev, &cfg.indirection_table_mask);
>     > @@ -1289,9 +1366,30 @@ static uint16_t
>     virtio_net_handle_rss(VirtIONet *n,
>     >           goto error;
>     >       }
>     >       n->rss_data.enabled = true;
>     > +
>     > +    if (!n->rss_data.populate_hash) {
>     > +        /* load EBPF RSS */
>     > +        if (!virtio_net_load_epbf_rss(n)) {
>     > +            /* EBPF mast 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 unload eBPF if was loaded before */
>     > +        virtio_net_unload_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);
>     > +
>     >       return queues;
>     >   error:
>     >       trace_virtio_net_rss_error(err_msg, err_value);
>     > @@ -1674,7 +1772,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);
>     > @@ -2780,6 +2878,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_load_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));
>     > @@ -3453,6 +3563,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 f4852ac27b..4d29a577eb 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;
>
>
>     We probably need a better name of this since "software" is kind of
>     confusing.
>
> No problem
>
>
>     >       bool    redirect;
>     >       bool    populate_hash;
>     >       uint32_t hash_types;
>     > @@ -214,6 +217,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 99c476db8c..feb5fa8624 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,
>
>
>     This is fine but looks unrelated to eBPF RSS support.
>
>     Usually it means the hardware can support RSS/hash reporting.
>
> As I see from 
> https://github.com/qemu/qemu/blob/master/hw/virtio/vhost.c#L1526:
> If we do not add these features this means they are transparent to 
> specific vhost solution.
> When in fact they are not. We need to ensure these features are not in 
> use in case of
> vhost-vdpa or vhost-user.
>
>     Thanks
>
>
>     >       VIRTIO_NET_F_GUEST_ANNOUNCE,
>     >       VIRTIO_NET_F_STATUS,
>     >       VHOST_INVALID_FEATURE_BIT
>
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 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);
+
+    return ret;
+}
+
 static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
                                         Error **errp)
 {
@@ -732,9 +745,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 &&
@@ -1169,12 +1182,75 @@  static int virtio_net_handle_announce(VirtIONet *n, uint8_t cmd,
     }
 }
 
+static void virtio_net_unload_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;
+
+    if (!n->rss_data.enabled_software_rss && ebpf_rss_is_loaded(&n->ebpf_rss)) {
+        virtio_net_unload_epbf_rss(n);
+    }
+}
+
+static bool virtio_net_attach_steering_ebpf(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_load_epbf_rss(VirtIONet *n)
+{
+    struct EBPFRSSConfig config = {};
+
+    if (!n->rss_data.enabled) {
+        if (ebpf_rss_is_loaded(&n->ebpf_rss)) {
+            ebpf_rss_unload(&n->ebpf_rss);
+        }
+        return true;
+    }
+
+    if (!ebpf_rss_is_loaded(&n->ebpf_rss) && !ebpf_rss_load(&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)) {
+        ebpf_rss_unload(&n->ebpf_rss);
+        return false;
+    }
+
+    if (!virtio_net_attach_steering_ebpf(n->nic, n->ebpf_rss.program_fd)) {
+        ebpf_rss_unload(&n->ebpf_rss);
+        return false;
+    }
+
+    return true;
+}
+
+static void virtio_net_unload_epbf_rss(VirtIONet *n)
+{
+    virtio_net_attach_steering_ebpf(n->nic, -1);
+    ebpf_rss_unload(&n->ebpf_rss);
 }
 
 static uint16_t virtio_net_handle_rss(VirtIONet *n,
@@ -1208,6 +1284,7 @@  static uint16_t virtio_net_handle_rss(VirtIONet *n,
         err_value = (uint32_t)s;
         goto error;
     }
+    n->rss_data.enabled_software_rss = false;
     n->rss_data.hash_types = virtio_ldl_p(vdev, &cfg.hash_types);
     n->rss_data.indirections_len =
         virtio_lduw_p(vdev, &cfg.indirection_table_mask);
@@ -1289,9 +1366,30 @@  static uint16_t virtio_net_handle_rss(VirtIONet *n,
         goto error;
     }
     n->rss_data.enabled = true;
+
+    if (!n->rss_data.populate_hash) {
+        /* load EBPF RSS */
+        if (!virtio_net_load_epbf_rss(n)) {
+            /* EBPF mast 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 unload eBPF if was loaded before */
+        virtio_net_unload_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);
+
     return queues;
 error:
     trace_virtio_net_rss_error(err_msg, err_value);
@@ -1674,7 +1772,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);
@@ -2780,6 +2878,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_load_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));
@@ -3453,6 +3563,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 f4852ac27b..4d29a577eb 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;
@@ -214,6 +217,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 99c476db8c..feb5fa8624 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