Date:	Tue, 17 May 2011 15:30:07 +0200
From:	Veaceslav Falico <vfalico@...hat.com>
To:	David Stevens <dlstevens@...ibm.com>
Cc:	David Miller <davem@...emloft.net>, jmorris@...ei.org,
	kaber@...sh.net, kuznet@....inr.ac.ru,
	linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
	mmarek@...e.cz, netdev@...r.kernel.org, pekkas@...core.fi,
	yoshfuji@...ux-ipv6.org
Subject: Re: [PATCH 1/1] igmp: fix ip_mc_clear_src to not reset
 ip_mc_list->sf{mode,count}

On Mon, May 16, 2011 at 01:42:11PM -0700, David Stevens wrote:
> 
>         On NETDEV_DOWN, all group memberships are dropped. 
> ip_mc_clear_src()
> is simply freeing all the source filters and turning it into an "EXCLUDE 
> nobody"
> membership (ie, the same as an ordinary join without source filtering). 
> This
> ordinarily happens when you are deleting the group entirely (when the 
> reference
> count goes to 0), but is also called on device down.
>         This patch is not appropriate; when the groups are deleted, the 
> source
> filters are deleted, and the filter counts have to reflect the source 
> filters
> in the list. If you had an "INCLUDE A" filter, for example, that would 
> become
> an "INCLUDE nobody" filter and drop all traffic (from A or not). The 
> number
> of source filters is not related to the number of listener sockets, and 
> the
> function of ip_mc_clear_src() is to make it 0 (with the special case of 1 
> for
> EXCLUDE), so setting the counts has to be done for proper functioning.
>         I don't quite understand the problem you're trying to solve here 
> --
> when the device comes back up, the group should be re-added with 
> {EXCLUDE,nobody} and
> ip_check_mc() should therefore return 1. Of course, while the interface is
> down, the mc_list is empty and it'd return 0 in that case.
>         Do you have a small test program to demonstrate the problem?

Yes, attached are two programs, one sender and one receiver, they bind both
to localhost and send each other traffic. To reproduce, start the sender
and two instances of receivers, then do an ifconfig lo up; ifconfig lo
down;, restart the sender program (both of the receivers should once again
receive the multicast traffic). Then kill one receiver (the MCAST_EXCLUDE
will become 0), and do an "ip route flush cache". The new route cache will
be without the local flag on, and the remaining receiver will stop
receiving traffic.

What happens:

1) When both receivers start, ip_mc_list->sfcount[MCAST_EXCLUDE] == 2
2) On NETDEV_DOWN event, groups are dropped and sfmode = MCAST_EXCLUDE,
	sfcount[MCAST_EXCLUDE] = 1
3) On NETDEV_UP, the group is re-joined, but kernel thinks that there's
	only one listener (sfcount[MCAST_EXCLUDE]).
4) On socket destroy (when one receiver is terminated), the count is 0.
5) On route cache flush, __mkroute_output() doesn't see the remaining
	listener, and creates a route cache without RTCF_LOCAL flag, thus not
	allowing any traffic on that group to local listeners.

The igmp_group_dropped() (the actual routine that drops a group) is called
when:

1) ip_mc_dec_group() is called and im->users == 0
2) ip_mc_unmap()
3) ip_mc_down()
4) ip_mc_destroy_dev()

The 1) we call either on socket destroy or when the socket actually asks to
leave a group. In this case, we need to "reset" the state on no listeners.

2),3),4) are called on various device modifications
(NETDEV_PRE_TYPE_CHANGE, NETDEV_DOWN and NETDEV_UNREGISTER) - but the group
can be rejoined on their next events - NETDEV_POST_TYPE_CHANGE, NETDEV_UP
and NETDEV_REGISTER, which will cause the ip_mc_list to loose track of
existing listeners.

So, I tend to think that we must clear the sources only on 1).

Will send the patch shortly.

Thank you!

View attachment "mcsend.c" of type "text/plain" (3595 bytes)

View attachment "mcreceive.c" of type "text/plain" (3822 bytes)

Powered by blists - more mailing lists