[lwip-devel] IGMP - Adding Random Delay

[Edward Kerekes]

Hi again,

I thought this discussion would be more appropriate in a separate thread. 

The following discussion pertains to the current HEAD (also V1.3.0RC1) version

of igmp.c.

 

I’d like to add a macro “hook” into the igmp.c module to allow

the user to specify a random number function for setting the delay timer.

I’m using a macro in the same spirit as the existing lwIP implementation of 

MEMCPY() which could have a simple default and could be overridden by the user

in his lwipopts.h file.

 

As I read RFC 2236 the delaying member should use a random number between 0 and

the “Max Response Time”.  The existing igmp_delaying_member() function

simply starts the timer with (maxresp)/2, rather than a random value. (This works fine but

doesn’t match the RFC.   I’m planning on using a large number of “members” and would

prefer the “random delay”.)

 

1.         The enhancement:

I’m basically replacing the (maxresp)/2 with a macro that will default

to the existing (maxresp)/2 that can be overridden by the user to provide a random delay. 

 

I’m also adding an “if” statement to start the timer with 1 if the macro returns 0.

This trick avoids having to send the message directly by calling igmp_send() – That would

be fine also, however I’m not sure of the consequence of calling igmp_send() directly in a

preemptive environment.  This trick might also result in a few less bytes of code.

 

The big question is: Should any other calls use a random delay?  I don’t see that

indicated in the RFC, but I’m certainly not an expert on IGMP…

 

2.         Bug Fix:

On line 669, the term

 

            “&& (maxresp > group->timer)”

 

seems wrong based on my interpretation of RFC 2236.  I believe it should be replaced

with:

 

       && ((group->timer == 0) || (maxresp < group->timer))

 

For reference, I’m basing this on RFC 2236 Section 3 Protocol Description.

The 4^th paragraph contains this sentence:

 

If a timer for the group is already running, 

it is reset to the random value only if the 

requested Max Response Time is less than the 

remaining value of the running timer.

 

I believe the intent of the RFC is to let any existing running timers

CONTINUE TO RUN if they are less than the maxresp and only start

stopped timers or timers with values that exceed the maxresp time.

 

I’m hoping to get this or some similar implementation incorporated into the lwIP source.

So if anyone has any comments or suggestions or has a preference in how this is

implemented, please let me know,

 

See the attached patch file for reference. 

For convenience, I have also provided the original and modified routines below.

 

Thanks again to all contributors!

Ed

 

 

 

Original function for reference:

 

void

igmp_delaying_member( struct igmp_group *group, u8_t maxresp)

{

  if (   (group->group_state == IGMP_GROUP_IDLE_MEMBER)

      || ((group->group_state == IGMP_GROUP_DELAYING_MEMBER)

      && (maxresp > group->timer))                                          <<<< wrong ????

     ) {

    igmp_start_timer(group, (maxresp)/2);

    group->group_state = IGMP_GROUP_DELAYING_MEMBER;

  }

}

 

New proposed macro:

 

#ifndef LWIP_IGMP_RANDOM

/* The end user can override this macro to provide his own random number routine

   For example, to use the standard c rand() function, include the following

   in your lwipopts.h file:

      #define IGMP_RANDOM(max) ((u8_t) ((long) max * rand()/RAND_MAX))

   The user routine should return a random u8_t ranging from 0 to max.  

   Default to the original source, which simply used max/2: 

*/

#define IGMP_RANDOM(max) ((u8_t) ((max)/2))

#endif

 

New proposed function:

void

igmp_delaying_member( struct igmp_group *group, u8_t maxresp)

{

  u8_t time;

  if (   (group->group_state == IGMP_GROUP_IDLE_MEMBER)

      || ((group->group_state == IGMP_GROUP_DELAYING_MEMBER) && ((group->timer==0) || (maxresp > group->timer)))

     ) {

    time = LWIP_IGMP_RANDOM(maxresp);  /* Generate a random u8_t integer 0 to maxresp */

    if (time==0) {                       /* For now change 0 to 1 to force a send the next timer tick. */

      time = 1;                            /* In the future, a developer can change this to send message  */

    }                                          /* right now if time == 0 without macro breakage. */                              

    igmp_start_timer(group, time);

    group->group_state = IGMP_GROUP_DELAYING_MEMBER;

  }

} 

 

 

igmp2.txt
Description: Text document