virtio_net_receive_rcu does not populate hash value and
hash type in case it needs to change queue number from the
initially defined by tap/tun to one calculated according
to the packet hash. Earlier commit a4c960e places the hash
value/type into local extra_hdr structure but the extra_hdr
is not filled in recursive call to virtio_net_receive_rcu.
Current commit discards the recursive call and just continues
execution with changed net client state and uses correct
virtio-net queue for the packet.

Fixes: a4c960e ("virtio-net: Do not write hashes to peer buffer")

Jira link: https://issues.redhat.com/browse/RHEL-59572

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 hw/net/virtio-net.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index f2104ed364..ed4e33b48c 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1898,7 +1898,7 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
                                       size_t size, bool no_rss)
 {
     VirtIONet *n = qemu_get_nic_opaque(nc);
-    VirtIONetQueue *q = virtio_net_get_subqueue(nc);
+    VirtIONetQueue *q;
     VirtIODevice *vdev = VIRTIO_DEVICE(n);
     VirtQueueElement *elems[VIRTQUEUE_MAX_SIZE];
     size_t lens[VIRTQUEUE_MAX_SIZE];
@@ -1915,12 +1915,13 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
     if (!no_rss && n->rss_data.enabled && n->rss_data.enabled_software_rss) {
         int index = virtio_net_process_rss(nc, buf, size, &extra_hdr);
         if (index >= 0) {
-            NetClientState *nc2 =
-                qemu_get_subqueue(n->nic, index % n->curr_queue_pairs);
-            return virtio_net_receive_rcu(nc2, buf, size, true);
+            /* change nc to proper one */
+            nc = qemu_get_subqueue(n->nic, index % n->curr_queue_pairs);
         }
     }
 
+    q = virtio_net_get_subqueue(nc);
+
     /* hdr_len refers to the header we supply to the guest */
     if (!virtio_net_has_buffers(q, size + n->guest_hdr_len - n->host_hdr_len)) {
         return 0;
-- 
2.43.0

On 2024/10/30 21:13, Yuri Benditovich wrote:
> virtio_net_receive_rcu does not populate hash value and
> hash type in case it needs to change queue number from the
> initially defined by tap/tun to one calculated according
> to the packet hash. Earlier commit a4c960e places the hash
> value/type into local extra_hdr structure but the extra_hdr
> is not filled in recursive call to virtio_net_receive_rcu.
> Current commit discards the recursive call and just continues
> execution with changed net client state and uses correct
> virtio-net queue for the packet.
> 
> Fixes: a4c960e ("virtio-net: Do not write hashes to peer buffer")
> 
> Jira link: https://issues.redhat.com/browse/RHEL-59572
> 
> Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>

Hi,

I have submitted a similar patch:
https://patchew.org/QEMU/20240915-queue-v1-0-b49bd49b926d@daynix.com/20240915-queue-v1-4-b49bd49b926d@daynix.com/

Jason, please pick the buglink, which was missing in my patch.

Regards,
Akihiko Odaki

> ---
>   hw/net/virtio-net.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index f2104ed364..ed4e33b48c 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1898,7 +1898,7 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
>                                         size_t size, bool no_rss)
>   {
>       VirtIONet *n = qemu_get_nic_opaque(nc);
> -    VirtIONetQueue *q = virtio_net_get_subqueue(nc);
> +    VirtIONetQueue *q;
>       VirtIODevice *vdev = VIRTIO_DEVICE(n);
>       VirtQueueElement *elems[VIRTQUEUE_MAX_SIZE];
>       size_t lens[VIRTQUEUE_MAX_SIZE];
> @@ -1915,12 +1915,13 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
>       if (!no_rss && n->rss_data.enabled && n->rss_data.enabled_software_rss) {
>           int index = virtio_net_process_rss(nc, buf, size, &extra_hdr);
>           if (index >= 0) {
> -            NetClientState *nc2 =
> -                qemu_get_subqueue(n->nic, index % n->curr_queue_pairs);
> -            return virtio_net_receive_rcu(nc2, buf, size, true);
> +            /* change nc to proper one */
> +            nc = qemu_get_subqueue(n->nic, index % n->curr_queue_pairs);
>           }
>       }
>   
> +    q = virtio_net_get_subqueue(nc);
> +
>       /* hdr_len refers to the header we supply to the guest */
>       if (!virtio_net_has_buffers(q, size + n->guest_hdr_len - n->host_hdr_len)) {
>           return 0;