| Message ID | 49A5ADB3.2010709@hp.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
Brian Haley <brian.haley@hp.com> wrote: [...] >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. > >Also, have the IPv6 address notifier request that the bonding_ipv6 >module be loaded when an IFF_MASTER device is first brought-up. >This avoids users from having to do this explicitly with modprobe. I'm not entirely sure what the right solution for all of this is, but it doesn't seem to me that cranking on bonding and adding a special case to ipv6 is the best way to go. This patch won't resolve the reported similar (but presumably lower profile) issues with SCTP or qeth, and it seems unlikely that this is the last time some driver will gain a run time dependence on ipv6 after being compiled with CONFIG_IPV6. Theodore Tso <tytso@mit.edu> wrote (in a different thread): >I think I can pretty much guarantee that distro users will be >clamoring for a quick and easy way to block ipv6, and it's in our >interest to document the recomended way to block it that doesn't cause >weird problems with bonding, etc. I agree with this. I've been fooling with the disable_ipv6 sysctl, and one issue is that, at least on the distro I'm testing on (SLES), it's not picked up from /etc/sysctl.conf at boot time (presumably because ipv6 isn't loaded yet, although I haven't really checked). -J --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Jay Vosburgh <fubar@us.ibm.com> Date: Wed, 25 Feb 2009 14:10:58 -0800 > I've been fooling with the disable_ipv6 sysctl, and one issue is > that, at least on the distro I'm testing on (SLES), it's not picked up > from /etc/sysctl.conf at boot time (presumably because ipv6 isn't loaded > yet, although I haven't really checked). Correct, that's the problem. We could create a blocker bitmap. Two sysctls, "block_af" and "unblock_af". You write the AF_foo value for the protocol there and it sets or clears the assosciated bit in the internal blocker bitmap. Things like sys_socket() et al. key off of this. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
David Miller wrote: > From: Jay Vosburgh <fubar@us.ibm.com> > Date: Wed, 25 Feb 2009 14:10:58 -0800 > >> I've been fooling with the disable_ipv6 sysctl, and one issue is >> that, at least on the distro I'm testing on (SLES), it's not picked up >> from /etc/sysctl.conf at boot time (presumably because ipv6 isn't loaded >> yet, although I haven't really checked). > > Correct, that's the problem. > > We could create a blocker bitmap. Two sysctls, "block_af" and > "unblock_af". You write the AF_foo value for the protocol there and > it sets or clears the assosciated bit in the internal blocker bitmap. > > Things like sys_socket() et al. key off of this. I'm open to suggestions at this point in time, I just don't see how this will solve the bonding problem since it still wouldn't load, right? Dave - do you feel I need to fix this regression? If not I can try to work on this AF blocker thing. My only other thought if we want to fix this is to have the IPv6 module register these five functions into an ops structure that bonding can call. It doesn't fix SCTP, qeth, etc, but it gets these "blacklist ipv6" configs working again, and gets me out of the crosshairs :) -Brian -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Brian Haley <brian.haley@hp.com> wrote: >David Miller wrote: >> From: Jay Vosburgh <fubar@us.ibm.com> >> Date: Wed, 25 Feb 2009 14:10:58 -0800 >> >>> I've been fooling with the disable_ipv6 sysctl, and one issue is >>> that, at least on the distro I'm testing on (SLES), it's not picked up >>> from /etc/sysctl.conf at boot time (presumably because ipv6 isn't loaded >>> yet, although I haven't really checked). >> >> Correct, that's the problem. >> >> We could create a blocker bitmap. Two sysctls, "block_af" and >> "unblock_af". You write the AF_foo value for the protocol there and >> it sets or clears the assosciated bit in the internal blocker bitmap. >> >> Things like sys_socket() et al. key off of this. > >I'm open to suggestions at this point in time, I just don't see how this >will solve the bonding problem since it still wouldn't load, right? It would permit users to load ipv6 (thus allowing bonding to load), but prevent ipv6 from actually doing anything. (because sys_socket, e.g., won't open an ipv6 socket if block_af includes ipv6). Actually, __sock_create might be the better place to put the hook for "create a socket"; there would probably need to be a check within the protocol code as well, so that, e.g., ipv6 addrconf won't run if AF_INET6 is disabled. >Dave - do you feel I need to fix this regression? If not I can try to >work on this AF blocker thing. My only other thought if we want to fix >this is to have the IPv6 module register these five functions into an ops >structure that bonding can call. It doesn't fix SCTP, qeth, etc, but it >gets these "blacklist ipv6" configs working again, and gets me out of the >crosshairs :) I think the problem (customers want to disable ipv6 and use bonding, sctp, qeth, whatever) needs to be fixed. If it's not, I'm sure I'll be getting lots of cards and letters from customers. I don't think the solution needs to preserve the current solution (preventing the ipv6 module from loading). Ipv6 being unusable should be sufficient. Except perhaps in an embedded environment, but they're probably in a position to compile their kernel without ipv6. Another possible resolution is to modify the initscripts in the distros to perform sysctl -p (read sysctls from /etc/sysctl.conf) after ipv6 is loaded, so that the disable_ipv6 sysctl can be set. That seems like more work, and is limited to ipv6, so I don't see it as being better than a "kernel shut off AF_xxx" type of solution. -J --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jay Vosburgh wrote: > Brian Haley <brian.haley@hp.com> wrote: > >> David Miller wrote: >>> From: Jay Vosburgh <fubar@us.ibm.com> >>> Date: Wed, 25 Feb 2009 14:10:58 -0800 >>> >>>> I've been fooling with the disable_ipv6 sysctl, and one issue is >>>> that, at least on the distro I'm testing on (SLES), it's not picked up >>>> from /etc/sysctl.conf at boot time (presumably because ipv6 isn't loaded >>>> yet, although I haven't really checked). >>> Correct, that's the problem. >>> >>> We could create a blocker bitmap. Two sysctls, "block_af" and >>> "unblock_af". You write the AF_foo value for the protocol there and >>> it sets or clears the assosciated bit in the internal blocker bitmap. >>> >>> Things like sys_socket() et al. key off of this. >> I'm open to suggestions at this point in time, I just don't see how this >> will solve the bonding problem since it still wouldn't load, right? > > It would permit users to load ipv6 (thus allowing bonding to > load), but prevent ipv6 from actually doing anything. (because > sys_socket, e.g., won't open an ipv6 socket if block_af includes ipv6). > > Actually, __sock_create might be the better place to put the > hook for "create a socket"; there would probably need to be a check > within the protocol code as well, so that, e.g., ipv6 addrconf won't run > if AF_INET6 is disabled. But addrconf_init doesn't care about AF_INET6 sockets... Additionally, why is it absolutely necessary to block AF_INET6 sockets. I never understood that requirement? I can see people blocking IPv6 from loading because the module automatically configures IPv6 addresses and thus opens another communication channel that may not be monitored/controlled. AF_INET6 sockets, on the other hand, are simply relegated to IPv4 protocol, when there are no IPv6 addresses. > >> Dave - do you feel I need to fix this regression? If not I can try to >> work on this AF blocker thing. My only other thought if we want to fix >> this is to have the IPv6 module register these five functions into an ops >> structure that bonding can call. It doesn't fix SCTP, qeth, etc, but it >> gets these "blacklist ipv6" configs working again, and gets me out of the >> crosshairs :) > > I think the problem (customers want to disable ipv6 and use > bonding, sctp, qeth, whatever) needs to be fixed. If it's not, I'm sure > I'll be getting lots of cards and letters from customers. > > I don't think the solution needs to preserve the current > solution (preventing the ipv6 module from loading). Ipv6 being unusable > should be sufficient. Except perhaps in an embedded environment, but > they're probably in a position to compile their kernel without ipv6. Yes. The system must not be reachable using IPv6. > > Another possible resolution is to modify the initscripts in the > distros to perform sysctl -p (read sysctls from /etc/sysctl.conf) after > ipv6 is loaded, so that the disable_ipv6 sysctl can be set. That seems > like more work, and is limited to ipv6, so I don't see it as being > better than a "kernel shut off AF_xxx" type of solution. This not enough. You need to disable parts of IPv6 at module initiation time and the only way to do that is with a parameter. Otherwise, you will have a small window of time when the system has ipv6 configured and is potentially vulnerable. We can have our own sysfs parameter calls that can turn the functionality back on to get back to a fully functional ipv6 implementation. -vlad > > -J > > --- > -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Vlad Yasevich <vladislav.yasevich@hp.com> wrote: >Jay Vosburgh wrote: >> Brian Haley <brian.haley@hp.com> wrote: >> >>> David Miller wrote: >>>> From: Jay Vosburgh <fubar@us.ibm.com> >>>> Date: Wed, 25 Feb 2009 14:10:58 -0800 >>>> >>>>> I've been fooling with the disable_ipv6 sysctl, and one issue is >>>>> that, at least on the distro I'm testing on (SLES), it's not picked up >>>>> from /etc/sysctl.conf at boot time (presumably because ipv6 isn't loaded >>>>> yet, although I haven't really checked). >>>> Correct, that's the problem. >>>> >>>> We could create a blocker bitmap. Two sysctls, "block_af" and >>>> "unblock_af". You write the AF_foo value for the protocol there and >>>> it sets or clears the assosciated bit in the internal blocker bitmap. >>>> >>>> Things like sys_socket() et al. key off of this. >>> I'm open to suggestions at this point in time, I just don't see how this >>> will solve the bonding problem since it still wouldn't load, right? >> >> It would permit users to load ipv6 (thus allowing bonding to >> load), but prevent ipv6 from actually doing anything. (because >> sys_socket, e.g., won't open an ipv6 socket if block_af includes ipv6). >> >> Actually, __sock_create might be the better place to put the >> hook for "create a socket"; there would probably need to be a check >> within the protocol code as well, so that, e.g., ipv6 addrconf won't run >> if AF_INET6 is disabled. > >But addrconf_init doesn't care about AF_INET6 sockets... > >Additionally, why is it absolutely necessary to block AF_INET6 sockets. >I never understood that requirement? I don't know that it is, but it's the current behavior if ipv6 is prevented from loading. >I can see people blocking IPv6 from loading because the module automatically >configures IPv6 addresses and thus opens another communication channel that >may not be monitored/controlled. AF_INET6 sockets, on the other hand, are >simply relegated to IPv4 protocol, when there are no IPv6 addresses. I believe that's only true if the ipv6 module is loaded. If ipv6 is not loaded, then socket(AF_INET6, ...) returns failure with EAFNOSUPPORT. If ipv6 is loaded, socket(AF_INET6, ...) succeeds (apparently no matter if there are ipv6 addresses configured or not). >>> Dave - do you feel I need to fix this regression? If not I can try to >>> work on this AF blocker thing. My only other thought if we want to fix >>> this is to have the IPv6 module register these five functions into an ops >>> structure that bonding can call. It doesn't fix SCTP, qeth, etc, but it >>> gets these "blacklist ipv6" configs working again, and gets me out of the >>> crosshairs :) >> >> I think the problem (customers want to disable ipv6 and use >> bonding, sctp, qeth, whatever) needs to be fixed. If it's not, I'm sure >> I'll be getting lots of cards and letters from customers. >> >> I don't think the solution needs to preserve the current >> solution (preventing the ipv6 module from loading). Ipv6 being unusable >> should be sufficient. Except perhaps in an embedded environment, but >> they're probably in a position to compile their kernel without ipv6. > >Yes. The system must not be reachable using IPv6. > >> >> Another possible resolution is to modify the initscripts in the >> distros to perform sysctl -p (read sysctls from /etc/sysctl.conf) after >> ipv6 is loaded, so that the disable_ipv6 sysctl can be set. That seems >> like more work, and is limited to ipv6, so I don't see it as being >> better than a "kernel shut off AF_xxx" type of solution. > >This not enough. You need to disable parts of IPv6 at module initiation >time and the only way to do that is with a parameter. Otherwise, you will >have a small window of time when the system has ipv6 configured and is potentially >vulnerable. > >We can have our own sysfs parameter calls that can turn the functionality >back on to get back to a fully functional ipv6 implementation. > >-vlad -J --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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) { diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index f8f76d6..ec24e0e 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -59,6 +59,7 @@ #include <linux/delay.h> #include <linux/notifier.h> #include <linux/string.h> +#include <linux/kmod.h> #include <net/net_namespace.h> #include <net/sock.h> @@ -2452,6 +2453,7 @@ static int addrconf_notify(struct notifier_block *this, unsigned long event, struct inet6_dev *idev = __in6_dev_get(dev); int run_pending = 0; int err; + static int bond_ipv6 = 0; switch(event) {
[Possible fix v2 for bonding IPv6 regression reported by Andrey Borzenkov, added automatic loading of bonding_ipv6] 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. Also, have the IPv6 address notifier request that the bonding_ipv6 module be loaded when an IFF_MASTER device is first brought-up. This avoids users from having to do this explicitly with modprobe. 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 +++-- net/ipv6/addrconf.c | 6 ++++++ 6 files changed, 75 insertions(+), 13 deletions(-) case NETDEV_REGISTER: @@ -2519,6 +2521,10 @@ static int addrconf_notify(struct notifier_block *this, unsigned long event, break; default: + if ((dev->flags & IFF_MASTER) && !bond_ipv6) { + request_module("bonding_ipv6"); + bond_ipv6 = 1; + } addrconf_dev_config(dev); break; }