Skip to content

Commit c0f5ba0

Browse files
bjsowaSkptaktony-josi-aws
authored
Fix network down up process (#1030)
* Disable DHCP timer when network goes down * Don't stop checking the Network Timer * Fix network down when using RA * Revert "Don't stop checking the Network Timer" This reverts commit f5d8d98. * Add vSetNotAllNetworksUp function * Fix unit tests * Update comments * Store DHCPv4 socket locally for all endpoints * Add vDHCPStop and vDHCPv6Stop functions * Fix IP Utils tests * Fix most of DHCP unit tests * Fix formatting * Change vSetNotAllNetworksUp into a more generic vSetAllNetworksUp * Fix almost all of DHCP unit tests * Fix formatting * Add tests for vDHCPStop and vDHCPv6Stop functions * Fix formatting * Remove redundant MISRA comment * Set all fields of xAddress when binding DHCP socket * Fix unit test coverage of prvCreateDHCPSocket * Fix DHCP CBMC memory proof --------- Co-authored-by: Soren Ptak <[email protected]> Co-authored-by: Tony Josi <[email protected]>
1 parent 3d5ee0e commit c0f5ba0

File tree

13 files changed

+493
-75
lines changed

13 files changed

+493
-75
lines changed

‎source/FreeRTOS_DHCP.c‎

Lines changed: 49 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,12 @@
118118
/*
119119
* Create the DHCP socket, if it has not been created already.
120120
*/
121-
_static void prvCreateDHCPSocket( const NetworkEndPoint_t * pxEndPoint );
121+
_static void prvCreateDHCPSocket( NetworkEndPoint_t * pxEndPoint );
122122

123123
/*
124124
* Close the DHCP socket, only when not in use anymore (i.e. xDHCPSocketUserCount = 0).
125125
*/
126-
static void prvCloseDHCPSocket( const NetworkEndPoint_t * pxEndPoint );
126+
static void prvCloseDHCPSocket( NetworkEndPoint_t * pxEndPoint );
127127

128128
static void vDHCPProcessEndPoint( BaseType_t xReset,
129129
BaseType_t xDoCheck,
@@ -214,20 +214,20 @@
214214
FreeRTOS_debug_printf( ( "DHCP wrong state: expect: %d got: %d : ignore\n",
215215
EP_DHCPData.eExpectedState, EP_DHCPData.eDHCPState ) );
216216
}
217-
else if( xDHCPv4Socket != NULL ) /* If there is a socket, check for incoming messages first. */
217+
else if( EP_DHCPData.xDHCPSocket != NULL ) /* If there is a socket, check for incoming messages first. */
218218
{
219219
/* No need to initialise 'pucUDPPayload', it just looks nicer. */
220220
uint8_t * pucUDPPayload = NULL;
221221
const DHCPMessage_IPv4_t * pxDHCPMessage;
222222
int32_t lBytes;
223223

224-
while( xDHCPv4Socket != NULL )
224+
while( EP_DHCPData.xDHCPSocket != NULL )
225225
{
226226
BaseType_t xRecvFlags = FREERTOS_ZERO_COPY + FREERTOS_MSG_PEEK;
227227
NetworkEndPoint_t * pxIterator = NULL;
228228

229229
/* Peek the next UDP message. */
230-
lBytes = FreeRTOS_recvfrom( xDHCPv4Socket, &( pucUDPPayload ), 0, xRecvFlags, NULL, NULL );
230+
lBytes = FreeRTOS_recvfrom( EP_DHCPData.xDHCPSocket, &( pucUDPPayload ), 0, xRecvFlags, NULL, NULL );
231231

232232
if( lBytes < ( ( int32_t ) sizeof( DHCPMessage_IPv4_t ) ) )
233233
{
@@ -282,7 +282,7 @@
282282
{
283283
/* Target not found, fetch the message and delete it. */
284284
/* PAss the address of a pointer pucUDPPayload, because zero-copy is used. */
285-
lBytes = FreeRTOS_recvfrom( xDHCPv4Socket, &( pucUDPPayload ), 0, FREERTOS_ZERO_COPY, NULL, NULL );
285+
lBytes = FreeRTOS_recvfrom( EP_DHCPData.xDHCPSocket, &( pucUDPPayload ), 0, FREERTOS_ZERO_COPY, NULL, NULL );
286286

287287
if( ( lBytes > 0 ) && ( pucUDPPayload != NULL ) )
288288
{
@@ -305,6 +305,20 @@
305305
}
306306
}
307307

308+
/**
309+
* @brief Stop the DHCP process. Close the DHCP socket when it's no longer used by any end-point.
310+
*
311+
* @param[in] pxEndPoint The end-point for which we want to stop the DHCP process.
312+
*/
313+
void vDHCPStop( struct xNetworkEndPoint * pxEndPoint )
314+
{
315+
/* Disable the DHCP timer. */
316+
vIPSetDHCP_RATimerEnableState( pxEndPoint, pdFALSE );
317+
318+
/* Close socket to ensure packets don't queue on it. */
319+
prvCloseDHCPSocket( pxEndPoint );
320+
}
321+
308322
/**
309323
* @brief Called by vDHCPProcessEndPoint(), this function handles the state 'eWaitingOffer'.
310324
* If there is a reply, it will be examined, if there is a time-out, there may be a new
@@ -564,7 +578,7 @@
564578
#endif /* ipconfigUSE_DHCP_HOOK */
565579
{
566580
/* See if prvInitialiseDHCP() has creates a socket. */
567-
if( xDHCPv4Socket == NULL )
581+
if( EP_DHCPData.xDHCPSocket == NULL )
568582
{
569583
xGivingUp = pdTRUE;
570584
}
@@ -618,7 +632,7 @@
618632
/* Resend the request at the appropriate time to renew the lease. */
619633
prvCreateDHCPSocket( pxEndPoint );
620634

621-
if( xDHCPv4Socket != NULL )
635+
if( EP_DHCPData.xDHCPSocket != NULL )
622636
{
623637
uint32_t ulID = 0U;
624638

@@ -829,11 +843,13 @@
829843
* using it.
830844
* @param[in] pxEndPoint The end-point that stops using the socket.
831845
*/
832-
static void prvCloseDHCPSocket( const NetworkEndPoint_t * pxEndPoint )
846+
void prvCloseDHCPSocket( NetworkEndPoint_t * pxEndPoint )
833847
{
834-
( void ) pxEndPoint;
835-
836-
if( ( xDHCPv4Socket != NULL ) && ( xDHCPSocketUserCount > 0 ) )
848+
if( ( EP_DHCPData.xDHCPSocket == NULL ) || ( EP_DHCPData.xDHCPSocket != xDHCPv4Socket ) )
849+
{
850+
/* the socket can not be closed. */
851+
}
852+
else if( xDHCPSocketUserCount > 0 )
837853
{
838854
xDHCPSocketUserCount--;
839855

@@ -844,6 +860,8 @@
844860
( void ) vSocketClose( xDHCPv4Socket );
845861
xDHCPv4Socket = NULL;
846862
}
863+
864+
EP_DHCPData.xDHCPSocket = NULL;
847865
}
848866
else
849867
{
@@ -862,13 +880,17 @@
862880
* @brief Create a DHCP socket with the defined timeouts. The same socket
863881
* will be shared among all end-points that need DHCP.
864882
*/
865-
_static void prvCreateDHCPSocket( const NetworkEndPoint_t * pxEndPoint )
883+
_static void prvCreateDHCPSocket( NetworkEndPoint_t * pxEndPoint )
866884
{
867885
struct freertos_sockaddr xAddress;
868886
BaseType_t xReturn;
869887
TickType_t xTimeoutTime = ( TickType_t ) 0;
870888

871-
if( xDHCPv4Socket == NULL ) /* Create the socket, if it has not already been created. */
889+
if( ( xDHCPv4Socket != NULL ) && ( EP_DHCPData.xDHCPSocket == xDHCPv4Socket ) )
890+
{
891+
/* the socket is still valid. */
892+
}
893+
else if( xDHCPv4Socket == NULL ) /* Create the socket, if it has not already been created. */
872894
{
873895
xDHCPv4Socket = FreeRTOS_socket( FREERTOS_AF_INET, FREERTOS_SOCK_DGRAM, FREERTOS_IPPROTO_UDP );
874896
configASSERT( xSocketValid( xDHCPv4Socket ) == pdTRUE );
@@ -883,10 +905,14 @@
883905
( void ) FreeRTOS_setsockopt( xDHCPv4Socket, 0, FREERTOS_SO_RCVTIMEO, &( xTimeoutTime ), sizeof( TickType_t ) );
884906
( void ) FreeRTOS_setsockopt( xDHCPv4Socket, 0, FREERTOS_SO_SNDTIMEO, &( xTimeoutTime ), sizeof( TickType_t ) );
885907

908+
memset( &xAddress, 0, sizeof( xAddress ) );
909+
xAddress.sin_family = FREERTOS_AF_INET4;
910+
xAddress.sin_len = ( uint8_t ) sizeof( xAddress );
886911
/* Bind to the standard DHCP client port. */
887912
xAddress.sin_port = ( uint16_t ) dhcpCLIENT_PORT_IPv4;
888913
xReturn = vSocketBind( xDHCPv4Socket, &xAddress, sizeof( xAddress ), pdFALSE );
889914
xDHCPSocketUserCount = 1;
915+
EP_DHCPData.xDHCPSocket = xDHCPv4Socket;
890916
FreeRTOS_printf( ( "DHCP-socket[%02x-%02x]: DHCP Socket Create\n",
891917
pxEndPoint->xMACAddress.ucBytes[ 4 ],
892918
pxEndPoint->xMACAddress.ucBytes[ 5 ] ) );
@@ -901,11 +927,13 @@
901927
{
902928
/* Change to NULL for easier testing. */
903929
xDHCPv4Socket = NULL;
930+
EP_DHCPData.xDHCPSocket = NULL;
904931
}
905932
}
906933
else
907934
{
908935
xDHCPSocketUserCount++;
936+
EP_DHCPData.xDHCPSocket = xDHCPv4Socket;
909937
}
910938

911939
FreeRTOS_printf( ( "prvCreateDHCPSocket[%02x-%02x]: %s, user count %d\n",
@@ -1248,7 +1276,7 @@
12481276
( void ) memset( &( xSet ), 0, sizeof( xSet ) );
12491277

12501278
/* Passing the address of a pointer (pucUDPPayload) because FREERTOS_ZERO_COPY is used. */
1251-
lBytes = FreeRTOS_recvfrom( xDHCPv4Socket, &pucUDPPayload, 0U, FREERTOS_ZERO_COPY, NULL, NULL );
1279+
lBytes = FreeRTOS_recvfrom( EP_DHCPData.xDHCPSocket, &pucUDPPayload, 0U, FREERTOS_ZERO_COPY, NULL, NULL );
12521280

12531281
if( lBytes > 0 )
12541282
{
@@ -1504,10 +1532,7 @@
15041532
&( uxOptionsLength ),
15051533
pxEndPoint );
15061534

1507-
/* MISRA Ref 11.4.1 [Socket error and integer to pointer conversion] */
1508-
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-114 */
1509-
/* coverity[misra_c_2012_rule_11_4_violation] */
1510-
if( ( xSocketValid( xDHCPv4Socket ) == pdTRUE ) && ( pucUDPPayloadBuffer != NULL ) )
1535+
if( ( xSocketValid( EP_DHCPData.xDHCPSocket ) == pdTRUE ) && ( pucUDPPayloadBuffer != NULL ) )
15111536
{
15121537
/* Copy in the IP address being requested. */
15131538

@@ -1528,9 +1553,9 @@
15281553
FreeRTOS_debug_printf( ( "vDHCPProcess: reply %xip\n", ( unsigned ) FreeRTOS_ntohl( EP_DHCPData.ulOfferedIPAddress ) ) );
15291554
iptraceSENDING_DHCP_REQUEST();
15301555

1531-
xDHCPv4Socket->pxEndPoint = pxEndPoint;
1556+
EP_DHCPData.xDHCPSocket->pxEndPoint = pxEndPoint;
15321557

1533-
if( FreeRTOS_sendto( xDHCPv4Socket, pucUDPPayloadBuffer, sizeof( DHCPMessage_IPv4_t ) + uxOptionsLength, FREERTOS_ZERO_COPY, &xAddress, ( socklen_t ) sizeof( xAddress ) ) == 0 )
1558+
if( FreeRTOS_sendto( EP_DHCPData.xDHCPSocket, pucUDPPayloadBuffer, sizeof( DHCPMessage_IPv4_t ) + uxOptionsLength, FREERTOS_ZERO_COPY, &xAddress, ( socklen_t ) sizeof( xAddress ) ) == 0 )
15341559
{
15351560
/* The packet was not successfully queued for sending and must be
15361561
* returned to the stack. */
@@ -1579,7 +1604,7 @@
15791604
/* MISRA Ref 11.4.1 [Socket error and integer to pointer conversion] */
15801605
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-114 */
15811606
/* coverity[misra_c_2012_rule_11_4_violation] */
1582-
if( ( xSocketValid( xDHCPv4Socket ) == pdTRUE ) && ( pucUDPPayloadBuffer != NULL ) )
1607+
if( ( xSocketValid( EP_DHCPData.xDHCPSocket ) == pdTRUE ) && ( pucUDPPayloadBuffer != NULL ) )
15831608
{
15841609
const void * pvCopySource;
15851610
void * pvCopyDest;
@@ -1608,9 +1633,9 @@
16081633
uxOptionsLength -= dhcpOPTION_50_SIZE;
16091634
}
16101635

1611-
xDHCPv4Socket->pxEndPoint = pxEndPoint;
1636+
EP_DHCPData.xDHCPSocket->pxEndPoint = pxEndPoint;
16121637

1613-
if( FreeRTOS_sendto( xDHCPv4Socket,
1638+
if( FreeRTOS_sendto( EP_DHCPData.xDHCPSocket,
16141639
pucUDPPayloadBuffer,
16151640
sizeof( DHCPMessage_IPv4_t ) + uxOptionsLength,
16161641
FREERTOS_ZERO_COPY,

‎source/FreeRTOS_DHCPv6.c‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,20 @@ void vDHCPv6Process( BaseType_t xReset,
464464
vDHCPv6ProcessEndPoint( xReset, pxEndPoint, pxEndPoint->pxDHCPMessage );
465465
}
466466
}
467+
468+
/**
469+
* @brief Stop the DHCP process. Close the DHCP socket when it's no longer used by any end-point.
470+
*
471+
* @param[in] pxEndPoint The end-point for which we want to stop the DHCP process.
472+
*/
473+
void vDHCPv6Stop( struct xNetworkEndPoint * pxEndPoint )
474+
{
475+
/* Disable the DHCP timer. */
476+
vIPSetDHCP_RATimerEnableState( pxEndPoint, pdFALSE );
477+
478+
/* Close socket to ensure packets don't queue on it. */
479+
prvCloseDHCPv6Socket( pxEndPoint );
480+
}
467481
/*-----------------------------------------------------------*/
468482

469483
/**

‎source/FreeRTOS_IP_Timers.c‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@
5757
#include "FreeRTOS_DNS.h"
5858
/*-----------------------------------------------------------*/
5959

60-
/** @brief 'xAllNetworksUp' becomes pdTRUE as soon as all network interfaces have
61-
* been initialised. */
60+
/** @brief 'xAllNetworksUp' becomes pdTRUE when all network interfaces are initialised
61+
* and becomes pdFALSE when any network interface goes down. */
6262
/* MISRA Ref 8.9.1 [File scoped variables] */
6363
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-89 */
6464
/* coverity[misra_c_2012_rule_8_9_violation] */
@@ -328,7 +328,7 @@ void vCheckNetworkTimers( void )
328328
}
329329
}
330330

331-
xAllNetworksUp = xUp;
331+
vSetAllNetworksUp( xUp );
332332
}
333333
}
334334
}
@@ -604,3 +604,12 @@ void vIPSetARPResolutionTimerEnableState( BaseType_t xEnableState )
604604

605605
#endif /* ipconfigDNS_USE_CALLBACKS == 1 */
606606
/*-----------------------------------------------------------*/
607+
608+
/**
609+
* @brief Mark whether all interfaces are up or at least one interface is down.
610+
* If all interfaces are up, the 'xNetworkTimer' will not be checked.
611+
*/
612+
void vSetAllNetworksUp( BaseType_t xIsAllNetworksUp )
613+
{
614+
xAllNetworksUp = xIsAllNetworksUp;
615+
}

‎source/FreeRTOS_IP_Utils.c‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -861,6 +861,31 @@ void prvProcessNetworkDownEvent( struct xNetworkInterface * pxInterface )
861861
* treat network down as a "delivery problem" and flush the ARP cache for this
862862
* interface. */
863863
FreeRTOS_ClearARP( pxEndPoint );
864+
865+
#if ( ipconfigUSE_DHCP == 1 )
866+
if( END_POINT_USES_DHCP( pxEndPoint ) )
867+
{
868+
#if ( ( ipconfigUSE_DHCPv6 != 0 ) && ( ipconfigUSE_IPv6 != 0 ) )
869+
if( pxEndPoint->bits.bIPv6 != pdFALSE_UNSIGNED )
870+
{
871+
vDHCPv6Stop( pxEndPoint );
872+
}
873+
else
874+
#endif /* (( ipconfigUSE_DHCPv6 != 0 ) && ( ipconfigUSE_IPv6 != 0 )) */
875+
{
876+
/* Stop the DHCP process for this end-point. */
877+
vDHCPStop( pxEndPoint );
878+
}
879+
}
880+
#endif /* ( ipconfigUSE_DHCP == 1 ) */
881+
882+
#if ( ( ipconfigUSE_RA != 0 ) && ( ipconfigUSE_IPv6 != 0 ) )
883+
if( END_POINT_USES_RA( pxEndPoint ) )
884+
{
885+
/* Stop the RA/SLAAC process for this end-point. */
886+
vIPSetDHCP_RATimerEnableState( pxEndPoint, pdFALSE );
887+
}
888+
#endif /* ( (ipconfigUSE_RA != 0) && ( ipconfigUSE_IPv6 != 0 )) */
864889
}
865890

866891
/* The network has been disconnected (or is being initialised for the first
@@ -936,7 +961,10 @@ void prvProcessNetworkDownEvent( struct xNetworkInterface * pxInterface )
936961
}
937962
else
938963
{
939-
/* Nothing to do. When the 'xNetworkTimer' expires, all interfaces
964+
/* At least one interface is down. */
965+
vSetAllNetworksUp( pdFALSE );
966+
967+
/* Nothing else to do. When the 'xNetworkTimer' expires, all interfaces
940968
* with bits.bInterfaceUp cleared will get a new 'eNetworkDownEvent' */
941969
}
942970
}

‎source/include/FreeRTOS_DHCP.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,12 @@ eDHCPState_t eGetDHCPState( const struct xNetworkEndPoint * pxEndPoint );
237237
void vDHCPProcess( BaseType_t xReset,
238238
struct xNetworkEndPoint * pxEndPoint );
239239

240+
/*
241+
* NOT A PUBLIC API FUNCTION.
242+
* It will be called when the network interface, that the endpoint is associated with, goes down.
243+
*/
244+
void vDHCPStop( struct xNetworkEndPoint * pxEndPoint );
245+
240246
/* Internal call: returns true if socket is the current DHCP socket */
241247
BaseType_t xIsDHCPSocket( const ConstSocket_t xSocket );
242248

‎source/include/FreeRTOS_DHCPv6.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,12 @@
152152
void vDHCPv6Process( BaseType_t xReset,
153153
struct xNetworkEndPoint * pxEndPoint );
154154

155+
/*
156+
* NOT A PUBLIC API FUNCTION.
157+
* It will be called when the network interface, that the endpoint is associated with, goes down.
158+
*/
159+
void vDHCPv6Stop( struct xNetworkEndPoint * pxEndPoint );
160+
155161
#ifdef __cplusplus
156162
} /* extern "C" */
157163
#endif

‎source/include/FreeRTOS_IP_Private.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -882,6 +882,9 @@ BaseType_t xIsCallingFromIPTask( void );
882882
/* Send the network-up event and start the ARP timer. */
883883
void vIPNetworkUpCalls( struct xNetworkEndPoint * pxEndPoint );
884884

885+
/* Mark whether all interfaces are up or at least one interface is down. */
886+
void vSetAllNetworksUp( BaseType_t xIsAllNetworksUp );
887+
885888
/* *INDENT-OFF* */
886889
#ifdef __cplusplus
887890
} /* extern "C" */

0 commit comments

Comments
 (0)