[PATCH 1/5] bonding: Changed hashing function to just provide hash

[PATCH 1/5] bonding: Changed hashing function to just provide hash

[Mahesh Bandewar]

Modified the hash function to return just hash separating from the
modulo operation that can be performed by the caller. This is to
make way for the tlb mode to use the same hashing policies that
are used in the 802.3ad and Xor mode.

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
 drivers/net/bonding/bond_3ad.c  |  3 ++-
 drivers/net/bonding/bond_main.c | 10 ++++------
 drivers/net/bonding/bonding.h   |  2 +-
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index b667a51ed215..ccf2639e916f 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -30,6 +30,7 @@
 #include <linux/etherdevice.h>
 #include <linux/if_bonding.h>
 #include <linux/pkt_sched.h>
+#include <net/sock.h>
 #include <net/net_namespace.h>
 #include "bonding.h"
 #include "bond_3ad.h"
@@ -2440,7 +2441,7 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
 		goto err_free;
 	}
 
-	slave_agg_no = bond_xmit_hash(bond, skb, slaves_in_agg);
+	slave_agg_no = bond_xmit_hash(bond, skb) % slaves_in_agg;
 	first_ok_slave = NULL;
 
 	bond_for_each_slave_rcu(bond, slave, iter) {
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 5be34b72a048..06024abcffcf 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3015,20 +3015,18 @@ static bool bond_flow_dissect(struct bonding *bond, struct sk_buff *skb,
  * bond_xmit_hash - generate a hash value based on the xmit policy
  * @bond: bonding device
  * @skb: buffer to use for headers
- * @count: modulo value
  *
  * This function will extract the necessary headers from the skb buffer and use
  * them to generate a hash based on the xmit_policy set in the bonding device
- * which will be reduced modulo count before returning.
  */
-int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb, int count)
+int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb)
 {
 	struct flow_keys flow;
 	u32 hash;
 
 	if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER2 ||
 	    !bond_flow_dissect(bond, skb, &flow))
-		return bond_eth_hash(skb) % count;
+		return bond_eth_hash(skb);
 
 	if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER23 ||
 	    bond->params.xmit_policy == BOND_XMIT_POLICY_ENCAP23)
@@ -3039,7 +3037,7 @@ int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb, int count)
 	hash ^= (hash >> 16);
 	hash ^= (hash >> 8);
 
-	return hash % count;
+	return hash;
 }
 
 /*-------------------------- Device entry points ----------------------------*/
@@ -3666,7 +3664,7 @@ static int bond_xmit_xor(struct sk_buff *skb, struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
 
-	bond_xmit_slave_id(bond, skb, bond_xmit_hash(bond, skb, bond->slave_cnt));
+	bond_xmit_slave_id(bond, skb, bond_xmit_hash(bond, skb) % bond->slave_cnt);
 
 	return NETDEV_TX_OK;
 }
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index b8bdd0acc8f3..f91f5dd219d6 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -499,7 +499,7 @@ int bond_sysfs_slave_add(struct slave *slave);
 void bond_sysfs_slave_del(struct slave *slave);
 int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev);
 int bond_release(struct net_device *bond_dev, struct net_device *slave_dev);
-int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb, int count);
+int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb);
 void bond_select_active_slave(struct bonding *bond);
 void bond_change_active_slave(struct bonding *bond, struct slave *new_active);
 void bond_create_debugfs(void);
-- 
1.9.1.423.g4596e3a

Re: [PATCH 1/5] bonding: Changed hashing function to just provide hash

[Eric Dumazet]

On Fri, 2014-03-28 at 22:28 -0700, Mahesh Bandewar wrote:
> Modified the hash function to return just hash separating from the
> modulo operation that can be performed by the caller. This is to
> make way for the tlb mode to use the same hashing policies that
> are used in the 802.3ad and Xor mode.
> 
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> ---


> -int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb, int count)
> +int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb)
>  {
>  	struct flow_keys flow;
>  	u32 hash;
>  
>  	if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER2 ||
>  	    !bond_flow_dissect(bond, skb, &flow))
> -		return bond_eth_hash(skb) % count;
> +		return bond_eth_hash(skb);
>  
>  	if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER23 ||
>  	    bond->params.xmit_policy == BOND_XMIT_POLICY_ENCAP23)
> @@ -3039,7 +3037,7 @@ int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb, int count)
>  	hash ^= (hash >> 16);
>  	hash ^= (hash >> 8);
>  
> -	return hash % count;
> +	return hash;
>  }


Sounds good, but I would change  bond_xmit_hash() to return an u32
instead of 'int'

There is an interesting mix of u32 and int in this bonding code :(

slave_cnt is signed, so a  (signed_hash % signed_slave_cnt) could
potentially be negative ?

Re: [PATCH 1/5] bonding: Changed hashing function to just provide hash

[Mahesh Bandewar]

On Mon, Mar 31, 2014 at 8:44 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2014-03-28 at 22:28 -0700, Mahesh Bandewar wrote:
>> Modified the hash function to return just hash separating from the
>> modulo operation that can be performed by the caller. This is to
>> make way for the tlb mode to use the same hashing policies that
>> are used in the 802.3ad and Xor mode.
>>
>> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>> ---
>
>
>> -int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb, int count)
>> +int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb)
>>  {
>>       struct flow_keys flow;
>>       u32 hash;
>>
>>       if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER2 ||
>>           !bond_flow_dissect(bond, skb, &flow))
>> -             return bond_eth_hash(skb) % count;
>> +             return bond_eth_hash(skb);
>>
>>       if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER23 ||
>>           bond->params.xmit_policy == BOND_XMIT_POLICY_ENCAP23)
>> @@ -3039,7 +3037,7 @@ int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb, int count)
>>       hash ^= (hash >> 16);
>>       hash ^= (hash >> 8);
>>
>> -     return hash % count;
>> +     return hash;
>>  }
>
>
> Sounds good, but I would change  bond_xmit_hash() to return an u32
> instead of 'int'
>
> There is an interesting mix of u32 and int in this bonding code :(
>
> slave_cnt is signed, so a  (signed_hash % signed_slave_cnt) could
> potentially be negative ?
>
>
Yes, it does make sense to change it to u32. Thanks.