[PATCH] bonding: move IPv6 support into a separate kernel module

[PATCH] bonding: move IPv6 support into a separate kernel module

[Brian Haley]

[Possible fix for bonding IPv6 regression reported by Andrey Borzenkov,
tried to keep all Cc's]

This patch moves the IPv6 bonding code into a separate kernel module
called bonding_ipv6 if either bonding or IPv6 are built as modules.
If both are built into the kernel then this is as well.  Bonding_ipv6.ko
registers an "send_unsol_na" function pointer for the unsolicited
advertisement function to be called on a failover - the default action
is to do nothing.  The notifier callbacks are now registered in this
module and not in the base bonding module.

Signed-off-by: Brian Haley <brian.haley@hp.com>
---
  Documentation/networking/bonding.txt |    3 ++
  drivers/net/bonding/Makefile         |    5 ++-
  drivers/net/bonding/bond_ipv6.c      |   13 +++++++-
  drivers/net/bonding/bond_main.c      |   52
++++++++++++++++++++++++++++++---
  drivers/net/bonding/bonding.h        |    9 +++--
  5 files changed, 69 insertions(+), 13 deletions(-)

diff --git a/Documentation/networking/bonding.txt
b/Documentation/networking/bonding.txt
index 5ede747..c8b1c1f 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -603,6 +603,9 @@ num_unsol_na
  	affects only the active-backup mode.  This option was added for
  	bonding version 3.4.0.

+	In order to get this functionality, you will need to load the
+	Bonding IPv6 module with 'modprobe bonding_ipv6'.
+
  primary

  	A string (eth0, eth2, etc) specifying which slave is the
diff --git a/drivers/net/bonding/Makefile b/drivers/net/bonding/Makefile
index 6f9c6fa..d4f6338 100644
--- a/drivers/net/bonding/Makefile
+++ b/drivers/net/bonding/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_BONDING) += bonding.o

  bonding-objs := bond_main.o bond_3ad.o bond_alb.o bond_sysfs.o

-ipv6-$(subst m,y,$(CONFIG_IPV6)) += bond_ipv6.o
-bonding-objs += $(ipv6-y)
+# build bonding_ipv6 as module whenever either IPv6 or Bonding is a module
+obj-$(subst y,$(CONFIG_BONDING),$(CONFIG_IPV6)) += bonding_ipv6.o
+bonding_ipv6-y := bond_ipv6.o

diff --git a/drivers/net/bonding/bond_ipv6.c
b/drivers/net/bonding/bond_ipv6.c
index 0d73bf5..2f10514 100644
--- a/drivers/net/bonding/bond_ipv6.c
+++ b/drivers/net/bonding/bond_ipv6.c
@@ -20,6 +20,9 @@
   *
   */

+#include <linux/module.h>
+#include <linux/init.h>
+
  #include <linux/types.h>
  #include <linux/if_vlan.h>
  #include <net/ipv6.h>
@@ -204,13 +207,19 @@ static struct notifier_block
bond_inet6addr_notifier = {
  	.notifier_call = bond_inet6addr_event,
  };

-void bond_register_ipv6_notifier(void)
+static int __init bonding_ipv6_init(void)
  {
+	bond_register_ipv6_na(bond_send_unsolicited_na);
  	register_inet6addr_notifier(&bond_inet6addr_notifier);
+	return 0;
  }

-void bond_unregister_ipv6_notifier(void)
+static void __exit bonding_ipv6_exit(void)
  {
  	unregister_inet6addr_notifier(&bond_inet6addr_notifier);
+	bond_unregister_ipv6_na();
  }

+module_init(bonding_ipv6_init)
+module_exit(bonding_ipv6_exit)
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/bonding/bond_main.c
b/drivers/net/bonding/bond_main.c
index 2c96b93..ff61add 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -149,6 +149,12 @@ static const char * const version =
  	DRV_DESCRIPTION ": v" DRV_VERSION " (" DRV_RELDATE ")\n";

  LIST_HEAD(bond_dev_list);
+EXPORT_SYMBOL(bond_dev_list);
+
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+static DEFINE_SPINLOCK(bond_v6_na_lock);
+#endif
+static void (*bond_send_unsol_na)(struct bonding *bond);

  #ifdef CONFIG_PROC_FS
  static struct proc_dir_entry *bond_proc_dir = NULL;
@@ -1201,6 +1207,8 @@ void bond_change_active_slave(struct bonding
*bond, struct slave *new_active)
  		}

  		if (new_active) {
+			void (*send_unsol_na)(struct bonding *bond);
+
  			bond_set_slave_active_flags(new_active);

  			if (bond->params.fail_over_mac)
@@ -1211,7 +1219,11 @@ void bond_change_active_slave(struct bonding
*bond, struct slave *new_active)
  			bond_send_gratuitous_arp(bond);

  			bond->send_unsol_na = bond->params.num_unsol_na;
-			bond_send_unsolicited_na(bond);
+			rcu_read_lock();
+			send_unsol_na = rcu_dereference(bond_send_unsol_na);
+			if (send_unsol_na)
+				send_unsol_na(bond);
+			rcu_read_unlock();

  			write_unlock_bh(&bond->curr_slave_lock);
  			read_unlock(&bond->lock);
@@ -2464,8 +2476,14 @@ void bond_mii_monitor(struct work_struct *work)
  	}

  	if (bond->send_unsol_na) {
+		void (*send_unsol_na)(struct bonding *bond);
+
  		read_lock(&bond->curr_slave_lock);
-		bond_send_unsolicited_na(bond);
+		rcu_read_lock();
+		send_unsol_na = rcu_dereference(bond_send_unsol_na);
+		if (send_unsol_na)
+			send_unsol_na(bond);
+		rcu_read_unlock();
  		read_unlock(&bond->curr_slave_lock);
  	}

@@ -3165,8 +3183,14 @@ void bond_activebackup_arp_mon(struct work_struct
*work)
  	}

  	if (bond->send_unsol_na) {
+		void (*send_unsol_na)(struct bonding *bond);
+
  		read_lock(&bond->curr_slave_lock);
-		bond_send_unsolicited_na(bond);
+		rcu_read_lock();
+		send_unsol_na = rcu_dereference(bond_send_unsol_na);
+		if (send_unsol_na)
+			send_unsol_na(bond);
+		rcu_read_unlock();
  		read_unlock(&bond->curr_slave_lock);
  	}

@@ -5203,6 +5227,26 @@ out_rtnl:
  	return res;
  }

