buglwIP - A Lightweight TCP/IP stack - Bugs: bug #39145, IGMP membership report for...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #39145: IGMP membership report for 224.0.0.1

Submitted by:  Michel van de Wetering <mvdwetering>
Submitted on:  Mon 03 Jun 2013 12:29:36 PM UTC  
 
Category: IPv4Severity: 3 - Normal
Item Group: Faulty BehaviourStatus: Fixed
Privacy: PublicAssigned to: Dirk Ziegelmeier <dziegel>
Open/Closed: ClosedPlanned Release: 2.0.1
lwIP version: 2.0.0

Sat 17 Dec 2016 09:10:53 PM UTC, comment #5:

Thanks for being my backup brain!

Dirk Ziegelmeier <dziegel>
Project MemberIn charge of this item.
Sat 17 Dec 2016 02:43:37 PM UTC, comment #4:

igmp: Fix optimized code for igmp_remove_group

The code in the for loop checks tmp_group->next == group, so current code
actually checks from the 3rd entry in the linked groups list. Fix it.

Fixes: 5c1dd6a4c65b ("Optimization in igmp_remove_group() pointed out by Axel Lin")

(file #39259)

Axel Lin <axellin>
Sat 17 Dec 2016 01:52:52 PM UTC, comment #3:

Hi Dirk,
I see your commit for this fix.
However, I'm wondering if it's possible the allsystems group is not the first entry:

static err_t
igmp_remove_group(struct netif* netif, struct igmp_group *group)
{
err_t err = ERR_OK;

/* Is it the first group? */
if (netif_igmp_data(netif) == group) {
netif_set_client_data(netif, LWIP_NETIF_CLIENT_DATA_INDEX_IGMP, group->next);
}

Axel Lin <axellin>
Sat 17 Dec 2016 10:14:10 AM UTC, comment #2:

Hi,

I use lwip 1.4.1. I move the fix of this problem from lwip 2.0.0, it is in igmp_report_groups function:

/* Skip the first group in the list, it is always the allsystems group added in igmp_start() */
if(group != NULL) {
group = group->next;
}

I join to six multicast groups in my app. The last joined group is assigned to igmp_group_list. When netif is down and up it reports a the allsystems group (because it is last no first in list) + my five multicast IPs. The sixth IP address is not reported.

When I take a look on lwip 2.0.0 source code, there shoud be the same problem.

Michaels fix is working and finaly i have to use

while (group != NULL) {
if ((group->netif == netif) && (group->next != NULL)) {
igmp_delaying_member(group, IGMP_JOIN_DELAYING_MEMBER_TMR);
}
group = group->next;
}

Regards

Tomas Poruba

Tomáš Poruba <porubat>
Tue 25 Feb 2014 09:21:59 PM UTC, comment #1:

Right. Thanks for the report. Fixed.

Simon Goldschmidt <goldsimon>
Project Administrator
Mon 03 Jun 2013 12:29:36 PM UTC, original submission:

We got reports from users of our product that there were issues with IGMP membership reports that we are sending. The only group we join is "239.255.255.250" for UPnP broadcasts, but additional membership reports are generated for 224.0.0.1 (the all systems group). The problematic membership reports are the ones to 224.0.0.1. These membership reports are automatically generated by lwIP.

This is a problem because the router that our system is connected to has a bug with the handling of the membership report for 224.0.0.1 which effectively breaks the all systems group for the router.

According to the RFC2236 (IGMPv2) sending the membership report for the allsystems group is invalid.

>The all-systems group (address 224.0.0.1) is handled as a
>special case. The host starts in Idle Member state for that
>group on every interface, never transitions to another state,
>and never sends a report for that group.


The reason why the reports are sent is because in the igmp_report_groups function all groups are added. The grouplist also contains the allsystems group. So this causes the membership reports to be sent.

We are using lwIP 1.4.0, but I checked the code and there are no changes in that part of the code so it will also be in 1.4.1.

I made the following change to avoid the membership reports for the allsystems group (also in attached patch). In igmp.c / igmp_report_groups() replace:
if (group->netif == netif) {
by:
if ((group->netif == netif) && (!(ip_addr_cmp(&(group->group_address), &allsystems)))) {

I am not sure if this completely covers the statement from the RFC2236 that it never should transition to another state. But at least the membership reports for 224.0.0.1 are not sent anymore and it seems to have no negative impact.

Michel van de Wetering <mvdwetering>

 

Attached Files
file #28232:  igmp_allsystems_report.patch added by mvdwetering (742B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by axellin (Posted a comment)
  • -unavailable- added by dziegel (Updated the item)
  • -unavailable- added by porubat (Posted a comment)
  • -unavailable- added by goldsimon (Updated the item)
  • -unavailable- added by mvdwetering (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 12 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat 17 Dec 2016 02:43:37 PM UTCaxellinAttached File-=>Added 0001-igmp-Fix-optimized-code-for-igmp_remove_group.patch, #39259
    Sat 17 Dec 2016 12:37:42 PM UTCdziegelStatusConfirmed=>Fixed
      Open/ClosedOpen=>Closed
    Sat 17 Dec 2016 12:35:09 PM UTCdziegelStatusFixed=>Confirmed
      Assigned togoldsimon=>dziegel
      Open/ClosedClosed=>Open
      Planned ReleaseNone=>2.0.1
    Tue 25 Feb 2014 09:21:59 PM UTCgoldsimonStatusNone=>Fixed
      Assigned toNone=>goldsimon
      Open/ClosedOpen=>Closed
    Fri 28 Jun 2013 08:06:03 PM UTCgoldsimonPlanned Release=>1.5.0
    Mon 03 Jun 2013 12:29:36 PM UTCmvdweteringAttached File-=>Added igmp_allsystems_report.patch, #28232

    Back to the top


    Powered by Savane 3.1-cleanup1