Unified STM32 Network Interface

[HTRamsey]

New Unified STM32 driver for F4, F7, H5, H7.

Description

Implementation of new STM32 drivers. Does not support f2.

Test Steps

Checklist:

• I have tested my changes. No regression in existing tests.
• I have modified and/or added unit-tests to cover the code changes in this Pull Request.

Related Issue

#766

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[HTRamsey]

@htibosch @AniruddhaKanhere Looks like this one NetworkInterface should work for all of Fxx & Hxx. The biggest difference between the HAL drivers are the usage of a few register macros that are named differently, but we can easily abstract that out. I've tested on a F767ZI and can test a H755ZI-Q, but don't have a way to test H5 at the moment.

[htibosch]

@holden-zenith wrote:

I've tested on a F767ZI and can test a H755ZI-Q, but don't have a way to test H5 at the moment.

If I'm not mistaken, I have an H747, F407, and F746 for testing.

So e.g. when testing F407, I use the latest HAL drivers for that part?

[HTRamsey]

@htibosch yes, last time I checked the latest F4 & F7 Eth drivers were 100% the same, so it should work the same. Before you test, I just noticed the call to HAL_ETH_ReleaseTxPacket needs to be moved to EMAC_IF_TX_EVENT because it calls the non interrupt safe version of vReleaseNetworkBufferAndDescriptor. I was using vNetworkBufferReleaseFromISR in the HAL_ETH_TxFreeCallback, but I guess it needs to be handled in the task to be compatible with both buffer allocation methods.

[htibosch]

but I guess it needs to be handled in the task to be compatible with both buffer allocation methods.

As you may have read, I also prefer to handle network buffers from a normal task, rather than from an interrupt.

I just came across this test:

    bool isInterrupt()
    {
        return (SCB->ICSR & SCB_ICSR_VECTACTIVE_Msk) != 0 ;
    }

which could be used to choose the correct network buffer function.

last time I checked the latest F4 & F7 Eth drivers were 100% the same

Doesn't F7 have multiple TX buffers each with their own hardware priority? I never used that feature, but I did have to disable it explicitly.

Would you mind to send me an email so I can ask you little things now and then? hein [at] htibosch [dot] net

Thanks,

[htibosch]

Are you sure that HAL_ETH_RxAllocateCallback() will only be called from a task-context?

And also, do you always use BufferAllocation_2.c ? I think we'll have to use that to make sure that uncached memory will be used by DMA.

[shubnil]

HI @holden-zenith thanks for creating the change.
I wanted to get a little bit of context. Please help in explaining the thought behind coming up with this new driver.
I see that the older H and F series portable layers are moved inside legacy and the new driver has an integrated NetworkInterface.c file. Is the idea to have a single file to handle all the multiple stm32 SKUs.

[HTRamsey]

@shubnil Historically the STM32 HAL Eth drivers have been awful, which is part of the reason why edited HAL drivers were provided with the NetworkInterface.c. The newer HAL drivers are 'better' and all STM32's from F4, F7, H5, H7 essentially use the same driver now. So with this, edited HAL drivers are no longer necessary, it will be much easier to update to accommodate for future HAL updates, only one NetworkInterface is needed, and I am getting about 20mbps higher on iperf. I left the legacy drivers for now since anything prior to F4 won't be getting the HAL update, so technically I suppose only the Fxx driver would be necessary to leave.

[htibosch]

@holden-zenith , do you have a new version of NetworkInterface.c to test?

[HTRamsey]

@htibosch I'll probably push it in the next few days. I've been able to max out the bandwidth at 95mbps but it's not consistent and depends on compilation optimization. So I need to spend some time looking through the disassembly before I push it.

[bjsowa]

@holden-zenith What's the status of this? I have a Nucleo-H563ZI board and would like to use FreeRTOS+TCP but the current port implementation does not support H5 series.

[HTRamsey]

@bjsowa Actually it would be great if you could test it. I have used it successfully on an F7 but there is potential concern with how it compares to the older driver in terms of performance, at least when tested with an F4. I do not have an H5 at the moment to try it.

[bjsowa]

@holden-zenith I made the driver work for my nucleo board (at least DHCP and ping replies seem to work).
I posted the modified NetworkInterface.c with some comments here:
HTRamsey#1

I also ported it to work for version 4.0.0 of FreeRTOS-Plus-TCP (as I want IPv6 support) by mimicking the changes made in #602 for the current driver.

[bjsowa]

On bjsowa#1 I integrated the new driver with the CMake build system. You can use this for reference.

[HTRamsey]

@bjsowa oh nice that's great. I am working on testing an H7 too so all of F4, F7, H5, & H7 will have been tested. Seems to be at least usable on all of them for the time being. In theory it should perform at least as well as the last driver but iperf apparently has had mixed results so far.

[htibosch]

The earlier drivers for STM32Fxx and STM32Hxx used a copy of ST's HAL driver. The driver was optimised for efficiency/speed, and support for zero-copy was added.

@holden-zenith created a unified driver that is based on today's HAL drivers. The driver files must be obtained from ST.

I tried the new driver on STM32F4 and STM32F7, and found that iperf3 shows lower speeds, both TX and RX. I wonder what the influence is of the extra mutex?

I've got no testing equipment where I am now. In August I can do a real-time analyses of both drivers.

[amazonKamath]

@holden-zenith I made the driver work for my nucleo board (at least DHCP and ping replies seem to work). I posted the modified NetworkInterface.c with some comments here: holden-zenith#1

I also ported it to work for version 4.0.0 of FreeRTOS-Plus-TCP (as I want IPv6 support) by mimicking the changes made in #602 for the current driver.

