| Message ID | 20231017040932.62997-16-akihiko.odaki@daynix.com |
|---|---|
| State | New |
| Headers | show |
| Series | virtio-net RSS/hash report fixes and improvements | expand |
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.
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)) {
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(-)