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]
Date:	Thu, 1 Mar 2007 11:49:19 -0500
From:	Andy Gospodarek <andy@...yhouse.net>
To:	Jay Vosburgh <fubar@...ibm.com>
Cc:	netdev@...r.kernel.org, bonding-devel@...ts.sourceforge.net,
	Jeff Garzik <jgarzik@...ox.com>
Subject: Re: [PATCH 3/3] bonding: Improve IGMP join processing

On Wed, Feb 28, 2007 at 05:03:37PM -0800, Jay Vosburgh wrote:
> 
> 	In active-backup mode, the current bonding code duplicates IGMP
> traffic to all slaves, so that switches are up to date in case of a
> failover from an active to a backup interface.  If bonding then fails
> back to the original active interface, it is likely that the "active
> slave" switch's IGMP forwarding for the port will be out of date until
> some event occurs to refresh the switch (e.g., a membership query).
> 
> 	This patch alters the behavior of bonding to no longer flood
> IGMP to all ports, and to issue IGMP JOINs to the newly active port at
> the time of a failover.  This insures that switches are kept up to date
> for all cases.
> 
> 	"GOELLESCH Niels" <niels.goellesch@...ocontrol.int> originally
> reported this problem, and included a patch.  His original patch was
> modified by Jay Vosburgh to additionally remove the existing IGMP flood
> behavior, use RCU, streamline code paths, fix trailing white space, and
> adjust for style.
> 
> Signed-off-by: Jay Vosburgh <fubar@...ibm.com>
> 

My only concern is that this code assumes all mcast addresses stored in
dev->mc-list list are for ipv4 igmp mcast addresses and nothing was done
for ipv6.

But this is much better than what we have now, so... 

Acked-by: Andy Gospodarek <andy@...yhouse.net>


> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 7ec6121..338d452 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -60,6 +60,7 @@ #include <asm/uaccess.h>
>  #include <linux/errno.h>
>  #include <linux/netdevice.h>
>  #include <linux/inetdevice.h>
> +#include <linux/igmp.h>
>  #include <linux/etherdevice.h>
>  #include <linux/skbuff.h>
>  #include <net/sock.h>
> @@ -861,6 +862,28 @@ static void bond_mc_delete(struct bondin
>  	}
>  }
>  
> +
> +/*
> + * Retrieve the list of registered multicast addresses for the bonding
> + * device and retransmit an IGMP JOIN request to the current active
> + * slave.
> + */
> +static void bond_resend_igmp_join_requests(struct bonding *bond)
> +{
> +	struct in_device *in_dev;
> +	struct ip_mc_list *im;
> +
> +	rcu_read_lock();
> +	in_dev = __in_dev_get_rcu(bond->dev);
> +	if (in_dev) {
> +		for (im = in_dev->mc_list; im; im = im->next) {
> +			ip_mc_rejoin_group(im);
> +		}
> +	}
> +
> +	rcu_read_unlock();
> +}
> +
>  /*
>   * Totally destroys the mc_list in bond
>   */
> @@ -874,6 +897,7 @@ static void bond_mc_list_destroy(struct 
>  		kfree(dmi);
>  		dmi = bond->mc_list;
>  	}
> +        bond->mc_list = NULL;
>  }
>  
>  /*
> @@ -967,6 +991,7 @@ static void bond_mc_swap(struct bonding 
>  		for (dmi = bond->dev->mc_list; dmi; dmi = dmi->next) {
>  			dev_mc_add(new_active->dev, dmi->dmi_addr, dmi->dmi_addrlen, 0);
>  		}
> +		bond_resend_igmp_join_requests(bond);
>  	}
>  }
>  
> @@ -4017,42 +4042,6 @@ out:
>  	return 0;
>  }
>  
> -static void bond_activebackup_xmit_copy(struct sk_buff *skb,
> -                                        struct bonding *bond,
> -                                        struct slave *slave)
> -{
> -	struct sk_buff *skb2 = skb_copy(skb, GFP_ATOMIC);
> -	struct ethhdr *eth_data;
> -	u8 *hwaddr;
> -	int res;
> -
> -	if (!skb2) {
> -		printk(KERN_ERR DRV_NAME ": Error: "
> -		       "bond_activebackup_xmit_copy(): skb_copy() failed\n");
> -		return;
> -	}
> -
> -	skb2->mac.raw = (unsigned char *)skb2->data;
> -	eth_data = eth_hdr(skb2);
> -
> -	/* Pick an appropriate source MAC address
> -	 *	-- use slave's perm MAC addr, unless used by bond
> -	 *	-- otherwise, borrow active slave's perm MAC addr
> -	 *	   since that will not be used
> -	 */
> -	hwaddr = slave->perm_hwaddr;
> -	if (!memcmp(eth_data->h_source, hwaddr, ETH_ALEN))
> -		hwaddr = bond->curr_active_slave->perm_hwaddr;
> -
> -	/* Set source MAC address appropriately */
> -	memcpy(eth_data->h_source, hwaddr, ETH_ALEN);
> -
> -	res = bond_dev_queue_xmit(bond, skb2, slave->dev);
> -	if (res)
> -		dev_kfree_skb(skb2);
> -
> -	return;
> -}
>  
>  /*
>   * in active-backup mode, we know that bond->curr_active_slave is always valid if
> @@ -4073,21 +4062,6 @@ static int bond_xmit_activebackup(struct
>  	if (!bond->curr_active_slave)
>  		goto out;
>  
> -	/* Xmit IGMP frames on all slaves to ensure rapid fail-over
> -	   for multicast traffic on snooping switches */
> -	if (skb->protocol == __constant_htons(ETH_P_IP) &&
> -	    skb->nh.iph->protocol == IPPROTO_IGMP) {
> -		struct slave *slave, *active_slave;
> -		int i;
> -
> -		active_slave = bond->curr_active_slave;
> -		bond_for_each_slave_from_to(bond, slave, i, active_slave->next,
> -		                            active_slave->prev)
> -			if (IS_UP(slave->dev) &&
> -			    (slave->link == BOND_LINK_UP))
> -				bond_activebackup_xmit_copy(skb, bond, slave);
> -	}
> -
>  	res = bond_dev_queue_xmit(bond, skb, bond->curr_active_slave->dev);
>  
>  out:
> diff --git a/include/linux/igmp.h b/include/linux/igmp.h
> index 9dbb525..a113fe6 100644
> --- a/include/linux/igmp.h
> +++ b/include/linux/igmp.h
> @@ -218,5 +218,7 @@ extern void ip_mc_up(struct in_device *)
>  extern void ip_mc_down(struct in_device *);
>  extern void ip_mc_dec_group(struct in_device *in_dev, __be32 addr);
>  extern void ip_mc_inc_group(struct in_device *in_dev, __be32 addr);
> +extern void ip_mc_rejoin_group(struct ip_mc_list *im);
> +
>  #endif
>  #endif
> diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> index 0637213..1c6a084 100644
> --- a/net/ipv4/igmp.c
> +++ b/net/ipv4/igmp.c
> @@ -1251,6 +1251,28 @@ out:
>  }
>  
>  /*
> + *	Resend IGMP JOIN report; used for bonding.
> + */
> +void ip_mc_rejoin_group(struct ip_mc_list *im)
> +{
> +	struct in_device *in_dev = im->interface;
> +
> +#ifdef CONFIG_IP_MULTICAST
> +	if (im->multiaddr == IGMP_ALL_HOSTS)
> +		return;
> +
> +	if (IGMP_V1_SEEN(in_dev) || IGMP_V2_SEEN(in_dev)) {
> +		igmp_mod_timer(im, IGMP_Initial_Report_Delay);
> +		return;
> +	}
> +	/* else, v3 */
> +	im->crcount = in_dev->mr_qrv ? in_dev->mr_qrv :
> +		IGMP_Unsolicited_Report_Count;
> +	igmp_ifc_event(in_dev);
> +#endif
> +}
> +
> +/*
>   *	A socket has left a multicast group on device dev
>   */
>  
> @@ -2596,3 +2618,4 @@ #endif
>  EXPORT_SYMBOL(ip_mc_dec_group);
>  EXPORT_SYMBOL(ip_mc_inc_group);
>  EXPORT_SYMBOL(ip_mc_join_group);
> +EXPORT_SYMBOL(ip_mc_rejoin_group);
> -
> 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
-
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