From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4C61EC64E7A for ; Tue, 1 Dec 2020 07:43:00 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 39BFA2085B for ; Tue, 1 Dec 2020 07:42:59 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=daynix-com.20150623.gappssmtp.com header.i=@daynix-com.20150623.gappssmtp.com header.b="q1ApCZ1m" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 39BFA2085B Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=daynix.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:41768 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kk0JN-0001Kz-Ez for qemu-devel@archiver.kernel.org; Tue, 01 Dec 2020 02:42:57 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:58568) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kk0Hh-0000cu-6C for qemu-devel@nongnu.org; Tue, 01 Dec 2020 02:41:13 -0500 Received: from mail-ot1-x341.google.com ([2607:f8b0:4864:20::341]:40079) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kk0He-0001uY-0O for qemu-devel@nongnu.org; Tue, 01 Dec 2020 02:41:12 -0500 Received: by mail-ot1-x341.google.com with SMTP id 79so793259otc.7 for ; Mon, 30 Nov 2020 23:41:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=daynix-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=msmfZefb7reVOc/YpWo09SwHioVVaL8Ux1QkAc9Z2eE=; b=q1ApCZ1mtI86YZqEIJW7uJJmV3KJNXttl5pozv91SsEjJamwD22G0egBOVKtV5HQOC Edke6x651XOFLvKWeYuTpjldvd4txFKmQWZenVtJ3WKAKHgQdNjlOa5tiTr6ZcchoyUa /G6Qx+S085tm/xpygStWIeOg48ntZjF4RGN1Nu5FOseBcPYKOcb3/rFUhaFgkGtxG+9m noDvibl3RpsujG+7TEtJFzvajTpp2aX/jO0WQgqYVfIbxlqVD+7wS15yD3ALw9xZk/LC 9+cOc31KrXYYY8GSI3lLJEPPAl5/Sys3pfAPRHb/D0x/3VJRVL94kw5b105C94Zt0Jgj itGg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=msmfZefb7reVOc/YpWo09SwHioVVaL8Ux1QkAc9Z2eE=; b=TqbpjldT9/YvRwvn9p0Pg1N6o5WIRgYTloozQOo3TXLHUB0LZZURo84VUI6HLUe9jp PimMN16NXd8HyYV/ClfW5dfEGkqxGANLaU0GFaGuR0VDvtMYpML9DRvsRt86ECZ1DbMP gXpuj2sAsj8nRDh4fI95vdNEquKBYW65GRc/zU+hbhBBeVxxP9/jYKUDt5BWru3TXp1N Q6sBCMbgX8MgBzyUaMLK1YfQg3AuMI8hFyayrr/g12Dmh7TzTCdA3uJzbEXKcMWOpQHW Rm2et8VKrDwlZTN0UBUwLxaS6PV1jI2Zf//Ile6O7zY6cBcB9q4ahFmSCLWoPSHLcTBn /58A== X-Gm-Message-State: AOAM531QVNK9o6xGcDVjA+4KGnIzlGfxMZ6AIQf5wKNRegruMrXoxhjx xkKTl+D4T85bkh905b2JpoMiazpf6zcVIr/W5D/0pg== X-Google-Smtp-Source: ABdhPJxBvGBM10oMNDx10RT+WbiqNzDmhV6hHgbX7Kzr3eCjVMX+itUHRPcS3bHeJSofyKWG9wjRoLHE+1w+esjcdvk= X-Received: by 2002:a9d:ece:: with SMTP id 72mr919992otj.27.1606808468295; Mon, 30 Nov 2020 23:41:08 -0800 (PST) MIME-Version: 1.0 References: <20201119111305.485202-1-andrew@daynix.com> <20201119111305.485202-5-andrew@daynix.com> In-Reply-To: From: Yuri Benditovich Date: Tue, 1 Dec 2020 09:40:56 +0200 Message-ID: Subject: Re: [RFC PATCH v2 4/5] virtio-net: Added eBPF RSS to virtio-net. To: Jason Wang Content-Type: multipart/alternative; boundary="000000000000f9aa8f05b5623fdf" Received-SPF: none client-ip=2607:f8b0:4864:20::341; envelope-from=yuri.benditovich@daynix.com; helo=mail-ot1-x341.google.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_NONE=0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Yan Vugenfirer , Andrew Melnychenko , qemu-devel@nongnu.org, "Michael S . Tsirkin" Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --000000000000f9aa8f05b5623fdf Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Nov 24, 2020 at 10:49 AM Jason Wang wrote: > > On 2020/11/19 =E4=B8=8B=E5=8D=887:13, Andrew Melnychenko wrote: > > From: Andrew > > > > 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=3Doff when eBPF can't be > loaded > > or when hash population requested by the guest. > > > > Signed-off-by: Yuri Benditovich > > Signed-off-by: Andrew Melnychenko > > --- > > 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[] =3D { > > 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=3Don & CONFIG_EBPF doesn't set - disable RSS feature *= / > > + uint64_t ret =3D 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) > > > + > > + 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(VirtIODevic= e > *vdev, uint64_t features, > > return features; > > } > > > > - virtio_clear_feature(&features, VIRTIO_NET_F_RSS); > > - virtio_clear_feature(&features, VIRTIO_NET_F_HASH_REPORT); > > - features =3D vhost_net_get_features(get_vhost_net(nc->peer), > features); > > + features =3D fix_ebpf_vhost_features( > > + vhost_net_get_features(get_vhost_net(nc->peer), features))= ; > > + > > vdev->backend_features =3D 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 =3D 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 =3D qemu_get_peer(qemu_get_queue(nic), 0); > > + if (nc =3D=3D NULL || nc->info->set_steering_ebpf =3D=3D 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 =3D data->redirect; > > + config->populate_hash =3D data->populate_hash; > > + config->hash_types =3D data->hash_types; > > + config->indirections_len =3D data->indirections_len; > > + config->default_queue =3D data->default_queue; > > +} > > + > > +static bool virtio_net_load_epbf_rss(VirtIONet *n) > > +{ > > + struct EBPFRSSConfig config =3D {}; > > + > > + 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 =3D (uint32_t)s; > > goto error; > > } > > + n->rss_data.enabled_software_rss =3D false; > > n->rss_data.hash_types =3D virtio_ldl_p(vdev, &cfg.hash_types); > > n->rss_data.indirections_len =3D > > 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 =3D true; > > + > > + if (!n->rss_data.populate_hash) { > > + /* load EBPF RSS */ > > > The code explains itself, so the comment is no necessary. > > > > + if (!virtio_net_load_epbf_rss(n)) { > > > Any reason that we load eBPF RSS here? I thought it would be easier to > do it during set_features (e.g when RSS is negotiated but not > HASH_REPORT) and if we do that we don't need extra care about migration. > > > > + /* EBPF mast be loaded for vhost */ > > > Typo. > > > > + 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 =3D 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 =3D true; > > + } > > + > > trace_virtio_net_rss_enable(n->rss_data.hash_types, > > n->rss_data.indirections_len, > > temp.b); > > + > > > Unnecessary changes. > > > > 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 =3D virtio_net_process_rss(nc, buf, size); > > if (index >=3D 0) { > > NetClientState *nc2 =3D 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 =3D 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 t= o > software RSS"); > > + n->rss_data.enabled_software_rss =3D true; > > + } > > + } > > + } > > > btw, I don't see the save and restore of bpf maps, or is it unnecessary? > > Thanks > > > > + > > 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[] =3D { > > 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 > > --000000000000f9aa8f05b5623fdf Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Tue, Nov 24, 2020 at 10:49 AM Jaso= n Wang <jasowang@redhat.com&g= t; wrote:

On 2020/11/19 =E4=B8=8B=E5=8D=887:13, 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=3Doff 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>
> ---
>=C2=A0 =C2=A0hw/net/vhost_net.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0|=C2=A0 =C2=A02 +
>=C2=A0 =C2=A0hw/net/virtio-net.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 | 120 +++++++++++++++++++++++++++++++--
>=C2=A0 =C2=A0include/hw/virtio/virtio-net.h |=C2=A0 =C2=A04 ++
>=C2=A0 =C2=A0net/vhost-vdpa.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0|=C2=A0 =C2=A02 +
>=C2=A0 =C2=A04 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[] =3D {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0VIRTIO_NET_F_MTU,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0VIRTIO_F_IOMMU_PLATFORM,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0VIRTIO_F_RING_PACKED,
> +=C2=A0 =C2=A0 VIRTIO_NET_F_RSS,
> +=C2=A0 =C2=A0 VIRTIO_NET_F_HASH_REPORT,
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0 =C2=A0 =C2=A0/* This bit implies RARP isn't sent by Q= EMU out of band */
>=C2=A0 =C2=A0 =C2=A0 =C2=A0VIRTIO_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) >=C2=A0 =C2=A0
>=C2=A0 =C2=A0static void virtio_net_set_multiqueue(VirtIONet *n, int mu= ltiqueue);
>=C2=A0 =C2=A0
> +static uint64_t fix_ebpf_vhost_features(uint64_t features)
> +{
> +=C2=A0 =C2=A0 /* If vhost=3Don & CONFIG_EBPF doesn't set - di= sable RSS feature */
> +=C2=A0 =C2=A0 uint64_t ret =3D features;
> +#ifndef CONFIG_EBPF
> +=C2=A0 =C2=A0 virtio_clear_feature(&ret, VIRTIO_NET_F_RSS);
> +#endif
> +=C2=A0 =C2=A0 /* for now, there is no solution for populating the has= h from eBPF */
> +=C2=A0 =C2=A0 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 i= t 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 disab= le
vhost-net unconditionally.


I agree in general with this requireme= nt and I'm preparing an implementation of such fallback.

=
The problem is that qemu already uses the mechanism=C2=A0of turn= ing off host features
silently if they are not supported by the c= urrent vhost in kernel:

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)
=C2=A0

