Linuxmodule FromDevice: Use netdev_rx_handler_register when available.

In later versions of Linux, the Ethernet bridging hook used by Click to take packets has been replaced by a more generic rx_handler. Use that rx_handler to steal packets from Linux. Simultaneously simplify notification code for devices going up and down. Joonwoo Park provided an initial version of this patch. Signed-off-by: Eddie Kohler <[email protected]>
tbarbette · Jun 12, 2011 · a8bb01c · a8bb01c
1 parent 8e4ed30
commit a8bb01c
Show file tree

Hide file tree

Showing 7 changed files with 114 additions and 43 deletions.
diff --git a/config-linuxmodule.h.in b/config-linuxmodule.h.in
@@ -112,6 +112,9 @@
 /* Define if you have the netdev_uses_trailer_tags function. */
 #undef HAVE_NETDEV_USES_TRAILER_TAGS
 
+/* Define if your Linux kernel has netdev_rx_handler_register. */
+#undef HAVE_LINUX_NETDEV_RX_HANDLER_REGISTER
+
 /* Define if netif_receive_skb takes 3 arguments. */
 #undef HAVE_NETIF_RECEIVE_SKB_EXTENDED
 

diff --git a/configure b/configure
@@ -11241,6 +11241,21 @@ $as_echo "#define HAVE_NETDEV_USES_TRAILER_TAGS 1" >>confdefs.h
 
     fi
 
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking for netdev_rx_handler_register kernel symbol" >&5
+$as_echo_n "checking for netdev_rx_handler_register kernel symbol... " >&6; }
+if ${ac_cv_linux_netdev_rx_handler_register+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  if grep "__ksymtab_netdev_rx_handler_register" $linux_system_map >/dev/null 2>&1; then
+        ac_cv_linux_netdev_rx_handler_register=yes
+    else ac_cv_linux_netdev_rx_handler_register=no; fi
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_linux_netdev_rx_handler_register" >&5
+$as_echo "$ac_cv_linux_netdev_rx_handler_register" >&6; }
+    if test $ac_cv_linux_netdev_rx_handler_register = yes; then
+        $as_echo "#define HAVE_LINUX_NETDEV_RX_HANDLER_REGISTER 1" >>confdefs.h
+
+    fi
 
     CC="$save_cc"
     CXX="$save_cxx"

diff --git a/configure.in b/configure.in
@@ -1458,6 +1458,13 @@ void f1(int64_t) { // will fail if long long and int64_t are the same type
 	AC_DEFINE([HAVE_NETDEV_USES_TRAILER_TAGS], [1], [Define if you have the netdev_uses_trailer_tags function.])
     fi
 
+    AC_CACHE_CHECK(for netdev_rx_handler_register kernel symbol, ac_cv_linux_netdev_rx_handler_register,
+    [if grep "__ksymtab_netdev_rx_handler_register" $linux_system_map >/dev/null 2>&1; then
+        ac_cv_linux_netdev_rx_handler_register=yes
+    else ac_cv_linux_netdev_rx_handler_register=no; fi])
+    if test $ac_cv_linux_netdev_rx_handler_register = yes; then
+        AC_DEFINE(HAVE_LINUX_NETDEV_RX_HANDLER_REGISTER)
+    fi
 
     CC="$save_cc"
     CXX="$save_cxx"

diff --git a/elements/linuxmodule/anydevice.cc b/elements/linuxmodule/anydevice.cc
@@ -22,6 +22,7 @@
 #include <click/config.h>
 #include <click/glue.hh>
 #include "anydevice.hh"
+#include "fromdevice.hh"
 #include <click/args.hh>
 #include <click/error.hh>
 #include <click/handlercall.hh>
@@ -120,31 +121,44 @@ AnyDevice::alter_promiscuity(int delta)
 #endif
 }
 