@bjsowa Thanks for trying out the release candidate versions of 4.0.0. Did you face any difficulties or issues with IPv6? Also curious to know if you use the multiple endpoint feature?
PS: We will be soon doing a formal release (targeting mid August). The stack is currently undergoing security review and penetration testing.

[HTRamsey]

@htibosch Here are the ping results of the latest version. It seems to have much better latency, at least for me.
Old Driver:

New Driver:

[htibosch]

@holden-zenith wrote:

Here are the ping results of the latest version. It seems to have much better latency, at least for me.

That looks very promising. Dit you already commit and push the changes?
Do you want me to test it on a particular platform?

[HTRamsey]

/bot run formatting

[bjsowa]

@HTRamsey I see that you put a lot of effort lately on this PR. Are you planning to finish it in the near future?

[HTRamsey]

@bjsowa #1065 has to go in first which is being held until after v4.1

[bjsowa]

@bjsowa #1065 has to go in first which is being held until after v4.1

Ok, sounds promising. I will test the current version on my H5 and share the results. Are there any features yet to be implemented?

[HTRamsey]

@bjsowa multicasting and filtering will be finished after the other PR gets merged. I thought about abandoning parts of the HAL and using registers but I am trying to see if ST will accept my PRs to fix the HAL problems.

[bjsowa]

I tried the current version. Ping flood and iperf tests seem promising:

$ sudo ping -f 10.42.0.186 -w 10
PING 10.42.0.186 (10.42.0.186) 56(84) bytes of data.
 
--- 10.42.0.186 ping statistics ---
136305 packets transmitted, 136305 received, 0% packet loss, time 10000ms
rtt min/avg/max/mdev = 0.054/0.060/0.330/0.003 ms, ipg/ewma 0.073/0.063 ms   

$ iperf3 -c 10.42.0.186 -p 5001 -t 10
Connecting to host 10.42.0.186, port 5001
[  5] local 10.42.0.1 port 38542 connected to 10.42.0.186 port 5001
[ ID] Interval           Transfer     Bitrate         Retr  Cwnd
[  5]   0.00-1.00   sec  11.1 MBytes  93.2 Mbits/sec    0   52.8 KBytes       
[  5]   1.00-2.00   sec  11.2 MBytes  94.4 Mbits/sec    0   52.8 KBytes       
[  5]   2.00-3.00   sec  11.2 MBytes  94.4 Mbits/sec    0   52.8 KBytes       
[  5]   3.00-4.00   sec  11.2 MBytes  94.4 Mbits/sec    0   52.8 KBytes       
[  5]   4.00-5.00   sec  11.1 MBytes  93.3 Mbits/sec    0   52.8 KBytes       
[  5]   5.00-6.00   sec  11.2 MBytes  94.4 Mbits/sec    0   52.8 KBytes       
[  5]   6.00-7.00   sec  11.1 MBytes  93.3 Mbits/sec    0   52.8 KBytes       
[  5]   7.00-8.00   sec  11.2 MBytes  94.4 Mbits/sec    0   52.8 KBytes       
[  5]   8.00-9.00   sec  11.2 MBytes  94.4 Mbits/sec    0   52.8 KBytes       
[  5]   9.00-10.00  sec  11.2 MBytes  94.3 Mbits/sec    0   52.8 KBytes       
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-10.00  sec   112 MBytes  94.0 Mbits/sec    0             sender
[  5]   0.00-10.00  sec   112 MBytes  93.9 Mbits/sec                  receiver

With the default settings, I got ~70 Mbps on Iperf. I am able to achieve ~94 Mbps after increasing ipconfigIPERF_RX_WINSIZE to at least 17.

Now for the things that don't work:

• Network down event (even though previously I made effort to fix it)
• IPv6 - no response to Neighbor Solicitation packets
• MDNS - no response to queries

I assume IPv6 and MDNS might not work due to unfinished multicasting and filtering support.

[bjsowa]

Regarding the broken network down event, the problem lies on the prvPhyReadReg and prvPhyWriteReg functions. They should return non-zero value in case of a failure but instead they return pdTRUE (which is non-zero) when they succeed.

[HTRamsey]

Good catch on prvPhyReadReg, I see the results are ignored everywhere for these functions but that one line in xPhyCheckLinkStatus. Those test results look good. In regards to the parts not working, let's try again after the multicasting is implemented. They have a fix going in 4.1.0 for the Neighbor Solicitation issue that will hopefully solve your issue. I had noticed a handful of other issues when reviewing the base code that I had planned on providing fixes for after doing some cleanup but I no longer have a project that needs it.

[HTRamsey]

If anybody is using this with the F7, ST finally fixed some major issues here, here and here. Unfortunately these exact problems still exist with F4, but it's a start.

[bjsowa]

@HTRamsey Are you dropping the development of this driver or are planning on opening another PR in place of this?

[HTRamsey]

@bjsowa I still use it personally but I think with how inconsistent the stm HAL drivers are it is hard to keep all of them working together without creating certain fixes based on what HAL version the user has. @tony-josi-aws what are your thoughts

[bjsowa]

AFAIK this is the only driver that works on H5. Perhaps we could add this driver alongside existing STM drivers as an alternative and just note which versions of the HAL libraries it was tested with?

[Mixaill]

At least it works nicely with Hxx and could replace it.

[tony-josi-aws]

@HTRamsey

Maintaining the STM32 network interface up to date with respect to different versions of HAL drivers is not easy, as you mentioned. We were discussing internally what could be a better alternative.

What do you think about the idea of keeping the version of HAL Ethernet drivers that are tested to be working with this network interface part of the PR so that it could act as a reference for users who may want to use a different version of the HAL Ethernet driver?