netdev - Re: [PATCH net-next] ip_gre: remove CRC flag from dev features in gre_gso

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:   Wed, 18 Nov 2020 08:44:55 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Xin Long <lucien.xin@...il.com>
Cc:     network dev <netdev@...r.kernel.org>, linux-sctp@...r.kernel.org,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        davem <davem@...emloft.net>, Guillaume Nault <gnault@...hat.com>,
        Paolo Abeni <pabeni@...hat.com>, lorenzo@...nel.org,
        Alexander Duyck <alexander.duyck@...il.com>
Subject: Re: [PATCH net-next] ip_gre: remove CRC flag from dev features in
 gre_gso_segment

On Wed, 18 Nov 2020 14:14:49 +0800 Xin Long wrote:
> On Wed, Nov 18, 2020 at 8:29 AM Jakub Kicinski <kuba@...nel.org> wrote:
> > On Mon, 16 Nov 2020 17:15:47 +0800 Xin Long wrote:  
> > > This patch is to let it always do CRC checksum in sctp_gso_segment()
> > > by removing CRC flag from the dev features in gre_gso_segment() for
> > > SCTP over GRE, just as it does in Commit 527beb8ef9c0 ("udp: support
> > > sctp over udp in skb_udp_tunnel_segment") for SCTP over UDP.
> > >
> > > It could set csum/csum_start in GSO CB properly in sctp_gso_segment()
> > > after that commit, so it would do checksum with gso_make_checksum()
> > > in gre_gso_segment(), and Commit 622e32b7d4a6 ("net: gre: recompute
> > > gre csum for sctp over gre tunnels") can be reverted now.
> > >
> > > Signed-off-by: Xin Long <lucien.xin@...il.com>  
> >
> > Makes sense, but does GRE tunnels don't always have a csum.  
> Do you mean the GRE csum can be offloaded? If so, it seems for GRE tunnel
> we need the similar one as:
> 
> commit 4bcb877d257c87298aedead1ffeaba0d5df1991d
> Author: Tom Herbert <therbert@...gle.com>
> Date:   Tue Nov 4 09:06:52 2014 -0800
> 
>     udp: Offload outer UDP tunnel csum if available
> 
> I will confirm and implement it in another patch.
> 
> >
> > Is the current hardware not capable of generating CRC csums over
> > encapsulated patches at all?  
> There is, but very rare. The thing is after doing CRC csum, the outer
> GRE/UDP checksum will have to be recomputed, as it's NOT zero after
> all fields for CRC scum are summed, which is different from the
> common checksum. So if it's a GRE/UDP tunnel, the inner CRC csum
> has to be done there even if the HW supports its offload.

Ack, my point is that for UDP tunnels (at least with IPv4) the UDP
checksum is optional (should be ignored if the header field is 0),
and for GRE checksum is optional and it's presence is indicated by 
a bit in the header IIRC.

So if the HW can compute the CRC csum based on offsets, without parsing
the packet, it should be able to do the CRC on tunneled packets w/o
checksum in the outer header.

IDK how realistic this is, whether it'd work today, and whether we care
about it...