netdev - Re: [PATCH] bonding: fix enslaving in alb mode when link down

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <23580.1326937521@death>
Date:	Wed, 18 Jan 2012 17:45:21 -0800
From:	Jay Vosburgh <fubar@...ibm.com>
To:	Jiri Bohac <jbohac@...e.cz>
cc:	netdev@...r.kernel.org
Subject: Re: [PATCH] bonding: fix enslaving in alb mode when link down

Jiri Bohac <jbohac@...e.cz> wrote:

>bond_alb_init_slave() is called from bond_enslave() and sets the slave's MAC
>address. This is done differently for TLB and ALB modes.
>bond->alb_info.rlb_enabled is used to discriminate between the two modes but
>this flag may be uninitialized if the slave is being enslaved prior to calling
>bond_open() -> bond_alb_initialize() on the master.
>
>It turns out all the callers of alb_set_slave_mac_addr() pass
>bond->alb_info.rlb_enabled as the hw parameter.
>
>This patch cleans up the unnecessary parameter of alb_set_slave_mac_addr() and
>makes the function decide based on the bonding mode instead, which fixes the
>above problem.
>
>Signed-off-by: Jiri Bohac <jbohac@...e.cz>

	Looks reasonable.

	-J

Signed-off-by: Jay Vosburgh <fubar@...ibm.com>


>diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>index 342626f..f820b26 100644
>--- a/drivers/net/bonding/bond_alb.c
>+++ b/drivers/net/bonding/bond_alb.c
>@@ -909,16 +909,12 @@ static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[])
> 	}
> }
>
>-/* hw is a boolean parameter that determines whether we should try and
>- * set the hw address of the device as well as the hw address of the
>- * net_device
>- */
>-static int alb_set_slave_mac_addr(struct slave *slave, u8 addr[], int hw)
>+static int alb_set_slave_mac_addr(struct slave *slave, u8 addr[])
> {
> 	struct net_device *dev = slave->dev;
> 	struct sockaddr s_addr;
>
>-	if (!hw) {
>+	if (slave->bond->params.mode == BOND_MODE_TLB) {
> 		memcpy(dev->dev_addr, addr, dev->addr_len);
> 		return 0;
> 	}
>@@ -948,8 +944,8 @@ static void alb_swap_mac_addr(struct bonding *bond, struct slave *slave1, struct
> 	u8 tmp_mac_addr[ETH_ALEN];
>
> 	memcpy(tmp_mac_addr, slave1->dev->dev_addr, ETH_ALEN);
>-	alb_set_slave_mac_addr(slave1, slave2->dev->dev_addr, bond->alb_info.rlb_enabled);
>-	alb_set_slave_mac_addr(slave2, tmp_mac_addr, bond->alb_info.rlb_enabled);
>+	alb_set_slave_mac_addr(slave1, slave2->dev->dev_addr);
>+	alb_set_slave_mac_addr(slave2, tmp_mac_addr);
>
> }
>
>@@ -1096,8 +1092,7 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav
>
> 		/* Try setting slave mac to bond address and fall-through
> 		   to code handling that situation below... */
>-		alb_set_slave_mac_addr(slave, bond->dev->dev_addr,
>-				       bond->alb_info.rlb_enabled);
>+		alb_set_slave_mac_addr(slave, bond->dev->dev_addr);
> 	}
>
> 	/* The slave's address is equal to the address of the bond.
>@@ -1133,8 +1128,7 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav
> 	}
>
> 	if (free_mac_slave) {
>-		alb_set_slave_mac_addr(slave, free_mac_slave->perm_hwaddr,
>-				       bond->alb_info.rlb_enabled);
>+		alb_set_slave_mac_addr(slave, free_mac_slave->perm_hwaddr);
>
> 		pr_warning("%s: Warning: the hw address of slave %s is in use by the bond; giving it the hw address of %s\n",
> 			   bond->dev->name, slave->dev->name,
>@@ -1491,8 +1485,7 @@ int bond_alb_init_slave(struct bonding *bond, struct slave *slave)
> {
> 	int res;
>
>-	res = alb_set_slave_mac_addr(slave, slave->perm_hwaddr,
>-				     bond->alb_info.rlb_enabled);
>+	res = alb_set_slave_mac_addr(slave, slave->perm_hwaddr);
> 	if (res) {
> 		return res;
> 	}
>@@ -1643,8 +1636,7 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
> 		alb_swap_mac_addr(bond, swap_slave, new_slave);
> 	} else {
> 		/* set the new_slave to the bond mac address */
>-		alb_set_slave_mac_addr(new_slave, bond->dev->dev_addr,
>-				       bond->alb_info.rlb_enabled);
>+		alb_set_slave_mac_addr(new_slave, bond->dev->dev_addr);
> 	}
>
> 	if (swap_slave) {
>@@ -1704,8 +1696,7 @@ int bond_alb_set_mac_address(struct net_device *bond_dev, void *addr)
> 		alb_swap_mac_addr(bond, swap_slave, bond->curr_active_slave);
> 		alb_fasten_mac_swap(bond, swap_slave, bond->curr_active_slave);
> 	} else {
>-		alb_set_slave_mac_addr(bond->curr_active_slave, bond_dev->dev_addr,
>-				       bond->alb_info.rlb_enabled);
>+		alb_set_slave_mac_addr(bond->curr_active_slave, bond_dev->dev_addr);
>
> 		read_lock(&bond->lock);
> 		alb_send_learning_packets(bond->curr_active_slave, bond_dev->dev_addr);
>-- 
>Jiri Bohac <jbohac@...e.cz>
>SUSE Labs, SUSE CZ
>

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@...ibm.com

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html