[PATCH 2.6.5] (1/9) bridge - include file cleanup

[PATCH 2.6.5] (1/9) bridge - include file cleanup

[Stephen Hemminger]

Cleanup some of the include file's in the bridge code.
* if_bridge.h defines net_bridge, but not needed as part of the API.
* get rid of places that include if_bridge.h and uaccess.h but don't
  actually do API work.


diff -Nru a/include/linux/if_bridge.h b/include/linux/if_bridge.h
--- a/include/linux/if_bridge.h	Tue Apr 13 14:44:58 2004
+++ b/include/linux/if_bridge.h	Tue Apr 13 14:44:58 2004
@@ -99,9 +99,6 @@
 
 #include <linux/netdevice.h>
 
-struct net_bridge;
-struct net_bridge_port;
-
 extern void brioctl_set(int (*ioctl_hook)(unsigned long));
 extern int (*br_handle_frame_hook)(struct sk_buff *skb);
 extern int (*br_should_route_hook)(struct sk_buff **pskb);
diff -Nru a/net/bridge/br.c b/net/bridge/br.c
--- a/net/bridge/br.c	Tue Apr 13 14:44:58 2004
+++ b/net/bridge/br.c	Tue Apr 13 14:44:58 2004
@@ -20,8 +20,7 @@
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/init.h>
-#include <linux/if_bridge.h>
-#include <asm/uaccess.h>
+
 #include "br_private.h"
 
 #if defined(CONFIG_ATM_LANE) || defined(CONFIG_ATM_LANE_MODULE)
diff -Nru a/net/bridge/br_device.c b/net/bridge/br_device.c
--- a/net/bridge/br_device.c	Tue Apr 13 14:44:58 2004
+++ b/net/bridge/br_device.c	Tue Apr 13 14:44:58 2004
@@ -15,7 +15,6 @@
 
 #include <linux/kernel.h>
 #include <linux/netdevice.h>
-#include <linux/if_bridge.h>
 #include <linux/module.h>
 #include <asm/uaccess.h>
 #include "br_private.h"
diff -Nru a/net/bridge/br_forward.c b/net/bridge/br_forward.c
--- a/net/bridge/br_forward.c	Tue Apr 13 14:44:58 2004
+++ b/net/bridge/br_forward.c	Tue Apr 13 14:44:58 2004
@@ -15,9 +15,7 @@
 
 #include <linux/kernel.h>
 #include <linux/netdevice.h>
-#include <linux/inetdevice.h>
 #include <linux/skbuff.h>
-#include <linux/if_bridge.h>
 #include <linux/netfilter_bridge.h>
 #include "br_private.h"
 
diff -Nru a/net/bridge/br_if.c b/net/bridge/br_if.c
--- a/net/bridge/br_if.c	Tue Apr 13 14:44:58 2004
+++ b/net/bridge/br_if.c	Tue Apr 13 14:44:58 2004
@@ -15,13 +15,11 @@
 
 #include <linux/kernel.h>
 #include <linux/if_arp.h>
-#include <linux/if_bridge.h>
-#include <linux/inetdevice.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/rtnetlink.h>
 #include <net/sock.h>
-#include <asm/uaccess.h>
+
 #include "br_private.h"
 
 /* Limited to 256 ports because of STP protocol pdu */
diff -Nru a/net/bridge/br_input.c b/net/bridge/br_input.c
--- a/net/bridge/br_input.c	Tue Apr 13 14:44:58 2004
+++ b/net/bridge/br_input.c	Tue Apr 13 14:44:58 2004
@@ -16,7 +16,6 @@
 #include <linux/kernel.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
-#include <linux/if_bridge.h>
 #include <linux/netfilter_bridge.h>
 #include "br_private.h"
 
diff -Nru a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c
--- a/net/bridge/br_ioctl.c	Tue Apr 13 14:44:58 2004
+++ b/net/bridge/br_ioctl.c	Tue Apr 13 14:44:58 2004
@@ -15,7 +15,7 @@
 
 #include <linux/kernel.h>
 #include <linux/if_bridge.h>
-#include <linux/inetdevice.h>
+#include <linux/netdevice.h>
 #include <asm/uaccess.h>
 #include "br_private.h"
 
diff -Nru a/net/bridge/br_notify.c b/net/bridge/br_notify.c
--- a/net/bridge/br_notify.c	Tue Apr 13 14:44:58 2004
+++ b/net/bridge/br_notify.c	Tue Apr 13 14:44:58 2004
@@ -14,7 +14,7 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/if_bridge.h>
+
 #include "br_private.h"
 
 static int br_device_event(struct notifier_block *unused, unsigned long event, void *ptr);
diff -Nru a/net/bridge/br_stp.c b/net/bridge/br_stp.c
--- a/net/bridge/br_stp.c	Tue Apr 13 14:44:58 2004
+++ b/net/bridge/br_stp.c	Tue Apr 13 14:44:58 2004
@@ -13,9 +13,8 @@
  *	2 of the License, or (at your option) any later version.
  */
 #include <linux/kernel.h>
-#include <linux/if_bridge.h>
 #include <linux/smp_lock.h>
-#include <asm/uaccess.h>
+
 #include "br_private.h"
 #include "br_private_stp.h"
 
diff -Nru a/net/bridge/br_stp_bpdu.c b/net/bridge/br_stp_bpdu.c
--- a/net/bridge/br_stp_bpdu.c	Tue Apr 13 14:44:58 2004
+++ b/net/bridge/br_stp_bpdu.c	Tue Apr 13 14:44:58 2004
@@ -14,9 +14,8 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/if_ether.h>
-#include <linux/if_bridge.h>
 #include <linux/netfilter_bridge.h>
+
 #include "br_private.h"
 #include "br_private_stp.h"
 
diff -Nru a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
--- a/net/bridge/br_stp_if.c	Tue Apr 13 14:44:58 2004
+++ b/net/bridge/br_stp_if.c	Tue Apr 13 14:44:58 2004
@@ -14,9 +14,8 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/if_bridge.h>
 #include <linux/smp_lock.h>
-#include <asm/uaccess.h>
+
 #include "br_private.h"
 #include "br_private_stp.h"
 
diff -Nru a/net/bridge/br_stp_timer.c b/net/bridge/br_stp_timer.c
--- a/net/bridge/br_stp_timer.c	Tue Apr 13 14:44:58 2004
+++ b/net/bridge/br_stp_timer.c	Tue Apr 13 14:44:58 2004
@@ -14,9 +14,8 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/if_bridge.h>
 #include <linux/smp_lock.h>
-#include <asm/uaccess.h>
+
 #include "br_private.h"
 #include "br_private_stp.h"
 

[PATCH 2.6.5] (2/9) bridge - rmmod race

[Stephen Hemminger]

Fix observed race between removing bridge module and ip packets
in flight.  Need to remove the hook last, after all bridges are gone
not the other way around.

diff -Nru a/net/bridge/br.c b/net/bridge/br.c
--- a/net/bridge/br.c	Mon Apr 12 16:09:56 2004
+++ b/net/bridge/br.c	Mon Apr 12 16:09:56 2004
@@ -54,16 +54,17 @@
 #endif
 	unregister_netdevice_notifier(&br_device_notifier);
 	brioctl_set(NULL);
-	br_handle_frame_hook = NULL;
+
+	br_cleanup_bridges();
+
+	synchronize_net();
 
 #if defined(CONFIG_ATM_LANE) || defined(CONFIG_ATM_LANE_MODULE)
 	br_fdb_get_hook = NULL;
 	br_fdb_put_hook = NULL;
 #endif
 
-	br_cleanup_bridges();
-
-	synchronize_net();
+	br_handle_frame_hook = NULL;
 }
 
 EXPORT_SYMBOL(br_should_route_hook);

[PATCH 2.6.5] (3/9) bridge - jiffies_to_clock

[Stephen Hemminger]

Rather than doing the math directly, use jiffies_to_clock functions
to convert from user hz to jiffies.

diff -Nru a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
--- a/net/bridge/br_fdb.c	Mon Apr 12 16:10:09 2004
+++ b/net/bridge/br_fdb.c	Mon Apr 12 16:10:09 2004
@@ -16,6 +16,7 @@
 #include <linux/kernel.h>
 #include <linux/spinlock.h>
 #include <linux/if_bridge.h>
+#include <linux/times.h>
 #include <asm/atomic.h>
 #include <asm/uaccess.h>
 #include "br_private.h"
@@ -48,7 +49,7 @@
 	ent->compat_port_no = ent->port_no;
 	ent->is_local = f->is_local;
 	ent->ageing_timer_value = f->is_static ? 0 
-		: ((jiffies - f->ageing_timer) * USER_HZ) / HZ;
+		: jiffies_to_clock_t(jiffies - f->ageing_timer);
 }
 
 static __inline__ int br_mac_hash(const unsigned char *mac)
diff -Nru a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c
--- a/net/bridge/br_ioctl.c	Mon Apr 12 16:10:09 2004
+++ b/net/bridge/br_ioctl.c	Mon Apr 12 16:10:09 2004
@@ -16,26 +16,15 @@
 #include <linux/kernel.h>
 #include <linux/if_bridge.h>
 #include <linux/netdevice.h>
+#include <linux/times.h>
 #include <asm/uaccess.h>
 #include "br_private.h"
 
-/* import values in USER_HZ  */
-static inline unsigned long user_to_ticks(unsigned long utick)
-{
-	return (utick * HZ) / USER_HZ;
-}
-
-/* export values in USER_HZ */
-static inline unsigned long ticks_to_user(unsigned long tick)
-{
-	return (tick * USER_HZ) / HZ;
-}
-
 /* Report time remaining in user HZ  */
 static unsigned long timer_residue(const struct timer_list *timer)
 {
-	return ticks_to_user(timer_pending(timer) 
-			     ? (timer->expires - jiffies) : 0);
+	return timer_pending(timer) 
+		? jiffies_to_clock_t(timer->expires - jiffies) : 0;
 }
 
 int br_ioctl_device(struct net_bridge *br, unsigned int cmd,
@@ -79,17 +68,17 @@
 		memcpy(&b.designated_root, &br->designated_root, 8);
 		memcpy(&b.bridge_id, &br->bridge_id, 8);
 		b.root_path_cost = br->root_path_cost;
-		b.max_age = ticks_to_user(br->max_age);
-		b.hello_time = ticks_to_user(br->hello_time);
+		b.max_age = jiffies_to_clock_t(br->max_age);
+		b.hello_time = jiffies_to_clock_t(br->hello_time);
 		b.forward_delay = br->forward_delay;
 		b.bridge_max_age = br->bridge_max_age;
 		b.bridge_hello_time = br->bridge_hello_time;
-		b.bridge_forward_delay = ticks_to_user(br->bridge_forward_delay);
+		b.bridge_forward_delay = jiffies_to_clock_t(br->bridge_forward_delay);
 		b.topology_change = br->topology_change;
 		b.topology_change_detected = br->topology_change_detected;
 		b.root_port = br->root_port;
 		b.stp_enabled = br->stp_enabled;
-		b.ageing_time = ticks_to_user(br->ageing_time);
+		b.ageing_time = jiffies_to_clock_t(br->ageing_time);
 		b.hello_timer_value = timer_residue(&br->hello_timer);
 		b.tcn_timer_value = timer_residue(&br->tcn_timer);
 		b.topology_change_timer_value = timer_residue(&br->topology_change_timer);
@@ -126,7 +115,7 @@
 			return -EPERM;
 
 		spin_lock_bh(&br->lock);
-		br->bridge_forward_delay = user_to_ticks(arg0);
+		br->bridge_forward_delay = clock_t_to_jiffies(arg0);
 		if (br_is_root_bridge(br))
 			br->forward_delay = br->bridge_forward_delay;
 		spin_unlock_bh(&br->lock);
@@ -137,7 +126,7 @@
 			return -EPERM;
 
 		spin_lock_bh(&br->lock);
-		br->bridge_hello_time = user_to_ticks(arg0);
+		br->bridge_hello_time = clock_t_to_jiffies(arg0);
 		if (br_is_root_bridge(br))
 			br->hello_time = br->bridge_hello_time;
 		spin_unlock_bh(&br->lock);
@@ -148,7 +137,7 @@
 			return -EPERM;
 
 		spin_lock_bh(&br->lock);
-		br->bridge_max_age = user_to_ticks(arg0);
+		br->bridge_max_age = clock_t_to_jiffies(arg0);
 		if (br_is_root_bridge(br))
 			br->max_age = br->bridge_max_age;
 		spin_unlock_bh(&br->lock);
@@ -158,7 +147,7 @@
 		if (!capable(CAP_NET_ADMIN))
 			return -EPERM;
 
-		br->ageing_time = user_to_ticks(arg0);
+		br->ageing_time = clock_t_to_jiffies(arg0);
 		return 0;
 
 	case BRCTL_GET_PORT_INFO:

[PATCH 2.6.5] (4/9) bridge - use ethtool to get port speed

[Stephen Hemminger]

The bridge code needs to keep track of a cost estimate for each
port in the bridge.  Instead of a hack based on device name, try
and use ethtool to get port speed from device.  This has been tested
on e100 (uses ethtool_ops) and e1000 (does ethtool the hard way)
and dummy (no ethtool).

Need to export dev_ethtool() to allow bridge module to get to
it easily.

Code takes care to maintain same locking and semantics as if ioctl
was being done from application.

diff -Nru a/net/bridge/br_if.c b/net/bridge/br_if.c
--- a/net/bridge/br_if.c	Mon Apr 12 16:10:23 2004
+++ b/net/bridge/br_if.c	Mon Apr 12 16:10:23 2004
@@ -14,6 +14,8 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/netdevice.h>
+#include <linux/ethtool.h>
 #include <linux/if_arp.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -25,18 +27,55 @@
 /* Limited to 256 ports because of STP protocol pdu */
 #define  BR_MAX_PORTS	256
 
+/*
+ * Determine initial path cost based on speed.
+ * using recommendations from 802.1d standard
+ *
+ * Need to simulate user ioctl because not all device's that support
+ * ethtool, use ethtool_ops.  Also, since driver might sleep need to
+ * not be holding any locks.
+ */
 static int br_initial_port_cost(struct net_device *dev)
 {
+
+	struct ethtool_cmd ecmd = { ETHTOOL_GSET };
+	struct ifreq ifr;
+	mm_segment_t old_fs;
+	int err;
+
+	strncpy(ifr.ifr_name, dev->name, IFNAMSIZ);
+	ifr.ifr_data = (void *) &ecmd;
+
+	old_fs = get_fs();
+	set_fs(KERNEL_DS);
+	err = dev_ethtool(&ifr);
+	set_fs(old_fs);
+	
+	if (!err) {
+		switch(ecmd.speed) {
+		case SPEED_100:
+			return 19;
+		case SPEED_1000:
+			return 4;
+		case SPEED_10000:
+			return 2;
+		case SPEED_10:
+			return 100;
+		default:
+			pr_info("bridge: can't decode speed from %s: %d\n",
+				dev->name, ecmd.speed);
+			return 100;
+		}
+	}
+
+	/* Old silly heuristics based on name */
 	if (!strncmp(dev->name, "lec", 3))
 		return 7;
 
-	if (!strncmp(dev->name, "eth", 3))
-		return 100;			/* FIXME handle 100Mbps */
-
 	if (!strncmp(dev->name, "plip", 4))
 		return 2500;
 
-	return 100;
+	return 100;	/* assume old 10Mbps */
 }
 
 static void destroy_nbp(void *arg)
@@ -147,7 +186,9 @@
 }
 
 /* called under bridge lock */
-static struct net_bridge_port *new_nbp(struct net_bridge *br, struct net_device *dev)
+static struct net_bridge_port *new_nbp(struct net_bridge *br, 
+				       struct net_device *dev,
+				       unsigned long cost)
 {
 	int index;
 	struct net_bridge_port *p;
@@ -162,16 +203,15 @@
 
 	memset(p, 0, sizeof(*p));
 	p->br = br;
+	dev_hold(dev);
 	p->dev = dev;
-	p->path_cost = br_initial_port_cost(dev);
+	p->path_cost = cost;
 	p->priority = 0x80;
 	dev->br_port = p;
 	p->port_no = index;
 	br_init_port(p);
 	p->state = BR_STATE_DISABLED;
 
-	list_add_rcu(&p->list, &br->port_list);
-
 	return p;
 }
 
@@ -216,13 +256,11 @@
 	return ret;
 }
 
-/* called under bridge lock */
 int br_add_if(struct net_bridge *br, struct net_device *dev)
 {
 	struct net_bridge_port *p;
-
-	if (dev->br_port != NULL)
-		return -EBUSY;
+	unsigned long cost;
+	int err = 0;
 
 	if (dev->flags & IFF_LOOPBACK || dev->type != ARPHRD_ETHER)
 		return -EINVAL;
@@ -230,34 +268,46 @@
 	if (dev->hard_start_xmit == br_dev_xmit)
 		return -ELOOP;
 
-	dev_hold(dev);
-	p = new_nbp(br, dev);
-	if (IS_ERR(p)) {
-		dev_put(dev);
-		return PTR_ERR(p);
-	}
+	cost = br_initial_port_cost(dev);
 
-	dev_set_promiscuity(dev, 1);
+	spin_lock_bh(&br->lock);
+	if (dev->br_port != NULL)
+		err = -EBUSY;
+
+	else if (IS_ERR(p = new_nbp(br, dev, cost)))
+		err = PTR_ERR(p);
 
-	br_stp_recalculate_bridge_id(br);
-	br_fdb_insert(br, p, dev->dev_addr, 1);
-	if ((br->dev->flags & IFF_UP) && (dev->flags & IFF_UP))
-		br_stp_enable_port(p);
+	else {
+		dev_set_promiscuity(dev, 1);
 
-	return 0;
+		list_add_rcu(&p->list, &br->port_list);
+
+		br_stp_recalculate_bridge_id(br);
+		br_fdb_insert(br, p, dev->dev_addr, 1);
+		if ((br->dev->flags & IFF_UP) && (dev->flags & IFF_UP))
+			br_stp_enable_port(p);
+
+	}
+	spin_unlock_bh(&br->lock);
+	return err;
 }
 
-/* called under bridge lock */
 int br_del_if(struct net_bridge *br, struct net_device *dev)
 {
 	struct net_bridge_port *p;
+	int err = 0;
 
-	if ((p = dev->br_port) == NULL || p->br != br)
-		return -EINVAL;
+	spin_lock_bh(&br->lock);
+	p = dev->br_port;
+	if (!p || p->br != br) 
+		err = -EINVAL;
+	else {
+		del_nbp(p);
+		br_stp_recalculate_bridge_id(br);
+	}
+	spin_unlock_bh(&br->lock);
 
-	del_nbp(p);
-	br_stp_recalculate_bridge_id(br);
-	return 0;
+	return err;
 }
 
 int br_get_bridge_ifindices(int *indices, int num)
diff -Nru a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c
--- a/net/bridge/br_ioctl.c	Mon Apr 12 16:10:23 2004
+++ b/net/bridge/br_ioctl.c	Mon Apr 12 16:10:23 2004
@@ -48,12 +48,10 @@
 		if (dev == NULL)
 			return -EINVAL;
 
-		spin_lock_bh(&br->lock);
 		if (cmd == BRCTL_ADD_IF)
 			ret = br_add_if(br, dev);
 		else
 			ret = br_del_if(br, dev);
-		spin_unlock_bh(&br->lock);
 
 		dev_put(dev);
 		return ret;
diff -Nru a/net/core/ethtool.c b/net/core/ethtool.c
--- a/net/core/ethtool.c	Mon Apr 12 16:10:23 2004
+++ b/net/core/ethtool.c	Mon Apr 12 16:10:23 2004
@@ -740,6 +740,7 @@
 	return -EOPNOTSUPP;
 }
 
+EXPORT_SYMBOL(dev_ethtool);
 EXPORT_SYMBOL(ethtool_op_get_link);
 EXPORT_SYMBOL(ethtool_op_get_sg);
 EXPORT_SYMBOL(ethtool_op_get_tso);

[PATCH 2.6.5] (5/9) bridge - multicast address as const

[Stephen Hemminger]

Trivial change.  For places where bridge multicast address is defined,
or compared use constant.

diff -Nru a/net/bridge/br_input.c b/net/bridge/br_input.c
--- a/net/bridge/br_input.c	Mon Apr 12 16:10:36 2004
+++ b/net/bridge/br_input.c	Mon Apr 12 16:10:36 2004
@@ -19,7 +19,7 @@
 #include <linux/netfilter_bridge.h>
 #include "br_private.h"
 
-unsigned char bridge_ula[6] = { 0x01, 0x80, 0xc2, 0x00, 0x00, 0x00 };
+const unsigned char bridge_ula[6] = { 0x01, 0x80, 0xc2, 0x00, 0x00, 0x00 };
 
 static int br_pass_frame_up_finish(struct sk_buff *skb)
 {
diff -Nru a/net/bridge/br_private.h b/net/bridge/br_private.h
--- a/net/bridge/br_private.h	Mon Apr 12 16:10:36 2004
+++ b/net/bridge/br_private.h	Mon Apr 12 16:10:36 2004
@@ -111,7 +111,7 @@
 };
 
 extern struct notifier_block br_device_notifier;
-extern unsigned char bridge_ula[6];
+extern const unsigned char bridge_ula[6];
 
 /* called under bridge lock */
 static inline int br_is_root_bridge(const struct net_bridge *br)
diff -Nru a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
--- a/net/bridge/br_stp_if.c	Mon Apr 12 16:10:36 2004
+++ b/net/bridge/br_stp_if.c	Mon Apr 12 16:10:36 2004
@@ -110,7 +110,8 @@
 }
 
 /* called under bridge lock */
-static void br_stp_change_bridge_id(struct net_bridge *br, unsigned char *addr)
+static void br_stp_change_bridge_id(struct net_bridge *br, 
+				    const unsigned char *addr)
 {
 	unsigned char oldaddr[6];
 	struct net_bridge_port *p;
@@ -137,15 +138,13 @@
 		br_become_root_bridge(br);
 }
 
-static unsigned char br_mac_zero[6];
+static const unsigned char br_mac_zero[6];
 
 /* called under bridge lock */
 void br_stp_recalculate_bridge_id(struct net_bridge *br)
 {
-	unsigned char *addr;
+	const unsigned char *addr = br_mac_zero;
 	struct net_bridge_port *p;
-
-	addr = br_mac_zero;
 
 	list_for_each_entry(p, &br->port_list, list) {
 		if (addr == br_mac_zero ||

[PATCH 2.6.5] (6/9) bridge - forwarding database changes

[Stephen Hemminger]

Make forwarding database more robust.  
  + Don't insert invalid ether address,
  + Report errors back so adding an interface to bridge can fail
  + get rid of unneeded explicit pads in data structure
  + replace bitfields with byte's for simple booleans.

diff -Nru a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
--- a/net/bridge/br_fdb.c	Mon Apr 12 16:10:50 2004
+++ b/net/bridge/br_fdb.c	Mon Apr 12 16:10:50 2004
@@ -17,6 +17,7 @@
 #include <linux/spinlock.h>
 #include <linux/if_bridge.h>
 #include <linux/times.h>
+#include <linux/etherdevice.h>
 #include <asm/atomic.h>
 #include <asm/uaccess.h>
 #include "br_private.h"
@@ -243,12 +244,16 @@
 	return num;
 }
 
-void br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
-		   const unsigned char *addr, int is_local)
+int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
+		  const unsigned char *addr, int is_local)
 {
 	struct hlist_node *h;
 	struct net_bridge_fdb_entry *fdb;
 	int hash = br_mac_hash(addr);
+	int ret = 0;
+
+	if (!is_valid_ether_addr(addr))
+		return -EADDRNOTAVAIL;
 
 	write_lock_bh(&br->hash_lock);
 	hlist_for_each(h, &br->hash[hash]) {
@@ -264,6 +269,7 @@
 					printk(KERN_WARNING "%s: received packet with "
 					       " own address as source address\n",
 					       source->dev->name);
+				ret = -EEXIST;
 				goto out;
 			}
 
@@ -278,8 +284,10 @@
 	}
 
 	fdb = kmalloc(sizeof(*fdb), GFP_ATOMIC);
-	if (fdb == NULL) 
+	if (unlikely(fdb == NULL)) {
+		ret = -ENOMEM;
 		goto out;
+	}
 
 	memcpy(fdb->addr.addr, addr, ETH_ALEN);
 	atomic_set(&fdb->use_count, 1);
@@ -298,4 +306,6 @@
 	list_add_tail(&fdb->age_list, &br->age_list);
  out:
 	write_unlock_bh(&br->hash_lock);
+
+	return ret;
 }
diff -Nru a/net/bridge/br_if.c b/net/bridge/br_if.c
--- a/net/bridge/br_if.c	Mon Apr 12 16:10:50 2004
+++ b/net/bridge/br_if.c	Mon Apr 12 16:10:50 2004
@@ -277,13 +277,15 @@
 	else if (IS_ERR(p = new_nbp(br, dev, cost)))
 		err = PTR_ERR(p);
 
+ 	else if ((err = br_fdb_insert(br, p, dev->dev_addr, 1)))
+ 		 destroy_nbp(p);
+ 
 	else {
 		dev_set_promiscuity(dev, 1);
 
 		list_add_rcu(&p->list, &br->port_list);
 
 		br_stp_recalculate_bridge_id(br);
-		br_fdb_insert(br, p, dev->dev_addr, 1);
 		if ((br->dev->flags & IFF_UP) && (dev->flags & IFF_UP))
 			br_stp_enable_port(p);
 
diff -Nru a/net/bridge/br_private.h b/net/bridge/br_private.h
--- a/net/bridge/br_private.h	Mon Apr 12 16:10:50 2004
+++ b/net/bridge/br_private.h	Mon Apr 12 16:10:50 2004
@@ -37,7 +37,6 @@
 struct mac_addr
 {
 	unsigned char	addr[6];
-	unsigned char	pad[2];
 };
 
 struct net_bridge_fdb_entry
@@ -48,8 +47,8 @@
 	atomic_t			use_count;
 	unsigned long			ageing_timer;
 	mac_addr			addr;
-	unsigned			is_local:1;
-	unsigned			is_static:1;
+	unsigned char			is_local;
+	unsigned char			is_static;
 };
 
 struct net_bridge_port
@@ -135,10 +134,10 @@
 extern void br_fdb_put(struct net_bridge_fdb_entry *ent);
 extern int  br_fdb_get_entries(struct net_bridge *br, void __user *buf,
 			       int maxnum, int offset);
-extern void br_fdb_insert(struct net_bridge *br,
-			  struct net_bridge_port *source,
-			  const unsigned char *addr,
-			  int is_local);
+extern int br_fdb_insert(struct net_bridge *br,
+			 struct net_bridge_port *source,
+			 const unsigned char *addr,
+			 int is_local);
 
 /* br_forward.c */
 extern void br_deliver(const struct net_bridge_port *to,

[PATCH 2.6.5] (7/9) bridge - STP unsigned fields

[Stephen Hemminger]

Use correct types for fields related to spanning tree protocols.
  * costs are 32 bit unsigned
  * ports are 16 bit unsigned
  * booleans are bytes rather than bitfield
  * arrange for better packing

diff -Nru a/net/bridge/br_private.h b/net/bridge/br_private.h
--- a/net/bridge/br_private.h	Mon Apr 12 16:11:03 2004
+++ b/net/bridge/br_private.h	Mon Apr 12 16:11:03 2004
@@ -56,19 +56,19 @@
 	struct net_bridge		*br;
 	struct net_device		*dev;
 	struct list_head		list;
-	__u8				port_no;
-	__u8				priority;
 
 	/* STP */
+	u8				priority;
+	u8				state;
+	u16				port_no;
+	unsigned char			topology_change_ack;
+	unsigned char			config_pending;
 	port_id				port_id;
-	int				state;
-	int				path_cost;
+	port_id				designated_port;
 	bridge_id			designated_root;
-	int				designated_cost;
 	bridge_id			designated_bridge;
-	port_id				designated_port;
-	unsigned			topology_change_ack:1;
-	unsigned			config_pending:1;
+	u32				path_cost;
+	u32				designated_cost;
 
 	struct timer_list		forward_delay_timer;
 	struct timer_list		hold_timer;
@@ -88,25 +88,25 @@
 
 	/* STP */
 	bridge_id			designated_root;
-	int				root_path_cost;
-	int				root_port;
-	int				max_age;
-	int				hello_time;
-	int				forward_delay;
 	bridge_id			bridge_id;
-	int				bridge_max_age;
-	int				bridge_hello_time;
-	int				bridge_forward_delay;
-	unsigned			stp_enabled:1;
-	unsigned			topology_change:1;
-	unsigned			topology_change_detected:1;
+	u32				root_path_cost;
+	unsigned long			max_age;
+	unsigned long			hello_time;
+	unsigned long			forward_delay;
+	unsigned long			bridge_max_age;
+	unsigned long			ageing_time;
+	unsigned long			bridge_hello_time;
+	unsigned long			bridge_forward_delay;
+
+	u16				root_port;
+	unsigned char			stp_enabled;
+	unsigned char			topology_change;
+	unsigned char			topology_change_detected;
 
 	struct timer_list		hello_timer;
 	struct timer_list		tcn_timer;
 	struct timer_list		topology_change_timer;
 	struct timer_list		gc_timer;
-
-	int				ageing_time;
 };
 
 extern struct notifier_block br_device_notifier;
@@ -185,7 +185,7 @@
 /* br_stp.c */
 extern void br_log_state(const struct net_bridge_port *p);
 extern struct net_bridge_port *br_get_port(struct net_bridge *br,
-				    int port_no);
+				    	   u16 port_no);
 extern void br_init_port(struct net_bridge_port *p);
 extern void br_become_designated_port(struct net_bridge_port *p);
 
@@ -196,11 +196,11 @@
 extern void br_stp_disable_port(struct net_bridge_port *p);
 extern void br_stp_recalculate_bridge_id(struct net_bridge *br);
 extern void br_stp_set_bridge_priority(struct net_bridge *br,
-				int newprio);
+				       u16 newprio);
 extern void br_stp_set_port_priority(struct net_bridge_port *p,
-			      int newprio);
+				     u8 newprio);
 extern void br_stp_set_path_cost(struct net_bridge_port *p,
-			  int path_cost);
+				 u32 path_cost);
 
 /* br_stp_bpdu.c */
 extern int br_stp_handle_bpdu(struct sk_buff *skb);
diff -Nru a/net/bridge/br_stp.c b/net/bridge/br_stp.c
--- a/net/bridge/br_stp.c	Mon Apr 12 16:11:03 2004
+++ b/net/bridge/br_stp.c	Mon Apr 12 16:11:03 2004
@@ -35,7 +35,7 @@
 }
 
 /* called under bridge lock */
-struct net_bridge_port *br_get_port(struct net_bridge *br, int port_no)
+struct net_bridge_port *br_get_port(struct net_bridge *br, u16 port_no)
 {
 	struct net_bridge_port *p;
 
@@ -49,7 +49,7 @@
 
 /* called under bridge lock */
 static int br_should_become_root_port(const struct net_bridge_port *p, 
-				      int root_port)
+				      u16 root_port)
 {
 	struct net_bridge *br;
 	struct net_bridge_port *rp;
@@ -102,9 +102,7 @@
 static void br_root_selection(struct net_bridge *br)
 {
 	struct net_bridge_port *p;
-	int root_port;
-
-	root_port = 0;
+	u16 root_port = 0;
 
 	list_for_each_entry(p, &br->port_list, list) {
 		if (br_should_become_root_port(p, root_port))
diff -Nru a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
--- a/net/bridge/br_stp_if.c	Mon Apr 12 16:11:03 2004
+++ b/net/bridge/br_stp_if.c	Mon Apr 12 16:11:03 2004
@@ -158,7 +158,7 @@
 }
 
 /* called under bridge lock */
-void br_stp_set_bridge_priority(struct net_bridge *br, int newprio)
+void br_stp_set_bridge_priority(struct net_bridge *br, u16 newprio)
 {
 	struct net_bridge_port *p;
 	int wasroot;
@@ -183,7 +183,7 @@
 }
 
 /* called under bridge lock */
-void br_stp_set_port_priority(struct net_bridge_port *p, int newprio)
+void br_stp_set_port_priority(struct net_bridge_port *p, u8 newprio)
 {
 	__u16 new_port_id;
 
@@ -202,7 +202,7 @@
 }
 
 /* called under bridge lock */
-void br_stp_set_path_cost(struct net_bridge_port *p, int path_cost)
+void br_stp_set_path_cost(struct net_bridge_port *p, u32 path_cost)
 {
 	p->path_cost = path_cost;
 	br_configuration_update(p->br);

[PATCH 2.6.5] (8/9) bridge - support lots of 1k ports

[Stephen Hemminger]

Support >256 ports on a bridge.  Use the suggestion of reducing
the number of bits of priority and increasing the number of bits
for port number.

Easy to increase to even larger if necessary.

diff -Nru a/net/bridge/br_if.c b/net/bridge/br_if.c
--- a/net/bridge/br_if.c	Tue Apr 13 13:16:56 2004
+++ b/net/bridge/br_if.c	Tue Apr 13 13:16:56 2004
@@ -24,9 +24,6 @@
 
 #include "br_private.h"
 
-/* Limited to 256 ports because of STP protocol pdu */
-#define  BR_MAX_PORTS	256
-
 /*
  * Determine initial path cost based on speed.
  * using recommendations from 802.1d standard
@@ -166,23 +163,27 @@
 	return br;
 }
 
-static int free_port(struct net_bridge *br)
+/* find an available port number */
+static int find_portno(struct net_bridge *br)
 {
 	int index;
 	struct net_bridge_port *p;
-	long inuse[BR_MAX_PORTS/(sizeof(long)*8)];
+	unsigned long *inuse;
+
+	inuse = kmalloc(BITS_TO_LONGS(BR_MAX_PORTS)*sizeof(unsigned long),
+			GFP_ATOMIC);
+	if (!inuse)
+		return -ENOMEM;
 
-	/* find free port number */
-	memset(inuse, 0, sizeof(inuse));
+	CLEAR_BITMAP(inuse, BR_MAX_PORTS);
+	set_bit(0, inuse);	/* zero is reserved */
 	list_for_each_entry(p, &br->port_list, list) {
 		set_bit(p->port_no, inuse);
 	}
-
 	index = find_first_zero_bit(inuse, BR_MAX_PORTS);
-	if (index >= BR_MAX_PORTS)
-		return -EXFULL;
+	kfree(inuse);
 
-	return index;
+	return (index >= BR_MAX_PORTS) ? -EXFULL : index;
 }
 
 /* called under bridge lock */
@@ -193,7 +194,7 @@
 	int index;
 	struct net_bridge_port *p;
 	
-	index = free_port(br);
+	index = find_portno(br);
 	if (index < 0)
 		return ERR_PTR(index);
 
@@ -206,7 +207,7 @@
 	dev_hold(dev);
 	p->dev = dev;
 	p->path_cost = cost;
-	p->priority = 0x80;
+ 	p->priority = 0x8000 >> BR_PORT_BITS;
 	dev->br_port = p;
 	p->port_no = index;
 	br_init_port(p);
diff -Nru a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c
--- a/net/bridge/br_ioctl.c	Tue Apr 13 13:16:56 2004
+++ b/net/bridge/br_ioctl.c	Tue Apr 13 13:16:56 2004
@@ -91,9 +91,15 @@
 
 	case BRCTL_GET_PORT_LIST:
 	{
-		int num = arg1 ? arg1 : 256;	/* compatiablity */
-		int ret = 0;
-		int *indices;
+		int num, *indices;
+
+		num = arg1;
+		if (num < 0)
+			return -EINVAL;
+		if (num == 0)
+			num = 256;
+		if (num > BR_MAX_PORTS)
+			num = BR_MAX_PORTS;
 
 		indices = kmalloc(num*sizeof(int), GFP_KERNEL);
 		if (indices == NULL)
@@ -103,9 +109,9 @@
 
 		br_get_port_ifindices(br, indices, num);
 		if (copy_to_user((void *)arg0, indices, num*sizeof(int)))
-			ret =  -EFAULT;
+			num =  -EFAULT;
 		kfree(indices);
-		return ret;
+		return num;
 	}
 
 	case BRCTL_SET_BRIDGE_FORWARD_DELAY:
@@ -204,6 +210,9 @@
 
 		if (!capable(CAP_NET_ADMIN))
 			return -EPERM;
+
+		if (arg1 >= (1<<(16-BR_PORT_BITS)))
+			return -ERANGE;
 
 		spin_lock_bh(&br->lock);
 		if ((p = br_get_port(br, arg0)) == NULL) 
diff -Nru a/net/bridge/br_private.h b/net/bridge/br_private.h
--- a/net/bridge/br_private.h	Tue Apr 13 13:16:56 2004
+++ b/net/bridge/br_private.h	Tue Apr 13 13:16:56 2004
@@ -24,6 +24,9 @@
 
 #define BR_HOLD_TIME (1*HZ)
 
+#define BR_PORT_BITS	10
+#define BR_MAX_PORTS	(1<<BR_PORT_BITS)
+
 typedef struct bridge_id bridge_id;
 typedef struct mac_addr mac_addr;
 typedef __u16 port_id;
diff -Nru a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
--- a/net/bridge/br_stp_if.c	Tue Apr 13 13:16:56 2004
+++ b/net/bridge/br_stp_if.c	Tue Apr 13 13:16:56 2004
@@ -19,15 +19,21 @@
 #include "br_private.h"
 #include "br_private_stp.h"
 
-static inline __u16 br_make_port_id(const struct net_bridge_port *p)
+
+/* Port id is composed of priority and port number.
+ * NB: least significant bits of priority are dropped to
+ *     make room for more ports.
+ */
+static inline port_id br_make_port_id(__u8 priority, __u16 port_no)
 {
-	return (p->priority << 8) | p->port_no;
+	return ((u16)priority << BR_PORT_BITS) 
+		| (port_no & ((1<<BR_PORT_BITS)-1));
 }
 
 /* called under bridge lock */
 void br_init_port(struct net_bridge_port *p)
 {
-	p->port_id = br_make_port_id(p);
+	p->port_id = br_make_port_id(p->priority, p->port_no);
 	br_become_designated_port(p);
 	p->state = BR_STATE_BLOCKING;
 	p->topology_change_ack = 0;
@@ -185,15 +191,13 @@
 /* called under bridge lock */
 void br_stp_set_port_priority(struct net_bridge_port *p, u8 newprio)
 {
-	__u16 new_port_id;
-
-	p->priority = newprio & 0xFF;
-	new_port_id = br_make_port_id(p);
+	port_id new_port_id = br_make_port_id(newprio, p->port_no);
 
 	if (br_is_designated_port(p))
 		p->designated_port = new_port_id;
 
 	p->port_id = new_port_id;
+	p->priority = newprio;
 	if (!memcmp(&p->br->bridge_id, &p->designated_bridge, 8) &&
 	    p->port_id < p->designated_port) {
 		br_become_designated_port(p);

[PATCH 2.6.5] (9/9) bridge - fdb cache alloc

[Stephen Hemminger]

Since forwarding database gets a lot of memory alloc/free on a busy
bridge, use kmem_cache_alloc to provide cache and better stats.

diff -Nru a/net/bridge/br.c b/net/bridge/br.c
--- a/net/bridge/br.c	Tue Apr 13 13:17:16 2004
+++ b/net/bridge/br.c	Tue Apr 13 13:17:16 2004
@@ -31,6 +31,8 @@
 
 static int __init br_init(void)
 {
+	br_fdb_init();
+
 #ifdef CONFIG_BRIDGE_NETFILTER
 	if (br_netfilter_init())
 		return 1;
@@ -65,6 +67,7 @@
 #endif
 
 	br_handle_frame_hook = NULL;
+	br_fdb_fini();
 }
 
 EXPORT_SYMBOL(br_should_route_hook);
diff -Nru a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
--- a/net/bridge/br_fdb.c	Tue Apr 13 13:17:16 2004
+++ b/net/bridge/br_fdb.c	Tue Apr 13 13:17:16 2004
@@ -22,6 +22,22 @@
 #include <asm/uaccess.h>
 #include "br_private.h"
 
+static kmem_cache_t *br_fdb_cache;
+
+void __init br_fdb_init(void)
+{
+	br_fdb_cache = kmem_cache_create("bridge_fdb_cache",
+					 sizeof(struct net_bridge_fdb_entry),
+					 0,
+					 SLAB_HWCACHE_ALIGN, NULL, NULL);
+}
+
+void __exit br_fdb_fini(void)
+{
+	kmem_cache_destroy(br_fdb_cache);
+}
+
+
 /* if topology_changing then use forward_delay (default 15 sec)
  * otherwise keep longer (default 5 minutes)
  */
@@ -37,7 +53,7 @@
 		&& time_before_eq(fdb->ageing_timer + hold_time(br), jiffies);
 }
 
-static __inline__ void copy_fdb(struct __fdb_entry *ent, 
+static inline void copy_fdb(struct __fdb_entry *ent, 
 				const struct net_bridge_fdb_entry *f)
 {
 	memset(ent, 0, sizeof(struct __fdb_entry));
@@ -180,7 +196,7 @@
 void br_fdb_put(struct net_bridge_fdb_entry *ent)
 {
 	if (atomic_dec_and_test(&ent->use_count))
-		kfree(ent);
+		kmem_cache_free(br_fdb_cache, ent);
 }
 
 int br_fdb_get_entries(struct net_bridge *br, void __user *buf,
@@ -224,7 +240,7 @@
 			
 			/* entry was deleted during copy_to_user */
 			if (atomic_dec_and_test(&f->use_count)) {
-				kfree(f);
+				kmem_cache_free(br_fdb_cache, f);
 				num = -EAGAIN;
 				goto out;
 			}
@@ -283,7 +299,7 @@
 		}
 	}
 
-	fdb = kmalloc(sizeof(*fdb), GFP_ATOMIC);
+	fdb = kmem_cache_alloc(br_fdb_cache, GFP_ATOMIC);
 	if (unlikely(fdb == NULL)) {
 		ret = -ENOMEM;
 		goto out;
diff -Nru a/net/bridge/br_private.h b/net/bridge/br_private.h
--- a/net/bridge/br_private.h	Tue Apr 13 13:17:16 2004
+++ b/net/bridge/br_private.h	Tue Apr 13 13:17:16 2004
@@ -127,6 +127,8 @@
 extern int br_dev_xmit(struct sk_buff *skb, struct net_device *dev);
 
 /* br_fdb.c */
+extern void br_fdb_init(void);
+extern void br_fdb_fini(void);
 extern void br_fdb_changeaddr(struct net_bridge_port *p,
 			      const unsigned char *newaddr);
 extern void br_fdb_cleanup(unsigned long arg);

[PATCH 2.6.5] bridge - replace CLEAR_BITMAP with memset

[Stephen Hemminger]

It looks like a recent patch to 2.6.5 got rid of CLEAR_BITMAP.

diff -Nru a/net/bridge/br_if.c b/net/bridge/br_if.c
--- a/net/bridge/br_if.c	Wed Apr 14 12:01:21 2004
+++ b/net/bridge/br_if.c	Wed Apr 14 12:01:21 2004
@@ -175,7 +175,7 @@
 	if (!inuse)
 		return -ENOMEM;
 
-	CLEAR_BITMAP(inuse, BR_MAX_PORTS);
+	memset(inuse, 0, BITS_TO_LONGS(BR_MAX_PORTS)*sizeof(unsigned long));
 	set_bit(0, inuse);	/* zero is reserved */
 	list_for_each_entry(p, &br->port_list, list) {
 		set_bit(p->port_no, inuse);

Re: [PATCH 2.6.5] (1/9) bridge - include file cleanup

[David S. Miller]

On Tue, 13 Apr 2004 15:16:30 -0700
Stephen Hemminger <shemminger@osdl.org> wrote:

> Cleanup some of the include file's in the bridge code.
> * if_bridge.h defines net_bridge, but not needed as part of the API.
> * get rid of places that include if_bridge.h and uaccess.h but don't
>   actually do API work.

Applied.

Re: [PATCH 2.6.5] (2/9) bridge - rmmod race

[David S. Miller]

On Tue, 13 Apr 2004 15:18:47 -0700
Stephen Hemminger <shemminger@osdl.org> wrote:

> Fix observed race between removing bridge module and ip packets
> in flight.  Need to remove the hook last, after all bridges are gone
> not the other way around.

Applied.

Re: [PATCH 2.6.5] (3/9) bridge - jiffies_to_clock

[David S. Miller]

On Tue, 13 Apr 2004 15:20:37 -0700
Stephen Hemminger <shemminger@osdl.org> wrote:

> Rather than doing the math directly, use jiffies_to_clock functions
> to convert from user hz to jiffies.

Applied.

Re: [PATCH 2.6.5] (4/9) bridge - use ethtool to get port speed

[David S. Miller]

On Tue, 13 Apr 2004 15:24:40 -0700
Stephen Hemminger <shemminger@osdl.org> wrote:

> The bridge code needs to keep track of a cost estimate for each
> port in the bridge.  Instead of a hack based on device name, try
> and use ethtool to get port speed from device.  This has been tested
> on e100 (uses ethtool_ops) and e1000 (does ethtool the hard way)
> and dummy (no ethtool).

Looks fine, applied.

Re: [PATCH 2.6.5] (5/9) bridge - multicast address as const

[David S. Miller]

On Tue, 13 Apr 2004 15:26:20 -0700
Stephen Hemminger <shemminger@osdl.org> wrote:

> Trivial change.  For places where bridge multicast address is defined,
> or compared use constant.

Applied.

Re: [PATCH 2.6.5] (6/9) bridge - forwarding database changes

[David S. Miller]

On Tue, 13 Apr 2004 15:28:38 -0700
Stephen Hemminger <shemminger@osdl.org> wrote:

> Make forwarding database more robust.  
>   + Don't insert invalid ether address,
>   + Report errors back so adding an interface to bridge can fail
>   + get rid of unneeded explicit pads in data structure
>   + replace bitfields with byte's for simple booleans.

Applied.

Re: [PATCH 2.6.5] (7/9) bridge - STP unsigned fields

[David S. Miller]

On Tue, 13 Apr 2004 15:30:46 -0700
Stephen Hemminger <shemminger@osdl.org> wrote:

> Use correct types for fields related to spanning tree protocols.
>   * costs are 32 bit unsigned
>   * ports are 16 bit unsigned
>   * booleans are bytes rather than bitfield
>   * arrange for better packing

Applied.

Re: [PATCH 2.6.5] (8/9) bridge - support lots of 1k ports

[David S. Miller]

On Tue, 13 Apr 2004 15:33:08 -0700
Stephen Hemminger <shemminger@osdl.org> wrote:

> Support >256 ports on a bridge.  Use the suggestion of reducing
> the number of bits of priority and increasing the number of bits
> for port number.

Seems sane, applied.

Re: [PATCH 2.6.5] (9/9) bridge - fdb cache alloc

[David S. Miller]

On Tue, 13 Apr 2004 15:34:53 -0700
Stephen Hemminger <shemminger@osdl.org> wrote:

> Since forwarding database gets a lot of memory alloc/free on a busy
> bridge, use kmem_cache_alloc to provide cache and better stats.

Applied.

Re: [PATCH 2.6.5] bridge - replace CLEAR_BITMAP with memset

[David S. Miller]

On Wed, 14 Apr 2004 12:04:34 -0700
Stephen Hemminger <shemminger@osdl.org> wrote:

> It looks like a recent patch to 2.6.5 got rid of CLEAR_BITMAP.

Applied.