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: <CACRpkdYPS7ox=rkAwwF3UCnh-HXqKmMh0CUmeG1QQMLTeQsZ9Q@mail.gmail.com>
Date: Tue, 14 May 2024 23:11:20 +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>, Andrew Lunn <andrew@...n.ch>, 
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next v3 1/5] net: ethernet: cortina: Restore TSO support

On Tue, May 14, 2024 at 11:13 AM Eric Dumazet <edumazet@...gle.com> wrote:
> On Mon, May 13, 2024 at 3:38 PM Linus Walleij <linus.walleij@...aro.org> wrote:

> > +               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.

It's actually easy to provoke with UDP jumboframes.

Bump the device with gemini ethernet MTU to max:
# ifconfig eth0 mtu 2029

Bump on the receiving side so jumbo frames will be used across
the network:
# ifconfig enp2s0 mtu 9193

Add a patch:

diff --git a/drivers/net/ethernet/cortina/gemini.c
b/drivers/net/ethernet/cortina/gemini.c
index 4ae25a064407..06f2d30179c1 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -1185,6 +1185,7 @@ static int gmac_map_tx_bufs(struct net_device
*netdev, struct sk_buff *skb,
                word1 |= TSS_MTU_ENABLE_BIT;
                word3 |= mss;
        } else if (skb->len >= ETH_FRAME_LEN) {
+               netdev_info(netdev, "skb length: %d\n", skb->len);
                /* Hardware offloaded checksumming isn't working on frames
                 * bigger than 1514 bytes. A hypothesis about this is that the
                 * checksum buffer is only 1518 bytes, so when the frames get

Ping with something big:
# ping -s 1800 192.168.1.2
PING 192.168.1.2 (192.168.1.2): 1800 data bytes
gemini-ethernet-port 60008000.ethernet-port eth0: skb length: 1842
1808 bytes from 192.168.1.2: seq=0 ttl=64 time=46.127 ms
gemini-ethernet-port 60008000.ethernet-port eth0: skb length: 1842
1808 bytes from 192.168.1.2: seq=1 ttl=64 time=38.329 ms
gemini-ethernet-port 60008000.ethernet-port eth0: skb length: 1842
1808 bytes from 192.168.1.2: seq=2 ttl=64 time=38.859 ms
gemini-ethernet-port 60008000.ethernet-port eth0: skb length: 1842

This kind of big frames is what upsets the checksumming hardware over
a certain size as we discussed in the earlier patches when we eventually
disabled TSO and how I ran into the problem.

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

Thanks, I will send this separately once net-next opens again in
two weeks, then split off the phy stuff in its own series and
send the controversial patch as RFC or something.

Yours,
Linus Walleij