qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Tobias Fiebig" <tobias@fiebig.nl>
To: "'Stefan Hajnoczi'" <stefanha@redhat.com>, <qemu-devel@nongnu.org>
Cc: <jasowang@redhat.com>, <qemu-stable@nongnu.org>,
	"'Russell King - ARM Linux'" <linux@armlinux.org.uk>
Subject: RE: [PATCH for-7.2] rtl8139: honor large send MSS value
Date: Wed, 16 Nov 2022 00:36:25 +0100	[thread overview]
Message-ID: <010001d8f94b$13bd5bf0$3b3813d0$@fiebig.nl> (raw)
In-Reply-To: <20221115163659.1595865-1-stefanha@redhat.com>

Heho,
Just to keep you in the loop; Just applied the patch, but things didn't really get better; I am currently doing a 'make clean; make' for good measure (had built head first), and will also double-check that there is no accidental use of system-qemu libs. 

If that still doesn't show an effect, I'll hold tcpdump to the wire again.

With best regards,
Tobias 

-----Original Message-----
From: Stefan Hajnoczi <stefanha@redhat.com> 
Sent: Tuesday, 15 November 2022 17:37
To: qemu-devel@nongnu.org
Cc: jasowang@redhat.com; qemu-stable@nongnu.org; Stefan Hajnoczi <stefanha@redhat.com>; Russell King - ARM Linux <linux@armlinux.org.uk>; Tobias Fiebig <tobias+git@fiebig.nl>
Subject: [PATCH for-7.2] rtl8139: honor large send MSS value

The Large-Send Task Offload Tx Descriptor (9.2.1 Transmit) has a Large-Send MSS value where the driver specifies the MSS. See the datasheet here:
http://realtek.info/pdf/rtl8139cp.pdf

The code ignores this value and uses a hardcoded MSS of 1500 bytes instead. When the MTU is less than 1500 bytes the hardcoded value results in IP fragmentation and poor performance.

Use the Large-Send MSS value to correctly size Large-Send packets.

This issue was discussed in the past here:
https://lore.kernel.org/all/20161114162505.GD26664@stefanha-x1.localdomain/

Reported-by: Russell King - ARM Linux <linux@armlinux.org.uk>
Reported-by: Tobias Fiebig <tobias+git@fiebig.nl>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1312
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/net/rtl8139.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Tobias: Please test this fix. Thanks!

diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index e6643e3c9d..ecc4dcb07f 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -77,7 +77,6 @@
     ( ( input ) & ( size - 1 )  )
 
 #define ETHER_TYPE_LEN 2
-#define ETH_MTU     1500
 
 #define VLAN_TCI_LEN 2
 #define VLAN_HLEN (ETHER_TYPE_LEN + VLAN_TCI_LEN) @@ -2151,8 +2150,8 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 
                 int large_send_mss = (txdw0 >> 16) & CP_TC_LGSEN_MSS_MASK;
 
-                DPRINTF("+++ C+ mode offloaded task TSO MTU=%d IP data %d "
-                    "frame data %d specified MSS=%d\n", ETH_MTU,
+                DPRINTF("+++ C+ mode offloaded task TSO IP data %d "
+                    "frame data %d specified MSS=%d\n",
                     ip_data_len, saved_size - ETH_HLEN, large_send_mss);
 
                 int tcp_send_offset = 0; @@ -2177,9 +2176,13 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
                     goto skip_offload;
                 }
 
-                /* ETH_MTU = ip header len + tcp header len + payload */
+                /* MSS too small? */
+                if (tcp_hlen + hlen >= large_send_mss) {
+                    goto skip_offload;
+                }
+
                 int tcp_data_len = ip_data_len - tcp_hlen;
-                int tcp_chunk_size = ETH_MTU - hlen - tcp_hlen;
+                int tcp_chunk_size = large_send_mss - hlen - tcp_hlen;
 
                 DPRINTF("+++ C+ mode TSO IP data len %d TCP hlen %d TCP "
                     "data len %d TCP chunk size %d\n", ip_data_len,
--
2.38.1




  reply	other threads:[~2022-11-16  1:48 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-15 16:36 [PATCH for-7.2] rtl8139: honor large send MSS value Stefan Hajnoczi
2022-11-15 23:36 ` Tobias Fiebig [this message]
2022-11-16  4:19   ` Jason Wang
2022-11-16  2:58 ` Tobias Fiebig
2022-11-16  6:54   ` Jason Wang
2022-11-16 10:04     ` Tobias Fiebig
2022-11-16 11:20     ` Tobias Fiebig
2022-11-16 15:47       ` Stefan Hajnoczi
2022-11-17  2:49         ` Tobias Fiebig
2022-11-17 11:07           ` Stefan Hajnoczi
2022-11-17 11:15             ` Stefan Hajnoczi
2022-11-17 11:26               ` Tobias Fiebig
2022-11-17 16:56                 ` Stefan Hajnoczi
2022-11-17 17:02                   ` Tobias Fiebig
2022-11-17 20:42                   ` Tobias Fiebig
2022-11-17 22:51                     ` Stefan Hajnoczi
2022-11-18  7:10                 ` Jason Wang
2022-11-16  4:13 ` Jason Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='010001d8f94b$13bd5bf0$3b3813d0$@fiebig.nl' \
    --to=tobias@fiebig.nl \
    --cc=jasowang@redhat.com \
    --cc=linux@armlinux.org.uk \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).