netxduo icon indicating copy to clipboard operation
netxduo copied to clipboard

_nx_ip_checksum_compute calculates invalid checksum when packet chunk is not aligned to machine natural boundary

Open elrafoon opened this issue 3 years ago • 8 comments

Description

My target system:

  • MCU Renesas R7FA6M3AH (Cortex-M4)
  • azure rtos version is 4e62226eeaf870827facbeef40bd1767db5cf9f0
  • netxduo version is aad43453310a1668029bd69d8edbce8198fbca59 (tried with v6.2.0_rel and v6.1.12_rel with the same result)
  • gcc 11.2 on linux

Function _nx_ip_checksum_compute calculates invalid checksum when packet chunk is not aligned to machine natural boundary (4 or 8 bytes).

Packet chunk is defined by pointers nx_packet_prepend_ptr and nx_packet_append_ptr.

I run to this when debugging ICMP packets being dropped on Renesas RA6M3 MCU (it's cortex-m4) - IPv4 header checksums were incorrectly calculated by _nx_ip_checksum_compute.

There is no way to guarantee that nx_packet_prepend_ptr will point to 4-byte aligned memory location, because that ipv4 header is part of ethernet packet received.

Packet looks like this:

  • ethernet header (6B destination MAC, 6B source MAC, 2B type)
  • ipv4 header
  • icmp message

So ipv4 header always starts on offset 14. Since packet received by ethernet mac is stored on 32-byte aligned memory location, after adding that offset it's impossible for ipv4 header to start on 4-byte aligned memory location.

To reproduce

To ease bug reproduction I wrote small C program nx_ip_cs_test.c containing just the function _nx_ip_checksum_compute and a few typedefs. nx_ip_cs_test.tar.gz

I also added very simple implementation of ipv4 checksum in function ipv4_simple_crc_calculate which works in all cases (but its performance is suboptimal).

Program calculates checksum of a sample IPv4 header located on misaligned memory location using both _nx_ip_checksum_compute and ipv4_simple_crc_calculate, prints the result and asserts if calculated inverted checksum is non-zero. (ipv4 header checksum calculation shall yield zero after final bit inversion, because the header already contains the crc).

To reproduce, just compile and run the program (you can use run.sh script). I tried x86, x86-64 and ARM Cortex-A7 (using iMX6UL machine). With optimizations enabled (-O3), it fails on all three architectures. With optimizations disabled (-O0), it succeeds only on x86-64.

Expected behaviour

_nx_ip_checksum_compute shall yield 0xFFFF for correct ipv4 headers.

Impact

IPv4 does not work at all.

elrafoon avatar Jan 26 '23 11:01 elrafoon

I made a fix for myself, until this is resolved somehow.

It certainly has lower performance then original 32-bit-addition code, but works: fix-nx_ip_checksum_compute.tar.gz

elrafoon avatar Jan 26 '23 12:01 elrafoon

Can you tag Renesas here too please? I am not sure who wrote the network driver (Renesas or Microsoft), but it sounds like maybe the driver isn't inserting padding or taking the alignment into account...

goldscott avatar Jan 26 '23 17:01 goldscott

I would gladly tag Renesas, but I don't know how to do that. Could you help me with that please?

But essentially, how can network driver guarantee that IPv4 header is 4-byte aligned? It's embedded in a ethernet packet received and driver can't do anything about it - receive buffer is 4-byte aligned, but that doesn't help. Also in this case IPv4 header is followed by ICMP payload, so padding the header from back isn't possible either.

Also, IPv4 header can be received from ppp link, or from some kind of tunnel, where 4-byte alignment of the header can not be forced.

elrafoon avatar Jan 26 '23 22:01 elrafoon

We have been using the Renesas Ethernet drivers and never had that issue, FSP related issues can be logged here https://github.com/renesas/fsp

hwmaier avatar Jan 26 '23 22:01 hwmaier

I don't think this has anything to do with renesas driver - did you read my reasoning?

And why would the test program containing just _nx_ip_checksum_compute function and main() fail? (did you look at it?)

Maybe someone modified _nx_ip_checksum_compute function recently, but it's impossible to tell from git-log of this repository.

elrafoon avatar Jan 27 '23 08:01 elrafoon

@elrafoon I read your reasoning but I also know that we had no issues with running TCP/IP protocol on Ethernet and that's the experience I wanted to share as fellow user. I checked our FSP configuration which shows the driver to be configured to add 2 bytes of padding at offset 14 which is the default setting by Renesas. Maybe try this setting. Otherwise ask in the Renesas forums, Renesas is very helpful and responsive with their support.

2023-01-27_19-21-37

hwmaier avatar Jan 27 '23 09:01 hwmaier

Thank you @hwmaier, it shall work with this padding, I will try it. I'm new to Renesas hardware, still learning, didn't know about this.

But still I think that depending on 4-byte alignment in _nx_ip_checksum_compute is wrong - do you think it's always possible to guarantee such alignment? Not just in ethernet case, but in general? (ppp, tunnels, ip-in-something, etc.) (I think it's not).

Also, if such alignment is strictly required, then _nx_ip_checksum_compute should assert that, at least in debug build, not just fail silently.

elrafoon avatar Jan 27 '23 09:01 elrafoon

Good suggestion, @elrafoon ! We will add such assert in the future.

TiejunZhou-CN avatar Jan 28 '23 02:01 TiejunZhou-CN