Skip to content

Conversation

htibosch
Copy link
Contributor

@htibosch htibosch commented Sep 1, 2025

Description

A visitor on the FreeRTOS forum, trantam found that sometimes a TCP connection has non-active moments of 60 ms or longer. See this long conversation.

I did an earlier attempt in PR 1283, but it didn't solve all.

Changes how to handle notification value:

In the current driver, the EMAC task is woken up using a generic task notify:

vTaskNotifyGiveFromISR( xEMACTaskHandles[ xEMACIndex ], &xTaskWoken );

and the notification value is stored in a separate variable.

In the new version, the notification value is stored in the task:

xTaskNotifyFromISR( xEMACTaskHandles[ xEMACIndex ], EMAC_IF_RX_EVENT, eSetBits, &( xTaskWoken ) );

This guarantees 'atomic' updates of the notification value, both when setting or clearing.

Introduced a new mutex:

For received packets, there is only a single path: a RX interrupt setting EMAC_IF_RX_EVENT, the EMAC task calls emacps_check_rx(), which sends the messages to a queue, owned by the IP-task.

For transmissions, there are two paths, handled by these functions:

  • emacps_send_message() which sends packets.
  • emacps_check_tx() which cleared used tx descriptors.

Test Steps

Please refer to the forum post

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

Be careful when using Python socket for testing purposes, the behaviour may be unexpected. See forum post as well.

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

@htibosch htibosch requested a review from a team as a code owner September 1, 2025 05:40
@htibosch htibosch mentioned this pull request Sep 1, 2025
2 tasks
@htibosch
Copy link
Contributor Author

htibosch commented Sep 4, 2025

@tony-josi-aws and others:
Hello, could anyone start the CI checks of this PR please?

@kstribrnAmzn
Copy link
Member

@htibosch - I've started the checks!

@htibosch
Copy link
Contributor Author

htibosch commented Sep 5, 2025

Thanks, the checks are now done. Is there currently anyone who could review/test this PR for Zynq-7000?
Thanks

@tony-josi-aws
Copy link
Member

tony-josi-aws commented Sep 5, 2025

@htibosch I should be able to do that by early next week.

@tony-josi-aws
Copy link
Member

Thanks @htibosch for this PR, I was able to test this on the Zynq, looks good to me.

@moninom1 moninom1 merged commit 8492a50 into FreeRTOS:main Sep 10, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants