netdev - Re: [PATCH] net: gro: respect nf_conntrack_checksum for skipping csum verification

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] [day] [month] [year] [list]

Message-ID: <CANn89iLJwdKXcfCHuEkRT7tknsXpD=UgFh-f61M1UAL9b8JMJw@mail.gmail.com>
Date:   Fri, 10 Jun 2022 07:08:28 -0700
From:   Eric Dumazet <edumazet@...gle.com>
To:     Rafał Miłecki <zajec5@...il.com>
Cc:     openwrt-devel@...ts.openwrt.org,
        "David S . Miller" <davem@...emloft.net>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        David Ahern <dsahern@...nel.org>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        netdev <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Rafał Miłecki <rafal@...ecki.pl>
Subject: Re: [PATCH] net: gro: respect nf_conntrack_checksum for skipping csum verification

On Fri, Jun 10, 2022 at 3:37 AM Rafał Miłecki <zajec5@...il.com> wrote:
>
> From: Rafał Miłecki <rafal@...ecki.pl>
>
> Netfilter allows disabling checksum verification of incoming packets by
> setting nf_conntrack_checksum variable. That feature is very useful for
> home routers which:
> 1. Most of the time just /forward/ network traffic
> 2. Have slow CPU(s) and csum calculation is a challenge
>
> Some projects like OpenWrt set nf_conntrack_checksum to 0 by default.
>
> It would be nice to allow similar optimization in the GRO code paths.
> This patch simply reuses nf_conntrack_checksum variable to skip
> skb_gro_checksum_validate() calls if applicable.
>

Problem is that GRO will be followed by TSO on the egress side.

TSO will generate segments with recomputed checksums for each one of them.

GRO only keeps one copy of the headers, so does not track original
checksums for all
segments at ingress side.

So if you want to use TSO, GRO has to validate checksums.

I am afraid this nf_conntrack_checksum idea can not be transposed to GRO.

> Signed-off-by: Rafał Miłecki <rafal@...ecki.pl>
> ---
> Hi guys,
>
> I'm not very familiar with net subsystem, please let me know if there is
> a better way of implementing such a feature.
> ---
>  net/ipv4/tcp_offload.c   | 3 +++
>  net/ipv6/tcpv6_offload.c | 3 +++
>  2 files changed, 6 insertions(+)
>
> diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
> index 30abde86db45..734a3c0f3d4a 100644
> --- a/net/ipv4/tcp_offload.c
> +++ b/net/ipv4/tcp_offload.c
> @@ -311,6 +311,9 @@ struct sk_buff *tcp4_gro_receive(struct list_head *head, struct sk_buff *skb)
>  {
>         /* Don't bother verifying checksum if we're going to flush anyway. */
>         if (!NAPI_GRO_CB(skb)->flush &&
> +#if IS_ENABLED(CONFIG_NF_CONNTRACK)
> +           dev_net(skb->dev)->ct.sysctl_checksum &&
> +#endif
>             skb_gro_checksum_validate(skb, IPPROTO_TCP,
>                                       inet_gro_compute_pseudo)) {
>                 NAPI_GRO_CB(skb)->flush = 1;
> diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c
> index 39db5a226855..2144afa56fa3 100644
> --- a/net/ipv6/tcpv6_offload.c
> +++ b/net/ipv6/tcpv6_offload.c
> @@ -18,6 +18,9 @@ struct sk_buff *tcp6_gro_receive(struct list_head *head, struct sk_buff *skb)
>  {
>         /* Don't bother verifying checksum if we're going to flush anyway. */
>         if (!NAPI_GRO_CB(skb)->flush &&
> +#if IS_ENABLED(CONFIG_NF_CONNTRACK)
> +           dev_net(skb->dev)->ct.sysctl_checksum &&
> +#endif
>             skb_gro_checksum_validate(skb, IPPROTO_TCP,
>                                       ip6_gro_compute_pseudo)) {
>                 NAPI_GRO_CB(skb)->flush = 1;
> --
> 2.34.1
>