> +
> +=C2=A0 =C2=A0 return ret;
> +}
> +
>=C2=A0 =C2=A0static uint64_t virtio_net_get_features(VirtIODevice *vdev= , uint64_t features,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0Error **errp)
>=C2=A0 =C2=A0{
> @@ -732,9 +745,9 @@ static uint64_t virtio_net_get_features(VirtIODevi= ce *vdev, uint64_t features,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return features;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0}
>=C2=A0 =C2=A0
> -=C2=A0 =C2=A0 virtio_clear_feature(&features, VIRTIO_NET_F_RSS);<= br> > -=C2=A0 =C2=A0 virtio_clear_feature(&features, VIRTIO_NET_F_HASH_R= EPORT);
> -=C2=A0 =C2=A0 features =3D vhost_net_get_features(get_vhost_net(nc-&g= t;peer), features);
> +=C2=A0 =C2=A0 features =3D fix_ebpf_vhost_features(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 vhost_net_get_features(get_= vhost_net(nc->peer), features));
> +
>=C2=A0 =C2=A0 =C2=A0 =C2=A0vdev->backend_features =3D features;
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (n->mtu_bypass_backend &&
> @@ -1169,12 +1182,75 @@ static int virtio_net_handle_announce(VirtIONe= t *n, uint8_t cmd,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0}
>=C2=A0 =C2=A0}
>=C2=A0 =C2=A0
> +static void virtio_net_unload_epbf_rss(VirtIONet *n);
> +
>=C2=A0 =C2=A0static void virtio_net_disable_rss(VirtIONet *n)
>=C2=A0 =C2=A0{
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (n->rss_data.enabled) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0trace_virtio_net_rss_disable()= ;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0}
>=C2=A0 =C2=A0 =C2=A0 =C2=A0n->rss_data.enabled =3D false;
> +
> +=C2=A0 =C2=A0 if (!n->rss_data.enabled_software_rss && ebp= f_rss_is_loaded(&n->ebpf_rss)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 virtio_net_unload_epbf_rss(n);
> +=C2=A0 =C2=A0 }
> +}
> +
> +static bool virtio_net_attach_steering_ebpf(NICState *nic, int prog_f= d)
> +{
> +=C2=A0 =C2=A0 NetClientState *nc =3D qemu_get_peer(qemu_get_queue(nic= ), 0);
> +=C2=A0 =C2=A0 if (nc =3D=3D NULL || nc->info->set_steering_ebpf= =3D=3D NULL) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return false;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 return nc->info->set_steering_ebpf(nc, prog_fd);<= br> > +}
> +
> +static void rss_data_to_rss_config(struct VirtioNetRssData *data,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0struct EBPFRSSConfi= g *config)
> +{
> +=C2=A0 =C2=A0 config->redirect =3D data->redirect;
> +=C2=A0 =C2=A0 config->populate_hash =3D data->populate_hash; > +=C2=A0 =C2=A0 config->hash_types =3D data->hash_types;
> +=C2=A0 =C2=A0 config->indirections_len =3D data->indirections_l= en;
> +=C2=A0 =C2=A0 config->default_queue =3D data->default_queue; > +}
> +
> +static bool virtio_net_load_epbf_rss(VirtIONet *n)
> +{
> +=C2=A0 =C2=A0 struct EBPFRSSConfig config =3D {};
> +
> +=C2=A0 =C2=A0 if (!n->rss_data.enabled) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (ebpf_rss_is_loaded(&n->ebpf_rs= s)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ebpf_rss_unload(&n->= ebpf_rss);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return true;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 if (!ebpf_rss_is_loaded(&n->ebpf_rss) &&= !ebpf_rss_load(&n->ebpf_rss)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return false;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 rss_data_to_rss_config(&n->rss_data, &config= );
> +
> +=C2=A0 =C2=A0 if (!ebpf_rss_set_all(&n->ebpf_rss, &config,=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 n->rss_data.indirections_table, n->rss_data.key= )) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 ebpf_rss_unload(&n->ebpf_rss);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return false;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 if (!virtio_net_attach_steering_ebpf(n->nic, n->e= bpf_rss.program_fd)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 ebpf_rss_unload(&n->ebpf_rss);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return false;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 return true;
> +}
> +
> +static void virtio_net_unload_epbf_rss(VirtIONet *n)
> +{
> +=C2=A0 =C2=A0 virtio_net_attach_steering_ebpf(n->nic, -1);
> +=C2=A0 =C2=A0 ebpf_rss_unload(&n->ebpf_rss);
>=C2=A0 =C2=A0}
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0static uint16_t virtio_net_handle_rss(VirtIONet *n,
> @@ -1208,6 +1284,7 @@ static uint16_t virtio_net_handle_rss(VirtIONet = *n,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0err_value =3D (uint32_t)s;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0goto error;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0}
> +=C2=A0 =C2=A0 n->rss_data.enabled_software_rss =3D false;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0n->rss_data.hash_types =3D virtio_ldl_p(v= dev, &cfg.hash_types);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0n->rss_data.indirections_len =3D
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0virtio_lduw_p(vdev, &cfg.i= ndirection_table_mask);
> @@ -1289,9 +1366,30 @@ static uint16_t virtio_net_handle_rss(VirtIONet= *n,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0goto error;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0}
>=C2=A0 =C2=A0 =C2=A0 =C2=A0n->rss_data.enabled =3D true;
> +
> +=C2=A0 =C2=A0 if (!n->rss_data.populate_hash) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* load EBPF RSS */


The code explains itself, so the comment is no necessary.


> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!virtio_net_load_epbf_rss(n)) {


Any reason that we load eBPF RSS here? I thought it would be easier to
do it during set_features (e.g when RSS is negotiated but not
HASH_REPORT) and if we do that we don't need extra care about migration= .


> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* EBPF mast be loaded for = vhost */


Typo.


> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (get_vhost_net(qemu_get_= queue(n->nic)->peer)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 warn_report(&= quot;Can't load eBPF RSS for vhost");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 goto error; > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* fallback to software RSS= */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 warn_report("Can't= load eBPF RSS - fallback to software RSS");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 n->rss_data.enabled_soft= ware_rss =3D true;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 } else {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* use software RSS for hash populating *= /
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* and unload eBPF if was loaded before *= /
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 virtio_net_unload_epbf_rss(n);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 n->rss_data.enabled_software_rss =3D t= rue;
> +=C2=A0 =C2=A0 }
> +
>=C2=A0 =C2=A0 =C2=A0 =C2=A0trace_virtio_net_rss_enable(n->rss_data.h= ash_types,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0n->rss_data.indir= ections_len,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0temp.b);
> +


Unnecessary changes.


>=C2=A0 =C2=A0 =C2=A0 =C2=A0return queues;
>=C2=A0 =C2=A0error:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0trace_virtio_net_rss_error(err_msg, err_valu= e);
> @@ -1674,7 +1772,7 @@ static ssize_t virtio_net_receive_rcu(NetClientS= tate *nc, const uint8_t *buf,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -1;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0}
>=C2=A0 =C2=A0
> -=C2=A0 =C2=A0 if (!no_rss && n->rss_data.enabled) {
> +=C2=A0 =C2=A0 if (!no_rss && n->rss_data.enabled &&= ; n->rss_data.enabled_software_rss) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0int index =3D virtio_net_proce= ss_rss(nc, buf, size);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (index >=3D 0) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0NetClientState *= nc2 =3D qemu_get_subqueue(n->nic, index);
> @@ -2780,6 +2878,18 @@ static int virtio_net_post_load_device(void *op= aque, int version_id)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0}
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (n->rss_data.enabled) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 n->rss_data.enabled_software_rss =3D n= ->rss_data.populate_hash;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!n->rss_data.populate_hash) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!virtio_net_load_epbf_r= ss(n)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (get_vhost= _net(qemu_get_queue(n->nic)->peer)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= error_report("Can't post-load eBPF RSS for vhost");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 } else {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= warn_report("Can't post-load eBPF RSS - fallback to software RSS&= quot;);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= n->rss_data.enabled_software_rss =3D true;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }


btw, I don't see the save and restore of bpf maps, or is it unnecessary= ?

Thanks


> +
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0trace_virtio_net_rss_enable(n-= >rss_data.hash_types,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0n->= rss_data.indirections_len,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sizeof= (n->rss_data.key));
> @@ -3453,6 +3563,8 @@ static void virtio_net_instance_init(Object *obj= )
>=C2=A0 =C2=A0 =C2=A0 =C2=A0device_add_bootindex_property(obj, &n-&g= t;nic_conf.bootindex,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"bootind= ex", "/ethernet-phy@0",
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0DEVICE(n)); > +
> +=C2=A0 =C2=A0 ebpf_rss_init(&n->ebpf_rss);
>=C2=A0 =C2=A0}
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0static 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 @@
>=C2=A0 =C2=A0#include "qemu/option_int.h"
>=C2=A0 =C2=A0#include "qom/object.h"
>=C2=A0 =C2=A0
> +#include "ebpf/ebpf_rss.h"
> +
>=C2=A0 =C2=A0#define TYPE_VIRTIO_NET "virtio-net-device"
>=C2=A0 =C2=A0OBJECT_DECLARE_SIMPLE_TYPE(VirtIONet, VIRTIO_NET)
>=C2=A0 =C2=A0
> @@ -130,6 +132,7 @@ typedef struct VirtioNetRscChain {
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0typedef struct VirtioNetRssData {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0bool=C2=A0 =C2=A0 enabled;
> +=C2=A0 =C2=A0 bool=C2=A0 =C2=A0 enabled_software_rss;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0bool=C2=A0 =C2=A0 redirect;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0bool=C2=A0 =C2=A0 populate_hash;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0uint32_t hash_types;
> @@ -214,6 +217,7 @@ struct VirtIONet {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0Notifier migration_state;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0VirtioNetRssData rss_data;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0struct NetRxPkt *rx_pkt;
> +=C2=A0 =C2=A0 struct EBPFRSSContext ebpf_rss;
>=C2=A0 =C2=A0};
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0void virtio_net_set_netclient_name(VirtIONet *n, const cha= r *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[] =3D {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0VIRTIO_NET_F_MTU,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0VIRTIO_F_IOMMU_PLATFORM,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0VIRTIO_F_RING_PACKED,
> +=C2=A0 =C2=A0 VIRTIO_NET_F_RSS,
> +=C2=A0 =C2=A0 VIRTIO_NET_F_HASH_REPORT,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0VIRTIO_NET_F_GUEST_ANNOUNCE,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0VIRTIO_NET_F_STATUS,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0VHOST_INVALID_FEATURE_BIT

--000000000000f9aa8f05b5623fdf--