virtio-net can report hash values even if the peer does not have a
virtio-net header.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 hw/net/virtio-net.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index c6a92ba3db..dc2b7b8ee7 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -763,8 +763,6 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
         virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_TSO4);
         virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_TSO6);
         virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_ECN);
-
-        virtio_clear_feature(&features, VIRTIO_NET_F_HASH_REPORT);
     }
 
     if (!peer_has_vnet_hdr(n) || !peer_has_ufo(n)) {
-- 
2.42.0

On Tue, Oct 17, 2023 at 12:14 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> virtio-net can report hash values even if the peer does not have a
> virtio-net header.

Do we need a compat flag for this?

Thanks

>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  hw/net/virtio-net.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index c6a92ba3db..dc2b7b8ee7 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -763,8 +763,6 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
>          virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_TSO4);
>          virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_TSO6);
>          virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_ECN);
> -
> -        virtio_clear_feature(&features, VIRTIO_NET_F_HASH_REPORT);
>      }
>
>      if (!peer_has_vnet_hdr(n) || !peer_has_ufo(n)) {
> --
> 2.42.0
>

On 2023/10/27 16:14, Jason Wang wrote:
> On Tue, Oct 17, 2023 at 12:14 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>>
>> virtio-net can report hash values even if the peer does not have a
>> virtio-net header.
> 
> Do we need a compat flag for this?

I don't think so. This change actually fixes the migration from a system 
with tap devices that support virtio-net headers to a system with tap 
devices that do not support virtio-net headers. Such a compatibility 
flag will revert the fix.

Regards,
Akihiko Odaki

This patch allows  VIRTIO_NET_F_HASH_REPORT feature to the adapter whose
backend does not have a virtio header and does not have offload features
that depend on it.
The migration between such different systems is very problematic even if it
seems successful, such setups are not performance-oriented and especially
supporting the hash delivery for them is (IMHO) redundant, it just requires
more testing and does not bring any advantage.


On Fri, Oct 27, 2023 at 11:07 AM Akihiko Odaki <akihiko.odaki@daynix.com>
wrote:

> On 2023/10/27 16:14, Jason Wang wrote:
> > On Tue, Oct 17, 2023 at 12:14 PM Akihiko Odaki <akihiko.odaki@daynix.com>
> wrote:
> >>
> >> virtio-net can report hash values even if the peer does not have a
> >> virtio-net header.
> >
> > Do we need a compat flag for this?
>
> I don't think so. This change actually fixes the migration from a system
> with tap devices that support virtio-net headers to a system with tap
> devices that do not support virtio-net headers. Such a compatibility
> flag will revert the fix.
>
> Regards,
> Akihiko Odaki
>

On 2023/10/30 6:56, Yuri Benditovich wrote:
> This patch allows  VIRTIO_NET_F_HASH_REPORT feature to the adapter whose 
> backend does not have a virtio header and does not have offload features 
> that depend on it.
> The migration between such different systems is very problematic even if 
> it seems successful, such setups are not performance-oriented and 
> especially supporting the hash delivery for them is (IMHO) redundant, it 
> just requires more testing and does not bring any advantage.

Whether the peer has virtio headers or not is irrelevant if 
VIRTIO_NET_F_HASH_REPORT is offloaded or not. Currently QEMU always 
implements hash reporting by itself and does not offload at all.