netdev - Re: [PATCH net-next v3 1/5] net: ethernet: cortina: Restore TSO support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89iKX0Gk8J=QVe5JwNi39zNzzfb2mP9tD4E5NLdimfrj5-w@mail.gmail.com>
Date: Tue, 14 May 2024 11:13:19 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Linus Walleij <linus.walleij@...aro.org>
Cc: Hans Ulli Kroll <ulli.kroll@...glemail.com>, "David S. Miller" <davem@...emloft.net>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Andrew Lunn <andrew@...n.ch>, 
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next v3 1/5] net: ethernet: cortina: Restore TSO support

On Mon, May 13, 2024 at 3:38 PM Linus Walleij <linus.walleij@...aro.org> wrote:
>
> An earlier commit deleted the TSO support in the Cortina Gemini
> driver because the driver was confusing gso_size and MTU,
> probably because what the Linux kernel calls "gso_size" was
> called "MTU" in the datasheet.
>
> Restore the functionality properly reading the gso_size from
> the skbuff.
>
> Tested with iperf3, running a server on a different machine
> and client on the device with the cortina gemini ethernet:
>
> Connecting to host 192.168.1.2, port 5201
> 60008000.ethernet-port eth0: segment offloading mss = 05ea len=1c8a
> 60008000.ethernet-port eth0: segment offloading mss = 05ea len=1c8a
> 60008000.ethernet-port eth0: segment offloading mss = 05ea len=27da
> 60008000.ethernet-port eth0: segment offloading mss = 05ea len=0b92
> 60008000.ethernet-port eth0: segment offloading mss = 05ea len=2bda
> (...)
>
> (The hardware MSS 0x05ea here includes the ethernet headers.)
>
> If I disable all segment offloading on the receiving host and
> dump packets using tcpdump -xx like this:
>
> ethtool -K enp2s0 gro off gso off tso off
> tcpdump -xx -i enp2s0 host 192.168.1.136
>
> I get segmented packages such as this when running iperf3:
>
> 23:16:54.024139 IP OpenWrt.lan.59168 > Fecusia.targus-getdata1:
> Flags [.], seq 1486:2934, ack 1, win 4198,
> options [nop,nop,TS val 3886192908 ecr 3601341877], length 1448
> 0x0000:  fc34 9701 a0c6 14d6 4da8 3c4f 0800 4500
> 0x0010:  05dc 16a0 4000 4006 9aa1 c0a8 0188 c0a8
> 0x0020:  0102 e720 1451 ff25 9822 4c52 29cf 8010
> 0x0030:  1066 ac8c 0000 0101 080a e7a2 990c d6a8
> (...)
> 0x05c0:  5e49 e109 fe8c 4617 5e18 7a82 7eae d647
> 0x05d0:  e8ee ae64 dc88 c897 3f8a 07a4 3a33 6b1b
> 0x05e0:  3501 a30f 2758 cc44 4b4a
>
> Several such packets often follow after each other verifying
> the segmentation into 0x05a8 (1448) byte packages also on the
> reveiving end. As can be seen, the ethernet frames are
> 0x05ea (1514) in size.
>
> Performance with iperf3 before this patch: ~15.5 Mbit/s
> Performance with iperf3 after this patch: ~175 Mbit/s
>
> This was running a 60 second test (twice) the best measurement
> was 179 Mbit/s.
>
> For comparison if I run iperf3 with UDP I get around 1.05 Mbit/s
> both before and after this patch.
>
> While this is a gigabit ethernet interface, the CPU is a cheap
> D-Link DIR-685 router (based on the ARMv5 Faraday FA526 at
> ~50 MHz), and the software is not supposed to drive traffic,
> as the device has a DSA chip, so this kind of numbers can be
> expected.
>
> Fixes: ac631873c9e7 ("net: ethernet: cortina: Drop TSO support")
> Signed-off-by: Linus Walleij <linus.walleij@...aro.org>
> ---

> +               mss += skb_tcp_all_headers(skb);
> +               netdev_dbg(netdev, "segment offloading mss = %04x len=%04x\n",
> +                          mss, skb->len);
> +               word1 |= TSS_MTU_ENABLE_BIT;
> +               word3 |= mss;
> +       } else if (skb->len >= ETH_FRAME_LEN) {

Note that this code path should be dead, because the upper layers
should drop the packets before reaching this point.
I wonder how you have tested it.

Reviewed-by: Eric Dumazet <edumazet@...gle.com>