netdev - Re: [PATCH net v2 2/2] net: ethernet: cortina: Bypass checksumming engine of alien ethertypes

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: <CANn89iLu_vE_S++5Q6Re4c6DZOD7GD-pLFC61VjYGcjFnKDWCw@mail.gmail.com>
Date: Mon, 18 Dec 2023 15:50:43 +0100
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>, netdev@...r.kernel.org
Subject: Re: [PATCH net v2 2/2] net: ethernet: cortina: Bypass checksumming
 engine of alien ethertypes

On Sat, Dec 16, 2023 at 8:36 PM Linus Walleij <linus.walleij@...aro.org> wrote:
>
> We had workarounds were the ethernet checksumming engine would be bypassed
> for larger frames, this fixed devices using DSA, but regressed devices
> where the ethernet was connected directly to a PHY.
>
> The devices with a PHY connected directly can't handle large frames
> either way, with or without bypass. Looking at the size of the frame
> is probably just wrong.
>
> Rework the workaround such that we just bypass the checksumming engine if
> the ethertype inside the actual frame is something else than 0x0800
> (IPv4) or 0x86dd (IPv6). These are the only frames the checksumming engine
> can actually handle. VLAN framing (0x8100) also works fine.
>
> We can't inspect skb->protocol because DSA frames will sometimes have a
> custom ethertype despite skb->protocol is e.g. 0x0800.
>
> After this both devices with direct ethernet attached such as D-Link
> DNS-313 and devices with a DSA switch with a custom ethertype such as
> D-Link DIR-685 work fine.
>
> Fixes: d4d0c5b4d279 ("net: ethernet: cortina: Handle large frames")
> Signed-off-by: Linus Walleij <linus.walleij@...aro.org>
> ---
>  drivers/net/ethernet/cortina/gemini.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
> index 6a7ea051391a..1400f19bf05b 100644
> --- a/drivers/net/ethernet/cortina/gemini.c
> +++ b/drivers/net/ethernet/cortina/gemini.c
> @@ -1143,7 +1143,9 @@ static int gmac_map_tx_bufs(struct net_device *netdev, struct sk_buff *skb,
>         skb_frag_t *skb_frag;
>         dma_addr_t mapping;
>         unsigned short mtu;
> +       u16 ethertype;
>         void *buffer;
> +       __be16 *p;
>
>         mtu  = ETH_HLEN;
>         mtu += netdev->mtu;
> @@ -1158,7 +1160,24 @@ static int gmac_map_tx_bufs(struct net_device *netdev, struct sk_buff *skb,
>                 word3 |= mtu;
>         }
>
> -       if (skb->ip_summed == CHECKSUM_PARTIAL) {
> +       /* Dig out the the ethertype actually in the buffer and not what the
> +        * protocol claims to be. This is the raw data that the checksumming
> +        * offload engine will have to deal with.
> +        */
> +       p = (__be16 *)(skb->data + 2 * ETH_ALEN);
> +       ethertype = ntohs(*p);
> +       if (ethertype == ETH_P_8021Q) {
> +               p += 2; /* +2 sizeof(__be16) */
> +               ethertype = ntohs(*p);
> +       }

Presumably all you need is to call vlan_get_protocol() ?

> +
> +       if (ethertype != ETH_P_IP && ethertype != ETH_P_IPV6) {
> +               /* Hardware offloaded checksumming isn't working on non-IP frames.
> +                * This happens for example on some DSA switches using a custom
> +                * ethertype. Just bypass the engine for those.
> +                */
> +               word1 |= TSS_BYPASS_BIT;
> +       } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
>                 int tcp = 0;
>
>                 /* We do not switch off the checksumming on non TCP/UDP
>
> --
> 2.34.1
>