From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Warren Date: Mon, 18 Aug 2008 19:57:07 -0700 Subject: [U-Boot] ARM: net.c: UDP Checksum code failing every packet In-Reply-To: <48AA2119.30509@ceos.com.au> References: <48AA2119.30509@ceos.com.au> Message-ID: <48AA3683.3020705@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Tom, Tom Evans wrote: > UDP Checksumming is enabled with the configuration variable > CONFIG_UDP_CHECKSUM. This is only enabled in 7 out of 437 > include/configs/*.h files. Enabling UDP checksumming can be useful, as > it allows any errors (in downloading via TFTP) to be retried rather than > resulting in an image CRC error when the whole download has finished. > The download isn't meant to fail, but I'm seeing intermittent corrupted > downloads currently. > > If CONFIG_UDP_CHECKSUM is enabled on an ARM core, and the CPU is prior > to ARM Core Version 6, all packets are reported as having checksum errors. > > This is due to the ARM's 32-bit alignment requirements and the following > four lines of code in net/net.c::NetReceive() > > xsum += (ntohl(ip->ip_src) >> 16) & 0x0000ffff; > xsum += (ntohl(ip->ip_src) >> 0) & 0x0000ffff; > xsum += (ntohl(ip->ip_dst) >> 16) & 0x0000ffff; > xsum += (ntohl(ip->ip_dst) >> 0) & 0x0000ffff; > > The original Ethernet packet is (usually, this is not controlled) 32-bit > aligned, the Ethernet header is 14 bytes long, so by design the aligned > fields in the IP (and other) headers are all misaligned, at least with > the NE2000 driver we're using. > > ARM (prior to V6) "silently corrupts" misaligned reads. For 32-bit reads > offset by 16 bits it reads 32-bits from the aligned address two bytes > below that requested, and then swaps the words. > > If the above lines are changed to match other related ARM-related > modifications as follows, then the UDP checksum code works: > > tmp = NetReadIP(&ip->ip_src); > xsum += (ntohl(tmp) >> 16) & 0x0000ffff; > xsum += (ntohl(tmp) >> 0) & 0x0000ffff; > tmp = NetReadIP(&ip->ip_dst); > xsum += (ntohl(tmp) >> 16) & 0x0000ffff; > xsum += (ntohl(tmp) >> 0) & 0x0000ffff; > > I'm afraid I can't generate a patch to do this. Could someone else > please incorporate this change if required? > > Everything seems logical until this point. Why can't you create a patch? regards, Ben