netdev - Re: [PATCH net-next 2/8] udp: skip fwd/list GRO for tunnel packets

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: <CA+FuTSc=V_=behQ0MKX3oYdDzZN=V7_CdeNOFXUAa-4TuU5ztA@mail.gmail.com>
Date:   Mon, 22 Mar 2021 09:24:51 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Paolo Abeni <pabeni@...hat.com>
Cc:     Network Development <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Steffen Klassert <steffen.klassert@...unet.com>,
        Alexander Lobakin <alobakin@...me>
Subject: Re: [PATCH net-next 2/8] udp: skip fwd/list GRO for tunnel packets

On Sun, Mar 21, 2021 at 1:01 PM Paolo Abeni <pabeni@...hat.com> wrote:
>
> If UDP GRO forwarding (or list) is enabled,

Please explicitly mention the gso type SKB_GSO_FRAGLIST. I, at least,
didn't immediately grasp that gro forwarding is an alias for that.

> and there are
> udp tunnel available in the system, we could end-up doing L4
> aggregation for packets targeting the UDP tunnel.

Is this specific to UDP tunnels, or can this also occur with others,
such as GRE? (not implying that this patchset needs to address those
at the same time)

> That could inner protocol corruption, as no overaly network
> parameters is taken in account at aggregation time.

nit: overaly .. is taken -> overlay .. are taken

You mean the packets on the frag list may have mtu exceeding the mtu
of the tunnel? Please make the constraint more explicit.

> Just skip the fwd GRO if this packet could land in an UDP
> tunnel.

Could you make more clear that this does not skip UDP GRO, only
switches from fraglist-based to pure SKB_GSO_UDP_L4.

> The current check is broader than what is strictly
> needed, as the UDP tunnel could be e.g. on top of a different
> device, but is simple and the performance downside looks not
> relevant.
>
> Fixes: 9fd1ff5d2ac7 ("udp: Support UDP fraglist GRO/GSO.")
> Fixes: 36707061d6ba ("udp: allow forwarding of plain (non-fraglisted) UDP GRO packets")
> Signed-off-by: Paolo Abeni <pabeni@...hat.com>