[bug fixes] STM32H7 Ethernet

[alister]

@Amel NASRI​, @ranran​, @Piranha​, @Harrold​, @Pavel A.​ 

V2 of my fixes and improvements to H7_FW V1.5.0/V1.6.0 Ethernet...

Changes include

• Decoupling receive buffers from receive descriptors so buffers may be held any length time without choking receive.
• Optionally queuing transmit, so transmit doesn't need to block until complete.
• Many bug fixes.

Find full details and source in the attached zip. V1 was posted to another developer's question. Please post any questions about V2 here.

[alister]

netmask's type is ip4_addr_t?

[alister]

Porting guide for Cube users. Supplements section 9 of my document...

1. BEFORE porting my bug-fixes & improvements, integrate and test your PHY code with the stock Cube code. Test using ICMP (ping). The stock Cube code operates well enough for this. This is to avoid integrating everything at once and possibly finding you’re unsure what’s working. Ideally, you’d start using my bug-fixes & improvements after discovering the stock Cube is unreliable.
2. Copy my ethernetif.c and stm32h7xx_hal_eth.c to a source directory of your application, i.e. away from Cube’s LWIP/Target or Drivers/STM32H7xx_HAL_Driver/Src directories. The purpose of this and following steps is to isolate changes to the Cube code so they won’t be trashed when you generate the Cube again later.
3. Exclude Cube’s LWIP/Target/ethernetif.c and Drivers/STM32H7xx_HAL_Driver/Src/stm32h7xx_hal_eth.c from your build.
4. Copy my stm32h7xx_hal_conf.h and stm32h7xx_hal_eth.h to a source directory of your application, away from ../Inc or ../Core/Inc, or ../Drivers/STM32H7xx_HAL_Driver/Inc. Configure your compiler’s include paths so that directory’s searched before Cube’s ../Inc or ../Core/Inc, or Drivers/STM32H7xx_HAL_Driver/Inc.
5. My stm32h7xx_hal_conf.h contains macros that Cube doesn’t configure correctly, e.g. ETH_TX_DESC_CNT and ETH_RX_DESC_CNT, or doesn’t configure yet.
6. Keep your lwipopts.h at LWIP/Target so Cube continues to control your lwIP configuration. Reconcile your Cube configuration with the parts of my lwipopts.h you want to keep. Keep all my changes between /* USER CODE BEGIN 1 */ and /* USER CODE END 1 */ is easiest. But drop the part commented "Relocate the lwIP heap..." if you don't want to do that, and if you don't do that, you'll have to remove the part with a similar comment in ethernetif.c.
7. Configure an MPU region so accesses of the rx and tx DMA descriptors are completed in program order. This is mandatory. Use my mpu.c as a guide. You may configure regions to make your rx and tx buffers uncached too. Refer my discussion on the MPU/caching choices earlier in this community post. The MPU configuration should be performed before calling MX_LWIP_Init(). If you’re unsure, use my regions.
8. Integrate your earlier LWIP/Target/ethernetif.c with my ethernetif.c (in the directory you’d moved it in step 1)…
   1. In the low_level_init function…
      1. Change MII to RMII if that's what your PHY uses.
      2. Remove the lines of code between /* USER CODE BEGIN low_level_init Code 1 for User BSP */ and /* USER CODE END low_level_init Code 1 for User BSP */. Those lines create my PHY task (which I haven’t shared). But Cube creates a PHY task named ethernet_link_thread from MX_LWIP_Init. I regret my PHY deviates from Cube. I suggest you follow Cube. And if Cube doesn’t work for you, tell ST.
      3. Modify or remove my calls of HAL_ETH_SetMACConfig and HAL_ETH_SetMACFilterConfig as appropriate for you.
   2. Copy in the ethernet_link_thread. That's the PHY task described a few steps earlier.
   3. Dimension your rx buffers: ETH_RX_BUFFER_SIZE, ETH_RX_BUFFER_COUNT.
   4. Check ETH_RX_BUFFERS_ARE_CACHED and ETH_TX_BUFFERS_ARE_CACHED are consistent with your MPU configuration.
9. Integrate your linker script file...
   1. Position the sections for the DMA descriptors and the receive buffer area named in ethernetif.c. Use my mapping_eth.ld as a guide.
   2. Check your configured MCU region sizes match your section sizes. Check their start addresses are aligned their sizes.
10. I’ve only read/tested GCC (__GNUC__). If you’re using a different compiler check the compiler-specific parts carefully.

[DOsbo]

Thanks @alister​ for sharing your solution, although I'm really disappointed you had to do it in the first place. I ran into the 'overwriting buffer' issue 5 seconds into my first test.

Just letting you know I'm using the socket interface (maintaining legacy code) and it's working well with your modifications. I haven't done a lot of testing, but so far it appears robust.

Hi @Amel NASRI​ , do these defects need to be logged as issues in the GitHub repository, or have they been captured here?

Cheers David

[p2399]

Thanks @alister​  for your work. I would like to ask You about another aspect of H7 ethernet controller. H7 ETH controller support TCP hardwere segmentation. Do you know, how to modified LWIP to use this functionality. Maybe You heard about this implementation? In my project I require high data transfer over ethernet between STMH7 and NationalInstrument SbRIO controler. To achive maksimum bandwith I've used direct transfering UDP datagrams omiting LWIP (tx data throwoutput on the level of 98%). I've discovered that this solution leads to hi cpu consumption on SbRIO side (50% usage of cpu only for receiving datagrams). So to deal with this issue I've made code that supported datagram segmentation in IP level (one UDP datagram can transfer around 65kB of data in this configuration). This partially solve problem on SbRIO (Ethernet driver joins ethernet frames in Kernel layer what is much faster). I know that ARM SbRIO procesor was equipded in ethernet controller supporting tcp segmentation so the next logic step in development would be moving with comunication between devices to TCP and hardwer supported segmentation but the question is how to do it on H7 side?

[alister]

I can't help much.

The H7 ETH driver described here does

1. Tx IP header and payload checksum calculation and insertion (refer RM0433, TDESC3 CIC) .
2. Appears to have coded support of TSO.

You might check these:

1. The STM32H7 reference manual, RM0433... https://www.st.com/content/ccc/resource/technical/document/reference_manual/group0/c9/a3/76/fa/55/46/45/fa/DM00314099/files/DM00314099.pdf/jcr:content/translations/en.DM00314099.pdf.
2. A Google search of lwIP and TSO... https://pdfs.semanticscholar.org/8f5a/de59cea788141047f5ed19b99d62fc24d0cb.pdf

[ADunc.1]

I have also used this code successfully. After a detailed review of the modified driver code, I was impressed with some of the subtle bugs and mistakes that alister found in the ST code.

It will be interesting to see if ST ever take on board the suggestion of a zero copy driver that abstracts buffers away from Ethernet DMA descriptors. The complexity of all the buffer callbacks would seem inconsistent with their typical 'dumb' drivers, but hopefully they do as it is essential for a high performance Ethernet solution.

Initially I tried to use the modified driver alongside the the other driver code so I could still cleanly regenerate STM32CubeMx code but because of changes in stm32h7xx_hal_eth.h, which is included in the auto generated files it was not practical. Replacement of the stm32h7xx_hal_eth files is necessary, then manual intervention each time STM32CubeMX code is generated after that. A small price to pay given that it actually works.

This application uses a hardware abstraction layer and OS abstraction (as it runs on embedded hardware or a PC) so took a bit of tweaking. All the code that is required is supplied in the zip folder. On custom hardware, STM32H753 @ 480MHz, using Windows iperf 2 and lwiperf server, I was able to obtain a repeatable 50MBps. I did zero investigation into what was limiting the rate to 50 as that is already 50 times more than this application needs!

I really wanted to find a bug or improvement in this code so I could submit some feedback but was unable to!

Thanks alister

[alister]

Thanks for the kind thanks. If you put your modified stm32h7xx_hal_conf.h, stm32h7xx_hal_eth.h and stm32h7xx_hal_eth.c files in different directories and configure the compiler to search your includes directory before the HAL driver includes directory, and you exclude stm32h7xx_hal_eth.c in the HAL drivers source directory, you should be able to regenerate Cube without its trashing your changes. That's what I'd tried to explain in the earlier porting-guide post, or am I missing something?

[AFair]

I've been following the development and progress that you and others have been making on the H7 ethernet drivers since last November -- only now have I finally had the time to sit down and incorporate all your recommendations. Just wanted to comment to say I'm incredibly thankful for the hard work that you've done here and for generously sharing that with the community. Working with the template code provided was quick and painless to get working, but that was made easier from knowing where to look after spending a lot of time on previous attempts at fixing the ethernet stack last year.

Now we just have to sit and wait, I'm hopeful that ST will incorporate your changes to their code and make life easier for others.

[ADunc.1]

Actually, since writing that post last night (and forgetting to click submit until this morning), I have managed to do as you said and have it build without modifying the STM32CubeMX generated files. The trick was to realising that STM32CubeMX will delete files that it thinks shouldn't be there, but not folders. Oh, and as you mentioned, making sure the include path to the modified files comes first!

I never actually saw your post describing how to do it and wrote my own notes (attached) for future reference.

[alister]

I wouldn't rename these files. Other files already including stm32h7xx_hal_eth.h are best unchanged, and renaming a .c file won't change its symbol names. Only excluding the unwanted .c files ensure the correct file's are used and avoids duplicate symbols during linking.