Hi,
I am using FreeRTOS + tcp v4.0.0 for ethernet application where I use dynamic ip address. I find that when it’s lease timeout has expired and it has sent a DHCP Request and waiting for an ACK, if any other server sends NAK to it, it does not reject it. I mean if you already have a leased address and you want to renew the lease, then your communication should be with the specific dhcp server, then why accept any message from other server.
Thanks for reporting this issue with multiple DHCP servers. Looking at the code, it seems to be the case. Can you try updating V4.0.0 with the following rough patch file to see if it helps with the issue?
multi_nack.patch (3 KB)
Diff:
--- a/source/FreeRTOS_DHCP.c
+++ b/source/FreeRTOS_DHCP.c
@@ -975,20 +975,7 @@
* state machine is expecting. */
pxSet->ulProcessed++;
}
- else
- {
- if( pxSet->pucByte[ pxSet->uxIndex ] == ( uint8_t ) dhcpMESSAGE_TYPE_NACK )
- {
- if( xExpectedMessageType == ( BaseType_t ) dhcpMESSAGE_TYPE_ACK )
- {
- /* Start again. */
- EP_DHCPData.eDHCPState = eInitialWait;
- }
- }
-
- /* Stop processing further options. */
- pxSet->uxLength = 0;
- }
+ pxSet->ucMessageOptionCode = pxSet->pucByte[ pxSet->uxIndex ];
break;
@@ -1077,6 +1064,7 @@
if( EP_DHCPData.ulDHCPServerAddress == pxSet->ulParameter )
{
pxSet->ulProcessed++;
+ pxSet->ucMatchingServer = pdTRUE;
}
}
}
@@ -1328,6 +1316,23 @@
pxEndPoint->xMACAddress.ucBytes[ 5 ] ) );
xReturn = pdPASS;
}
+
+ if(xExpectedMessageType == (BaseType_t) dhcpMESSAGE_TYPE_ACK )
+ {
+ if(xSet.ucMessageOptionCode == (BaseType_t) dhcpMESSAGE_TYPE_NACK )
+ {
+ if((xSet.ucMatchingServer != pdPASS))
+ {
+ FreeRTOS_printf( ( "vDHCPProcess: Received dhcpMESSAGE_TYPE_NACK from different server. Ignore ") );
+ xReturn = pdFALSE;
+ }
+ else
+ {
+ /* Start again. */
+ EP_DHCPData.eDHCPState = eInitialWait;
+ }
+ }
+ }
}
}
diff --git a/source/include/FreeRTOS_DHCP.h b/source/include/FreeRTOS_DHCP.h
index 26fef7de..1b14f148 100644
--- a/source/include/FreeRTOS_DHCP.h
+++ b/source/include/FreeRTOS_DHCP.h
@@ -229,6 +229,8 @@ typedef struct xProcessSet
uint32_t ulParameter; /**< The uint32 value of the answer, if available. */
uint32_t ulProcessed; /**< The number of essential options that were parsed. */
const uint8_t * pucByte; /**< A pointer to the data to be analysed. */
+ uint8_t ucMessageOptionCode;
+ uint8_t ucMatchingServer;
} ProcessSet_t;
/* Returns the current state of a DHCP process. */
If the patch helps, I will create a PR that is more refined and aligned with DHCP RFC and share it here.
1 Like
@tony-josi-aws
I would like to share a improvement in the code from my side. And it works. Please see if it can be added. The change is in vProcessHandleOption() function
/* some code */
case dhcpIPv4_MESSAGE_TYPE_OPTION_CODE:
if( pxSet->pucByte[ pxSet->uxIndex ] == ( uint8_t ) xExpectedMessageType )
{
/* The message type is the message type the
* state machine is expecting. */
pxSet->ulProcessed++;
}
else
{
if( pxSet->pucByte[ pxSet->uxIndex ] == ( uint8_t ) dhcpMESSAGE_TYPE_NACK )
{
if( xExpectedMessageType == ( BaseType_t ) dhcpMESSAGE_TYPE_ACK )
{
// Raghav 5mar25 - First store the present state to previous state
// ePreviousState = EP_DHCPData.eDHCPState;
/* Start again. */
EP_DHCPData.eDHCPState = eInitialWait;
APP_USR_PRINT("\n****** Received NAK - set eInitialWait *******\n");
}
}
/* Stop processing further options. */
// pxSet->uxLength = 0; /* Removed so that it processes further options */
}
break;
/* Some Code */
case dhcpIPv4_SERVER_IP_ADDRESS_OPTION_CODE:
if( pxSet->uxLength == sizeof( uint32_t ) )
{
if( xExpectedMessageType == ( BaseType_t ) dhcpMESSAGE_TYPE_OFFER )
{
/* Offers state the replying server. */
pxSet->ulProcessed++;
EP_DHCPData.ulDHCPServerAddress = pxSet->ulParameter;
}
else
{
/* The ack must come from the expected server. */
if( EP_DHCPData.ulDHCPServerAddress == pxSet->ulParameter )
{
pxSet->ulProcessed++;
}
/* Addition from my side */
else // added in case unexpected server responded
{ // Raghav 5mar25 - when Server sending NAK is unexpected, change back to previous state and stop processing further
EP_DHCPData.eDHCPState = eWaitingAcknowledge;
pxSet->uxLength = 0;
}
}
}
break;
/* some code */
default:
/* Not interested in this field. */
break;
}
}
Thanks for sharing your changes.
The code improvement seems to be addressing your specific issue, but I have a comment on the following line:
EP_DHCPData.eDHCPState = eWaitingAcknowledge;
Unless ulProcessed is not incremented to ulMandatoryOptions which is the case in your improvement, the eDHCPState should be eWaitingAcknowledge already, so I’m not sure why its re assigned.
Actually I haven’t added your patch into my code and secondly the code will reject any other packet also from any other server.
That’s because when NACK is received, eDHCPState is changed to eInitial.
So, if server does not match, it needs to be changed to previous state and further processing of options is stopped by setting length to 0.
I was talking about your code improvements.
That’s because when NACK is received, eDHCPState is changed to eInitial.
The problem is that RFC doesn’t mandate a requirement that other options be listed in any particular order. Based on how the DHCP server is implemented, there is a chance that you might get the NACK option after the server identifier. Which will cause the state set back to eInitialWait.
Oh, I didn’t take into account this scenario.
Then, I thinks it’s better to use the patch. I have tried it and it works normally.
But one thing I want to add here.
We need to reset the ucMatchingServer after or before handling all the options.
So, the line pxSet->ucMatchingServer = pdFALSE can be added before the while loop.
Thanks for checking the patch.
That’s taken care inside the prvProcessDHCPReplies by the memset here:
( void ) memset( &( xSet ), 0, sizeof( xSet ) );