[RFC] Add priority queues #925

go2sh · 2023-06-15T20:19:07Z

Add priority queues.

Description

This PR adds priorities to socket and packets and multiple event queues to handle packets. This change enables multiple things with in the time sensitive networking space:

The stack can handle packets based on their priority to allow overtakes or idle background traffic using the remaining bandwidth. An example would be audio/video data or PTP message as high priority or a data dump in the background.
The packet priority can be used by network drivers to assign the packet to a different hardware queue.
The HW can add VLAN tags with priority fields. (And maybe later FreeRTOS-Plus-TCP :) )

Test Steps

Enable the feature via ipconfigPRIORITIES && ipconfigEVENT_QUEUES and add two udp sockets. The first socket gets a higher priority (e.g. 5). The second socket sends a lot of packets and the first socket afterwards one. This packet should overtake some of the packets of the second socket.

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

#894

TODOs

Make TCP sockets adhere the priority. This would basicly mean, that a TCP packet before sending must be placed back into the Queues and cannot be send directly.
Add unit tests for the new features.
Add a distinction between packet and socket priorities including separate mapping.

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

This enables strict priority based scheduling of packets within the stack. The user can define a socket/packet priority mapping for each queue. This allows certain high priority traffic to overtake packets or low priority traffic to fill the rest of the bandwidth.

AniruddhaKanhere

Hello @go2sh,

This is a very interesting prospect/suggestion.
While we discuss this PR with our team, I have an initial review with a couple of questions/concerns.

Thanks for taking the time to contribute to FreeRTOS+TCP.

-Aniruddha

source/FreeRTOS_Sockets.c

source/FreeRTOS_IP.c

AniruddhaKanhere · 2023-06-15T23:31:56Z

source/include/FreeRTOSIPConfigDefaults.h

+
+#ifndef ipconfigPACKET_PRIORITY_MAPPING
+    #if ipconfigPACKET_PRIORITIES == 8 && ipconfigEVENT_QUEUES == 3
+        #define ipconfigPACKET_PRIORITY_MAPPING    { 0, 1, 1, 1, 2, 2, 2, 2, }


Just trying to understand this scheme here. Please do correct me if I am wrong.

All messages with priority 0 will be added to Queue 0.
And all messages with priority 1-3 will be added to Queue 1.
And all messages with priority 4-7 will be added to Queue 2.

Is that right?
If so, then what is the point of having 8 different priorites if you just have 3 queues?

The idea is based on the current linux implementation, where tc-prio has 3 fifos. The 8 comes from the size of the vlan pcp which has 3 bits so 8 priorities. I do hw mappings inside of the driver and add vlan tags with the priority.

Could you please add a bit of comment about the solution here? It is confusing to see 3 queues, and 8 different priorities.

Add added a big comment below. I will add comment in the code as well.

Wouldn't it be better to have the number of priorities and the number queues to be equal in the default configuration (if we are keeping a default mapping), to avoid users getting confused about packets with different priorites getting into the same queue (depending on the mapping) and processed together regardless of their actual priorities.

AniruddhaKanhere · 2023-06-15T23:39:21Z

source/FreeRTOS_IP.c

+                BaseType_t xQueue = ipconfigEVENT_QUEUES - 1;
+
+                if( ( pxEvent->eEventType == eNetworkRxEvent ) || ( pxEvent->eEventType == eNetworkTxEvent ) || ( pxEvent->eEventType == eStackTxEvent ) )
+                {
+                    NetworkBufferDescriptor_t * pxBuffer = ( NetworkBufferDescriptor_t * ) pxEvent->pvData;
+                    xQueue = xQueueMapping[ pxBuffer->ucPriority ];
+                }
+
+                xReturn = xQueueSendToBack( xNetworkEventQueues[ xQueue ], pxEvent, uxUseTimeout );


Does this mean that all non-data-related events (like a eNetworkDown event) are highest priority and will be processed first?
I am not sure whether that will cause any unforeseen race conditions... It should not... But I am still thinking about it :)

Exactly. Initially I thought to flush the other queues but that is not possible. , since there might be other interfaces.

htibosch

Hi @go2sh, this looks like a useful addition: priorities. It is a good idea to use TaskNotify in combination with uxQueueMessagesWaiting().

Do you have an application that needs priorities? I am curious to hear how it works for you, with and without multiple queues.

As I write here below, eNetworkRxEvent packets have just arrived in the interface, and they do not have a specific priority. So you don't have to test it, just let them travel with the highest priority in order to avoid congestion.

Thank you very much for this contribution.
Hein

source/FreeRTOS_IP.c

htibosch · 2023-06-16T11:40:31Z

source/FreeRTOS_IP.c

+            #if ipconfigEVENT_QUEUES > 1
+                BaseType_t xQueue = ipconfigEVENT_QUEUES - 1;
+
+                if( ( pxEvent->eEventType == eNetworkRxEvent ) || ( pxEvent->eEventType == eNetworkTxEvent ) || ( pxEvent->eEventType == eStackTxEvent ) )


