[Feature Request] Configuration option for Task affinity

[mike919192]

I have been doing testing with using the TCP library with the SMP kernel (zynq port, 2 cores). I've already reported over on the FreeRTOS forums that the TCP library right now requires configRUN_MULTIPLE_PRIORITIES to be set to 0, otherwise an assertion is triggered during initialization. Right now I'm not too concerned with investigating the assertion, I have just been finding configuration that allows TCP to run with the SMP kernel as it is.

I have been testing for several weeks now with configRUN_MULTIPLE_PRIORITIES set to 0 and everything seems to be working properly. Now the next step is that I want to set configRUN_MULTIPLE_PRIORITIES to 1, and use the task core affinity feature to prevent the necessary tasks from running concurrently.

Just for backround information, these tasks seem to need affinity set:

• User task which creates sockets (pinned to core 0)
• IP Task (pinned to core 0)
• Zynq port GEM task (pinned to core 0)

Notably I have 1 task for reading and 1 task for writing to a socket, and these do need seem to require set affinity, but I will continue to test to be sure.

Currently there is no configuration to set the affinity for the IP task and the Zynq port GEM task. Would it be OK to allow a configuration for setting the task affinity of those 2 tasks?

[htibosch]

Thank you very much for sharing your investigation.

Some information:

You probably know that it is recommended to use the following priorities:

• Higher: EMAC task
• Normal: IP-task
• Lower: tasks that make use of the network
• Any: tasks that do not use the network

That means that when I call FreeRTOS_send(), it will be handled by the IP-task before the function returns.
If the IP-task has a low priority, it may get overloaded by commands, leading to buffer overflow and a shortage of resources.

I suppose that you are using this Zynq driver?

If so, it might be worth to switch the latest driver. That PR has everything to do with synchronisation. It should be more robust. The IP-task will send messages, and the EMAC task will clear TX descriptors after a packet has been sent.
In the new version, the two actions will not run simultaneously by using a mutex called xemacpsif_s::tx_mutex.

Notably I have 1 task for reading and 1 task for writing to a socket, and these do need seem to require set affinity, but I will continue to test to be sure.

Both in recv() as well as in send(), the socket's TCP stream buffer will be accessed without a lock or mutex. Stream buffers are supposed to be thread-save as long one condition is met: the buffer has one reader and one sender.

After TCP data has been sent or received, an event will be sent to the socket called eTCPTimerEvent. The IP-task will handle that event.

Currently there is no configuration to set the affinity for the IP task and the Zynq port GEM task. Would it be OK to allow a configuration for setting the task affinity of those 2 tasks?

Sure, that can be added. Maybe keep them together in FreeRTOSIPConfigDefaults.h

ipconfigIP_TASK_STACK_SIZE_WORDS
ipconfigIP_TASK_PRIORITY

Unfortunately the EMAC handler macro's do not have conformity of naming:

configEMAC_TASK_STACK_SIZE
niEMAC_HANDLER_TASK_PRIORITY

[mike919192]

Thanks for the response.

You probably know that it is recommended to use the following priorities:

* Higher: EMAC task

* Normal: IP-task

* Lower: tasks that make use of the network

* Any: tasks that do not use the network

Yep. I left EMAC task and IP task as default, so I think they are something like EMAC: 7 and IP: 6. Then my Rx and Tx tasks which use the network are 4.

I suppose that you are using this Zynq driver?

I am using the zynq driver merged into main. Once I'm satisfied testing and finding the right configuration, I'd be happy to test out the new driver changes.

Notably I have 1 task for reading and 1 task for writing to a socket, and these do need seem to require set affinity, but I will continue to test to be sure.

I did eventually get a test failure after a few hours without setting affinity for Rx task and Tx task. No asserts were raised but the connection sort of locked up and all traffic dropped to zero. So now I have also set the affinity for the Rx and Tx task to core 0 and so far (several days) have not experienced the issue again.

Sure, that can be added. Maybe keep them together in FreeRTOSIPConfigDefaults.h

I have started making the changes in a fork. It's a pretty small change, I'm just also in the middle of a few other things. So far the names I am using are.

ipconfigIP_TASK_AFFINITY
niEMAC_HANDLER_TASK_AFFINITY

[ActoryOu]

IMO, since the FreeRTOSIPConfigDefault.h has below check, I believe it makes sense to depend on configRUN_MULTIPLE_PRIORITIES.

#ifndef FREERTOS_CONFIG_H
    #error FreeRTOSConfig.h has not been included yet
#endif

Maybe we can define something like below to help configure SMP in FreeRTOS-Plus-TCP.

#ifdef configRUN_MULTIPLE_PRIORITIES && configRUN_MULTIPLE_PRIORITIES != 0
    #define ipconfigRUN_MULTIPLE_PRIORITIES    ipconfigENABLE
#endif

/* define other definitions based on `ipconfigRUN_MULTIPLE_PRIORITIES` */

Thank you.

[mike919192]

#ifdef configRUN_MULTIPLE_PRIORITIES && configRUN_MULTIPLE_PRIORITIES != 0
#define ipconfigRUN_MULTIPLE_PRIORITIES ipconfigENABLE
#endif

I'm not sure I see what would be the point of defining a new macro ipconfigRUN_MULTIPLE_PRIORITIES. The current macro configRUN_MULTIPLE_PRIORITIES is not really used anywhere in user code (or library code). Its basically just exclusively used by the scheduler for its algorithm. Also in the kernel it is defaulted to 0 (which is the more conservative setting).

I could see maybe a warning or overridable error is printed in the library if configRUN_MULTIPLE_PRIORITIES is set to 1, since this is all experimental.

The reason task affinity is needed in the library is that because it is a parameter in task creation. The functions xTaskCreateAffinitySet and xTaskCreateStaticAffinitySet take a task affinity parameter along with all the usual parameters.

[ActoryOu]

That sounds good to me! Thanks for clarifiing!

[ActoryOu]

Closing since #1288 is merged.