netdev - Re: [Bonding-devel] [PATCH 3/3] bonding: Improve IGMP join processing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <OF5C8C5CA8.4BC1B722-ON88257297.00029119-88257297.00042D61@us.ibm.com>
Date:	Tue, 6 Mar 2007 16:45:45 -0800
From:	David Stevens <dlstevens@...ibm.com>
To:	fubar@...ux.vnet.ibm.com
Cc:	"Andy Gospodarek" <andy@...yhouse.net>,
	bonding-devel@...ts.sourceforge.net,
	Brian Haley <brian.haley@...com>, netdev@...r.kernel.org,
	netdev-owner@...r.kernel.org
Subject: Re: [Bonding-devel] [PATCH 3/3] bonding: Improve IGMP join processing

fubar@...ux.vnet.ibm.com wrote on 03/06/2007 03:15:41 PM:

> 
> David Stevens <dlstevens@...ibm.com> wrote:
> 
> >It looks to me like "rejoin" is essentially ip_mc_up(), and it'd be 
better
> >to call that than add a nearly identical function.
> 
>    Won't ip_mc_up() acquire an additional reference (via
> ip_mc_inc_group) to the IGMP_ALL_HOSTS im->users that would never be
> released (in the case of bonding calling the function out of the blue)?

        Yes. I'm not sure that matters-- destroy_dev doesn't care how many
references to a group, and IGMP_ALL_HOSTS isn't advertised (so wouldn't
get a "leave" when you only down the interface, like other groups do).
But since ip_mc_up() is *entirely* that join plus group_added() on all
the existing groups, there really shouldn't be another. But the new device
will need the all-hosts group in its hardware multicast filter, too, if
it hasn't already been using multicasting. Your "reload" caller could just
dec_group that group after calling ip_mc_up().

>    In looking at it, the ip_mc_rejoin_group function (the new one
> added with the patch) is a lot more like igmp_group_added() than
> ip_mc_up().
No, group_added() is one group. mc_up() just calls group_added on all
of them, which I think is what the rejoin was trying to do.

>I'm not sure if the extra bits in igmp_group_added() are
> worthy of concern; I'm thinking not, since im->loaded shouldn't be zero
> coming in for the bonding case.
        "im->loaded" means the device has it in its multicast address 
filter.
If you're switching devices, and didn't do all the multicast stuff on all
the devices originally, then you want it to be 0 (and should make it so,
like ip_mc_down() does). :-)

>    I think the meat that the "rejoin" wants is what's in
> igmpv3_send_cr(), which appears to do the actual sending stuff.  I'm not
> sure if that's better to call directly (and risk locking adventures) or
> to just trip the timer via igmp_ifc_event().

        No, no, no -- please don't mess with those directly. It'd be a 
maintenance
nightmare, and multicasting is device independent right now. :-) I'd hope 
there
wouldn't be any bonding-specific code needed at this layer, which is why I 
hope
something like using up/down would work out.

                                                                +-DLS

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html