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.
Id
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.
Im
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
doesnt
match the RFC. Im planning on using a large number
of members and would
prefer
the random delay.)
1.
The enhancement:
Im
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.
Im
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 Im 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 dont see
that
indicated
in the RFC, but Im 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, Im basing this on RFC 2236 Section 3 Protocol
Description.
The
4th 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.
Im
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;
}
}