When pxEvent->eEventType == eNetworkRxEvent, I think that pxBuffer->ucPriority always has its default value of ipconfigPACKET_PRIORITY_DEFAULT. It was assigned in BufferAllocation_x.c

Here, the packet has just been received by NetworkInterface.c, and it has not been matched with a receiving socket yet. I think it is useless to test its priority.

In general it is good to give a higher priority to incoming traffic in order to avoid congestion.

See the big comment below. The incoming priority in my case is set by the NetworkInterface based on the VLAN PCP field.

source/FreeRTOS_Sockets.c

htibosch · 2023-06-16T12:08:54Z

source/include/FreeRTOSIPConfigDefaults.h

+
+#ifndef ipconfigPACKET_PRIORITY_MAPPING
+    #if ipconfigPACKET_PRIORITIES == 8 && ipconfigEVENT_QUEUES == 3
+        #define ipconfigPACKET_PRIORITY_MAPPING    { 0, 1, 1, 1, 2, 2, 2, 2, }


Could you please add a bit of comment about the solution here? It is confusing to see 3 queues, and 8 different priorities.

go2sh · 2023-06-16T15:10:28Z

A general comment about priorities and queues:

The number of priorities is chosen based on the VLAN Tag PCP field with 3 bit. (See https://en.wikipedia.org/wiki/IEEE_P802.1p) Other possibilities are TOS field from IP header (4 bit / 16 numbers) or simply the number queues. Its really application dependent. I think the most useful is the VLAN PCP variant with 8. In my case: The NetworkInterface (a Infineon TC3 and TC4 port) inserts and removes a vlan tag via the descriptors and the PCP value is used to assign dedicated tx and rx hardware queues. Also the PCP field for RX packets is written as priority into the NetworkBufferDescriptor_t field of a received packet overwriting the default value and thus the priority handling of rx events.
The number of queues is chosen based on the default number 3 from the tc-prio scheduler of the linux kernel. (https://www.man7.org/linux/man-pages/man8/tc-prio.8.html) The idea behind 3 is: One queue for best effort. No garuantees at all. Gets what is left. One queue for normal app traffic like HTTP/FTP/MQTT etc. One queue for high priority traffic like low latency traffic or real time traffic. (e.g. Audio/Video, Real-Time Sense and Control or Time Sync). The number of queues might be equal to the number of priorities, but for most applications the number of packets with a high priority are rather small compared to the rest of the traffic. So more queues might waste resources, since only a few messages are stored in there.

I don't mind the numbers at all. A just wanted to choose a reasonable default, since anybody can overwrite it relatively easy.

A comment on the application: I work at Infineon and I port FreeRTOS+TCP on our TC3 and TC4 microcontroller. These are rather powerful devices with a lot of cores (up to 6), a lot of SRAM (a few MB) and up to 5G Ethernet MACs with 8 DMA and 8 Hardware Queues each for RX and TX). We run relativly complex applications with multiple tasks working on network traffic with diffrent application domains. From SOTA, CAN Tunneling, Service Handling (like SOMEIP or MQTT) and real time data like Audio and Video (or Radar) streams. These patch is a first step off removing some bottlenecks with the stack and to fulfill the real-time requirements.

As an concrete example: I have an app which runs some low priority Debug Instrumentation, an MQTT Client and some Audio Processing. Since the system has a lot of ram, the number of NetworkBuffers is high (eg. 128). This lead to the instrumentaion filling up the single queue and the audio task had no chance of meeting the real time latency requirement. With this multiple queue approach: The instrumentation runs in the lowest priority and gets rest of the bandwith. Its no problem if a packets waits a bit. The overall bandwidth of the system is sufficient. The MQTT traffic runs in the middle priority and the audio and timesync traffic in the highest. (Only a few 100 kBit/s but very latency sensitive). With this multiple queues approach, I don't need take any measures within the software to meet my real-time goals.

I hope this makes this a bit more clear. :)

go2sh · 2023-06-16T19:05:20Z

One more comment regarding: RX Events. The problem why I also added the priorities for RX events is the following: Think about a lot of NetworkBuffers and your IP Tasks start blocking until a descriptor entry is free inside the descriptor table. During that time no RX packet handling will happen as the IP task is blocking. The high priority queue makes it possible, that an high priority packet is processed a bit earlier than otherwise. The ideal solution would be to split RX and TX Packet processing. Then no extra queues for RX would be needed, as the traffic comes in prioritized already.

shubnil · 2023-06-17T04:27:20Z

Thanks Christoph for creating the PR.
We will discuss internally more on the design and , if needed, I will synch up with you as well more on the design part and take the change forward.
Please note that the actual merge might happen only in August. We have an important GA release coming up and we will be freezing the branch in a week. However, we will make sure that we merge the change as soon as the release tagging is done.
Thanks for the patience.

amazonKamath · 2023-06-19T16:23:55Z

A general comment about priorities and queues:

