+TCP DHCP xGivingUp and timeouts

Libraries
1

Is there a good way to have a seamless retry on DHCP? The stack calls vIPNetworkUpCalls and sets a default address, then immediately resets the IP to 0 on an eAnswer of eDHCPContinue. This is a bit hard for downstream network threads to detect that it is still trying DHCP. Edit: I was restarting this in my application, so wasn’t really correct.

Somewhat related, but with

#define dhcpINITIAL_TIMER_PERIOD (pdMS_TO_TICKS(250))
#define dhcpINITIAL_DHCP_TX_PERIOD (pdMS_TO_TICKS(500))
#define ipconfigMAXIMUM_DISCOVER_TX_PERIOD		( pdMS_TO_TICKS( 120000 ) )

the output ends up like this:

prvInitialiseDHCP: start after 250 ticks
vDHCPProcess: discover
vDHCPProcess: discover
vDHCPProcess: timeout 1000 ticks
vDHCPProcess: discover
vDHCPProcess: timeout 2000 ticks
vDHCPProcess: discover
vDHCPProcess: timeout 4000 ticks
vDHCPProcess: discover
vDHCPProcess: timeout 8000 ticks
vDHCPProcess: discover
vDHCPProcess: timeout 16000 ticks
vDHCPProcess: discover
vDHCPProcess: timeout 32000 ticks
vDHCPProcess: discover
vDHCPProcess: timeout 64000 ticks
vDHCPProcess: giving up 128000 > 120000 ticks

However, the network was made live after 64000 ticks, but in reality in never actually tried again after 64000. Is there some workarounds?

2

@htibosch

Any opinion on something like this?

In FreeRTOSIPConfig.h

#define ipconfigDHCP_EARLY_TIMEOUT            1

#define ipconfigMAXIMUM_DISCOVER_TX_PERIOD              ( pdMS_TO_TICKS( 8000 ) )
#define ipconfigMAXIMUM_DISCOVER_RX_PERIOD              ( pdMS_TO_TICKS( 1000 ) )

In FreeRTOS_DHCP.c vDHCPProcess()

#if ( ipconfigDHCP_EARLY_TIMEOUT != 0 )
else if( ( xTaskGetTickCount() - EP_DHCPData.xDHCPTxTime ) > ipconfigMAXIMUM_DISCOVER_RX_PERIOD && 
         EP_DHCPData.xDHCPTxPeriod * 2 > ipconfigMAXIMUM_DISCOVER_TX_PERIOD)
{
    FreeRTOS_debug_printf( ( "vDHCPProcess: Last try timeout %lu ticks\n", ipconfigMAXIMUM_DISCOVER_RX_PERIOD ) );
    xGivingUp = pdTRUE;
}
#endif
/* Is it time to send another Discover? */
else if( ( xTaskGetTickCount() - EP_DHCPData.xDHCPTxTime ) > EP_DHCPData.xDHCPTxPeriod )
...
3

Hello @friesen,

What if we do this:

@@ -444,11 +444,6 @@
                     /* Is it time to send another Discover? */
                     else if( ( xTaskGetTickCount() - EP_DHCPData.xDHCPTxTime ) > EP_DHCPData.xDHCPTxPeriod )
                     {
-                        /* It is time to send another Discover.  Increase the time
-                         * period, and if it has not got to the point of giving up - send
-                         * another discovery. */
-                        EP_DHCPData.xDHCPTxPeriod <<= 1;
-
                         if( EP_DHCPData.xDHCPTxPeriod <= ( TickType_t ) ipconfigMAXIMUM_DISCOVER_TX_PERIOD )
                         {
                             if( xApplicationGetRandomNumber( &( EP_DHCPData.ulTransactionId ) ) != pdFALSE )
@@ -503,6 +498,11 @@
                                 }
                             #endif /* ipconfigDHCP_FALL_BACK_AUTO_IP */
                         }
+
+                        /* It is time to send another Discover.  Increase the time
+                                                * period, and if it has not got to the point of giving up - send
+                                                * another discovery. */
+                                               EP_DHCPData.xDHCPTxPeriod <<= 1;
                     }
                     else
                     {

Basically move the update to the variable after the test has been done?

Let me know if that works for you.

Thanks

P.S., this is a git diff, please make sure that you put the changes in correct location :slightly_smiling_face:

4

Isn’t that still going to delay the complete discover period? The problem gets more pronounced the longer it delays, so that the last delay is always pointless after a 1 second or so.

This is my present code I’m testing with.

if( EP_DHCPData.xDHCPTxPeriod <= ( TickType_t ) ipconfigMAXIMUM_DISCOVER_TX_PERIOD )
{
    if( xApplicationGetRandomNumber( &( EP_DHCPData.ulTransactionId ) ) != pdFALSE )
    {
        EP_DHCPData.xDHCPTxTime = xTaskGetTickCount();

+        #if ( ipconfigDHCP_EARLY_TIMEOUT != 0 )
+        if( EP_DHCPData.xDHCPTxPeriod * 2 > ipconfigMAXIMUM_DISCOVER_TX_PERIOD)
+        {
+          EP_DHCPData.xDHCPTxTime -= (ipconfigMAXIMUM_DISCOVER_TX_PERIOD - ipconfigMAXIMUM_DISCOVER_RX_PERIOD);
+          FreeRTOS_debug_printf( ( "vDHCPProcess: Last short try\n" ) );
+        }
+        #endif

Some things I wonder, perhaps dhcp should be more flexible. Right now its a bit hard to make it act like windows for example, where it autoconf’s, then continues to try dhcp periodically, yet without setting the IP to 0.

Things I have considered:

#1 Allow a total ticks timeout instead of timeout when xDHCPTxPeriod>=ipconfigMAXIMUM_DISCOVER_TX_PERIOD

#2 Allow user defined period shifting instead of EP_DHCPData.xDHCPTxPeriod <<= 1

#3 Use eAnswer from last eDHCPPhaseFailed hook call to restart dhcp without starting the network and setting default addresses.

5

Hello @friesen,

Apologies, I did not understand you problem statement correctly. Are you trying to get around the problem of longer delays? Or are you saying that the last delay should be cut short if the DHCP server has not responded within a given timeframe?

Would you mind clarifying your issue more?

As for your suggestions, they all sound wonderful. If you have the changes, would you like to create a PR for that?

Thanks,

6

On a network power cycle (which would include the freertos device)it is very likely that the router won’t be dhcp ready for at least 1 minute.

If I set the ipconfigMAXIMUM_DISCOVER_TX_PERIOD to 120000 ticks, here is the time sequence.

xDHCPTxPeriod Total ticks
1000 1000
2000 3000
4000 7000
8000 15000
16000 31000
32000 63000
64000 127000
128000 255000

So in this case, the last real try happened at 127000 ticks, but then the dhcp handler goes into lala land for another wasted 128k ticks, when I’d rather it time out and fail.

7

Apologies for the late reply. I was pulled into some other work.

I see your point. Yes, we should correct that. I will try to come up with a fix.