netdev - Re: net: xilinx: axienet: Query about checksum partial implementation

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: <20240731085442.GN1967603@kernel.org>
Date: Wed, 31 Jul 2024 09:54:42 +0100
From: Simon Horman <horms@...nel.org>
To: "Pandey, Radhey Shyam" <radhey.shyam.pandey@....com>
Cc: Daniel Borkmann <daniel@...earbox.net>,
	Ariane Keller <ariane.keller@....ee.ethz.ch>,
	"Simek, Michal" <michal.simek@....com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
	"Katakam, Harini" <harini.katakam@....com>
Subject: Re: net: xilinx: axienet: Query about checksum partial implementation

On Tue, Jul 30, 2024 at 07:15:13PM +0000, Pandey, Radhey Shyam wrote:
> > -----Original Message-----
> > From: Simon Horman <horms@...nel.org>
> > Sent: Friday, July 26, 2024 5:37 PM
> > To: Pandey, Radhey Shyam <radhey.shyam.pandey@....com>
> > Cc: Daniel Borkmann <daniel@...earbox.net>; Ariane Keller
> > <ariane.keller@....ee.ethz.ch>; Simek, Michal <michal.simek@....com>;
> > netdev@...r.kernel.org; linux-arm-kernel@...ts.infradead.org
> > Subject: net: xilinx: axienet: Query about checksum partial implementation
> > 
> > Hi Radhey, all,
> > 
> > I am wondering if you could shed some light on the following checksum
> > partial handling in the axienet_rx_poll():
> > 
> >                         /* if we're doing Rx csum offload, set it up */
> >                         if (lp->features & XAE_FEATURE_FULL_RX_CSUM) {
> > 				...
> >                         } else if ((lp->features & XAE_FEATURE_PARTIAL_RX_CSUM) != 0
> > &&
> >                                    skb->protocol == htons(ETH_P_IP) &&
> >                                    skb->len > 64) {
> >                                 skb->csum = be32_to_cpu(cur_p->app3 & 0xFFFF);
> >                                 ...
> >                         }
> > 
> > In particluar the "skb->csum =" line.
> > 
> > The type of cur_p->app3 is u32, and 0xFFFF is also host byte order.
> > So far so good. But after the bitwise operation it is treated as a big-endian
> > value by passing it to be32_to_cpu.
> > 
> > Perhaps I am missing something obvious, but my question is how does that
> > work?
> > 
> > * Was it only tested on big endian sysgtems where be32_to_cpu() is a no-op
> > 
> > * Was it only tested on little endian systems where be32_to_cpu()
> >   is a byteswap and somehow that works (how?).
> > 
> > * Is the code unecessised because the XAE_FEATURE_FULL_RX_CSUM branch
> > is
> >   always taken?
> > 
> >   A grep of dts files shows up arch/microblaze/boot/dts/system.dts which
> >   sets sets xlnx,rxcsum to 0, which corresponds to XAE_NO_CSUM_OFFLOAD.
> 
> + Harini
> 
> Yes, IIRC default AXI Ethernet IP RX checksum is set to "No checksum offload"
> so, it is default case and being set in most designs. Have added Harini to this
> thread to confirm on partial checksum verification results.
> 
> Assuming partial implementation is functional then likely DMA IP updates
> application field in big endian format and that is the reason we have this
> be32 to CPU conversion in place. will dig a bit more and get back on it.

Thanks, much appreciated.

FWIIW, I do agree that the scenario you describe would mostly explain
things, although the mask with 0xFFFF still seems off.