-
Notifications
You must be signed in to change notification settings - Fork 7.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
esp_eth_transmit
tries to transmit packets even if the link is down. (IDFGH-9490)
#10851
Comments
esp_eth_transmit
tries to transmit packets even if the link is down.esp_eth_transmit
tries to transmit packets even if the link is down. (IDFGH-9490)
@AxelLin thanks for the reminder. I am aware of this issue. The update is ready for some time but non-related CI constantly failed so it didn't make it to the latest release :/ |
@kostaond |
@AxelLin |
@kostaond : Sharing a data point on 4.4 in case it helps My codebase uses esp-idf at https://github.com/espressif/esp-idf/commit/6a7d83af1984b93ebaefa7ca9be36304806c3dc8/ I am seeing the issue on 4.4 and added an
Call stack on assert:
In this setup we use LWIP with IPv6 and the trigger is frequent link-up <-> link-down event on the ethernet port. |
@AxelLin : Thanks for the fix. Do you know if your fix can be applied on 4.4 as-is? (it seems to compile okay for me) or is there other patches needed to make sure Also, did you come across a setup that can |
@rretanubun thanks for sharing your observation. The fix will be backported to v4.4 too.
That sounds possible since there is 10 Tx buffers by default. |
Just note that there is a follow up fix 5ebe6b5 needs backport as well. |
Fixed by 0872feb in v5.0. |
@kostaond Just remind that the fix is not yet available in v4.4 branch. |
The MR with this commit was stuck in review for some time and I thought I did not include the "link up" fix when I mentioned it here. |
@rretanubun I think you should upgrade to v4.4.5. |
The fix is present in v4.4. release branch. However, unfortunately, it didn't get to v4.4.5 release :/ |
Thank you both for the updates, I'll open an internal tracker ticket on our in-house project to monitor this thread. |
I think I am having this issue on 4.3.5.
EDIT - I upgraded to 4.3.6 and it seems to be fixed. I didn't see any mention of it in the changelog for that release, so maybe it was another issue that's now fixed. |
Answers checklist.
IDF version.
ESP-IDF v5.0.1-3-g886e98a2c1
Operating System used.
macOS
How did you build your project?
Command line with idf.py
If you are using Windows, please specify command line type.
None
What is the expected behavior?
Assuming the following state
esp_netif_attach
If any traffic occurs on the network stack, the function
esp_eth_transmit
should prevent it's transmission, due to the link not being up.The expected behaviour is implemented in current IDF 5.1 master, commit
256d4579695a67d5fabaa3b678da46c59757790e
.esp-idf/components/esp_eth/src/esp_eth.c
Line 346 in 256d457
What is the actual behavior?
In
ESP-IDF v5.0.1-3-g886e98a2c1
, functionesp_eth_transmit
only checks if the interface is started, hence allowing for transmission on an interface that is not capable to do so. This causes undefined behaviour ofesp.emac: emac_esp32_transmit(232): insufficient TX buffer size
further down.https://github.com/espressif/esp-idf/blob/release/v5.0/components/esp_eth/src/esp_eth.c#L339
Issue in ESP-IDF v5.0.1 can be resolved by applying the same logic as IDF 5.1 does.
Steps to reproduce.
Explained in expected behaviour section
Build or installation Logs.
No response
More Information.
No response
The text was updated successfully, but these errors were encountered: