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.

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

Fix network down up process #1030

Merged
merged 25 commits into from
Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
5058d3f
Disable DHCP timer when network goes down
bjsowa Sep 25, 2023
f5d8d98
Don't stop checking the Network Timer
bjsowa Sep 22, 2023
9132077
Fix network down when using RA
bjsowa Sep 25, 2023
9e17182
Merge branch 'main' into fix-network-down-up
Skptak Sep 26, 2023
3c69c91
Revert "Don't stop checking the Network Timer"
bjsowa Sep 22, 2023
0975315
Add vSetNotAllNetworksUp function
bjsowa Sep 27, 2023
07b23ea
Fix unit tests
bjsowa Sep 27, 2023
83a3d9e
Update comments
bjsowa Sep 27, 2023
4bb3d52
Store DHCPv4 socket locally for all endpoints
bjsowa Oct 5, 2023
6e7f2b9
Add vDHCPStop and vDHCPv6Stop functions
bjsowa Oct 5, 2023
47a5d02
Fix IP Utils tests
bjsowa Oct 5, 2023
6b304cd
Fix most of DHCP unit tests
bjsowa Oct 5, 2023
e0a861d
Fix formatting
bjsowa Oct 5, 2023
6408ee7
Merge branch 'main' into fix-network-down-up
bjsowa Oct 5, 2023
5a70705
Change vSetNotAllNetworksUp into a more generic vSetAllNetworksUp
bjsowa Oct 5, 2023
21ca710
Fix almost all of DHCP unit tests
bjsowa Oct 6, 2023
3f49d08
Fix formatting
bjsowa Oct 6, 2023
a645291
Add tests for vDHCPStop and vDHCPv6Stop functions
bjsowa Oct 6, 2023
04a6ad2
Fix formatting
bjsowa Oct 6, 2023
e9dddae
Merge branch 'main' into fix-network-down-up
tony-josi-aws Oct 9, 2023
f92294a
Remove redundant MISRA comment
bjsowa Oct 9, 2023
26ecb07
Set all fields of xAddress when binding DHCP socket
bjsowa Oct 9, 2023
cb86c13
Fix unit test coverage of prvCreateDHCPSocket
bjsowa Oct 9, 2023
65517de
Fix DHCP CBMC memory proof
tony-josi-aws Oct 10, 2023
919f450
Merge branch 'main' into fix-network-down-up
tony-josi-aws Oct 10, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 46 additions & 21 deletions source/FreeRTOS_DHCP.c
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,12 @@
/*
* Create the DHCP socket, if it has not been created already.
*/
_static void prvCreateDHCPSocket( const NetworkEndPoint_t * pxEndPoint );
_static void prvCreateDHCPSocket( NetworkEndPoint_t * pxEndPoint );

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

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

while( xDHCPv4Socket != NULL )
while( EP_DHCPData.xDHCPSocket != NULL )
{
BaseType_t xRecvFlags = FREERTOS_ZERO_COPY + FREERTOS_MSG_PEEK;
NetworkEndPoint_t * pxIterator = NULL;

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

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

if( ( lBytes > 0 ) && ( pucUDPPayload != NULL ) )
{
Expand All @@ -305,6 +305,20 @@
}
}

/**
* @brief Stop the DHCP process. Close the DHCP socket when it's no longer used by any end-point.
*
* @param[in] pxEndPoint The end-point for which we want to stop the DHCP process.
*/
void vDHCPStop( struct xNetworkEndPoint * pxEndPoint )
{
/* Disable the DHCP timer. */
vIPSetDHCP_RATimerEnableState( pxEndPoint, pdFALSE );

/* Close socket to ensure packets don't queue on it. */
prvCloseDHCPSocket( pxEndPoint );
}

/**
* @brief Called by vDHCPProcessEndPoint(), this function handles the state 'eWaitingOffer'.
* If there is a reply, it will be examined, if there is a time-out, there may be a new
Expand Down Expand Up @@ -564,7 +578,7 @@
#endif /* ipconfigUSE_DHCP_HOOK */
{
/* See if prvInitialiseDHCP() has creates a socket. */
if( xDHCPv4Socket == NULL )
if( EP_DHCPData.xDHCPSocket == NULL )
{
xGivingUp = pdTRUE;
}
Expand Down Expand Up @@ -618,7 +632,7 @@
/* Resend the request at the appropriate time to renew the lease. */
prvCreateDHCPSocket( pxEndPoint );

if( xDHCPv4Socket != NULL )
if( EP_DHCPData.xDHCPSocket != NULL )
{
uint32_t ulID = 0U;

Expand Down Expand Up @@ -829,11 +843,13 @@
* using it.
* @param[in] pxEndPoint The end-point that stops using the socket.
*/
static void prvCloseDHCPSocket( const NetworkEndPoint_t * pxEndPoint )
void prvCloseDHCPSocket( NetworkEndPoint_t * pxEndPoint )
{
( void ) pxEndPoint;

if( ( xDHCPv4Socket != NULL ) && ( xDHCPSocketUserCount > 0 ) )
if( ( EP_DHCPData.xDHCPSocket == NULL ) || ( EP_DHCPData.xDHCPSocket != xDHCPv4Socket ) )
{
/* the socket can not be closed. */
}
else if( xDHCPSocketUserCount > 0 )
{
xDHCPSocketUserCount--;

Expand All @@ -844,6 +860,8 @@
( void ) vSocketClose( xDHCPv4Socket );
xDHCPv4Socket = NULL;
}

EP_DHCPData.xDHCPSocket = NULL;
}
else
{
Expand All @@ -862,13 +880,17 @@
* @brief Create a DHCP socket with the defined timeouts. The same socket
* will be shared among all end-points that need DHCP.
*/
_static void prvCreateDHCPSocket( const NetworkEndPoint_t * pxEndPoint )
_static void prvCreateDHCPSocket( NetworkEndPoint_t * pxEndPoint )
{
struct freertos_sockaddr xAddress;
BaseType_t xReturn;
TickType_t xTimeoutTime = ( TickType_t ) 0;

if( xDHCPv4Socket == NULL ) /* Create the socket, if it has not already been created. */
if( ( xDHCPv4Socket != NULL ) && ( EP_DHCPData.xDHCPSocket == xDHCPv4Socket ) )
{
/* the socket is still valid. */
}
else if( xDHCPv4Socket == NULL ) /* Create the socket, if it has not already been created. */
{
xDHCPv4Socket = FreeRTOS_socket( FREERTOS_AF_INET, FREERTOS_SOCK_DGRAM, FREERTOS_IPPROTO_UDP );
configASSERT( xSocketValid( xDHCPv4Socket ) == pdTRUE );
Expand All @@ -887,6 +909,7 @@
xAddress.sin_port = ( uint16_t ) dhcpCLIENT_PORT_IPv4;
bjsowa marked this conversation as resolved.
Show resolved Hide resolved
xReturn = vSocketBind( xDHCPv4Socket, &xAddress, sizeof( xAddress ), pdFALSE );
xDHCPSocketUserCount = 1;
EP_DHCPData.xDHCPSocket = xDHCPv4Socket;
FreeRTOS_printf( ( "DHCP-socket[%02x-%02x]: DHCP Socket Create\n",
pxEndPoint->xMACAddress.ucBytes[ 4 ],
pxEndPoint->xMACAddress.ucBytes[ 5 ] ) );
Expand All @@ -901,11 +924,13 @@
{
/* Change to NULL for easier testing. */
xDHCPv4Socket = NULL;
EP_DHCPData.xDHCPSocket = NULL;
}
}
else
{
xDHCPSocketUserCount++;
EP_DHCPData.xDHCPSocket = xDHCPv4Socket;
}

FreeRTOS_printf( ( "prvCreateDHCPSocket[%02x-%02x]: %s, user count %d\n",
Expand Down Expand Up @@ -1248,7 +1273,7 @@
( void ) memset( &( xSet ), 0, sizeof( xSet ) );

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

if( lBytes > 0 )
{
Expand Down Expand Up @@ -1507,7 +1532,7 @@
/* MISRA Ref 11.4.1 [Socket error and integer to pointer conversion] */
bjsowa marked this conversation as resolved.
Show resolved Hide resolved
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-114 */
/* coverity[misra_c_2012_rule_11_4_violation] */
if( ( xSocketValid( xDHCPv4Socket ) == pdTRUE ) && ( pucUDPPayloadBuffer != NULL ) )
if( ( xSocketValid( EP_DHCPData.xDHCPSocket ) == pdTRUE ) && ( pucUDPPayloadBuffer != NULL ) )
{
/* Copy in the IP address being requested. */

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

xDHCPv4Socket->pxEndPoint = pxEndPoint;
EP_DHCPData.xDHCPSocket->pxEndPoint = pxEndPoint;

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

xDHCPv4Socket->pxEndPoint = pxEndPoint;
EP_DHCPData.xDHCPSocket->pxEndPoint = pxEndPoint;

if( FreeRTOS_sendto( xDHCPv4Socket,
if( FreeRTOS_sendto( EP_DHCPData.xDHCPSocket,
pucUDPPayloadBuffer,
sizeof( DHCPMessage_IPv4_t ) + uxOptionsLength,
FREERTOS_ZERO_COPY,
Expand Down
14 changes: 14 additions & 0 deletions source/FreeRTOS_DHCPv6.c
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,20 @@ void vDHCPv6Process( BaseType_t xReset,
vDHCPv6ProcessEndPoint( xReset, pxEndPoint, pxEndPoint->pxDHCPMessage );
}
}

/**
* @brief Stop the DHCP process. Close the DHCP socket when it's no longer used by any end-point.
*
* @param[in] pxEndPoint The end-point for which we want to stop the DHCP process.
*/
void vDHCPv6Stop( struct xNetworkEndPoint * pxEndPoint )
{
/* Disable the DHCP timer. */
vIPSetDHCP_RATimerEnableState( pxEndPoint, pdFALSE );

/* Close socket to ensure packets don't queue on it. */
prvCloseDHCPv6Socket( pxEndPoint );
}
/*-----------------------------------------------------------*/

/**
Expand Down
14 changes: 12 additions & 2 deletions source/FreeRTOS_IP_Timers.c
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@
#include "FreeRTOS_DNS.h"
/*-----------------------------------------------------------*/

/** @brief 'xAllNetworksUp' becomes pdTRUE as soon as all network interfaces have
* been initialised. */
/** @brief 'xAllNetworksUp' becomes pdTRUE when all network interfaces are initialised
* and becomes pdFALSE when any network interface goes down. */
/* MISRA Ref 8.9.1 [File scoped variables] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-89 */
/* coverity[misra_c_2012_rule_8_9_violation] */
Expand Down Expand Up @@ -604,3 +604,13 @@ void vIPSetARPResolutionTimerEnableState( BaseType_t xEnableState )

#endif /* ipconfigDNS_USE_CALLBACKS == 1 */
/*-----------------------------------------------------------*/

/**
* @brief Mark that at least one interface is down so that the 'xNetworkTimer' is checked.
* Whenever the timer expires, all interfaces that are down will get a new NetworkDown
* event.
*/
void vSetNotAllNetworksUp( void )
bjsowa marked this conversation as resolved.
Show resolved Hide resolved
{
xAllNetworksUp = pdFALSE;
}
29 changes: 28 additions & 1 deletion source/FreeRTOS_IP_Utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -861,6 +861,31 @@ void prvProcessNetworkDownEvent( struct xNetworkInterface * pxInterface )
* treat network down as a "delivery problem" and flush the ARP cache for this
* interface. */
FreeRTOS_ClearARP( pxEndPoint );

#if ( ipconfigUSE_DHCP == 1 )
if( END_POINT_USES_DHCP( pxEndPoint ) )
{
#if ( ( ipconfigUSE_DHCPv6 != 0 ) && ( ipconfigUSE_IPv6 != 0 ) )
if( pxEndPoint->bits.bIPv6 != pdFALSE_UNSIGNED )
{
vDHCPv6Stop( pxEndPoint );
}
else
#endif /* (( ipconfigUSE_DHCPv6 != 0 ) && ( ipconfigUSE_IPv6 != 0 )) */
{
/* Stop the DHCP process for this end-point. */
vDHCPStop( pxEndPoint );
}
}
#endif /* ( ipconfigUSE_DHCP == 1 ) */

#if ( ( ipconfigUSE_RA != 0 ) && ( ipconfigUSE_IPv6 != 0 ) )
if( END_POINT_USES_RA( pxEndPoint ) )
{
/* Stop the RA/SLAAC process for this end-point. */
vIPSetDHCP_RATimerEnableState( pxEndPoint, pdFALSE );
}
#endif /* ( (ipconfigUSE_RA != 0) && ( ipconfigUSE_IPv6 != 0 )) */
}

/* The network has been disconnected (or is being initialised for the first
Expand Down Expand Up @@ -936,7 +961,9 @@ void prvProcessNetworkDownEvent( struct xNetworkInterface * pxInterface )
}
else
{
/* Nothing to do. When the 'xNetworkTimer' expires, all interfaces
vSetNotAllNetworksUp();

/* Nothing else to do. When the 'xNetworkTimer' expires, all interfaces
* with bits.bInterfaceUp cleared will get a new 'eNetworkDownEvent' */
}
}
Expand Down
6 changes: 6 additions & 0 deletions source/include/FreeRTOS_DHCP.h
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,12 @@ eDHCPState_t eGetDHCPState( const struct xNetworkEndPoint * pxEndPoint );
void vDHCPProcess( BaseType_t xReset,
struct xNetworkEndPoint * pxEndPoint );

/*
* NOT A PUBLIC API FUNCTION.
* It will be called when the network interface, that the endpoint is associated with, goes down.
*/
void vDHCPStop( struct xNetworkEndPoint * pxEndPoint );

/* Internal call: returns true if socket is the current DHCP socket */
BaseType_t xIsDHCPSocket( const ConstSocket_t xSocket );

Expand Down
6 changes: 6 additions & 0 deletions source/include/FreeRTOS_DHCPv6.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,12 @@
void vDHCPv6Process( BaseType_t xReset,
struct xNetworkEndPoint * pxEndPoint );

/*
* NOT A PUBLIC API FUNCTION.
* It will be called when the network interface, that the endpoint is associated with, goes down.
*/
void vDHCPv6Stop( struct xNetworkEndPoint * pxEndPoint );

#ifdef __cplusplus
} /* extern "C" */
#endif
Expand Down
3 changes: 3 additions & 0 deletions source/include/FreeRTOS_IP_Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -882,6 +882,9 @@ BaseType_t xIsCallingFromIPTask( void );
/* Send the network-up event and start the ARP timer. */
void vIPNetworkUpCalls( struct xNetworkEndPoint * pxEndPoint );

/* Mark that at least one interface is down to trigger repeated NetworkDown events. */
void vSetNotAllNetworksUp( void );

/* *INDENT-OFF* */
#ifdef __cplusplus
} /* extern "C" */
Expand Down
12 changes: 12 additions & 0 deletions test/unit-test/FreeRTOS_IP_Utils/FreeRTOS_IP_Utils_utest.c
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,8 @@ void test_prvProcessNetworkDownEvent_Pass( void )

FreeRTOS_ClearARP_ExpectAnyArgs();

vDHCPStop_Expect( &xEndPoint );

vDHCPProcess_Expect( pdTRUE, &xEndPoint );

prvProcessNetworkDownEvent( &xInterfaces[ 0 ] );
Expand All @@ -497,6 +499,8 @@ void test_prvProcessNetworkDownEvent_Pass( void )

FreeRTOS_ClearARP_Expect( &xEndPoint );

vDHCPStop_Expect( &xEndPoint );

vDHCPProcess_Expect( pdTRUE, &xEndPoint );

prvProcessNetworkDownEvent( &xInterfaces[ 0 ] );
Expand Down Expand Up @@ -575,6 +579,10 @@ void test_prvProcessNetworkDownEvent_InterfaceInitFail( void )

FreeRTOS_ClearARP_ExpectAnyArgs();

vDHCPStop_Expect( &xEndPoint );

vSetNotAllNetworksUp_Expect();

prvProcessNetworkDownEvent( &xInterface );
}

Expand All @@ -601,6 +609,8 @@ void test_prvProcessNetworkDownEvent_PassDHCPv6( void )

FreeRTOS_ClearARP_ExpectAnyArgs();

vDHCPv6Stop_Expect( &xEndPoint );

vDHCPv6Process_Expect( pdTRUE, &xEndPoint );

prvProcessNetworkDownEvent( &xInterface );
Expand Down Expand Up @@ -629,6 +639,8 @@ void test_prvProcessNetworkDownEvent_PassRA( void )

FreeRTOS_ClearARP_ExpectAnyArgs();

vIPSetDHCP_RATimerEnableState_Expect( &xEndPoint, pdFALSE );

vRAProcess_Expect( pdTRUE, &xEndPoint );

prvProcessNetworkDownEvent( &xInterface );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ void test_prvProcessNetworkDownEvent_Pass_DHCP_Enabled( void )

FreeRTOS_ClearARP_Expect( &xEndPoint );

vDHCPStop_Expect( &xEndPoint );

FreeRTOS_NextEndPoint_ExpectAndReturn( &xInterface, &xEndPoint, NULL );

xInterface.pfInitialise = xNetworkInterfaceInitialise_test;
Expand Down
Loading