+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+void bond_register_ipv6_na(void (*send_unsol_na) (struct bonding *bond))
+{
+	spin_lock_bh(&bond_v6_na_lock);
+	rcu_assign_pointer(bond_send_unsol_na, send_unsol_na);
+	spin_unlock_bh(&bond_v6_na_lock);
+	synchronize_rcu();
+}
+EXPORT_SYMBOL_GPL(bond_register_ipv6_na);
+
+void bond_unregister_ipv6_na(void)
+{
+	spin_lock_bh(&bond_v6_na_lock);
+	rcu_assign_pointer(bond_send_unsol_na, NULL);
+	spin_unlock_bh(&bond_v6_na_lock);
+	synchronize_rcu();
+}
+EXPORT_SYMBOL(bond_unregister_ipv6_na);
+#endif
+
  static int __init bonding_init(void)
  {
  	int i;
@@ -5234,7 +5278,6 @@ static int __init bonding_init(void)

  	register_netdevice_notifier(&bond_netdev_notifier);
  	register_inetaddr_notifier(&bond_inetaddr_notifier);
-	bond_register_ipv6_notifier();

  	goto out;
  err:
@@ -5257,7 +5300,6 @@ static void __exit bonding_exit(void)
  {
  	unregister_netdevice_notifier(&bond_netdev_notifier);
  	unregister_inetaddr_notifier(&bond_inetaddr_notifier);
-	bond_unregister_ipv6_notifier();

  	bond_destroy_sysfs();

diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index ca849d2..9e5e092 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -23,15 +23,13 @@
  #include "bond_3ad.h"
  #include "bond_alb.h"

-#define DRV_VERSION	"3.5.0"
-#define DRV_RELDATE	"November 4, 2008"
+#define DRV_VERSION	"3.6.0"
+#define DRV_RELDATE	"February 20, 2009"
  #define DRV_NAME	"bonding"
  #define DRV_DESCRIPTION	"Ethernet Channel Bonding Driver"

  #define BOND_MAX_ARP_TARGETS	16

-extern struct list_head bond_dev_list;
-
  #define IS_UP(dev)					   \
  	      ((((dev)->flags & IFF_UP) == IFF_UP)	&& \
  	       netif_running(dev)			&& \
@@ -360,6 +358,9 @@ extern struct rw_semaphore bonding_rwsem;
  void bond_send_unsolicited_na(struct bonding *bond);
  void bond_register_ipv6_notifier(void);
  void bond_unregister_ipv6_notifier(void);
+
+void bond_register_ipv6_na(void (*send_unsol_na) (struct bonding *bond));
+void bond_unregister_ipv6_na(void);
  #else
  static inline void bond_send_unsolicited_na(struct bonding *bond)
  {
-- 
1.5.4.3

Re: [PATCH] bonding: move IPv6 support into a separate kernel module

[David Miller]

From: Brian Haley <brian.haley@hp.com>
Date: Fri, 20 Feb 2009 19:31:29 -0500

> [Possible fix for bonding IPv6 regression reported by Andrey Borzenkov,
> tried to keep all Cc's]
> 
> This patch moves the IPv6 bonding code into a separate kernel module
> called bonding_ipv6 if either bonding or IPv6 are built as modules.
> If both are built into the kernel then this is as well.  Bonding_ipv6.ko
> registers an "send_unsol_na" function pointer for the unsolicited
> advertisement function to be called on a failover - the default action
> is to do nothing.  The notifier callbacks are now registered in this
> module and not in the base bonding module.
> 
> Signed-off-by: Brian Haley <brian.haley@hp.com>

Thanks for taking the time to work on this Brian.

I wonder if we aren't just trading one evil for another.

Right now just configuring bonding will get the bonding
module loaded and the ipv6 facilities will be visible.

Now with your change, the user has to explicitly load
the module.  That's extremely user-unfriendly.

Re: [PATCH] bonding: move IPv6 support into a separate kernel module

[Willy Tarreau]

On Sat, Feb 21, 2009 at 11:59:01PM -0800, David Miller wrote:
> From: Brian Haley <brian.haley@hp.com>
> Date: Fri, 20 Feb 2009 19:31:29 -0500
> 
> > [Possible fix for bonding IPv6 regression reported by Andrey Borzenkov,
> > tried to keep all Cc's]
> > 
> > This patch moves the IPv6 bonding code into a separate kernel module
> > called bonding_ipv6 if either bonding or IPv6 are built as modules.
> > If both are built into the kernel then this is as well.  Bonding_ipv6.ko
> > registers an "send_unsol_na" function pointer for the unsolicited
> > advertisement function to be called on a failover - the default action
> > is to do nothing.  The notifier callbacks are now registered in this
> > module and not in the base bonding module.
> > 
> > Signed-off-by: Brian Haley <brian.haley@hp.com>
> 
> Thanks for taking the time to work on this Brian.
> 
> I wonder if we aren't just trading one evil for another.
> 
> Right now just configuring bonding will get the bonding
> module loaded and the ipv6 facilities will be visible.
> 
> Now with your change, the user has to explicitly load
> the module.  That's extremely user-unfriendly.

Especially since bonding is working between layers 1 and 2 and provides
a protocol-agnostic ethernet-like interface. It would seem very confusing
to load ethernet drivers depending on the protocols we expect to run on
top of them :-/

Willy

Re: [PATCH] bonding: move IPv6 support into a separate kernel module

[Brian Haley]

David Miller wrote:
> From: Brian Haley <brian.haley@hp.com>
> Date: Fri, 20 Feb 2009 19:31:29 -0500
> 
>> [Possible fix for bonding IPv6 regression reported by Andrey Borzenkov,
>> tried to keep all Cc's]
>>
>> This patch moves the IPv6 bonding code into a separate kernel module
>> called bonding_ipv6 if either bonding or IPv6 are built as modules.
>> If both are built into the kernel then this is as well.  Bonding_ipv6.ko
>> registers an "send_unsol_na" function pointer for the unsolicited
>> advertisement function to be called on a failover - the default action
>> is to do nothing.  The notifier callbacks are now registered in this
>> module and not in the base bonding module.
>>
>> Signed-off-by: Brian Haley <brian.haley@hp.com>
> 
> Thanks for taking the time to work on this Brian.
> 
> I wonder if we aren't just trading one evil for another.
> 
> Right now just configuring bonding will get the bonding
> module loaded and the ipv6 facilities will be visible.
> 
> Now with your change, the user has to explicitly load
> the module.  That's extremely user-unfriendly.

I agree, part of me just wanted to make sure it could be done, and figured it 
was just a matter of time before my hand was forced, or the original commit was 
reverted.

One idea is that the IPv6 notifier, addrconf_notify(), could try and load the 
bonding_ipv6 module if a device with IFF_MASTER registered.  I haven't 
prototyped it to see if it would actually work, seemed like a hack.

-Brian