Skip to content

Commit

Permalink
bonding: Check return of dev_set_promiscuity/allmulti
Browse files Browse the repository at this point in the history
dev_set_promiscuity/allmulti might overflow.
Commit: "netdevice: Fix promiscuity and allmulti overflow" in net-next makes
dev_set_promiscuity/allmulti return error number if overflow happened.

In bond_alb and bond_main, we check all positive increment for promiscuity
and allmulti to get error return.
But there are still two problems left.
1. Some code path has no mechanism to signal errors upstream.
2. If there are multi slaves, it's hard to tell which slaves increment
   promisc/allmulti successfully and which failed.
So I left these problems to be FIXME.
Fortunately, the overflow is very rare case.

Signed-off-by: Wang Chen <wangchen@cn.fujitsu.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Wang Chen authored and David S. Miller committed Jul 15, 2008
1 parent 2aeb0b8 commit 7e1a1ac
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 10 deletions.
6 changes: 4 additions & 2 deletions drivers/net/bonding/bond_alb.c
Original file line number Diff line number Diff line change
Expand Up @@ -419,8 +419,10 @@ static void rlb_teach_disabled_mac_on_primary(struct bonding *bond, u8 addr[])
}

if (!bond->alb_info.primary_is_promisc) {
bond->alb_info.primary_is_promisc = 1;
dev_set_promiscuity(bond->curr_active_slave->dev, 1);
if (!dev_set_promiscuity(bond->curr_active_slave->dev, 1))
bond->alb_info.primary_is_promisc = 1;
else
bond->alb_info.primary_is_promisc = 0;
}

bond->alb_info.rlb_promisc_timeout_counter = 0;
Expand Down
39 changes: 31 additions & 8 deletions drivers/net/bonding/bond_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -772,39 +772,49 @@ static struct dev_mc_list *bond_mc_list_find_dmi(struct dev_mc_list *dmi, struct
/*
* Push the promiscuity flag down to appropriate slaves
*/
static void bond_set_promiscuity(struct bonding *bond, int inc)
static int bond_set_promiscuity(struct bonding *bond, int inc)
{
int err = 0;
if (USES_PRIMARY(bond->params.mode)) {
/* write lock already acquired */
if (bond->curr_active_slave) {
dev_set_promiscuity(bond->curr_active_slave->dev, inc);
err = dev_set_promiscuity(bond->curr_active_slave->dev,
inc);
}
} else {
struct slave *slave;
int i;
bond_for_each_slave(bond, slave, i) {
dev_set_promiscuity(slave->dev, inc);
err = dev_set_promiscuity(slave->dev, inc);
if (err)
return err;
}
}
return err;
}

/*
* Push the allmulti flag down to all slaves
*/
static void bond_set_allmulti(struct bonding *bond, int inc)
static int bond_set_allmulti(struct bonding *bond, int inc)
{
int err = 0;
if (USES_PRIMARY(bond->params.mode)) {
/* write lock already acquired */
if (bond->curr_active_slave) {
dev_set_allmulti(bond->curr_active_slave->dev, inc);
err = dev_set_allmulti(bond->curr_active_slave->dev,
inc);
}
} else {
struct slave *slave;
int i;
bond_for_each_slave(bond, slave, i) {
dev_set_allmulti(slave->dev, inc);
err = dev_set_allmulti(slave->dev, inc);
if (err)
return err;
}
}
return err;
}

/*
Expand Down Expand Up @@ -965,6 +975,7 @@ static void bond_mc_swap(struct bonding *bond, struct slave *new_active, struct
}

if (new_active) {
/* FIXME: Signal errors upstream. */
if (bond->dev->flags & IFF_PROMISC) {
dev_set_promiscuity(new_active->dev, 1);
}
Expand Down Expand Up @@ -1544,12 +1555,16 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
if (!USES_PRIMARY(bond->params.mode)) {
/* set promiscuity level to new slave */
if (bond_dev->flags & IFF_PROMISC) {
dev_set_promiscuity(slave_dev, 1);
res = dev_set_promiscuity(slave_dev, 1);
if (res)
goto err_close;
}

/* set allmulti level to new slave */
if (bond_dev->flags & IFF_ALLMULTI) {
dev_set_allmulti(slave_dev, 1);
res = dev_set_allmulti(slave_dev, 1);
if (res)
goto err_close;
}

netif_tx_lock_bh(bond_dev);
Expand Down Expand Up @@ -4065,6 +4080,10 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
* Do promisc before checking multicast_mode
*/
if ((bond_dev->flags & IFF_PROMISC) && !(bond->flags & IFF_PROMISC)) {
/*
* FIXME: Need to handle the error when one of the multi-slaves
* encounters error.
*/
bond_set_promiscuity(bond, 1);
}

Expand All @@ -4074,6 +4093,10 @@ static void bond_set_multicast_list(struct net_device *bond_dev)

/* set allmulti flag to slaves */
if ((bond_dev->flags & IFF_ALLMULTI) && !(bond->flags & IFF_ALLMULTI)) {
/*
* FIXME: Need to handle the error when one of the multi-slaves
* encounters error.
*/
bond_set_allmulti(bond, 1);
}

Expand Down

0 comments on commit 7e1a1ac

Please sign in to comment.