diff -Nuarp a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c --- a/drivers/net/bonding/bond_alb.c Sun Dec 21 16:08:55 2003 +++ b/drivers/net/bonding/bond_alb.c Sun Dec 21 16:08:56 2003 @@ -88,12 +88,14 @@ */ #define RLB_PROMISC_TIMEOUT 10*ALB_TIMER_TICKS_PER_SEC +static const u8 mac_bcast[ETH_ALEN] = {0xff,0xff,0xff,0xff,0xff,0xff}; + #pragma pack(1) struct learning_pkt { u8 mac_dst[ETH_ALEN]; u8 mac_src[ETH_ALEN]; u16 type; - u8 padding[ETH_ZLEN - (2*ETH_ALEN + 2)]; + u8 padding[ETH_ZLEN - ETH_HLEN]; }; struct arp_pkt { @@ -117,7 +119,7 @@ static inline u8 _simple_hash(u8 *hash_s int i; u8 hash = 0; - for (i=0; itx_hashtbl = kmalloc(size, GFP_KERNEL); - if (bond_info->tx_hashtbl == NULL) { + if (!bond_info->tx_hashtbl) { printk(KERN_ERR DRV_NAME ": Error: %s: Failed to allocate TLB hash table\n", bond->dev->name); @@ -197,7 +199,7 @@ static int tlb_initialize(struct bonding } memset(bond_info->tx_hashtbl, 0, size); - for (i=0; itx_hashtbl[i], 1); } _unlock_tx_hashtbl(bond); @@ -236,14 +238,14 @@ static struct slave *tlb_get_least_loade } least_loaded = slave; - max_gap = (s64)(slave->speed * 1000000) - - (s64)(SLAVE_TLB_INFO(slave).load * 8); + max_gap = (s64)(slave->speed << 20) - /* Convert to Megabit per sec */ + (s64)(SLAVE_TLB_INFO(slave).load << 3); /* Bytes to bits */ /* Find the slave with the largest gap */ bond_for_each_slave_from(bond, slave, i, least_loaded) { if (SLAVE_IS_OK(slave)) { - s64 gap = (s64)(slave->speed * 1000000) - - (s64)(SLAVE_TLB_INFO(slave).load * 8); + s64 gap = (s64)(slave->speed << 20) - + (s64)(SLAVE_TLB_INFO(slave).load << 3); if (max_gap < gap) { least_loaded = slave; max_gap = gap; @@ -318,7 +320,7 @@ static void rlb_update_entry_from_arp(st _lock_rx_hashtbl(bond); - hash_index = _simple_hash((u8*)&(arp->ip_src), 4); + hash_index = _simple_hash((u8*)&(arp->ip_src), sizeof(arp->ip_src)); client_info = &(bond_info->rx_hashtbl[hash_index]); if ((client_info->assigned) && @@ -376,7 +378,7 @@ static struct slave *rlb_next_rx_slave(s int i = 0; slave = bond_info->next_rx_slave; - if (slave == NULL) { + if (!slave) { slave = bond->first_slave; } @@ -384,8 +386,7 @@ static struct slave *rlb_next_rx_slave(s if (SLAVE_IS_OK(slave)) { if (!rx_slave) { rx_slave = slave; - } - else if (slave->speed > rx_slave->speed) { + } else if (slave->speed > rx_slave->speed) { rx_slave = slave; } } @@ -425,7 +426,6 @@ static void rlb_clear_slave(struct bondi { struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond)); struct rlb_client_info *rx_hash_table; - u8 mac_bcast[ETH_ALEN] = {0xff,0xff,0xff,0xff,0xff,0xff}; u32 index, next_index; /* clear slave from rx_hashtbl */ @@ -474,11 +474,11 @@ static void rlb_update_client(struct rlb { int i; - if (client_info->slave == NULL) { + if (!client_info->slave) { return; } - for (i=0; iip_dst, client_info->slave->dev, @@ -521,7 +521,6 @@ static void rlb_update_rx_clients(struct static void rlb_req_update_slave_clients(struct bonding *bond, struct slave *slave) { struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond)); - u8 mac_bcast[ETH_ALEN] = {0xff,0xff,0xff,0xff,0xff,0xff}; struct rlb_client_info *client_info; int ntt = 0; u32 hash_index; @@ -553,7 +552,6 @@ static void rlb_req_update_slave_clients static void rlb_req_update_subnet_clients(struct bonding *bond, u32 src_ip) { struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond)); - u8 mac_bcast[ETH_ALEN] = {0xff,0xff,0xff,0xff,0xff,0xff}; struct rlb_client_info *client_info; u32 hash_index; @@ -592,14 +590,13 @@ struct slave *rlb_choose_channel(struct struct slave *assigned_slave; struct rlb_client_info *client_info; u32 hash_index = 0; - u8 mac_bcast[ETH_ALEN] = {0xff,0xff,0xff,0xff,0xff,0xff}; _lock_rx_hashtbl(bond); - hash_index = _simple_hash((u8 *)&arp->ip_dst, 4); + hash_index = _simple_hash((u8 *)&arp->ip_dst, sizeof(arp->ip_src)); client_info = &(bond_info->rx_hashtbl[hash_index]); - if (client_info->assigned == 1) { + if (client_info->assigned) { if ((client_info->ip_src == arp->ip_src) && (client_info->ip_dst == arp->ip_dst)) { /* the entry is already assigned to this client */ @@ -642,8 +639,7 @@ struct slave *rlb_choose_channel(struct if (memcmp(client_info->mac_dst, mac_bcast, ETH_ALEN)) { client_info->ntt = 1; bond->alb_info.rx_ntt = 1; - } - else { + } else { client_info->ntt = 0; } @@ -724,7 +720,7 @@ static void rlb_rebalance(struct bonding for (; hash_index != RLB_NULL_INDEX; hash_index = client_info->next) { client_info = &(bond_info->rx_hashtbl[hash_index]); assigned_slave = rlb_next_rx_slave(bond); - if (assigned_slave && (client_info->slave != assigned_slave)){ + if (assigned_slave && (client_info->slave != assigned_slave)) { client_info->slave = assigned_slave; client_info->ntt = 1; ntt = 1; @@ -758,7 +754,7 @@ static int rlb_initialize(struct bonding _lock_rx_hashtbl(bond); bond_info->rx_hashtbl = kmalloc(size, GFP_KERNEL); - if (bond_info->rx_hashtbl == NULL) { + if (!bond_info->rx_hashtbl) { printk(KERN_ERR DRV_NAME ": Error: %s: Failed to allocate RLB hash table\n", bond->dev->name); @@ -768,7 +764,7 @@ static int rlb_initialize(struct bonding bond_info->rx_hashtbl_head = RLB_NULL_INDEX; - for (i=0; irx_hashtbl + i); } _unlock_rx_hashtbl(bond); @@ -810,7 +806,7 @@ static void alb_send_learning_packets(st memcpy(pkt.mac_src, mac_addr, ETH_ALEN); pkt.type = __constant_htons(ETH_P_LOOP); - for (i=0; i < MAX_LP_RETRY; i++) { + for (i = 0; i < MAX_LP_RETRY; i++) { struct sk_buff *skb; char *data; @@ -883,8 +879,7 @@ static void alb_swap_mac_addr(struct bon */ rlb_req_update_slave_clients(bond, slave1); } - } - else { + } else { disabled_slave = slave1; } @@ -896,8 +891,7 @@ static void alb_swap_mac_addr(struct bon */ rlb_req_update_slave_clients(bond, slave2); } - } - else { + } else { disabled_slave = slave2; } @@ -1159,11 +1153,11 @@ int bond_alb_xmit(struct sk_buff *skb, s struct ethhdr *eth_data = (struct ethhdr *)skb->mac.raw = skb->data; struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond)); struct slave *tx_slave = NULL; - char do_tx_balance = 1; + static u32 ip_bcast = 0xffffffff; int hash_size = 0; + int do_tx_balance = 1; u32 hash_index = 0; u8 *hash_start = NULL; - u8 mac_bcast[ETH_ALEN] = {0xff,0xff,0xff,0xff,0xff,0xff}; if (!IS_UP(bond_dev)) { /* bond down */ dev_kfree_skb(skb); @@ -1187,12 +1181,12 @@ int bond_alb_xmit(struct sk_buff *skb, s switch (ntohs(skb->protocol)) { case ETH_P_IP: if ((memcmp(eth_data->h_dest, mac_bcast, ETH_ALEN) == 0) || - (skb->nh.iph->daddr == 0xffffffff)) { + (skb->nh.iph->daddr == ip_bcast)) { do_tx_balance = 0; break; } hash_start = (char*)&(skb->nh.iph->daddr); - hash_size = 4; + hash_size = sizeof(skb->nh.iph->daddr); break; case ETH_P_IPV6: @@ -1202,7 +1196,7 @@ int bond_alb_xmit(struct sk_buff *skb, s } hash_start = (char*)&(skb->nh.ipv6h->daddr); - hash_size = 16; + hash_size = sizeof(skb->nh.ipv6h->daddr); break; case ETH_P_IPX: @@ -1334,8 +1328,7 @@ void bond_alb_monitor(struct bonding *bo */ write_lock(&bond->curr_slave_lock); if (bond_info->primary_is_promisc && - (++bond_info->rlb_promisc_timeout_counter >= - RLB_PROMISC_TIMEOUT)) { + (++bond_info->rlb_promisc_timeout_counter >= RLB_PROMISC_TIMEOUT)) { bond_info->rlb_promisc_timeout_counter = 0; @@ -1348,7 +1341,7 @@ void bond_alb_monitor(struct bonding *bo } write_unlock(&bond->curr_slave_lock); - if (bond_info->rlb_rebalance == 1) { + if (bond_info->rlb_rebalance) { bond_info->rlb_rebalance = 0; rlb_rebalance(bond); } @@ -1533,7 +1526,7 @@ int bond_alb_set_mac_address(struct net_ * Otherwise we'll need to pass the new address to it and handle * duplications. */ - if (bond->curr_active_slave == NULL) { + if (!bond->curr_active_slave) { return 0; } diff -Nuarp a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c --- a/drivers/net/bonding/bond_main.c Sun Dec 21 16:08:55 2003 +++ b/drivers/net/bonding/bond_main.c Sun Dec 21 16:08:56 2003 @@ -886,7 +886,7 @@ static int bond_open(struct net_device * add_timer(alb_timer); } - if (miimon > 0) { /* link check interval, in milliseconds. */ + if (miimon) { /* link check interval, in milliseconds. */ init_timer(timer); timer->expires = jiffies + 1; timer->data = (unsigned long)bond_dev; @@ -894,7 +894,7 @@ static int bond_open(struct net_device * add_timer(timer); } - if (arp_interval> 0) { /* arp interval, in milliseconds. */ + if (arp_interval) { /* arp interval, in milliseconds. */ init_timer(arp_timer); arp_timer->expires = jiffies + 1; arp_timer->data = (unsigned long)bond_dev; @@ -943,11 +943,11 @@ static int bond_close(struct net_device * because a running timer might be trying to hold it too */ - if (miimon > 0) { /* link check interval, in milliseconds. */ + if (miimon) { /* link check interval, in milliseconds. */ del_timer_sync(&bond->mii_timer); } - if (arp_interval> 0) { /* arp interval, in milliseconds. */ + if (arp_interval) { /* arp interval, in milliseconds. */ del_timer_sync(&bond->arp_timer); } @@ -984,8 +984,9 @@ static void bond_mc_list_flush(struct ne { struct dev_mc_list *dmi; - for (dmi = bond_dev->mc_list; dmi != NULL; dmi = dmi->next) + for (dmi = bond_dev->mc_list; dmi; dmi = dmi->next) { dev_mc_delete(slave_dev, dmi->dmi_addr, dmi->dmi_addrlen, 0); + } if (bond_mode == BOND_MODE_8023AD) { /* del lacpdu mc addr from mc list */ @@ -1018,8 +1019,9 @@ static void bond_mc_add(struct bonding * { if (USES_PRIMARY(bond_mode)) { /* write lock already acquired */ - if (bond->curr_active_slave != NULL) + if (bond->curr_active_slave) { dev_mc_add(bond->curr_active_slave->dev, addr, alen, 0); + } } else { struct slave *slave; int i; @@ -1037,8 +1039,9 @@ static void bond_mc_delete(struct bondin { if (USES_PRIMARY(bond_mode)) { /* write lock already acquired */ - if (bond->curr_active_slave != NULL) + if (bond->curr_active_slave) { dev_mc_delete(bond->curr_active_slave->dev, addr, alen, 0); + } } else { struct slave *slave; int i; @@ -1055,10 +1058,11 @@ static int bond_mc_list_copy(struct dev_ { struct dev_mc_list *dmi, *new_dmi; - for (dmi = mc_list; dmi != NULL; dmi = dmi->next) { + for (dmi = mc_list; dmi; dmi = dmi->next) { new_dmi = kmalloc(sizeof(struct dev_mc_list), gpf_flag); - if (new_dmi == NULL) { + if (!new_dmi) { + /* FIXME: Potential memory leak !!! */ return -ENOMEM; } @@ -1109,8 +1113,9 @@ static void bond_set_allmulti(struct bon { if (USES_PRIMARY(bond_mode)) { /* write lock already acquired */ - if (bond->curr_active_slave != NULL) + if (bond->curr_active_slave) { dev_set_allmulti(bond->curr_active_slave->dev, inc); + } } else { struct slave *slave; int i; @@ -1127,7 +1132,7 @@ static struct dev_mc_list *bond_mc_list_ { struct dev_mc_list *idmi; - for (idmi = mc_list; idmi != NULL; idmi = idmi->next) { + for (idmi = mc_list; idmi; idmi = idmi->next) { if (bond_is_dmi_same(dmi, idmi)) { return idmi; } @@ -1145,31 +1150,37 @@ static void bond_set_multicast_list(stru /* * Do promisc before checking multicast_mode */ - if ( (bond_dev->flags & IFF_PROMISC) && !(bond->flags & IFF_PROMISC) ) + if ( (bond_dev->flags & IFF_PROMISC) && !(bond->flags & IFF_PROMISC) ) { bond_set_promiscuity(bond, 1); + } - if ( !(bond_dev->flags & IFF_PROMISC) && (bond->flags & IFF_PROMISC) ) + if ( !(bond_dev->flags & IFF_PROMISC) && (bond->flags & IFF_PROMISC) ) { bond_set_promiscuity(bond, -1); + } /* set allmulti flag to slaves */ - if ( (bond_dev->flags & IFF_ALLMULTI) && !(bond->flags & IFF_ALLMULTI) ) + if ( (bond_dev->flags & IFF_ALLMULTI) && !(bond->flags & IFF_ALLMULTI) ) { bond_set_allmulti(bond, 1); + } - if ( !(bond_dev->flags & IFF_ALLMULTI) && (bond->flags & IFF_ALLMULTI) ) + if ( !(bond_dev->flags & IFF_ALLMULTI) && (bond->flags & IFF_ALLMULTI) ) { bond_set_allmulti(bond, -1); + } bond->flags = bond_dev->flags; /* looking for addresses to add to slaves' mc list */ - for (dmi = bond_dev->mc_list; dmi != NULL; dmi = dmi->next) { - if (bond_mc_list_find_dmi(dmi, bond->mc_list) == NULL) - bond_mc_add(bond, dmi->dmi_addr, dmi->dmi_addrlen); + for (dmi = bond_dev->mc_list; dmi; dmi = dmi->next) { + if (!bond_mc_list_find_dmi(dmi, bond->mc_list)) { + bond_mc_add(bond, dmi->dmi_addr, dmi->dmi_addrlen); + } } /* looking for addresses to delete from slaves' list */ - for (dmi = bond->mc_list; dmi != NULL; dmi = dmi->next) { - if (bond_mc_list_find_dmi(dmi, bond_dev->mc_list) == NULL) - bond_mc_delete(bond, dmi->dmi_addr, dmi->dmi_addrlen); + for (dmi = bond->mc_list; dmi; dmi = dmi->next) { + if (!bond_mc_list_find_dmi(dmi, bond_dev->mc_list)) { + bond_mc_delete(bond, dmi->dmi_addr, dmi->dmi_addrlen); + } } @@ -1203,7 +1214,7 @@ static void bond_mc_swap(struct bonding if (bond->dev->flags & IFF_ALLMULTI) { dev_set_allmulti(old_active->dev, -1); } - for (dmi = bond->dev->mc_list; dmi != NULL; dmi = dmi->next) { + for (dmi = bond->dev->mc_list; dmi; dmi = dmi->next) { dev_mc_delete(old_active->dev, dmi->dmi_addr, dmi->dmi_addrlen, 0); } } @@ -1215,7 +1226,7 @@ static void bond_mc_swap(struct bonding if (bond->dev->flags & IFF_ALLMULTI) { dev_set_allmulti(new_active->dev, 1); } - for (dmi = bond->dev->mc_list; dmi != NULL; dmi = dmi->next) { + for (dmi = bond->dev->mc_list; dmi; dmi = dmi->next) { dev_mc_add(new_active->dev, dmi->dmi_addr, dmi->dmi_addrlen, 0); } } @@ -1283,7 +1294,7 @@ static int bond_enslave(struct net_devic } if ((bond_mode == BOND_MODE_8023AD) || - (bond_mode == BOND_MODE_TLB) || + (bond_mode == BOND_MODE_TLB) || (bond_mode == BOND_MODE_ALB)) { printk(KERN_ERR DRV_NAME ": Error: to use %s mode, you must upgrade " @@ -1293,7 +1304,8 @@ static int bond_enslave(struct net_devic } } - if ((new_slave = kmalloc(sizeof(struct slave), GFP_KERNEL)) == NULL) { + new_slave = kmalloc(sizeof(struct slave), GFP_KERNEL); + if (!new_slave) { return -ENOMEM; } memset(new_slave, 0, sizeof(struct slave)); @@ -1364,12 +1376,14 @@ static int bond_enslave(struct net_devic dev_set_promiscuity(slave_dev, 1); } /* set allmulti level to new slave */ - if (bond_dev->flags & IFF_ALLMULTI) + if (bond_dev->flags & IFF_ALLMULTI) { dev_set_allmulti(slave_dev, 1); + } /* upload master's mc_list to new slave */ - for (dmi = bond_dev->mc_list; dmi != NULL; dmi = dmi->next) + for (dmi = bond_dev->mc_list; dmi; dmi = dmi->next) { dev_mc_add (slave_dev, dmi->dmi_addr, dmi->dmi_addrlen, 0); + } } if (bond_mode == BOND_MODE_8023AD) { @@ -1385,10 +1399,10 @@ static int bond_enslave(struct net_devic new_slave->delay = 0; new_slave->link_failure_count = 0; - if (miimon > 0 && !use_carrier) { + if (miimon && !use_carrier) { link_reporting = bond_check_dev_link(slave_dev, 1); - if ((link_reporting == -1) && (arp_interval == 0)) { + if ((link_reporting == -1) && !arp_interval) { /* * miimon is set but a bonded network driver * does not support ETHTOOL/MII and @@ -1418,22 +1432,20 @@ static int bond_enslave(struct net_devic } /* check for initial state */ - if ((miimon <= 0) || + if (!miimon || (bond_check_dev_link(slave_dev, 0) == BMSR_LSTATUS)) { if (updelay) { dprintk("Initial state of slave_dev is " "BOND_LINK_BACK\n"); new_slave->link = BOND_LINK_BACK; new_slave->delay = updelay; - } - else { + } else { dprintk("Initial state of slave_dev is " "BOND_LINK_UP\n"); new_slave->link = BOND_LINK_UP; } new_slave->jiffies = jiffies; - } - else { + } else { dprintk("Initial state of slave_dev is " "BOND_LINK_DOWN\n"); new_slave->link = BOND_LINK_DOWN; @@ -1454,7 +1466,7 @@ static int bond_enslave(struct net_devic } } - if (USES_PRIMARY(bond_mode) && primary != NULL) { + if (USES_PRIMARY(bond_mode) && primary) { /* if there is a primary slave, remember it */ if (strcmp(primary, new_slave->dev->name) == 0) { bond->primary_slave = new_slave; @@ -1470,15 +1482,14 @@ static int bond_enslave(struct net_devic * since we guarantee that curr_active_slave always point to the last * usable interface, we just have to verify this interface's flag. */ - if (((bond->curr_active_slave == NULL) - || (bond->curr_active_slave->dev->flags & IFF_NOARP)) - && (new_slave->link != BOND_LINK_DOWN)) { + if (((!bond->curr_active_slave) || + (bond->curr_active_slave->dev->flags & IFF_NOARP)) && + (new_slave->link != BOND_LINK_DOWN)) { dprintk("This is the first active slave\n"); /* first slave or no active slave yet, and this link is OK, so make this interface the active one */ bond_change_active_slave(bond, new_slave); - } - else { + } else { dprintk("This is just a backup slave\n"); bond_set_slave_inactive_flags(new_slave); } @@ -1507,7 +1518,8 @@ static int bond_enslave(struct net_devic case BOND_MODE_TLB: case BOND_MODE_ALB: new_slave->state = BOND_STATE_ACTIVE; - if ((bond->curr_active_slave == NULL) && (new_slave->link != BOND_LINK_DOWN)) { + if ((!bond->curr_active_slave) && + (new_slave->link != BOND_LINK_DOWN)) { /* first slave or no active slave yet, and this link * is OK, so make this interface the active one */ @@ -1523,9 +1535,9 @@ static int bond_enslave(struct net_devic * anyway (it holds no special properties of the bond device), * so we can change it without calling change_active_interface() */ - if (bond->curr_active_slave == NULL) + if (!bond->curr_active_slave) { bond->curr_active_slave = new_slave; - + } break; } /* switch(bond_mode) */ @@ -1627,9 +1639,9 @@ static int bond_ioctl_change_active(stru return 0; } - if ((new_active != NULL)&& - (old_active != NULL)&& - (new_active->link == BOND_LINK_UP)&& + if ((new_active) && + (old_active) && + (new_active->link == BOND_LINK_UP) && IS_UP(new_active->dev)) { bond_change_active_slave(bond, new_active); } else { @@ -1654,7 +1666,7 @@ static struct slave *bond_find_best_slav new_active = old_active = bond->curr_active_slave; - if (new_active == NULL) { /* there were no active slaves left */ + if (!new_active) { /* there were no active slaves left */ if (bond->slave_cnt > 0) { /* found one slave */ new_active = bond->first_slave; } else { @@ -1669,9 +1681,10 @@ static struct slave *bond_find_best_slav * slaves between the curr_active_slave and primary_slave that may be up * and able to arp */ - if ((bond->primary_slave != NULL) && (arp_interval == 0)) { - if (IS_UP(bond->primary_slave->dev)) - new_active = bond->primary_slave; + if ((bond->primary_slave) && + (!arp_interval) && + (IS_UP(bond->primary_slave->dev))) { + new_active = bond->primary_slave; } /* remember where to stop iterating over the slaves */ @@ -1681,8 +1694,7 @@ static struct slave *bond_find_best_slav if (IS_UP(new_active->dev)) { if (new_active->link == BOND_LINK_UP) { return new_active; - } - else if (new_active->link == BOND_LINK_BACK) { + } else if (new_active->link == BOND_LINK_BACK) { /* link up, but waiting for stabilization */ if (new_active->delay < mintime) { mintime = new_active->delay; @@ -1824,7 +1836,7 @@ static int bond_release(struct net_devic write_lock_bh(&bond->lock); slave = bond_get_slave_by_dev(bond, slave_dev); - if (slave == NULL) { + if (!slave) { /* not a slave of this bond */ printk(KERN_INFO DRV_NAME ": %s: %s not enslaved\n", @@ -1910,9 +1922,9 @@ static int bond_release(struct net_devic dev_set_promiscuity(slave_dev, -1); } /* unset allmulti level from slave */ - if (bond_dev->flags & IFF_ALLMULTI) + if (bond_dev->flags & IFF_ALLMULTI) { dev_set_allmulti(slave_dev, -1); - + } /* flush master's mc_list from slave */ bond_mc_list_flush (slave_dev, bond_dev); } @@ -2005,9 +2017,9 @@ static int bond_release_all(struct net_d dev_set_promiscuity(slave_dev, -1); } /* unset allmulti level from slave */ - if (bond_dev->flags & IFF_ALLMULTI) + if (bond_dev->flags & IFF_ALLMULTI) { dev_set_allmulti(slave_dev, -1); - + } /* flush master's mc_list from slave */ bond_mc_list_flush(slave_dev, bond_dev); } @@ -2096,14 +2108,13 @@ static void bond_mii_monitor(struct net_ if (link_state == BMSR_LSTATUS) { /* link stays up, nothing more to do */ break; - } - else { /* link going down */ + } else { /* link going down */ slave->link = BOND_LINK_FAIL; slave->delay = downdelay; if (slave->link_failure_count < UINT_MAX) { slave->link_failure_count++; } - if (downdelay > 0) { + if (downdelay) { printk(KERN_INFO DRV_NAME ": %s: link status down for %s " "interface %s, disabling it in " @@ -2181,7 +2192,7 @@ static void bond_mii_monitor(struct net_ slave->link = BOND_LINK_BACK; slave->delay = updelay; - if (updelay > 0) { + if (updelay) { /* if updelay == 0, no need to advertise about a 0 ms delay */ printk(KERN_INFO DRV_NAME @@ -2216,8 +2227,7 @@ static void bond_mii_monitor(struct net_ if (bond_mode == BOND_MODE_8023AD) { /* prevent it from being the active one */ slave->state = BOND_STATE_BACKUP; - } - else if (bond_mode != BOND_MODE_ACTIVEBACKUP) { + } else if (bond_mode != BOND_MODE_ACTIVEBACKUP) { /* make it immediately active */ slave->state = BOND_STATE_ACTIVE; } else if (slave != bond->primary_slave) { @@ -2241,7 +2251,7 @@ static void bond_mii_monitor(struct net_ bond_alb_handle_link_change(bond, slave, BOND_LINK_UP); } - if ((oldcurrent == NULL) || + if ((!oldcurrent) || (slave == bond->primary_slave)) { do_failover = 1; } @@ -2250,7 +2260,12 @@ static void bond_mii_monitor(struct net_ } } break; - } /* end of switch */ + default: + /* Should not happen */ + printk(KERN_ERR "bonding: Error: %s Illegal value (link=%d)\n", + slave->dev->name, slave->link); + goto out; + } /* end of switch (slave->link) */ bond_update_speed_duplex(slave); @@ -2326,10 +2341,8 @@ static void bond_loadbalance_arp_mon(str if (slave->link != BOND_LINK_UP) { - if (((jiffies - slave->dev->trans_start) <= - delta_in_ticks) && - ((jiffies - slave->dev->last_rx) <= - delta_in_ticks)) { + if (((jiffies - slave->dev->trans_start) <= delta_in_ticks) && + ((jiffies - slave->dev->last_rx) <= delta_in_ticks)) { slave->link = BOND_LINK_UP; slave->state = BOND_STATE_ACTIVE; @@ -2339,7 +2352,7 @@ static void bond_loadbalance_arp_mon(str * curr_active_slave being null after enslaving * is closed. */ - if (oldcurrent == NULL) { + if (!oldcurrent) { printk(KERN_INFO DRV_NAME ": %s: link status definitely " "up for interface %s, ", @@ -2360,10 +2373,9 @@ static void bond_loadbalance_arp_mon(str * when the source ip is 0, so don't take the link down * if we don't know our ip yet */ - if (((jiffies - slave->dev->trans_start) >= - (2*delta_in_ticks)) || - (((jiffies - slave->dev->last_rx) >= - (2*delta_in_ticks)) && my_ip !=0)) { + if (((jiffies - slave->dev->trans_start) >= (2*delta_in_ticks)) || + (((jiffies - slave->dev->last_rx) >= (2*delta_in_ticks)) && + my_ip)) { slave->link = BOND_LINK_DOWN; slave->state = BOND_STATE_BACKUP; if (slave->link_failure_count < UINT_MAX) { @@ -2452,14 +2464,12 @@ static void bond_activebackup_arp_mon(st bond_for_each_slave(bond, slave, i) { if (slave->link != BOND_LINK_UP) { - if ((jiffies - slave->dev->last_rx) <= - delta_in_ticks) { + if ((jiffies - slave->dev->last_rx) <= delta_in_ticks) { slave->link = BOND_LINK_UP; write_lock(&bond->curr_slave_lock); - if ((bond->curr_active_slave == NULL) && - ((jiffies - slave->dev->trans_start) <= - delta_in_ticks)) { + if ((!bond->curr_active_slave) && + ((jiffies - slave->dev->trans_start) <= delta_in_ticks)) { bond_change_active_slave(bond, slave); bond->current_arp_slave = NULL; } else if (bond->curr_active_slave != slave) { @@ -2492,9 +2502,9 @@ static void bond_activebackup_arp_mon(st } else { read_lock(&bond->curr_slave_lock); if ((slave != bond->curr_active_slave) && - (bond->current_arp_slave == NULL) && - (((jiffies - slave->dev->last_rx) >= - 3*delta_in_ticks) && (my_ip != 0))) { + (!bond->current_arp_slave) && + (((jiffies - slave->dev->last_rx) >= 3*delta_in_ticks) && + my_ip)) { /* a backup slave has gone down; three times * the delta allows the current slave to be * taken out before the backup slave. @@ -2525,7 +2535,7 @@ static void bond_activebackup_arp_mon(st slave = bond->curr_active_slave; read_unlock(&bond->curr_slave_lock); - if (slave != NULL) { + if (slave) { /* if we have sent traffic in the past 2*arp_intervals but * haven't xmit and rx traffic in that time interval, select @@ -2535,10 +2545,9 @@ static void bond_activebackup_arp_mon(st * before being taken out. if a primary is being used, check * if it is up and needs to take over as the curr_active_slave */ - if ((((jiffies - slave->dev->trans_start) >= - (2*delta_in_ticks)) || - (((jiffies - slave->dev->last_rx) >= - (2*delta_in_ticks)) && (my_ip != 0))) && + if ((((jiffies - slave->dev->trans_start) >= (2*delta_in_ticks)) || + (((jiffies - slave->dev->last_rx) >= (2*delta_in_ticks)) && + my_ip)) && ((jiffies - slave->jiffies) >= 2*delta_in_ticks)) { slave->link = BOND_LINK_DOWN; @@ -2555,11 +2564,11 @@ static void bond_activebackup_arp_mon(st slave = bond->curr_active_slave; write_unlock(&bond->curr_slave_lock); bond->current_arp_slave = slave; - if (slave != NULL) { + if (slave) { slave->jiffies = jiffies; } - } else if ((bond->primary_slave != NULL) && + } else if ((bond->primary_slave) && (bond->primary_slave != slave) && (bond->primary_slave->link == BOND_LINK_UP)) { /* at this point, slave is the curr_active_slave */ @@ -2583,7 +2592,7 @@ static void bond_activebackup_arp_mon(st /* the current slave must tx an arp to ensure backup slaves * rx traffic */ - if ((slave != NULL) && (my_ip != 0)) { + if (slave && my_ip) { bond_arp_send_all(slave); } } @@ -2592,7 +2601,7 @@ static void bond_activebackup_arp_mon(st * backup slave from the current_arp_slave and make it the candidate * for becoming the curr_active_slave */ - if (slave == NULL) { + if (!slave) { if (bond->current_arp_slave == NULL) { bond->current_arp_slave = bond->first_slave; @@ -2621,8 +2630,7 @@ static void bond_activebackup_arp_mon(st */ if (slave->link == BOND_LINK_UP) { slave->link = BOND_LINK_DOWN; - if (slave->link_failure_count < - UINT_MAX) { + if (slave->link_failure_count < UINT_MAX) { slave->link_failure_count++; } @@ -2701,50 +2709,49 @@ static int bond_ethtool_ioctl(struct net { void *addr = ifr->ifr_data; uint32_t cmd; + struct ethtool_drvinfo info; + char *endptr; - if (get_user(cmd, (uint32_t *)addr)) + if (get_user(cmd, (uint32_t *)addr)) { return -EFAULT; + } switch (cmd) { case ETHTOOL_GDRVINFO: - { - struct ethtool_drvinfo info; - char *endptr; - - if (copy_from_user(&info, addr, sizeof(info))) - return -EFAULT; - - if (strcmp(info.driver, "ifenslave") == 0) { - int new_abi_ver; + if (copy_from_user(&info, addr, sizeof(info))) { + return -EFAULT; + } - new_abi_ver = simple_strtoul(info.fw_version, - &endptr, 0); - if (*endptr) { - printk(KERN_ERR DRV_NAME - ": Error: got invalid ABI " - "version from application\n"); + if (strcmp(info.driver, "ifenslave") == 0) { + int new_abi_ver; - return -EINVAL; - } + new_abi_ver = simple_strtoul(info.fw_version, + &endptr, 0); + if (*endptr) { + printk(KERN_ERR DRV_NAME + ": Error: got invalid ABI " + "version from application\n"); - if (orig_app_abi_ver == -1) { - orig_app_abi_ver = new_abi_ver; - } + return -EINVAL; + } - app_abi_ver = new_abi_ver; + if (orig_app_abi_ver == -1) { + orig_app_abi_ver = new_abi_ver; } - strncpy(info.driver, DRV_NAME, 32); - strncpy(info.version, DRV_VERSION, 32); - snprintf(info.fw_version, 32, "%d", BOND_ABI_VERSION); + app_abi_ver = new_abi_ver; + } - if (copy_to_user(addr, &info, sizeof(info))) - return -EFAULT; + strncpy(info.driver, DRV_NAME, 32); + strncpy(info.version, DRV_VERSION, 32); + snprintf(info.fw_version, 32, "%d", BOND_ABI_VERSION); - return 0; + if (copy_to_user(addr, &info, sizeof(info))) { + return -EFAULT; } - break; + + return 0; default: return -EOPNOTSUPP; } @@ -2768,7 +2775,7 @@ static int bond_do_ioctl(struct net_devi case SIOCGMIIPHY: mii = (struct mii_ioctl_data *)&ifr->ifr_data; - if (mii == NULL) { + if (!mii) { return -EINVAL; } mii->phy_id = 0; @@ -2779,7 +2786,7 @@ static int bond_do_ioctl(struct net_devi * instead of SIOCGMIIPHY. */ mii = (struct mii_ioctl_data *)&ifr->ifr_data; - if (mii == NULL) { + if (!mii) { return -EINVAL; } if (mii->reg_num == 1) { @@ -2820,6 +2827,9 @@ static int bond_do_ioctl(struct net_devi } } return res; + default: + /* Go on */ + break; } if (!capable(CAP_NET_ADMIN)) { @@ -2845,7 +2855,7 @@ static int bond_do_ioctl(struct net_devi dprintk("slave_dev=%p: \n", slave_dev); - if (slave_dev == NULL) { + if (!slave_dev) { res = -ENODEV; } else { dprintk("slave_dev->name=%s: \n", slave_dev->name); @@ -2866,8 +2876,7 @@ static int bond_do_ioctl(struct net_devi case SIOCBONDCHANGEACTIVE: if (USES_PRIMARY(bond_mode)) { res = bond_ioctl_change_active(bond_dev, slave_dev); - } - else { + } else { res = -EINVAL; } break; @@ -2916,7 +2925,7 @@ static int bond_xmit_broadcast(struct sk start_at = bond->curr_active_slave; read_unlock(&bond->curr_slave_lock); - if (start_at == NULL) { /* we're at the root, get the first slave */ + if (!start_at) { /* we're at the root, get the first slave */ /* no suitable interface, frame not sent */ read_unlock(&bond->lock); dev_kfree_skb(skb); @@ -2924,12 +2933,12 @@ static int bond_xmit_broadcast(struct sk } bond_for_each_slave_from(bond, slave, i, start_at) { - if (IS_UP(slave->dev) - && (slave->link == BOND_LINK_UP) - && (slave->state == BOND_STATE_ACTIVE)) { + if (IS_UP(slave->dev) && + (slave->link == BOND_LINK_UP) && + (slave->state == BOND_STATE_ACTIVE)) { if (tx_dev) { - struct sk_buff *skb2; - if ((skb2 = skb_clone(skb, GFP_ATOMIC)) == NULL) { + struct sk_buff *skb2 = skb_clone(skb, GFP_ATOMIC); + if (!skb2) { printk(KERN_ERR DRV_NAME ": Error: bond_xmit_broadcast(): " "skb_clone() failed\n"); @@ -2948,8 +2957,9 @@ static int bond_xmit_broadcast(struct sk skb->dev = tx_dev; skb->priority = 1; dev_queue_xmit(skb); - } else + } else { dev_kfree_skb(skb); + } /* frame sent to all suitable interfaces */ read_unlock(&bond->lock); @@ -2973,7 +2983,7 @@ static int bond_xmit_roundrobin(struct s slave = start_at = bond->curr_active_slave; read_unlock(&bond->curr_slave_lock); - if (slave == NULL) { /* we're at the root, get the first slave */ + if (!slave) { /* we're at the root, get the first slave */ /* no suitable interface, frame not sent */ dev_kfree_skb(skb); read_unlock(&bond->lock); @@ -2981,9 +2991,9 @@ static int bond_xmit_roundrobin(struct s } bond_for_each_slave_from(bond, slave, i, start_at) { - if (IS_UP(slave->dev) - && (slave->link == BOND_LINK_UP) - && (slave->state == BOND_STATE_ACTIVE)) { + if (IS_UP(slave->dev) && + (slave->link == BOND_LINK_UP) && + (slave->state == BOND_STATE_ACTIVE)) { skb->dev = slave->dev; skb->priority = 1; @@ -3043,9 +3053,9 @@ static int bond_xmit_xor(struct sk_buff start_at = slave; bond_for_each_slave_from(bond, slave, i, start_at) { - if (IS_UP(slave->dev) - && (slave->link == BOND_LINK_UP) - && (slave->state == BOND_STATE_ACTIVE)) { + if (IS_UP(slave->dev) && + (slave->link == BOND_LINK_UP) && + (slave->state == BOND_STATE_ACTIVE)) { skb->dev = slave->dev; skb->priority = 1; @@ -3077,11 +3087,11 @@ static int bond_xmit_activebackup(struct /* if we are sending arp packets, try to at least identify our own ip address */ - if ( (arp_interval > 0) && (my_ip == 0) && - (skb->protocol == __constant_htons(ETH_P_ARP) ) ) { - char *the_ip = (((char *)skb->data)) - + sizeof(struct ethhdr) - + sizeof(struct arphdr) + + if (arp_interval && !my_ip && + (skb->protocol == __constant_htons(ETH_P_ARP))) { + char *the_ip = (((char *)skb->data)) + + sizeof(struct ethhdr) + + sizeof(struct arphdr) + ETH_ALEN; memcpy(&my_ip, the_ip, 4); } @@ -3089,15 +3099,14 @@ static int bond_xmit_activebackup(struct read_lock(&bond->lock); read_lock(&bond->curr_slave_lock); - if (bond->curr_active_slave != NULL) { /* one usable interface */ + if (bond->curr_active_slave) { /* one usable interface */ skb->dev = bond->curr_active_slave->dev; read_unlock(&bond->curr_slave_lock); skb->priority = 1; dev_queue_xmit(skb); read_unlock(&bond->lock); return 0; - } - else { + } else { read_unlock(&bond->curr_slave_lock); } @@ -3616,7 +3625,7 @@ static int bond_slave_netdev_event(unsig switch (event) { case NETDEV_UNREGISTER: - if (bond_dev != NULL) { + if (bond_dev) { bond_release(bond_dev, slave_dev); } break; @@ -3791,7 +3800,7 @@ static int __init bond_init(struct net_d #endif printk(KERN_INFO DRV_NAME ": %s registered with", bond_dev->name); - if (miimon > 0) { + if (miimon) { printk(" MII link monitoring set to %d ms", miimon); updelay /= miimon; downdelay /= miimon; @@ -3804,8 +3813,9 @@ static int __init bond_init(struct net_d if (arp_interval > 0) { printk(" ARP monitoring set to %d ms with %d target(s):", arp_interval, arp_ip_count); - for (count=0 ; count 0) && (arp_ip_count==0)) { + if (arp_interval && !arp_ip_count) { /* don't allow arping if no arp_ip_target given... */ printk(KERN_WARNING DRV_NAME ": Warning: arp_interval module parameter (%d) " @@ -4031,7 +4041,7 @@ static int bond_check_params(void) arp_interval = 0; } - if ((miimon == 0) && (arp_interval == 0)) { + if (!miimon && !arp_interval) { /* miimon and arp_interval not set, we need one so things * work as expected, see bonding.txt for details */ @@ -4042,7 +4052,7 @@ static int bond_check_params(void) "bonding.txt for details.\n"); } - if ((primary != NULL) && !USES_PRIMARY(bond_mode)) { + if (primary && !USES_PRIMARY(bond_mode)) { /* currently, using a primary only makes sense * in active backup, TLB or ALB modes */