Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[RFC] Add priority queues #925

Open
wants to merge 9 commits into
base: dev/IPv6_integration
Choose a base branch
from
Open

[RFC] Add priority queues #925

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
090000a
Add priority fields for sockets and buffers.
go2sh Jun 15, 2023
43c522d
Add multiple event queues.
go2sh Jun 15, 2023
f731c9d
Add missing udp packet priority.
go2sh Jun 15, 2023
97d05ae
Fix priority check in setsockopt
go2sh Jun 15, 2023
680ae63
Add default priority config value.
go2sh Jun 15, 2023
890ef29
Address review comments.
go2sh Jun 16, 2023
9d8c6a0
Add spellchecking entry.
go2sh Jun 16, 2023
88cbabc
Fix define wording.
go2sh Jun 16, 2023
be084d1
Change priority value to BaseType_t.
go2sh Jun 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/lexicon.txt
View file
Open in desktop
Expand Up @@ -1345,6 +1345,7 @@ ucpeerwinscalefactor
ucpreference
ucprefix
ucprefixlength
ucpriority
ucprotocol
ucprotocoladdresslength
ucprotocolheadersize
Expand Down
131 changes: 113 additions & 18 deletions source/FreeRTOS_IP.c
View file
Open in desktop
Expand Up @@ -178,6 +178,11 @@ static eFrameProcessingResult_t prvProcessUDPPacket( NetworkBufferDescriptor_t *
/** @brief The queue used to pass events into the IP-task for processing. */
QueueHandle_t xNetworkEventQueue = NULL;

#if ( ipconfigEVENT_QUEUES > 1 )
QueueHandle_t xNetworkEventQueues[ ipconfigEVENT_QUEUES ] = { 0 };
uint8_t xQueueMapping[ ipconfigPACKET_PRIORITIES ] = ipconfigPACKET_PRIORITY_MAPPING;
#endif

/** @brief The IP packet ID. */
uint16_t usPacketIdentifier = 0U;

Expand Down Expand Up @@ -271,10 +276,31 @@ static void prvProcessIPEventsAndTimers( void )
/* Wait until there is something to do. If the following call exits
* due to a time out rather than a message being received, set a
* 'NoEvent' value. */
if( xQueueReceive( xNetworkEventQueue, ( void * ) &xReceivedEvent, xNextIPSleep ) == pdFALSE )
{
xReceivedEvent.eEventType = eNoEvent;
}
#if ( ipconfigEVENT_QUEUES > 1 )
if( ulTaskNotifyTake( pdFALSE, xNextIPSleep ) == pdFALSE )
tony-josi-aws marked this conversation as resolved.
Show resolved Hide resolved
{
xReceivedEvent.eEventType = eNoEvent;
}
else
{
BaseType_t xIndex;
BaseType_t xReturn;

for( xIndex = ipconfigEVENT_QUEUES - 1; xIndex >= 0; xIndex-- )
{
if( uxQueueMessagesWaiting( xNetworkEventQueues[ xIndex ] ) > 0 )
{
xQueueReceive( xNetworkEventQueues[ xIndex ], &xReceivedEvent, 0 );
break;
}
}
}
#else /* if ( ipconfigEVENT_QUEUES > 1 ) */
if( xQueueReceive( xNetworkEventQueue, ( void * ) &xReceivedEvent, xNextIPSleep ) == pdFALSE )
{
xReceivedEvent.eEventType = eNoEvent;
}
#endif /* if ( ipconfigEVENT_QUEUES > 1 ) */

#if ( ipconfigCHECK_IP_QUEUE_SPACE != 0 )
{
Expand Down Expand Up @@ -942,6 +968,11 @@ void * FreeRTOS_GetUDPPayloadBuffer_Multi( size_t uxRequestedSizeBytes,
BaseType_t FreeRTOS_IPInit_Multi( void )
{
BaseType_t xReturn = pdFALSE;
BaseType_t xAllocSuccessful = pdFALSE;

#if ( ipconfigEVENT_QUEUES > 1 )
BaseType_t xIndex;
#endif

/* There must be at least one interface and one end-point. */
configASSERT( FreeRTOS_FirstNetworkInterface() != NULL );
Expand All @@ -953,21 +984,54 @@ BaseType_t FreeRTOS_IPInit_Multi( void )
/* Attempt to create the queue used to communicate with the IP task. */
#if ( configSUPPORT_STATIC_ALLOCATION == 1 )
{
static StaticQueue_t xNetworkEventStaticQueue;
static uint8_t ucNetworkEventQueueStorageArea[ ipconfigEVENT_QUEUE_LENGTH * sizeof( IPStackEvent_t ) ];
xNetworkEventQueue = xQueueCreateStatic( ipconfigEVENT_QUEUE_LENGTH,
sizeof( IPStackEvent_t ),
ucNetworkEventQueueStorageArea,
&xNetworkEventStaticQueue );
#if ( ipconfigEVENT_QUEUES > 1 )
static StaticQueue_t xPacketEventStaticQueue[ ipconfigEVENT_QUEUES ];
static uint8_t ucPacketEventQueueStorageArea[ ipconfigEVENT_QUEUES ][ ipconfigEVENT_QUEUE_LENGTH * sizeof( IPStackEvent_t ) ];

xAllocSuccessful = pdTRUE;

for( xIndex = 0; xIndex < ipconfigEVENT_QUEUES; xIndex++ )
{
xNetworkEventQueues[ xIndex ] = xQueueCreateStatic( ipconfigEVENT_QUEUE_LENGTH,
sizeof( IPStackEvent_t ),
ucPacketEventQueueStorageArea[ xIndex ],
&xPacketEventStaticQueue[ xIndex ] );
xAllocSuccessful &= ( xNetworkEventQueues[ xIndex ] != NULL );
}

xNetworkEventQueue = xNetworkEventQueues[ ipconfigEVENT_QUEUES - 1 ];
#else /* if ( ipconfigEVENT_QUEUES > 1 ) */
static StaticQueue_t xNetworkEventStaticQueue;
static uint8_t ucNetworkEventQueueStorageArea[ ipconfigEVENT_QUEUE_LENGTH * sizeof( IPStackEvent_t ) ];
xNetworkEventQueue = xQueueCreateStatic( ipconfigEVENT_QUEUE_LENGTH,
sizeof( IPStackEvent_t ),
ucNetworkEventQueueStorageArea,
&xNetworkEventStaticQueue );
xAllocSuccessful = ( xNetworkEventQueue != NULL );
#endif /* if ( ipconfigEVENT_QUEUES > 1 ) */
}
#else
#else /* if ( configSUPPORT_STATIC_ALLOCATION == 1 ) */
{
xNetworkEventQueue = xQueueCreate( ipconfigEVENT_QUEUE_LENGTH, sizeof( IPStackEvent_t ) );
configASSERT( xNetworkEventQueue != NULL );
#if ( ipconfigEVENT_QUEUES > 1 )
xAllocSuccessful = pdTRUE;

for( xIndex = 0; xIndex < ipconfigEVENT_QUEUES; xIndex++ )
{
xNetworkEventQueues[ xIndex ] = xQueueCreate( ipconfigEVENT_QUEUE_LENGTH, sizeof( IPStackEvent_t ) );
configASSERT( xNetworkEventQueues[ xIndex ] != NULL );
xAllocSuccessful &= ( xNetworkEventQueues[ xIndex ] != NULL );
}

xNetworkEventQueue = xNetworkEventQueues[ ipconfigEVENT_QUEUES - 1 ];
#else /* if ( ipconfigEVENT_QUEUES > 1 ) */
xNetworkEventQueue = xQueueCreate( ipconfigEVENT_QUEUE_LENGTH, sizeof( IPStackEvent_t ) );
configASSERT( xNetworkEventQueue != NULL );
xAllocSuccessful = ( xNetworkEventQueue != NULL );
#endif /* if ( ipconfigEVENT_QUEUES > 1 ) */
}
#endif /* configSUPPORT_STATIC_ALLOCATION */

if( xNetworkEventQueue != NULL )
if( xAllocSuccessful != pdFALSE )
{
#if ( configQUEUE_REGISTRY_SIZE > 0 )
{
Expand Down Expand Up @@ -1017,8 +1081,18 @@ BaseType_t FreeRTOS_IPInit_Multi( void )
FreeRTOS_debug_printf( ( "FreeRTOS_IPInit_Multi: xNetworkBuffersInitialise() failed\n" ) );

/* Clean up. */
vQueueDelete( xNetworkEventQueue );
xNetworkEventQueue = NULL;
#if ( ipconfigEVENT_QUEUES > 1 )
for( xIndex = 0; xIndex < ipconfigEVENT_QUEUES; xIndex++ )
{
vQueueDelete( xNetworkEventQueues[ xIndex ] );
xNetworkEventQueues[ xIndex ] = NULL;
}

xNetworkEventQueue = NULL;
#else
vQueueDelete( xNetworkEventQueue );
xNetworkEventQueue = NULL;
#endif
}
}
else
Expand Down Expand Up @@ -1392,7 +1466,11 @@ BaseType_t xSendEventStructToIPTask( const IPStackEvent_t * pxEvent,
* IP task is already awake processing other message. */
vIPSetTCPTimerExpiredState( pdTRUE );

if( uxQueueMessagesWaiting( xNetworkEventQueue ) != 0U )
#if ( ipconfigEVENT_QUEUES > 1 )
if( ulTaskNotifyValueClear( xIPTaskHandle, 0 ) != 0U )
#else
if( uxQueueMessagesWaiting( xNetworkEventQueue ) != 0U )
#endif
{
/* Not actually going to send the message but this is not a
* failure as the message didn't need to be sent. */
Expand All @@ -1411,7 +1489,24 @@ BaseType_t xSendEventStructToIPTask( const IPStackEvent_t * pxEvent,
uxUseTimeout = ( TickType_t ) 0;
}

xReturn = xQueueSendToBack( xNetworkEventQueue, pxEvent, uxUseTimeout );
#if ( ipconfigEVENT_QUEUES > 1 )
BaseType_t xQueue = ipconfigEVENT_QUEUES - 1;

if( ( pxEvent->eEventType == eNetworkRxEvent ) || ( pxEvent->eEventType == eNetworkTxEvent ) || ( pxEvent->eEventType == eStackTxEvent ) )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

{
NetworkBufferDescriptor_t * pxBuffer = ( NetworkBufferDescriptor_t * ) pxEvent->pvData;
xQueue = xQueueMapping[ pxBuffer->ucPriority ];
}

xReturn = xQueueSendToBack( xNetworkEventQueues[ xQueue ], pxEvent, uxUseTimeout );
Comment on lines +1493 to +1501
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


if( xReturn != pdFAIL )
{
xTaskNotifyGive( xIPTaskHandle );
moninom1 marked this conversation as resolved.
Show resolved Hide resolved
}
#else /* if ( ipconfigEVENT_QUEUES > 1 ) */
xReturn = xQueueSendToBack( xNetworkEventQueue, pxEvent, uxUseTimeout );
#endif /* if ( ipconfigEVENT_QUEUES > 1 ) */

if( xReturn == pdFAIL )
{
Expand Down
24 changes: 24 additions & 0 deletions source/FreeRTOS_Sockets.c
View file
Open in desktop
Expand Up @@ -758,6 +758,9 @@ Socket_t FreeRTOS_socket( BaseType_t xDomain,
pxSocket->xSendBlockTime = ipconfigSOCK_DEFAULT_SEND_BLOCK_TIME;
pxSocket->ucSocketOptions = ( uint8_t ) FREERTOS_SO_UDPCKSUM_OUT;
pxSocket->ucProtocol = ( uint8_t ) xProtocolCpy; /* protocol: UDP or TCP */
#if ( ipconfigPACKET_PRIORITIES > 1 )
pxSocket->ucPriority = ipconfigPACKET_PRIORITY_DEFAULT;
#endif

xReturn = pxSocket;
}
Expand Down Expand Up @@ -1431,6 +1434,9 @@ static int32_t prvSendUDPPacket( const FreeRTOS_Socket_t * pxSocket,
pxNetworkBuffer->xDataLength = uxTotalDataLength + uxPayloadOffset;
pxNetworkBuffer->usPort = pxDestinationAddress->sin_port;
pxNetworkBuffer->usBoundPort = ( uint16_t ) socketGET_SOCKET_PORT( pxSocket );
#if ipconfigPACKET_PRIORITIES > 1
pxNetworkBuffer->ucPriority = pxSocket->ucPriority;
#endif

/* The socket options are passed to the IP layer in the
* space that will eventually get used by the Ethernet header. */
Expand Down Expand Up @@ -2928,6 +2934,24 @@ BaseType_t FreeRTOS_setsockopt( Socket_t xSocket,
xReturn = prvSetOptionStopRX( pxSocket, pvOptionValue );
break;
#endif /* ipconfigUSE_TCP == 1 */
#if ( ipconfigPACKET_PRIORITIES > 1 )
case FREERTOS_SO_PRIORITY:
{
BaseType_t * pxValue = ( BaseType_t * ) pvOptionValue;

if( ( pxValue == NULL ) || ( *pxValue >= ipconfigPACKET_PRIORITIES ) )
{
xReturn = pdFREERTOS_ERRNO_EINVAL;
}
else
{
pxSocket->ucPriority = *pxValue;
xReturn = pdPASS;
}

break;
}
#endif /* if ( ipconfigPACKET_PRIORITIES > 1 ) */

default:
/* No other options are handled. */
Expand Down
3 changes: 3 additions & 0 deletions source/FreeRTOS_TCP_Transmission_IPV4.c
View file
Open in desktop
Expand Up @@ -183,6 +183,9 @@ void prvTCPReturnPacket_IPV4( FreeRTOS_Socket_t * pxSocket,
prvTCPReturn_SetSequenceNumber( pxSocket, pxNetworkBuffer, uxIPHeaderSize, ulLen );
pxIPHeader->ulDestinationIPAddress = FreeRTOS_htonl( pxSocket->u.xTCP.xRemoteIP.ulIP_IPv4 );
pxIPHeader->ulSourceIPAddress = pxNetworkBuffer->pxEndPoint->ipv4_settings.ulIPAddress;
#if ( ipconfigPACKET_PRIORITIES > 1 )
pxNetworkBuffer->ucPriority = pxSocket->ucPriority;
#endif
}
else
{
Expand Down
3 changes: 3 additions & 0 deletions source/FreeRTOS_TCP_Transmission_IPV6.c
View file
Open in desktop
Expand Up @@ -193,6 +193,9 @@ void prvTCPReturnPacket_IPV6( FreeRTOS_Socket_t * pxSocket,
prvTCPReturn_SetSequenceNumber( pxSocket, pxNetworkBuffer, uxIPHeaderSize, ulLen );
( void ) memcpy( pxIPHeader->xDestinationAddress.ucBytes, pxSocket->u.xTCP.xRemoteIP.xIP_IPv6.ucBytes, ipSIZE_OF_IPv6_ADDRESS );
( void ) memcpy( pxIPHeader->xSourceAddress.ucBytes, pxNetworkBuffer->pxEndPoint->ipv6_settings.xIPAddress.ucBytes, ipSIZE_OF_IPv6_ADDRESS );
#if ( ipconfigPACKET_PRIORITIES > 1 )
pxNetworkBuffer->ucPriority = pxSocket->ucPriority;
#endif
}
else
{
Expand Down
28 changes: 28 additions & 0 deletions source/include/FreeRTOSIPConfigDefaults.h
View file
Open in desktop
Expand Up @@ -1161,4 +1161,32 @@
#define ipconfigRA_IP_TEST_TIME_OUT_MSEC ( 1500U )
#endif

#ifndef ipconfigEVENT_QUEUES
#define ipconfigEVENT_QUEUES 1
#endif

#ifndef ipconfigPACKET_PRIORITIES
#if ( ipconfigEVENT_QUEUES > 1 )
#define ipconfigPACKET_PRIORITIES 8
#else
#define ipconfigPACKET_PRIORITIES 1
#endif
#endif

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

#elif ipconfigPACKET_PRIORITIES > 1 && ipconfigEVENT_QUEUES > 1
#error "Please define your own ipconfigPACKET_PRIORITY_MAPPING."
#endif
#endif

#ifndef ipconfigPACKET_PRIORITY_DEFAULT
#define ipconfigPACKET_PRIORITY_DEFAULT 1
#endif

#if ipconfigPACKET_PRIORITIES == 1 && ipconfigEVENT_QUEUES > 1
#error "Network event queues need priority support."
#endif

#endif /* FREERTOS_DEFAULT_IP_CONFIG_H */
3 changes: 3 additions & 0 deletions source/include/FreeRTOS_IP.h
View file
Open in desktop
Expand Up @@ -168,6 +168,9 @@ typedef struct xNETWORK_BUFFER
#if ( ipconfigUSE_LINKED_RX_MESSAGES != 0 )
struct xNETWORK_BUFFER * pxNextBuffer; /**< Possible optimisation for expert users - requires network driver support. */
#endif
#if ( ipconfigPACKET_PRIORITIES > 1 )
uint8_t ucPriority; /**< Priority of the buffer*/
#endif

#define ul_IPAddress xIPAddress.xIP_IPv4
#define x_IPv6Address xIPAddress.xIP_IPv6
Expand Down
4 changes: 4 additions & 0 deletions source/include/FreeRTOS_IP_Private.h
View file
Open in desktop
Expand Up @@ -715,6 +715,10 @@ struct xSOCKET
*/
void * pvSocketID;

#if ( ipconfigPACKET_PRIORITIES > 1 )
uint8_t ucPriority; /**< Priority of the socket */
#endif

/* TCP/UDP specific fields: */
/* Before accessing any member of this structure, it should be confirmed */
/* that the protocol corresponds with the type of structure */
Expand Down
7 changes: 6 additions & 1 deletion source/include/FreeRTOS_Sockets.h
View file
Open in desktop
Expand Up @@ -148,8 +148,13 @@
#endif

#if ( ipconfigUSE_TCP == 1 )
#define FREERTOS_SO_SET_LOW_HIGH_WATER ( 18 )
#define FREERTOS_SO_SET_LOW_HIGH_WATER ( 18 )
#endif

#if ( ipconfigPACKET_PRIORITIES > 1 )
#define FREERTOS_SO_PRIORITY ( 19 )
#endif

#define FREERTOS_INADDR_ANY ( 0U ) /* The 0.0.0.0 IPv4 address. */

#if ( 0 ) /* Not Used */
Expand Down
3 changes: 3 additions & 0 deletions source/portable/BufferManagement/BufferAllocation_1.c
View file
Open in desktop
Expand Up @@ -292,6 +292,9 @@ NetworkBufferDescriptor_t * pxGetNetworkBufferWithDescriptor( size_t xRequestedS
pxReturn->xDataLength = xRequestedSizeBytes;
pxReturn->pxInterface = NULL;
pxReturn->pxEndPoint = NULL;
#if ( ipconfigPACKET_PRIORITIES > 1 )
pxReturn->ucPriority = ipconfigPACKET_PRIORITY_DEFAULT;
#endif

#if ( ipconfigTCP_IP_SANITY != 0 )
{
Expand Down
3 changes: 3 additions & 0 deletions source/portable/BufferManagement/BufferAllocation_2.c
View file
Open in desktop
Expand Up @@ -306,6 +306,9 @@ NetworkBufferDescriptor_t * pxGetNetworkBufferWithDescriptor( size_t xRequestedS
pxReturn->xDataLength = xRequestedSizeBytesCopy;
pxReturn->pxInterface = NULL;
pxReturn->pxEndPoint = NULL;
#if ( ipconfigPACKET_PRIORITIES > 1 )
pxReturn->ucPriority = ipconfigPACKET_PRIORITY_DEFAULT;
#endif

#if ( ipconfigUSE_LINKED_RX_MESSAGES != 0 )
{
Expand Down