-#if CLICK_FROMDEVICE_USE_BRIDGE
 void
 AnyDevice::alter_from_device(int delta)
 {
+#if HAVE_CLICK_KERNEL
+    (void) delta;
+#elif CLICK_FROMDEVICE_USE_BRIDGE
     if (!_dev)
 	return;
     fake_bridge *fb = reinterpret_cast<fake_bridge *>(_dev->br_port);
-    if (fb && fb->magic != fake_bridge::click_magic) {
+    if (fb && fb->magic != fake_bridge::click_magic)
 	printk("<1>%s: appears to be owned by the bridge module!", _devname.c_str());
-	return;
-    }
-
-    if (delta < 0 && fb && atomic_dec_and_test(&fb->refcount)) {
+    else if (delta < 0 && fb && atomic_dec_and_test(&fb->refcount)) {
 	delete fb;
 	rcu_assign_pointer(_dev->br_port, NULL);
-    } else if (delta > 0 && fb)
-	atomic_inc(&fb->refcount);
-    else if (delta > 0) {
+    } else if (delta > 0 && !fb) {
 	fb = new fake_bridge;
 	fb->magic = fake_bridge::click_magic;
 	atomic_set(&fb->refcount, 1);
 	rcu_assign_pointer(_dev->br_port, reinterpret_cast<struct net_bridge_port *>(fb));
-    }
-}
+    } else if (delta > 0)
+	atomic_inc(&fb->refcount);
+#elif HAVE_LINUX_NETDEV_RX_HANDLER_REGISTER
+    if (!_dev)
+	return;
+    rtnl_lock();
+    if (_dev->rx_handler && _dev->rx_handler != click_fromdevice_rx_handler)
+	printk("<1>%s: rx_handler already set!", _devname.c_str());
+    else if (delta < 0 && !_dev->rx_handler_data)
+	netdev_rx_handler_unregister(_dev);
+    else if (delta > 0 && !_dev->rx_handler)
+	netdev_rx_handler_register(_dev, click_fromdevice_rx_handler, 0);
+    else
+	_dev->rx_handler_data = (void *) ((uintptr_t) _dev->rx_handler_data + delta);
+    rtnl_unlock();
+#else
+    (void) delta;
 #endif
+}
 
 net_device *
 AnyDevice::lookup_device(ErrorHandler *errh)
@@ -204,10 +218,8 @@ AnyDevice::set_device(net_device *dev, AnyDeviceMap *adm, int flags)
     if (_dev && _timestamp)
 	net_enable_timestamp();
 #endif
-#if CLICK_FROMDEVICE_USE_BRIDGE
     if (_dev && (flags & anydev_from_device))
 	alter_from_device(1);
-#endif
     _carrier_ok = (_dev && netif_carrier_ok(_dev));
 
     // call going-up notifiers
@@ -228,10 +240,8 @@ AnyDevice::clear_device(AnyDeviceMap *adm, int flags)
     if (_dev && _timestamp)
 	net_disable_timestamp();
 #endif
-#if CLICK_FROMDEVICE_USE_BRIDGE
     if (_dev && (flags & anydev_from_device))
 	alter_from_device(-1);
-#endif
     if (adm && _in_map)
 	adm->remove(this, flags & anydev_change);
     if (_dev)
@@ -364,5 +374,5 @@ AnyDevice::get_by_ether_address(const String &name, Element *context)
     return dev;
 }
 
-ELEMENT_REQUIRES(linuxmodule)
+ELEMENT_REQUIRES(linuxmodule FromDevice)
 ELEMENT_PROVIDES(AnyDevice)
diff --git a/elements/linuxmodule/anydevice.hh b/elements/linuxmodule/anydevice.hh
@@ -73,7 +73,8 @@ DECLARE_PER_CPU(sk_buff *, click_device_unreceivable_sk_buff);
 
 #if !HAVE_CLICK_KERNEL && LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 24) \
 	&& LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 36) \
-	&& (defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE))
+	&& (defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE)) \
+	&& !HAVE_LINUX_NETDEV_RX_HANDLER_REGISTER
 # define CLICK_FROMDEVICE_USE_BRIDGE
 #endif
 
@@ -107,9 +108,7 @@ class AnyDevice : public Element { public:
     };
     void set_device(net_device *dev, AnyDeviceMap *map, int flags);
     void clear_device(AnyDeviceMap *map, int flags);
-#if CLICK_FROMDEVICE_USE_BRIDGE
     void alter_from_device(int delta);
-#endif
 
     static inline net_device *get_by_name(const char *name) {
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 24)

diff --git a/elements/linuxmodule/fromdevice.cc b/elements/linuxmodule/fromdevice.cc
@@ -9,7 +9,7 @@
  * Copyright (c) 1999-2000 Massachusetts Institute of Technology
  * Copyright (c) 2000 Mazu Networks, Inc.
  * Copyright (c) 2001 International Computer Science Institute
- * Copyright (c) 2007-2008 Regents of the University of California
+ * Copyright (c) 2007-2011 Regents of the University of California
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
@@ -31,13 +31,23 @@
 #include <click/standard/scheduleinfo.hh>
 #include <click/straccum.hh>
 
+#include <click/cxxprotect.h>
+CLICK_CXX_PROTECT
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 24)
+# include <linux/rtnetlink.h>
+#endif
+CLICK_CXX_UNPROTECT
+#include <click/cxxunprotect.h>
+
 static AnyDeviceMap from_device_map;
 static int registered_readers;
 #if HAVE_CLICK_KERNEL
 static struct notifier_block packet_notifier;
 #endif
+#if CLICK_FROMDEVICE_USE_BRIDGE
 static struct notifier_block device_notifier_early;
-static struct notifier_block device_notifier_late;
+#endif
+static struct notifier_block device_notifier;
 
 #if CLICK_FROMDEVICE_USE_BRIDGE
 # include <click/cxxprotect.h>
@@ -54,9 +64,9 @@ static int packet_notifier_hook(struct notifier_block *nb, unsigned long val, vo
 #elif CLICK_FROMDEVICE_USE_BRIDGE
 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_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);
+#endif
+static int device_notifier_hook(struct notifier_block *nb, unsigned long val, void *v);
 }
 
 void
@@ -68,15 +78,16 @@ FromDevice::static_initialize()
     packet_notifier.priority = 1;
     packet_notifier.next = 0;
 #endif
+#if CLICK_FROMDEVICE_USE_BRIDGE
     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);
+#endif
+    device_notifier.notifier_call = device_notifier_hook;
+    device_notifier.priority = INT_MIN;
+    device_notifier.next = 0;
+    register_netdevice_notifier(&device_notifier);
 }
 
 void
@@ -88,9 +99,9 @@ FromDevice::static_cleanup()
 #elif CLICK_FROMDEVICE_USE_BRIDGE
     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_late);
     unregister_netdevice_notifier(&device_notifier_early);
+#endif
+    unregister_netdevice_notifier(&device_notifier);
 }
 
 FromDevice::FromDevice()
@@ -151,7 +162,7 @@ FromDevice::initialize(ErrorHandler *errh)
     if (ifindex() >= 0) {
 	void *&used = router()->force_attachment("device_reader_" + String(ifindex()));
 	if (used)
-	    return errh->error("duplicate reader for device '%s'", _devname.c_str());
+	    return errh->error("device %<%s%> has duplicate reader", _devname.c_str());
 	used = this;
     }
 
@@ -162,6 +173,8 @@ FromDevice::initialize(ErrorHandler *errh)
 #elif CLICK_FROMDEVICE_USE_BRIDGE
 	real_br_handle_frame_hook = br_handle_frame_hook;
 	br_handle_frame_hook = click_br_handle_frame_hook;
