netdev - Re: [PATCH net-next 1/2] 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]

Date: Thu, 9 May 2024 16:38:30 +0200
From: Linus Walleij <linus.walleij@...aro.org>
To: Eric Dumazet <edumazet@...gle.com>
Cc: Hans Ulli Kroll <ulli.kroll@...glemail.com>, "David S. Miller" <davem@...emloft.net>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 1/2] net: ethernet: cortina: Restore TSO support

On Thu, May 9, 2024 at 10:21 AM Eric Dumazet <edumazet@...gle.com> wrote:
> On Thu, May 9, 2024 at 9:48 AM 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 = 05a8 len=1c8a
> > 60008000.ethernet-port eth0: segment offloading mss = 05a8 len=1c8a
> > 60008000.ethernet-port eth0: segment offloading mss = 05a8 len=27da
> > 60008000.ethernet-port eth0: segment offloading mss = 05a8 len=0b92
> > 60008000.ethernet-port eth0: segment offloading mss = 05a8 len=2bda
> > (...)
> >
> > It also performs well: ~268 MBit/s.
>
> This does not look very good to me ?

Oh it's pretty typical. This is an ARMv4 router from 2007, end-of-lifed
in 2015, and it is not meant to be stressed by the software like
this, the idea is that packets get routed by the DSA switch
(RTL8366RB).

> What number do you have when/if TSO is turned off ?

Around 187 MBit/s.

> > +       /* Translate to link layer size */
> > +       mss += ETH_HLEN;
> > +       if (skb->protocol == htons(ETH_P_8021Q))
> > +               mss += VLAN_HLEN;
>
> Are you sure this is needed at all ?
> Why not include IP and TCP header sizes as well, if the datasheet
> mentions 'link layer size' ?

Actually that code is just reusing the mss variable for
skb->len in the case where TSO is not used, so I'll try to
be more elaborate in the code :/

I guess I actually need to account for it if ->gso_size expand
to the MTU of the interface if I bump it up. But I don't
know if the the TSO code actually does this or if it is
more conservative?

> To double check, please disable GRO on the receive side and verify the
> packet sizes with tcpdump.
>
> Typically, for MTU=1500, IPv4, and TCP timestamp enabled,
> skb_shinfo(skb)->gso_size is 1448
>
> (Because 20 (ipv4 header) + 32 (tcp header with TS option) + 1448 = 1500)

I disabled all segment offloading on the receiving side:
ethtool -K enp2s0 gro off gso off tso off

The iperf3 -c generates segmens like in the commit message:
gemini-ethernet-port 60008000.ethernet-port eth0: segment offloading
mss = 05a8 len=2bda
gemini-ethernet-port 60008000.ethernet-port eth0: segment offloading
mss = 05a8 len=27da
gemini-ethernet-port 60008000.ethernet-port eth0: segment offloading
mss = 05a8 len=0b92

And 05a8 is 1448 so it is expected.

tcpdump -e -X enp2s0 gives this on a single segment in a segmented
iperf3 -c transfer:

16:24:09.182095 14:d6:4d:a8:3c:4f (oui Unknown) > fc:34:97:01:a0:c6
(oui Unknown), ethertype IPv4 (0x0800), length 1448: OpenWrt.lan.56624
> Fecusia.targus-getdata1: Flags [.], seq 18664:20046, ack 1, win
4198, options [nop,nop,TS val 2770370491 ecr 3490176978], length 1382
    0x0000:  4500 059a 8ff6 4000 4006 218d c0a8 0188  E.....@.@.......
    0x0010:  c0a8 0102 dd30 1451 a701 4f9d e809 8788  .....0.Q..O.....
    0x0020:  8010 1066 0b60 0000 0101 080a a520 7fbb  ...f.`..........
(...)
    0x0580:  de60 2081 5678 4f8b 31b1 6f85 87fe ae63  .`..VxO.1.o....c
    0x0590:  e2ca 8281 fa72 16aa 52e2                 .....r..R.

As can be seen in the header, it is indeed 1448 bytes when arriving
as well, so it seems to work!

Yours,
Linus Walleij