The number of priorities is chosen based on the VLAN Tag PCP field with 3 bit. (See https://en.wikipedia.org/wiki/IEEE_P802.1p) Other possibilities are TOS field from IP header (4 bit / 16 numbers) or simply the number queues. Its really application dependent. I think the most useful is the VLAN PCP variant with 8. In my case: The NetworkInterface (a Infineon TC3 and TC4 port) inserts and removes a vlan tag via the descriptors and the PCP value is used to assign dedicated tx and rx hardware queues. Also the PCP field for RX packets is written as priority into the NetworkBufferDescriptor_t field of a received packet overwriting the default value and thus the priority handling of rx events.

The number of queues is chosen based on the default number 3 from the tc-prio scheduler of the linux kernel. (https://www.man7.org/linux/man-pages/man8/tc-prio.8.html) The idea behind 3 is: One queue for best effort. No garuantees at all. Gets what is left. One queue for normal app traffic like HTTP/FTP/MQTT etc. One queue for high priority traffic like low latency traffic or real time traffic. (e.g. Audio/Video, Real-Time Sense and Control or Time Sync). The number of queues might be equal to the number of priorities, but for most applications the number of packets with a high priority are rather small compared to the rest of the traffic. So more queues might waste resources, since only a few messages are stored in there.

I don't mind the numbers at all. A just wanted to choose a reasonable default, since anybody can overwrite it relatively easy.

A comment on the application: I work at Infineon and I port FreeRTOS+TCP on our TC3 and TC4 microcontroller. These are rather powerful devices with a lot of cores (up to 6), a lot of SRAM (a few MB) and up to 5G Ethernet MACs with 8 DMA and 8 Hardware Queues each for RX and TX). We run relativly complex applications with multiple tasks working on network traffic with diffrent application domains. From SOTA, CAN Tunneling, Service Handling (like SOMEIP or MQTT) and real time data like Audio and Video (or Radar) streams. These patch is a first step off removing some bottlenecks with the stack and to fulfill the real-time requirements.

As an concrete example: I have an app which runs some low priority Debug Instrumentation, an MQTT Client and some Audio Processing. Since the system has a lot of ram, the number of NetworkBuffers is high (eg. 128). This lead to the instrumentaion filling up the single queue and the audio task had no chance of meeting the real time latency requirement. With this multiple queue approach: The instrumentation runs in the lowest priority and gets rest of the bandwith. Its no problem if a packets waits a bit. The overall bandwidth of the system is sufficient. The MQTT traffic runs in the middle priority and the audio and timesync traffic in the highest. (Only a few 100 kBit/s but very latency sensitive). With this multiple queues approach, I don't need take any measures within the software to meet my real-time goals.

I hope this makes this a bit more clear. :)

@go2sh thoughts on mq-prio?

go2sh · 2023-06-20T10:10:04Z

@go2sh thoughts on mq-prio?

That is my final step of my TSN chain.

I would see this functionality as part of the network interface. In my case, the driver checks the NetworkBuffer ucPriority and has a custom mapping array for mapping the packets to the different DMA queues. But each devices does it differently and I know that alot of other devices support multiple hw queues, which are already part of FreeRTOS+TCP.

go2sh added 5 commits June 15, 2023 21:58

Add priority fields for sockets and buffers.

090000a

Add missing udp packet priority.

f731c9d

Fix priority check in setsockopt

97d05ae

Add default priority config value.

680ae63

AniruddhaKanhere reviewed Jun 15, 2023

View reviewed changes

go2sh added 2 commits June 16, 2023 12:57

Address review comments.

890ef29

Add spellchecking entry.

9d8c6a0

htibosch reviewed Jun 16, 2023

View reviewed changes

go2sh added 2 commits June 16, 2023 20:20

Fix define wording.

88cbabc

Change priority value to BaseType_t.

be084d1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] Add priority queues #925

[RFC] Add priority queues #925

go2sh commented Jun 15, 2023 •

edited

Loading

AniruddhaKanhere left a comment

AniruddhaKanhere Jun 15, 2023

go2sh Jun 16, 2023

htibosch Jun 16, 2023

go2sh Jun 16, 2023

tony-josi-aws Jun 19, 2023

AniruddhaKanhere Jun 15, 2023

go2sh Jun 16, 2023

htibosch left a comment

htibosch Jun 16, 2023

go2sh Jun 16, 2023

htibosch Jun 16, 2023

go2sh commented Jun 16, 2023 •

edited

Loading

go2sh commented Jun 16, 2023

shubnil commented Jun 17, 2023 •

edited

Loading

amazonKamath commented Jun 19, 2023

go2sh commented Jun 20, 2023 •

edited

Loading

[RFC] Add priority queues #925

Are you sure you want to change the base?

[RFC] Add priority queues #925

Conversation

go2sh commented Jun 15, 2023 • edited Loading

Description

Test Steps

Checklist:

Related Issue

TODOs

AniruddhaKanhere left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

htibosch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

go2sh commented Jun 16, 2023 • edited Loading

go2sh commented Jun 16, 2023

shubnil commented Jun 17, 2023 • edited Loading

amazonKamath commented Jun 19, 2023

go2sh commented Jun 20, 2023 • edited Loading

go2sh commented Jun 15, 2023 •

edited

Loading

go2sh commented Jun 16, 2023 •

edited

Loading

shubnil commented Jun 17, 2023 •

edited

Loading

go2sh commented Jun 20, 2023 •

edited

Loading