On Fri, Feb 24, 2023 at 4:25 PM Michael S. Tsirkin [off-list ref] wrote:

On Thu, Feb 23, 2023 at 07:38:25PM +0000, Rob Bradford via B4 Relay wrote:

quoted

From: Rob Bradford <redacted>

kvmtool does not support the VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature
but does advertise the VIRTIO_NET_F_GUEST_TSO{4,6} features. Check that
the VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature is present before setting
the NETIF_F_GRO_HW feature bit as otherwise an attempt will be made to
program the virtio-net device using the ctrl queue which will fail.

This resolves the following error when running on kvmtool:

[    1.865992] net eth0: Fail to set guest offload.
[    1.872491] virtio_net virtio2 eth0: set_features() failed (-22); wanted 0x0000000000134829, left 0x0080000000134829

Signed-off-by: Rob Bradford <redacted>
---
Changes in v2:
- Use parentheses to group logical OR of features
- Link to v1:
  https://lore.kernel.org/r/20230223-virtio-net-kvmtool-v1-1-fc23d29b9d7a@rivosinc.com (local)
---
 drivers/net/virtio_net.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 61e33e4dd0cd..f8341d1a4ccd 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c

@@ -3780,10 +3780,9 @@ static int virtnet_probe(struct virtio_device *vdev)
      }
      if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_CSUM))
              dev->features |= NETIF_F_RXCSUM;
-     if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO4) ||
-         virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO6))
-             dev->features |= NETIF_F_GRO_HW;
-     if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS))
+     if ((virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO4) ||
+         virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO6)) &&
+         virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS))
              dev->hw_features |= NETIF_F_GRO_HW;

This will disable GRO/LRO on kvmtool completely causing a significant
performance regression.

Jason, isn't this what
        commit dbcf24d153884439dad30484a0e3f02350692e4c
        Author: Jason Wang [off-list ref]
        Date:   Tue Aug 17 16:06:59 2021 +0800

            virtio-net: use NETIF_F_GRO_HW instead of NETIF_F_LRO

was supposed to address?

Yes, I've asked a similar question in another thread.

And apropos this:

    Fix this by using NETIF_F_GRO_HW instead. Though the spec does not
    guarantee packets to be re-segmented as the original ones,
    we can add that to the spec, possibly with a flag for devices to
    differentiate between GRO and LRO.

this never happened. What's the plan exactly?

It's in the backlog, but I'm out of bandwidth for doing that now.

Thanks

quoted

      dev->vlan_features = dev->features;

---
base-commit: c39cea6f38eefe356d64d0bc1e1f2267e282cdd3
change-id: 20230223-virtio-net-kvmtool-87f37515be22

Best regards,
--
Rob Bradford [off-list ref]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization