Skip to contentSkip to conversation

bonding updates to net-next-2.6

RFC PATCH Applied [email protected] 2 participants 15 messages Dec 9, 2008 – Dec 10, 2008

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover bonding updates to net-next-2.6

Hi Jay,

these are some patches against the bonding code in net-next-2.6. I hope you find them usefull.

As you requested previously I tried to make bond_start_xmit() use a function table instead of the switch statement, but with some unexpected results which I'm currently analyzing. I therefore leave it as-is.

Thanks for your previous feedback.

/holger

#1 lore raw

PATCH 1/7 bonding: add and use bond_is_lb()

Introduce and use bond_is_lb(), it is usefull to shorten the repetitive check for either ALB or TLB mode.

drivers/net/bonding/bond_main.c +9 -20
Index: bonding-2.6-out/drivers/net/bonding/bond_main.c
===================================================================
@@ -294,10 +294,8 @@
 		if (vlan->vlan_id == vlan_id) {
 			list_del(&vlan->vlan_list);
 
-			if ((bond->params.mode == BOND_MODE_TLB) ||
-			    (bond->params.mode == BOND_MODE_ALB)) {
+			if (bond_is_lb(bond))
 				bond_alb_clear_vlan(bond, vlan_id);
-			}
 
 			dprintk("removed VLAN ID %d from bond %s\n", vlan_id,
 				bond->dev->name);
@@ -1174,10 +1172,8 @@
 				bond_3ad_handle_link_change(new_active, BOND_LINK_UP);
 			}
 
-			if ((bond->params.mode == BOND_MODE_TLB) ||
-			    (bond->params.mode == BOND_MODE_ALB)) {
+			if (bond_is_lb(bond))
 				bond_alb_handle_link_change(bond, new_active, BOND_LINK_UP);
-			}
 		} else {
 			if (USES_PRIMARY(bond->params.mode)) {
 				printk(KERN_INFO DRV_NAME
@@ -1192,8 +1188,7 @@
 		bond_mc_swap(bond, new_active, old_active);
 	}
 
-	if ((bond->params.mode == BOND_MODE_TLB) ||
-	    (bond->params.mode == BOND_MODE_ALB)) {
+	if (bond_is_lb(bond)) {
 		bond_alb_handle_active_change(bond, new_active);
 		if (old_active)
 			bond_set_slave_inactive_flags(old_active);
@@ -1554,8 +1549,7 @@
 	new_slave->dev = slave_dev;
 	slave_dev->priv_flags |= IFF_BONDING;
 
-	if ((bond->params.mode == BOND_MODE_TLB) ||
-	    (bond->params.mode == BOND_MODE_ALB)) {
+	if (bond_is_lb(bond)) {
 		/* bond_alb_init_slave() must be called before all other stages since
 		 * it might fail and we do not want to have to undo everything
 		 */
@@ -1871,8 +1865,7 @@
 		bond_change_active_slave(bond, NULL);
 	}
 
-	if ((bond->params.mode == BOND_MODE_TLB) ||
-	    (bond->params.mode == BOND_MODE_ALB)) {
+	if (bond_is_lb(bond)) {
 		/* Must be called only after the slave has been
 		 * detached from the list and the curr_active_slave
 		 * has been cleared (if our_slave == old_current),
@@ -2061,8 +2054,7 @@
 		 */
 		write_unlock_bh(&bond->lock);
 
-		if ((bond->params.mode == BOND_MODE_TLB) ||
-		    (bond->params.mode == BOND_MODE_ALB)) {
+		if (bond_is_lb(bond)) {
 			/* must be called only after the slave
 			 * has been detached from the list
 			 */
@@ -2389,8 +2381,7 @@
 			if (bond->params.mode == BOND_MODE_8023AD)
 				bond_3ad_handle_link_change(slave, BOND_LINK_UP);
 
-			if ((bond->params.mode == BOND_MODE_TLB) ||
-			    (bond->params.mode == BOND_MODE_ALB))
+			if (bond_is_lb(bond))
 				bond_alb_handle_link_change(bond, slave,
 							    BOND_LINK_UP);
 
@@ -3796,8 +3787,7 @@
 
 	bond->kill_timers = 0;
 
-	if ((bond->params.mode == BOND_MODE_TLB) ||
-	    (bond->params.mode == BOND_MODE_ALB)) {
+	if (bond_is_lb(bond)) {
 		/* bond_alb_initialize must be called before the timer
 		 * is started.
 		 */
@@ -3882,8 +3872,7 @@
 	}
 
 
-	if ((bond->params.mode == BOND_MODE_TLB) ||
-	    (bond->params.mode == BOND_MODE_ALB)) {
+	if (bond_is_lb(bond)) {
 		/* Must be called only after all
 		 * slaves have been released
 		 */
drivers/net/bonding/bonding.h +6 -0
Index: bonding-2.6-out/drivers/net/bonding/bonding.h
===================================================================
@@ -260,6 +260,12 @@
 	return (struct bonding *)netdev_priv(slave->dev->master);
 }
 
+static inline bool bond_is_lb(const struct bonding *bond)
+{
+	return bond->params.mode == BOND_MODE_TLB
+		|| bond->params.mode == BOND_MODE_ALB;
+}
+
 #define BOND_FOM_NONE			0
 #define BOND_FOM_ACTIVE			1
 #define BOND_FOM_FOLLOW			2
#3 lore raw

PATCH 2/7 bonding: use table for mode names

In reply to [email protected]

Use a small array in bond_mode_name() for the names, thus saving some space:

before

    text     data      bss      dec    hex filename
   57736     9372      344    67452  1077c drivers/net/bonding/bonding.ko

after

    text     data      bss      dec    hex filename
   57441     9372      344    67157  10655 drivers/net/bonding/bonding.ko
drivers/net/bonding/bond_main.c +13 -17
Index: bonding-2.6/drivers/net/bonding/bond_main.c
===================================================================
@@ -219,24 +219,20 @@
 
 static const char *bond_mode_name(int mode)
 {
-	switch (mode) {
-	case BOND_MODE_ROUNDROBIN :
-		return "load balancing (round-robin)";
-	case BOND_MODE_ACTIVEBACKUP :
-		return "fault-tolerance (active-backup)";
-	case BOND_MODE_XOR :
-		return "load balancing (xor)";
-	case BOND_MODE_BROADCAST :
-		return "fault-tolerance (broadcast)";
-	case BOND_MODE_8023AD:
-		return "IEEE 802.3ad Dynamic link aggregation";
-	case BOND_MODE_TLB:
-		return "transmit load balancing";
-	case BOND_MODE_ALB:
-		return "adaptive load balancing";
-	default:
+	static const char *names[] = {
+		[BOND_MODE_ROUNDROBIN] = "load balancing (round-robin)",
+		[BOND_MODE_ACTIVEBACKUP] = "fault-tolerance (active-backup)",
+		[BOND_MODE_XOR] = "load balancing (xor)",
+		[BOND_MODE_BROADCAST] = "fault-tolerance (broadcast)",
+		[BOND_MODE_8023AD]= "IEEE 802.3ad Dynamic link aggregation",
+		[BOND_MODE_TLB] = "transmit load balancing",
+		[BOND_MODE_ALB] = "adaptive load balancing",
+	};
+
+	if (mode < 0 || mode > BOND_MODE_ALB)
 		return "unknown";
-	}
+
+	return names[mode];
 }
 
 /*---------------------------------- VLAN -----------------------------------*/
maintainer Applied
#4 lore raw

Re: [patch 2/7] bonding: use table for mode names

From: [email protected]
Date: Tue, 09 Dec 2008 21:07:52 +0100

Quoted 5 lines

Use a small array in bond_mode_name() for the names, thus saving some space:

before

Quoted 8 lines
    text     data      bss      dec    hex filename
   57736     9372      344    67452  1077c drivers/net/bonding/bonding.ko

after

    text     data      bss      dec    hex filename
   57441     9372      344    67157  10655 drivers/net/bonding/bonding.ko

Applied.

#5 lore raw

PATCH 3/7 bonding: fix compile error if debug enabled

In reply to [email protected]

This is what I get if debug is enabled:

drivers/net/bonding/bond_ipv6.c: In function 'bond_na_send':
drivers/net/bonding/bond_ipv6.c:75: error: 'slave' undeclared (first use in this function) drivers/net/bonding/bond_ipv6.c:75: error: (Each undeclared identifier is reported only once drivers/net/bonding/bond_ipv6.c:75: error: for each function it appears in.)

This patch fixes that.

drivers/net/bonding/bond_ipv6.c +1 -1
Index: bonding-2.6/drivers/net/bonding/bond_ipv6.c
===================================================================
@@ -75,7 +75,7 @@
 	addrconf_addr_solict_mult(daddr, &mcaddr);
 
 	dprintk("ipv6 na on slave %s: dest %pI6, src %pI6\n",
-	       slave->name, &mcaddr, daddr);
+	       slave_dev->name, &mcaddr, daddr);
 
 	skb = ndisc_build_skb(slave_dev, &mcaddr, daddr, &icmp6h, daddr,
 			      ND_OPT_TARGET_LL_ADDR);
maintainer Applied
#6 lore raw

Re: [patch 3/7] bonding: fix compile error if debug enabled

From: [email protected]
Date: Tue, 09 Dec 2008 21:07:53 +0100

Quoted 10 lines

This is what I get if debug is enabled:

drivers/net/bonding/bond_ipv6.c: In function 'bond_na_send':
drivers/net/bonding/bond_ipv6.c:75: error: 'slave' undeclared (first use in this function) drivers/net/bonding/bond_ipv6.c:75: error: (Each undeclared identifier is reported only once drivers/net/bonding/bond_ipv6.c:75: error: for each function it appears in.)

This patch fixes that.

Applied.

#7 lore raw

PATCH 4/7 bonding: use pr_debug instead of own macros

In reply to [email protected]

Use pr_debug() instead of own macros.

drivers/net/bonding/bonding.h +0 -8
Index: bonding-2.6/drivers/net/bonding/bonding.h
===================================================================
@@ -32,14 +32,6 @@
 
 extern struct list_head bond_dev_list;
 
-#ifdef BONDING_DEBUG
-#define dprintk(fmt, args...) \
-	printk(KERN_DEBUG     \
-	       DRV_NAME ": %s() %d: " fmt, __func__, __LINE__ , ## args )
-#else
-#define dprintk(fmt, args...)
-#endif /* BONDING_DEBUG */
-
 #define IS_UP(dev)					   \
 	      ((((dev)->flags & IFF_UP) == IFF_UP)	&& \
 	       netif_running(dev)			&& \
drivers/net/bonding/bond_3ad.c +29 -31

227 lines not on this page · raw message

drivers/net/bonding/bond_alb.c +5 -7
Index: bonding-2.6/drivers/net/bonding/bond_alb.c
===================================================================
@@ -20,8 +20,6 @@
  *
  */
 
-//#define BONDING_DEBUG 1
-
 #include <linux/skbuff.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
@@ -361,12 +359,12 @@
 		goto out;
 
 	if (!arp) {
-		dprintk("Packet has no ARP data\n");
+		pr_debug("Packet has no ARP data\n");
 		goto out;
 	}
 
 	if (skb->len < sizeof(struct arp_pkt)) {
-		dprintk("Packet is too small to be an ARP\n");
+		pr_debug("Packet is too small to be an ARP\n");
 		goto out;
 	}
 
@@ -376,7 +374,7 @@
 		       bond_dev->name);
 		bond = netdev_priv(bond_dev);
 		rlb_update_entry_from_arp(bond, arp);
-		dprintk("Server received an ARP Reply from client\n");
+		pr_debug("Server received an ARP Reply from client\n");
 	}
 
 	res = NET_RX_SUCCESS;
@@ -730,7 +728,7 @@
 		if (tx_slave) {
 			memcpy(arp->mac_src,tx_slave->dev->dev_addr, ETH_ALEN);
 		}
-		dprintk("Server sent ARP Reply packet\n");
+		pr_debug("Server sent ARP Reply packet\n");
 	} else if (arp->op_code == htons(ARPOP_REQUEST)) {
 		/* Create an entry in the rx_hashtbl for this client as a
 		 * place holder.
@@ -750,7 +748,7 @@
 		 * updated with their assigned mac.
 		 */
 		rlb_req_update_subnet_clients(bond, arp->ip_src);
-		dprintk("Server sent ARP Request packet\n");
+		pr_debug("Server sent ARP Request packet\n");
 	}
 
 	return tx_slave;
drivers/net/bonding/bond_ipv6.c +2 -4
Index: bonding-2.6/drivers/net/bonding/bond_ipv6.c
===================================================================
@@ -20,8 +20,6 @@
  *
  */
 
-//#define BONDING_DEBUG 1
-
 #include <linux/types.h>
 #include <linux/if_vlan.h>
 #include <net/ipv6.h>
@@ -74,7 +72,7 @@
 
 	addrconf_addr_solict_mult(daddr, &mcaddr);
 
-	dprintk("ipv6 na on slave %s: dest %pI6, src %pI6\n",
+	pr_debug("ipv6 na on slave %s: dest %pI6, src %pI6\n",
 	       slave_dev->name, &mcaddr, daddr);
 
 	skb = ndisc_build_skb(slave_dev, &mcaddr, daddr, &icmp6h, daddr,
@@ -110,7 +108,7 @@
 	struct inet6_dev *idev;
 	int is_router;
 
-	dprintk("bond_send_unsol_na: bond %s slave %s\n", bond->dev->name,
+	pr_debug("bond_send_unsol_na: bond %s slave %s\n", bond->dev->name,
 				slave ? slave->dev->name : "NULL");
 
 	if (!slave || !bond->send_unsol_na ||
drivers/net/bonding/bond_main.c +45 -47

339 lines not on this page · raw message

drivers/net/bonding/bond_sysfs.c +1 -1
Index: bonding-2.6/drivers/net/bonding/bond_sysfs.c
===================================================================
@@ -36,8 +36,8 @@
 #include <linux/rtnetlink.h>
 #include <net/net_namespace.h>
 
-/* #define BONDING_DEBUG 1 */
 #include "bonding.h"
+
 #define to_dev(obj)	container_of(obj,struct device,kobj)
 #define to_bond(cd)	((struct bonding *)(netdev_priv(to_net_dev(cd))))
 
#9 lore raw

PATCH 5/7 bonding: remove duplicate declarations

In reply to [email protected]

Remove some declarations from bonding.c as they are declared in bonding.h already.

drivers/net/bonding/bond_sysfs.c +0 -7
Index: bonding-2.6/drivers/net/bonding/bond_sysfs.c
===================================================================
@@ -43,15 +43,8 @@
 
 /*---------------------------- Declarations -------------------------------*/
 
-
-extern struct list_head bond_dev_list;
 extern struct bond_params bonding_defaults;
-extern struct bond_parm_tbl bond_mode_tbl[];
-extern struct bond_parm_tbl bond_lacp_tbl[];
 extern struct bond_parm_tbl ad_select_tbl[];
-extern struct bond_parm_tbl xmit_hashtype_tbl[];
-extern struct bond_parm_tbl arp_validate_tbl[];
-extern struct bond_parm_tbl fail_over_mac_tbl[];
 
 static int expected_refcount = -1;
 /*--------------------------- Data Structures -----------------------------*/
#11 lore raw

PATCH 6/7 bonding: make tbl argument to bond_parse_parm() const

In reply to [email protected]

bond_parse_parm() parses a parameter table for a particular value and is therefore not modifying the table at all. Therefore make the 2nd argument const, thus allowing to make the tables const later.

drivers/net/bonding/bond_main.c +1 -1
Index: bonding-2.6-out/drivers/net/bonding/bond_main.c
===================================================================
@@ -4723,7 +4723,7 @@
  * some mode names are substrings of other names, and calls from sysfs
  * may have whitespace in the name (trailing newlines, for example).
  */
-int bond_parse_parm(const char *buf, struct bond_parm_tbl *tbl)
+int bond_parse_parm(const char *buf, const struct bond_parm_tbl *tbl)
 {
 	int mode = -1, i, rv;
 	char *p, modestr[BOND_MAX_MODENAME_LEN + 1] = { 0, };
drivers/net/bonding/bonding.h +1 -1
Index: bonding-2.6-out/drivers/net/bonding/bonding.h
===================================================================
@@ -337,7 +337,7 @@
 void bond_loadbalance_arp_mon(struct work_struct *);
 void bond_activebackup_arp_mon(struct work_struct *);
 void bond_set_mode_ops(struct bonding *bond, int mode);
-int bond_parse_parm(const char *mode_arg, struct bond_parm_tbl *tbl);
+int bond_parse_parm(const char *mode_arg, const struct bond_parm_tbl *tbl);
 void bond_select_active_slave(struct bonding *bond);
 void bond_change_active_slave(struct bonding *bond, struct slave *new_active);
 void bond_register_arp(struct bonding *);
maintainer Applied
#12 lore raw

Re: [patch 6/7] bonding: make tbl argument to bond_parse_parm() const

From: [email protected]
Date: Tue, 09 Dec 2008 21:07:56 +0100

Quoted 5 lines

bond_parse_parm() parses a parameter table for a particular value and is therefore not modifying the table at all. Therefore make the 2nd argument const, thus allowing to make the tables const later.

Applied.

#13 lore raw

PATCH 7/7 bonding: turn all bond_parm_tbls const

In reply to [email protected]

Turn all bond_parm_tbls const.

drivers/net/bonding/bond_main.c +5 -5
Index: bonding-2.6-out/drivers/net/bonding/bond_main.c
===================================================================
@@ -162,13 +162,13 @@
 static int lacp_fast	= 0;
 
 
-struct bond_parm_tbl bond_lacp_tbl[] = {
+const struct bond_parm_tbl bond_lacp_tbl[] = {
 {	"slow",		AD_LACP_SLOW},
 {	"fast",		AD_LACP_FAST},
 {	NULL,		-1},
 };
 
-struct bond_parm_tbl bond_mode_tbl[] = {
+const struct bond_parm_tbl bond_mode_tbl[] = {
 {	"balance-rr",		BOND_MODE_ROUNDROBIN},
 {	"active-backup",	BOND_MODE_ACTIVEBACKUP},
 {	"balance-xor",		BOND_MODE_XOR},
@@ -179,14 +179,14 @@
 {	NULL,			-1},
 };
 
-struct bond_parm_tbl xmit_hashtype_tbl[] = {
+const struct bond_parm_tbl xmit_hashtype_tbl[] = {
 {	"layer2",		BOND_XMIT_POLICY_LAYER2},
 {	"layer3+4",		BOND_XMIT_POLICY_LAYER34},
 {	"layer2+3",		BOND_XMIT_POLICY_LAYER23},
 {	NULL,			-1},
 };
 
-struct bond_parm_tbl arp_validate_tbl[] = {
+const struct bond_parm_tbl arp_validate_tbl[] = {
 {	"none",			BOND_ARP_VALIDATE_NONE},
 {	"active",		BOND_ARP_VALIDATE_ACTIVE},
 {	"backup",		BOND_ARP_VALIDATE_BACKUP},
@@ -194,7 +194,7 @@
 {	NULL,			-1},
 };
 
-struct bond_parm_tbl fail_over_mac_tbl[] = {
+const struct bond_parm_tbl fail_over_mac_tbl[] = {
 {	"none",			BOND_FOM_NONE},
 {	"active",		BOND_FOM_ACTIVE},
 {	"follow",		BOND_FOM_FOLLOW},
drivers/net/bonding/bonding.h +5 -5
Index: bonding-2.6-out/drivers/net/bonding/bonding.h
===================================================================
@@ -345,11 +345,11 @@
 
 /* exported from bond_main.c */
 extern struct list_head bond_dev_list;
-extern struct bond_parm_tbl bond_lacp_tbl[];
-extern struct bond_parm_tbl bond_mode_tbl[];
-extern struct bond_parm_tbl xmit_hashtype_tbl[];
-extern struct bond_parm_tbl arp_validate_tbl[];
-extern struct bond_parm_tbl fail_over_mac_tbl[];
+extern const struct bond_parm_tbl bond_lacp_tbl[];
+extern const struct bond_parm_tbl bond_mode_tbl[];
+extern const struct bond_parm_tbl xmit_hashtype_tbl[];
+extern const struct bond_parm_tbl arp_validate_tbl[];
+extern const struct bond_parm_tbl fail_over_mac_tbl[];
 
 #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
 void bond_send_unsolicited_na(struct bonding *bond);