[PATCH v2 0/2] virtio-net: Fix RSS

[PATCH v2 0/2] virtio-net: Fix RSS

[Akihiko Odaki]

Some recent changes made RSS unfunctional so here are fixes.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
Changes in v2:
- Fixed a potential problem with big-endian hosts.
- Link to v1: https://lore.kernel.org/r/20240326-vhost-v1-0-09bd85b1ba2c@daynix.com

---
Akihiko Odaki (2):
      virtio-net: Fix vhost virtqueue notifiers for RSS
      ebpf: Fix indirections table setting

 ebpf/ebpf_rss.c     | 9 +++++++--
 hw/net/virtio-net.c | 4 ++--
 2 files changed, 9 insertions(+), 4 deletions(-)
---
base-commit: ba49d760eb04630e7b15f423ebecf6c871b8f77b
change-id: 20240324-vhost-5e26c8a2da5a

Best regards,
-- 
Akihiko Odaki <akihiko.odaki@daynix.com>

[PATCH v2 1/2] virtio-net: Fix vhost virtqueue notifiers for RSS

[Akihiko Odaki]

virtio_net_guest_notifier_pending() and virtio_net_guest_notifier_mask()
checked VIRTIO_NET_F_MQ to know there are multiple queues, but
VIRTIO_NET_F_RSS also enables multiple queues. Refer to n->multiqueue,
which is set to true either of VIRTIO_NET_F_MQ or VIRTIO_NET_F_RSS is
enabled.

Fixes: 68b0a6395f36 ("virtio-net: align ctrl_vq index for non-mq guest for vhost_vdpa")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 hw/net/virtio-net.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 9959f1932b1b..a6ff000cd9d3 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3426,7 +3426,7 @@ static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx)
     VirtIONet *n = VIRTIO_NET(vdev);
     NetClientState *nc;
     assert(n->vhost_started);
-    if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ) && idx == 2) {
+    if (!n->multiqueue && idx == 2) {
         /* Must guard against invalid features and bogus queue index
          * from being set by malicious guest, or penetrated through
          * buggy migration stream.
@@ -3458,7 +3458,7 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx,
     VirtIONet *n = VIRTIO_NET(vdev);
     NetClientState *nc;
     assert(n->vhost_started);
-    if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ) && idx == 2) {
+    if (!n->multiqueue && idx == 2) {
         /* Must guard against invalid features and bogus queue index
          * from being set by malicious guest, or penetrated through
          * buggy migration stream.

-- 
2.44.0

[PATCH v2 2/2] ebpf: Fix indirections table setting

[Akihiko Odaki]

The kernel documentation says:
> The value stored can be of any size, however, all array elements are
> aligned to 8 bytes.
https://www.kernel.org/doc/html/v6.8/bpf/map_array.html

Fixes: 333b3e5fab75 ("ebpf: Added eBPF map update through mmap.")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 ebpf/ebpf_rss.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/ebpf/ebpf_rss.c b/ebpf/ebpf_rss.c
index 2e506f974357..d102f3dd0929 100644
--- a/ebpf/ebpf_rss.c
+++ b/ebpf/ebpf_rss.c
@@ -185,13 +185,18 @@ static bool ebpf_rss_set_indirections_table(struct EBPFRSSContext *ctx,
                                             uint16_t *indirections_table,
                                             size_t len)
 {
+    char *cursor = ctx->mmap_indirections_table;
+
     if (!ebpf_rss_is_loaded(ctx) || indirections_table == NULL ||
        len > VIRTIO_NET_RSS_MAX_TABLE_LEN) {
         return false;
     }
 
-    memcpy(ctx->mmap_indirections_table, indirections_table,
-            sizeof(*indirections_table) * len);
+    for (size_t i = 0; i < len; i++) {
+        *(uint16_t *)cursor = indirections_table[i];
+        cursor += 8;
+    }
+
     return true;
 }
 

-- 
2.44.0

Re: [PATCH v2 2/2] ebpf: Fix indirections table setting

[Yuri Benditovich]

Hi Andrew,
Can you please check the indirection table copy and ack on the patch
if the fix is correct

Thanks,
Yuri

On Wed, Mar 27, 2024 at 4:05 AM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> The kernel documentation says:
> > The value stored can be of any size, however, all array elements are
> > aligned to 8 bytes.
> https://www.kernel.org/doc/html/v6.8/bpf/map_array.html
>
> Fixes: 333b3e5fab75 ("ebpf: Added eBPF map update through mmap.")
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  ebpf/ebpf_rss.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/ebpf/ebpf_rss.c b/ebpf/ebpf_rss.c
> index 2e506f974357..d102f3dd0929 100644
> --- a/ebpf/ebpf_rss.c
> +++ b/ebpf/ebpf_rss.c
> @@ -185,13 +185,18 @@ static bool ebpf_rss_set_indirections_table(struct EBPFRSSContext *ctx,
>                                              uint16_t *indirections_table,
>                                              size_t len)
>  {
> +    char *cursor = ctx->mmap_indirections_table;
> +
>      if (!ebpf_rss_is_loaded(ctx) || indirections_table == NULL ||
>         len > VIRTIO_NET_RSS_MAX_TABLE_LEN) {
>          return false;
>      }
>
> -    memcpy(ctx->mmap_indirections_table, indirections_table,
> -            sizeof(*indirections_table) * len);
> +    for (size_t i = 0; i < len; i++) {
> +        *(uint16_t *)cursor = indirections_table[i];
> +        cursor += 8;
> +    }
> +
>      return true;
>  }
>
>
> --
> 2.44.0
>

Re: [PATCH v2 2/2] ebpf: Fix indirections table setting

[Andrew Melnichenko]

Hi all,
I've reviewed and checked - this patch is necessary!

Acked-by: andrew@daynix.com

On Thu, Mar 28, 2024 at 11:39 AM Yuri Benditovich
<yuri.benditovich@daynix.com> wrote:
>
> Hi Andrew,
> Can you please check the indirection table copy and ack on the patch
> if the fix is correct
>
> Thanks,
> Yuri
>
> On Wed, Mar 27, 2024 at 4:05 AM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> >
> > The kernel documentation says:
> > > The value stored can be of any size, however, all array elements are
> > > aligned to 8 bytes.
> > https://www.kernel.org/doc/html/v6.8/bpf/map_array.html
> >
> > Fixes: 333b3e5fab75 ("ebpf: Added eBPF map update through mmap.")
> > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> > ---
> >  ebpf/ebpf_rss.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/ebpf/ebpf_rss.c b/ebpf/ebpf_rss.c
> > index 2e506f974357..d102f3dd0929 100644
> > --- a/ebpf/ebpf_rss.c
> > +++ b/ebpf/ebpf_rss.c
> > @@ -185,13 +185,18 @@ static bool ebpf_rss_set_indirections_table(struct EBPFRSSContext *ctx,
> >                                              uint16_t *indirections_table,
> >                                              size_t len)
> >  {
> > +    char *cursor = ctx->mmap_indirections_table;
> > +
> >      if (!ebpf_rss_is_loaded(ctx) || indirections_table == NULL ||
> >         len > VIRTIO_NET_RSS_MAX_TABLE_LEN) {
> >          return false;
> >      }
> >
> > -    memcpy(ctx->mmap_indirections_table, indirections_table,
> > -            sizeof(*indirections_table) * len);
> > +    for (size_t i = 0; i < len; i++) {
> > +        *(uint16_t *)cursor = indirections_table[i];
> > +        cursor += 8;
> > +    }
> > +
> >      return true;
> >  }
> >
> >
> > --
> > 2.44.0
> >