Re: [PATCH net-next 3/4] bonding: allow use of tx hashing in balance-alb
Debabrata Banerjee wrote:
>The rx load balancing provided by balance-alb is not mutually
>exclusive with using hashing for tx selection, and should provide a decent
>speed increase because this eliminates spinlocks and cache contention.
>
>Signed-off-by: Debabrata Banerjee
>---
> drivers/net/bonding/bond_alb.c | 20 ++--
> drivers/net/bonding/bond_main.c| 25 +++--
> drivers/net/bonding/bond_options.c | 2 +-
> include/net/bonding.h | 10 +-
> 4 files changed, 43 insertions(+), 14 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>index 180e50f7806f..6228635880d5 100644
>--- a/drivers/net/bonding/bond_alb.c
>+++ b/drivers/net/bonding/bond_alb.c
>@@ -1478,8 +1478,24 @@ int bond_alb_xmit(struct sk_buff *skb, struct
>net_device *bond_dev)
> }
>
> if (do_tx_balance) {
>- hash_index = _simple_hash(hash_start, hash_size);
>- tx_slave = tlb_choose_channel(bond, hash_index, skb->len);
>+ if (bond->params.tlb_dynamic_lb) {
>+ hash_index = _simple_hash(hash_start, hash_size);
>+ tx_slave = tlb_choose_channel(bond, hash_index,
>skb->len);
>+ } else {
>+ /*
>+ * do_tx_balance means we are free to select the
>tx_slave
>+ * So we do exactly what tlb would do for hash selection
>+ */
>+
>+ struct bond_up_slave *slaves;
>+ unsigned int count;
>+
>+ slaves = rcu_dereference(bond->slave_arr);
>+ count = slaves ? READ_ONCE(slaves->count) : 0;
>+ if (likely(count))
>+ tx_slave = slaves->arr[bond_xmit_hash(bond,
>skb) %
>+ count];
>+ }
> }
>
> return bond_do_alb_xmit(skb, bond, tx_slave);
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 1f1e97b26f95..f7f8a49cb32b 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -159,7 +159,7 @@ module_param(min_links, int, 0);
> MODULE_PARM_DESC(min_links, "Minimum number of available links before turning
> on carrier");
>
> module_param(xmit_hash_policy, charp, 0);
>-MODULE_PARM_DESC(xmit_hash_policy, "balance-xor and 802.3ad hashing method; "
>+MODULE_PARM_DESC(xmit_hash_policy, "balance-alb, balance-tlb, balance-xor,
>802.3ad hashing method; "
> "0 for layer 2 (default), 1 for layer 3+4, "
> "2 for layer 2+3, 3 for encap layer 2+3, "
> "4 for encap layer 3+4");
>@@ -1735,7 +1735,7 @@ int bond_enslave(struct net_device *bond_dev, struct
>net_device *slave_dev,
> unblock_netpoll_tx();
> }
>
>- if (bond_mode_uses_xmit_hash(bond))
>+ if (bond_mode_can_use_xmit_hash(bond))
> bond_update_slave_arr(bond, NULL);
>
> bond->nest_level = dev_get_nest_level(bond_dev);
>@@ -1870,7 +1870,7 @@ static int __bond_release_one(struct net_device
>*bond_dev,
> if (BOND_MODE(bond) == BOND_MODE_8023AD)
> bond_3ad_unbind_slave(slave);
>
>- if (bond_mode_uses_xmit_hash(bond))
>+ if (bond_mode_can_use_xmit_hash(bond))
> bond_update_slave_arr(bond, slave);
>
> netdev_info(bond_dev, "Releasing %s interface %s\n",
>@@ -3102,7 +3102,7 @@ static int bond_slave_netdev_event(unsigned long event,
>* events. If these (miimon/arpmon) parameters are configured
>* then array gets refreshed twice and that should be fine!
>*/
>- if (bond_mode_uses_xmit_hash(bond))
>+ if (bond_mode_can_use_xmit_hash(bond))
> bond_update_slave_arr(bond, NULL);
> break;
> case NETDEV_CHANGEMTU:
>@@ -3322,7 +3322,7 @@ static int bond_open(struct net_device *bond_dev)
>*/
> if (bond_alb_initialize(bond, (BOND_MODE(bond) ==
> BOND_MODE_ALB)))
> return -ENOMEM;
>- if (bond->params.tlb_dynamic_lb)
>+ if (bond->params.tlb_dynamic_lb || BOND_MODE(bond) ==
>BOND_MODE_ALB)
> queue_delayed_work(bond->wq, &bond->alb_work, 0);
> }
>
>@@ -3341,7 +3341,7 @@ static int bond_open(struct net_device *bond_dev)
> bond_3ad_initiate_agg_selection(bond, 1);
> }
>
>- if (bond_mode_uses_xmit_hash(bond))
>+ if (bond_mode_can_use_xmit_hash(bond))
> bond_update_slave_arr(bond, NULL);
>
> return 0;
>@@ -3892,7 +3892,7 @@ static void bond_slave_arr_handler(struct work_struct
>*work)
> * to determine the slave interface -
> * (a) BOND_MODE_8023AD
> * (b) BOND_MODE_XOR
>- * (c) BOND_MODE_TLB && tlb_dynamic_lb == 0
>
[PATCH net-next 3/4] bonding: allow use of tx hashing in balance-alb
The rx load balancing provided by balance-alb is not mutually
exclusive with using hashing for tx selection, and should provide a decent
speed increase because this eliminates spinlocks and cache contention.
Signed-off-by: Debabrata Banerjee
---
drivers/net/bonding/bond_alb.c | 20 ++--
drivers/net/bonding/bond_main.c| 25 +++--
drivers/net/bonding/bond_options.c | 2 +-
include/net/bonding.h | 10 +-
4 files changed, 43 insertions(+), 14 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 180e50f7806f..6228635880d5 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1478,8 +1478,24 @@ int bond_alb_xmit(struct sk_buff *skb, struct net_device
*bond_dev)
}
if (do_tx_balance) {
- hash_index = _simple_hash(hash_start, hash_size);
- tx_slave = tlb_choose_channel(bond, hash_index, skb->len);
+ if (bond->params.tlb_dynamic_lb) {
+ hash_index = _simple_hash(hash_start, hash_size);
+ tx_slave = tlb_choose_channel(bond, hash_index,
skb->len);
+ } else {
+ /*
+* do_tx_balance means we are free to select the
tx_slave
+* So we do exactly what tlb would do for hash selection
+*/
+
+ struct bond_up_slave *slaves;
+ unsigned int count;
+
+ slaves = rcu_dereference(bond->slave_arr);
+ count = slaves ? READ_ONCE(slaves->count) : 0;
+ if (likely(count))
+ tx_slave = slaves->arr[bond_xmit_hash(bond,
skb) %
+ count];
+ }
}
return bond_do_alb_xmit(skb, bond, tx_slave);
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 1f1e97b26f95..f7f8a49cb32b 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -159,7 +159,7 @@ module_param(min_links, int, 0);
MODULE_PARM_DESC(min_links, "Minimum number of available links before turning
on carrier");
module_param(xmit_hash_policy, charp, 0);
-MODULE_PARM_DESC(xmit_hash_policy, "balance-xor and 802.3ad hashing method; "
+MODULE_PARM_DESC(xmit_hash_policy, "balance-alb, balance-tlb, balance-xor,
802.3ad hashing method; "
"0 for layer 2 (default), 1 for layer 3+4, "
"2 for layer 2+3, 3 for encap layer 2+3, "
"4 for encap layer 3+4");
@@ -1735,7 +1735,7 @@ int bond_enslave(struct net_device *bond_dev, struct
net_device *slave_dev,
unblock_netpoll_tx();
}
- if (bond_mode_uses_xmit_hash(bond))
+ if (bond_mode_can_use_xmit_hash(bond))
bond_update_slave_arr(bond, NULL);
bond->nest_level = dev_get_nest_level(bond_dev);
@@ -1870,7 +1870,7 @@ static int __bond_release_one(struct net_device *bond_dev,
if (BOND_MODE(bond) == BOND_MODE_8023AD)
bond_3ad_unbind_slave(slave);
- if (bond_mode_uses_xmit_hash(bond))
+ if (bond_mode_can_use_xmit_hash(bond))
bond_update_slave_arr(bond, slave);
netdev_info(bond_dev, "Releasing %s interface %s\n",
@@ -3102,7 +3102,7 @@ static int bond_slave_netdev_event(unsigned long event,
* events. If these (miimon/arpmon) parameters are configured
* then array gets refreshed twice and that should be fine!
*/
- if (bond_mode_uses_xmit_hash(bond))
+ if (bond_mode_can_use_xmit_hash(bond))
bond_update_slave_arr(bond, NULL);
break;
case NETDEV_CHANGEMTU:
@@ -3322,7 +3322,7 @@ static int bond_open(struct net_device *bond_dev)
*/
if (bond_alb_initialize(bond, (BOND_MODE(bond) ==
BOND_MODE_ALB)))
return -ENOMEM;
- if (bond->params.tlb_dynamic_lb)
+ if (bond->params.tlb_dynamic_lb || BOND_MODE(bond) ==
BOND_MODE_ALB)
queue_delayed_work(bond->wq, &bond->alb_work, 0);
}
@@ -3341,7 +3341,7 @@ static int bond_open(struct net_device *bond_dev)
bond_3ad_initiate_agg_selection(bond, 1);
}
- if (bond_mode_uses_xmit_hash(bond))
+ if (bond_mode_can_use_xmit_hash(bond))
bond_update_slave_arr(bond, NULL);
return 0;
@@ -3892,7 +3892,7 @@ static void bond_slave_arr_handler(struct work_struct
*work)
* to determine the slave interface -
* (a) BOND_MODE_8023AD
* (b) BOND_MODE_XOR
- * (c) BOND_MODE_TLB && tlb_dynamic_lb == 0
+ * (c) (BOND_MODE_TLB || BOND_MODE_ALB) && tlb_dynamic_lb == 0
*
* The caller is expec
