On Thu, Nov 25, 2021 at 3:26 PM Michael S. Tsirkin [off-list ref] wrote:

On Thu, Nov 25, 2021 at 03:20:07PM +0800, Jason Wang wrote:

quoted

On Thu, Nov 25, 2021 at 3:15 PM Michael S. Tsirkin [off-list ref] wrote:

quoted

On Thu, Nov 25, 2021 at 03:11:58PM +0800, Jason Wang wrote:

quoted

On Thu, Nov 25, 2021 at 3:00 PM Michael S. Tsirkin [off-list ref] wrote:

quoted

On Thu, Nov 25, 2021 at 02:05:47PM +0800, Jason Wang wrote:

quoted

When VIRTIO_NET_F_MTU feature is not negotiated, we assume a very
large max_mtu. In this case, using small packet mode is not correct
since it may breaks the networking when MTU is grater than
ETH_DATA_LEN.

To have a quick fix, simply enable the big packet mode when
VIRTIO_NET_F_MTU is not negotiated.

This will slow down dpdk hosts which disable mergeable buffers
and send standard MTU sized packets.

quoted

We can do optimization on top.

I don't think it works like this, increasing mtu
from guest >4k never worked,

Looking at add_recvbuf_small() it's actually GOOD_PACKET_LEN if I was not wrong.

OK, even more so then.

quoted

quoted

we can't regress everyone's
performance with a promise to maybe sometime bring it back.

So consider it never work before I wonder if we can assume a 1500 as
max_mtu value instead of simply using MAX_MTU?

Thanks

You want to block guests from setting MTU to a value >GOOD_PACKET_LEN?

Yes, or fix the issue to let large packets on RX work (e.g as the TODO
said, size the buffer: for <=4K mtu continue to work as
add_recvbuf_small(), for >= 4K switch to use big).

Right. The difficulty is with changing modes, current code isn't
designed for it.

I think it might work if we reset the device during the mode change.

Thanks

quoted

quoted

Maybe ... it will prevent sending large packets which did work ...

Yes, but it's strange to allow TX but not RX

quoted

I'd tread carefully here, and I don't think this kind of thing is net
material.

I agree consider it can't be fixed easily.

Thanks

quoted

quoted

quoted

quoted

Reported-by: Eli Cohen <redacted>
Cc: Eli Cohen <redacted>
Signed-off-by: Jason Wang <jasowang@redhat.com>

---
 drivers/net/virtio_net.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 7c43bfc1ce44..83ae3ef5eb11 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c

@@ -3200,11 +3200,12 @@ static int virtnet_probe(struct virtio_device *vdev)
              dev->mtu = mtu;
              dev->max_mtu = mtu;

-             /* TODO: size buffers correctly in this case. */
-             if (dev->mtu > ETH_DATA_LEN)
-                     vi->big_packets = true;
      }

+     /* TODO: size buffers correctly in this case. */
+     if (dev->max_mtu > ETH_DATA_LEN)
+             vi->big_packets = true;
+
      if (vi->any_header_sg)
              dev->needed_headroom = vi->hdr_len;

--

2.25.1

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