STM32 MAC Checksum offload bug + PTP Topic is solved

Report here problems in any of ChibiOS components. This forum is NOT for support.
drizzi
Posts: 4
Joined: Wed Apr 19, 2017 4:52 pm
Been thanked: 1 time

STM32 MAC Checksum offload bug + PTP  Topic is solved

Postby drizzi » Mon May 15, 2017 11:50 am

Hi,
I've been working with the STM32F4 MAC lately, and I have came up with a bug. ChibiOS 16.1.x.
When the checksum offload is enabled (IPCO=1, MACCR register bit 10), enhanced descriptor RDES4 must be used to check if the received frame checksum was OK, as the reference manual says (page 1185 of RM0090).

I have also PTP enabled the MAC: the code style is a little bit off the standard (some cut and paste from ST HAL), but the code works reliably.

I would be happy to provide the patches!

0001-Added-PTP-Fixed-Checksum-offload.patch.gz
Patch
(4.91 KiB) Downloaded 247 times


Ciao

User avatar
Giovanni
Site Admin
Posts: 14444
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1074 times
Been thanked: 921 times
Contact:

Re: STM32 MAC Checksum offload bug + PTP

Postby Giovanni » Mon May 29, 2017 10:47 am

bump

steved
Posts: 823
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 12 times
Been thanked: 135 times

Re: STM32 MAC Checksum offload bug + PTP

Postby steved » Wed Jul 12, 2017 10:11 am

I've been looking at this from the perspective of MAC Checksum offload. Couldn't apply the patch (seems to be git-specific), but looked through the relevant changes, and they make sense to me.
I've also tried the existing code with lwIP 2.0.2, and it appears to work fine without extended DMA descriptors, although I have no means of generating checksum errors.
While RM0090 clearly states that extended DMA descriptors must be enabled when offloading checksums, it doesn't appear to cause a problem in practice, and the definitions of the status flags suggest that the extended descriptors are not essential

This link suggests that maybe there are situations (such as fragmented packets) where the chacksum calculation should not be offloaded.

steved
Posts: 823
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 12 times
Been thanked: 135 times

Re: STM32 MAC Checksum offload bug + PTP

Postby steved » Wed Jul 12, 2017 2:40 pm

Another small comment on the patch - it introduces a new define STM32_MAC_USE_ENHANCED_DMA_DESCRIPTORS. Personally I would prefer that to be automatically defined if STM32_MAC_ENABLE_PTP or STM32_MAC_IP_CHECKSUM_OFFLOAD is defined

And assuming MAC_USE_PTP is a 'capabilities' flag, I didn't come across an error message if MAC_USE_PTP is FALSE and STM32_MAC_ENABLE_PTP is defined.

rasm
Posts: 11
Joined: Thu Aug 24, 2017 6:35 pm
Has thanked: 4 times
Been thanked: 3 times

Re: STM32 MAC Checksum offload bug + PTP

Postby rasm » Thu Aug 24, 2017 6:49 pm

I'm using the Olimex STM32-E407 board and when STM32_MAC_IP_CHECKSUM_OFFLOAD is enabled and the lwIP options CHECKSUM_GEN_IP, CHECKSUM_GEN_TCP, and CHECKSUM_GEN_UDP are disabled I can't even establish a TCP connection to the STM32-E407 board. Applying this patch (only the checksum offload part) solved it for me. Should I send a patch with only drizzi's checksum offload fix ?

steved
Posts: 823
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 12 times
Been thanked: 135 times

Re: STM32 MAC Checksum offload bug + PTP

Postby steved » Fri Aug 25, 2017 8:40 am

I found that there are a few more things to configure than are documented or obvious.

In mcuconf.h:

Code: Select all

#define STM32_MAC_IP_CHECKSUM_OFFLOAD       3


In lwipopts.h:

Code: Select all

#define CHECKSUM_GEN_ICMP               0
#define CHECKSUM_GEN_IP                 0
#define CHECKSUM_GEN_TCP                0
#define CHECKSUM_GEN_UDP                0
#define CHECKSUM_CHECK_ICMP             0
#define CHECKSUM_CHECK_IP               0
#define CHECKSUM_CHECK_TCP              0
#define CHECKSUM_CHECK_UDP              0


Without some of those, things did indeed lock up.

This is the configuration I am currently running - only on my development target ATM, but no noticeable problems.

Having said that, an update which conforms to the letter of the manual would be useful.

[As an aside, personally I prefer modified complete files rather than patches against a specific file version; I tend to do a file compare locally to look at the differences. I can see that patches can be advantageous when modifying a large codebase, but when it's a handful of files, less so, IMO.]

rasm
Posts: 11
Joined: Thu Aug 24, 2017 6:35 pm
Has thanked: 4 times
Been thanked: 3 times

Re: STM32 MAC Checksum offload bug + PTP

Postby rasm » Fri Aug 25, 2017 12:33 pm

Thank you for the information steved. Setting STM32_MAC_IP_CHECKSUM_OFFLOAD to 3 does indeed fix the issue I was having, my bad for not reading the hal_mac_lld.h header. I sent several ICMP packets with invalid checksums to test both implementations (mainline, drizzi's) and both are able to detect invalid IP checksums when checksum offload is enabled. By reading the manual I believe that both implementations are correct:

- ChibiOS 16.1.x does not enable enhanced descriptors and therefore the bit IPHCE signals a checksum error.
- drizzi's patch enables enhanced descriptors because of PTP but in this scenario the IPHCE bit takes on the function of TSV (time stamp valid), only in this case RDES4 must be read to check for checksum errors.

With that said I don't think the mainline code has to be fixed until the PTP code is merged.

steved
Posts: 823
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 12 times
Been thanked: 135 times

Re: STM32 MAC Checksum offload bug + PTP

Postby steved » Fri Aug 25, 2017 1:59 pm

rasm wrote:Thank you for the information steved. Setting STM32_MAC_IP_CHECKSUM_OFFLOAD to 3 does indeed fix the issue I was having

Been there too!
rasm wrote:I sent several ICMP packets with invalid checksums to test both implementations (mainline, drizzi's) and both are able to detect invalid IP checksums when checksum offload is enabled

Good that were able to test. As a matter of interest, how did you generate the faulty packets?
rasm wrote:By reading the manual I believe that both implementations are correct:

- ChibiOS 16.1.x does not enable enhanced descriptors and therefore the bit IPHCE signals a checksum error.
- drizzi's patch enables enhanced descriptors because of PTP but in this scenario the IPHCE bit takes on the function of TSV (time stamp valid), only in this case RDES4 must be read to check for checksum errors.

That was the impression I got; good that someone else thinks the same.
rasm wrote:With that said I don't think the mainline code has to be fixed until the PTP code is merged.

I agree

rasm
Posts: 11
Joined: Thu Aug 24, 2017 6:35 pm
Has thanked: 4 times
Been thanked: 3 times

Re: STM32 MAC Checksum offload bug + PTP

Postby rasm » Fri Aug 25, 2017 2:10 pm

steved wrote:Good that were able to test. As a matter of interest, how did you generate the faulty packets?


I used the hping3 tool with the -b flag (git://git.debian.org/collab-maint/hping3.git)

User avatar
Giovanni
Site Admin
Posts: 14444
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1074 times
Been thanked: 921 times
Contact:

Re: STM32 MAC Checksum offload bug + PTP

Postby Giovanni » Sun May 27, 2018 9:08 am

bump


Return to “Bug Reports”

Who is online

Users browsing this forum: No registered users and 7 guests