+#elif HAVE_LINUX_NETDEV_RX_HANDLER_REGISTER
+	/* OK */
 #else
 	errh->warning("can't get packets: not compiled for a Click kernel");
 #endif
@@ -188,13 +201,13 @@ FromDevice::cleanup(CleanupStage stage)
 {
     if (stage >= CLEANUP_INITIALIZED) {
 	--registered_readers;
+	if (registered_readers == 0) {
 #if HAVE_CLICK_KERNEL
-	if (registered_readers == 0)
 	    unregister_net_in(&packet_notifier);
 #elif CLICK_FROMDEVICE_USE_BRIDGE
-	if (registered_readers == 0)
 	    br_handle_frame_hook = real_br_handle_frame_hook;
 #endif
+	}
     }
 
     clear_device(&from_device_map, anydev_from_device);
@@ -251,6 +264,7 @@ packet_notifier_hook(struct notifier_block *nb, unsigned long backlog_len, void
     from_device_map.unlock(false, lock_flags);
     return (stolen ? NOTIFY_STOP_MASK : 0);
 }
+
 #elif CLICK_FROMDEVICE_USE_BRIDGE
 static struct sk_buff *
 click_br_handle_frame_hook(struct net_bridge_port *p, struct sk_buff *skb)
@@ -276,8 +290,33 @@ click_br_handle_frame_hook(struct net_bridge_port *p, struct sk_buff *skb)
     else
 	return skb;
 }
+
+#elif HAVE_LINUX_NETDEV_RX_HANDLER_REGISTER
+struct sk_buff *
+click_fromdevice_rx_handler(struct sk_buff *skb)
+{
+# if CLICK_DEVICE_UNRECEIVABLE_SK_BUFF
+    if (__get_cpu_var(click_device_unreceivable_sk_buff) == skb)
+	// This packet is being passed to Linux by ToHost.
+	return skb;
+# endif
+
+    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);
+    from_device_map.unlock(false, lock_flags);
+    if (stolen)
+	return 0;
+    else
+	return skb;
+}
 #endif
 
+#if CLICK_FROMDEVICE_USE_BRIDGE
 static int
 device_notifier_hook_early(struct notifier_block *nb, unsigned long flags, void *v)
 {
@@ -294,19 +333,17 @@ device_notifier_hook_early(struct notifier_block *nb, unsigned long flags, void
 	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++) {
-#if CLICK_FROMDEVICE_USE_BRIDGE
+	for (i = 0; i < nes; i++)
 	    ((FromDevice*)(es[i]))->alter_from_device(-1);
-#endif
-	}
 	from_device_map.unlock(true, lock_flags);
     }
 
     return 0;
 }
+#endif
 
 static int
-device_notifier_hook_late(struct notifier_block *nb, unsigned long flags, void *v)
+device_notifier_hook(struct notifier_block *nb, unsigned long flags, void *v)
 {
 #ifdef NETDEV_GOING_DOWN
     if (flags == NETDEV_GOING_DOWN)
@@ -319,12 +356,8 @@ device_notifier_hook_late(struct notifier_block *nb, unsigned long flags, void *
 	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++) {
-#if CLICK_FROMDEVICE_USE_BRIDGE
-	    ((FromDevice*)(es[i]))->alter_from_device(1);
-#endif
+	for (int i = 0; i < nes; i++)
 	    ((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;

diff --git a/elements/linuxmodule/fromdevice.hh b/elements/linuxmodule/fromdevice.hh
@@ -190,4 +190,8 @@ class FromDevice : public AnyTaskDevice, public Storage { public:
 
 };
 
+extern "C" {
+struct sk_buff *click_fromdevice_rx_handler(struct sk_buff *skb);
+}
+
 #endif
-Original file line number
+Diff line change
@@ Expand Up @@
     };
+    extern "C" {
+    struct sk_buff *click_fromdevice_rx_handler(struct sk_buff *skb);
+    }
     #endif