From b615c2664c9fd72b131216f2fbda4316cd7e2e7b Mon Sep 17 00:00:00 2001 From: Joonwoo Park Date: Sun, 30 Jan 2011 00:46:50 -0800 Subject: [PATCH] FromDevice: avoid kernel panic on device status change. When device status changes (e.g. link bounce), if dev->br_port is not null linux bridge's br_device_event will refer to dev->br_port. But dev->br_port is a invalid pointer to linux bridge as it's fake_bridge which was allocated by FromDevice. To make linux bridge happy, set br_port to null with early notification handler and set it again with late notification handler again. Note: this fix is under *not* ideal assumption as it assumes br_device_notifier's prioriy is higher than INT_MIN and less than INT_MAX. Signed-off-by: Joonwoo Park --- elements/linuxmodule/anydevice.cc | 2 ++ elements/linuxmodule/anydevice.hh | 2 +- elements/linuxmodule/fromdevice.cc | 53 +++++++++++++++++++++++++----- 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/elements/linuxmodule/anydevice.cc b/elements/linuxmodule/anydevice.cc index b77a7e9dea..fa4a2343cd 100644 --- a/elements/linuxmodule/anydevice.cc +++ b/elements/linuxmodule/anydevice.cc @@ -124,6 +124,8 @@ void AnyDevice::alter_from_device(int delta) { #if !HAVE_CLICK_KERNEL && (defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE)) && LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 24) + if (!_dev) + return; fake_bridge *fb = reinterpret_cast(_dev->br_port); if (fb && fb->magic != fake_bridge::click_magic) { printk("<1>%s: appears to be owned by the bridge module!", _devname.c_str()); diff --git a/elements/linuxmodule/anydevice.hh b/elements/linuxmodule/anydevice.hh index 15a37b3dbd..b502b5f7e0 100644 --- a/elements/linuxmodule/anydevice.hh +++ b/elements/linuxmodule/anydevice.hh @@ -101,6 +101,7 @@ class AnyDevice : public Element { public: }; void set_device(net_device *dev, AnyDeviceMap *map, int flags); void clear_device(AnyDeviceMap *map, int flags); + void alter_from_device(int delta); static inline net_device *get_by_name(const char *name) { #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 24) @@ -140,7 +141,6 @@ class AnyDevice : public Element { public: HandlerCall *_down_call; void alter_promiscuity(int delta); - void alter_from_device(int delta); friend class AnyDeviceMap; diff --git a/elements/linuxmodule/fromdevice.cc b/elements/linuxmodule/fromdevice.cc index 47e42aa0d9..f780dfa310 100644 --- a/elements/linuxmodule/fromdevice.cc +++ b/elements/linuxmodule/fromdevice.cc @@ -36,7 +36,8 @@ static int registered_readers; #if HAVE_CLICK_KERNEL static struct notifier_block packet_notifier; #endif -static struct notifier_block device_notifier; +static struct notifier_block device_notifier_early; +static struct notifier_block device_notifier_late; #if !HAVE_CLICK_KERNEL && (defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE)) # define CLICK_FROMDEVICE_USE_BRIDGE 1 @@ -58,7 +59,8 @@ static int packet_notifier_hook(struct notifier_block *nb, unsigned long val, vo static struct sk_buff *click_br_handle_frame_hook(struct net_bridge_port *p, struct sk_buff *skb); static struct sk_buff *(*real_br_handle_frame_hook)(struct net_bridge_port *p, struct sk_buff *skb); #endif -static int device_notifier_hook(struct notifier_block *nb, unsigned long val, void *v); +static int device_notifier_hook_early(struct notifier_block *nb, unsigned long val, void *v); +static int device_notifier_hook_late(struct notifier_block *nb, unsigned long val, void *v); } void @@ -70,10 +72,15 @@ FromDevice::static_initialize() packet_notifier.priority = 1; packet_notifier.next = 0; #endif - device_notifier.notifier_call = device_notifier_hook; - device_notifier.priority = 1; - device_notifier.next = 0; - register_netdevice_notifier(&device_notifier); + device_notifier_early.notifier_call = device_notifier_hook_early; + device_notifier_early.priority = INT_MAX; + device_notifier_early.next = 0; + register_netdevice_notifier(&device_notifier_early); + + device_notifier_late.notifier_call = device_notifier_hook_late; + device_notifier_late.priority = INT_MIN; + device_notifier_late.next = 0; + register_netdevice_notifier(&device_notifier_late); } void @@ -86,7 +93,8 @@ FromDevice::static_cleanup() if (br_handle_frame_hook == click_br_handle_frame_hook) br_handle_frame_hook = real_br_handle_frame_hook; #endif - unregister_netdevice_notifier(&device_notifier); + unregister_netdevice_notifier(&device_notifier_late); + unregister_netdevice_notifier(&device_notifier_early); } FromDevice::FromDevice() @@ -260,6 +268,7 @@ click_br_handle_frame_hook(struct net_bridge_port *p, struct sk_buff *skb) int stolen = 0; FromDevice *fd = 0; unsigned long lock_flags; + from_device_map.lock(false, lock_flags); while (stolen == 0 && (fd = (FromDevice *)from_device_map.lookup(skb->dev, fd))) stolen = fd->got_skb(skb); @@ -274,7 +283,31 @@ click_br_handle_frame_hook(struct net_bridge_port *p, struct sk_buff *skb) #endif static int -device_notifier_hook(struct notifier_block *nb, unsigned long flags, void *v) +device_notifier_hook_early(struct notifier_block *nb, unsigned long flags, void *v) +{ + unsigned long lock_flags; + net_device* dev = (net_device*)v; + AnyDevice *es[8]; + int i, nes; + +#ifdef NETDEV_GOING_DOWN + if (flags == NETDEV_GOING_DOWN) + flags = NETDEV_DOWN; +#endif + if (flags == NETDEV_DOWN || flags == NETDEV_UP || flags == NETDEV_CHANGE) { + bool exists = (flags != NETDEV_UP); + from_device_map.lock(true, lock_flags); + nes = from_device_map.lookup_all(dev, exists, es, 8); + for (i = 0; i < nes; i++) + ((FromDevice*)(es[i]))->alter_from_device(-1); + from_device_map.unlock(true, lock_flags); + } + + return 0; +} + +static int +device_notifier_hook_late(struct notifier_block *nb, unsigned long flags, void *v) { #ifdef NETDEV_GOING_DOWN if (flags == NETDEV_GOING_DOWN) @@ -287,8 +320,10 @@ device_notifier_hook(struct notifier_block *nb, unsigned long flags, void *v) from_device_map.lock(true, lock_flags); AnyDevice *es[8]; int nes = from_device_map.lookup_all(dev, exists, es, 8); - for (int i = 0; i < nes; i++) + for (int i = 0; i < nes; i++) { + ((FromDevice*)(es[i]))->alter_from_device(1); ((FromDevice*)(es[i]))->set_device(flags == NETDEV_DOWN ? 0 : dev, &from_device_map, AnyDevice::anydev_change | AnyDevice::anydev_from_device); + } from_device_map.unlock(true, lock_flags); } return 0;