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
| ||
|
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
Powered by blists - more mailing lists