* [PATCH 4/5] bonding: Added bond_tlb_xmit() for tlb mode.
@ 2014-03-29 5:29 Mahesh Bandewar
2014-03-31 15:52 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: Mahesh Bandewar @ 2014-03-29 5:29 UTC (permalink / raw)
To: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David Miller
Cc: netdev, Mahesh Bandewar, Eric Dumazet, Maciej Zenczykowski
Re-organized the xmit function for the lb mode separating tlb xmit
from the alb mode. This will enable use of the hashing policies
like 802.3ad mode. Also extended use of xmit-hash-policy to tlb mode.
Now the tlb-mode defaults to BOND_XMIT_POLICY_LAYER2 if the xmit policy
module parameter is not set (just like 802.3ad, or Xor mode).
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
drivers/net/bonding/bond_alb.c | 25 +++++++++++++++++++++++++
drivers/net/bonding/bond_alb.h | 1 +
drivers/net/bonding/bond_main.c | 6 ++++--
drivers/net/bonding/bond_options.c | 2 +-
4 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 5cd36016c393..8b7426ce6182 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1381,6 +1381,31 @@ out:
return NETDEV_TX_OK;
}
+int bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
+{
+ struct bonding *bond = netdev_priv(bond_dev);
+ struct ethhdr *eth_data;
+ struct slave *tx_slave = NULL;
+ u32 hash_index = 0;
+
+ skb_reset_mac_header(skb);
+ eth_data = eth_hdr(skb);
+
+ /* Do not TX balance any multicast or broadcast */
+ if (!is_multicast_ether_addr(eth_data->h_dest)) {
+ switch (ntohs(skb->protocol)) {
+ case ETH_P_IP:
+ case ETH_P_IPX: /* In case of IPX, it will falback to L2 hash */
+ case ETH_P_IPV6:
+ hash_index = bond_xmit_hash(bond, skb);
+ tx_slave = tlb_choose_channel(bond, hash_index & 0xFF, skb->len);
+ break;
+ }
+ }
+
+ return bond_do_alb_xmit(skb, bond, tx_slave);
+}
+
int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
{
struct bonding *bond = netdev_priv(bond_dev);
diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
index e09dd4bfafff..5fc76c01636c 100644
--- a/drivers/net/bonding/bond_alb.h
+++ b/drivers/net/bonding/bond_alb.h
@@ -175,6 +175,7 @@ void bond_alb_deinit_slave(struct bonding *bond, struct slave *slave);
void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char link);
void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave);
int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev);
+int bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev);
void bond_alb_monitor(struct work_struct *);
int bond_alb_set_mac_address(struct net_device *bond_dev, void *addr);
void bond_alb_clear_vlan(struct bonding *bond, unsigned short vlan_id);
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e4f062ddc23e..23e4073ec798 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3776,8 +3776,9 @@ static netdev_tx_t __bond_start_xmit(struct sk_buff *skb, struct net_device *dev
case BOND_MODE_8023AD:
return bond_3ad_xmit_xor(skb, dev);
case BOND_MODE_ALB:
- case BOND_MODE_TLB:
return bond_alb_xmit(skb, dev);
+ case BOND_MODE_TLB:
+ return bond_tlb_xmit(skb, dev);
default:
/* Should never happen, mode already checked */
pr_err("%s: Error: Unknown bonding mode %d\n",
@@ -3998,7 +3999,8 @@ static int bond_check_params(struct bond_params *params)
if (xmit_hash_policy) {
if ((bond_mode != BOND_MODE_XOR) &&
- (bond_mode != BOND_MODE_8023AD)) {
+ (bond_mode != BOND_MODE_8023AD) &&
+ (bond_mode != BOND_MODE_TLB)) {
pr_info("xmit_hash_policy param is irrelevant in mode %s\n",
bond_mode_name(bond_mode));
} else {
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 724e30fa20b9..dc3893841752 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -199,7 +199,7 @@ static const struct bond_option bond_opts[] = {
[BOND_OPT_XMIT_HASH] = {
.id = BOND_OPT_XMIT_HASH,
.name = "xmit_hash_policy",
- .desc = "balance-xor and 802.3ad hashing method",
+ .desc = "balance-xor, 802.3ad, and tlb hashing method",
.values = bond_xmit_hashtype_tbl,
.set = bond_option_xmit_hash_policy_set
},
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 4/5] bonding: Added bond_tlb_xmit() for tlb mode.
2014-03-29 5:29 [PATCH 4/5] bonding: Added bond_tlb_xmit() for tlb mode Mahesh Bandewar
@ 2014-03-31 15:52 ` Eric Dumazet
2014-03-31 16:00 ` David Laight
2014-03-31 22:16 ` Mahesh Bandewar
0 siblings, 2 replies; 6+ messages in thread
From: Eric Dumazet @ 2014-03-31 15:52 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David Miller,
netdev, Eric Dumazet, Maciej Zenczykowski
On Fri, 2014-03-28 at 22:29 -0700, Mahesh Bandewar wrote:
> Re-organized the xmit function for the lb mode separating tlb xmit
> from the alb mode. This will enable use of the hashing policies
> like 802.3ad mode. Also extended use of xmit-hash-policy to tlb mode.
>
> Now the tlb-mode defaults to BOND_XMIT_POLICY_LAYER2 if the xmit policy
> module parameter is not set (just like 802.3ad, or Xor mode).
>
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> ---
> drivers/net/bonding/bond_alb.c | 25 +++++++++++++++++++++++++
> drivers/net/bonding/bond_alb.h | 1 +
> drivers/net/bonding/bond_main.c | 6 ++++--
> drivers/net/bonding/bond_options.c | 2 +-
> 4 files changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index 5cd36016c393..8b7426ce6182 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -1381,6 +1381,31 @@ out:
> return NETDEV_TX_OK;
> }
>
> +int bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
> +{
> + struct bonding *bond = netdev_priv(bond_dev);
> + struct ethhdr *eth_data;
> + struct slave *tx_slave = NULL;
> + u32 hash_index = 0;
> +
> + skb_reset_mac_header(skb);
> + eth_data = eth_hdr(skb);
> +
> + /* Do not TX balance any multicast or broadcast */
> + if (!is_multicast_ether_addr(eth_data->h_dest)) {
> + switch (ntohs(skb->protocol)) {
Nit :
You can avoid the ntohs(skb->protocol)
> + case ETH_P_IP:
by using " case htons(ETH_P_IP)
> + case ETH_P_IPX: /* In case of IPX, it will falback to L2 hash */
> + case ETH_P_IPV6:
> + hash_index = bond_xmit_hash(bond, skb);
> + tx_slave = tlb_choose_channel(bond, hash_index & 0xFF, skb->len);
> + break;
> + }
> + }
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH 4/5] bonding: Added bond_tlb_xmit() for tlb mode.
2014-03-31 15:52 ` Eric Dumazet
@ 2014-03-31 16:00 ` David Laight
2014-03-31 16:22 ` Eric Dumazet
2014-03-31 22:16 ` Mahesh Bandewar
1 sibling, 1 reply; 6+ messages in thread
From: David Laight @ 2014-03-31 16:00 UTC (permalink / raw)
To: 'Eric Dumazet', Mahesh Bandewar
Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David Miller,
netdev, Eric Dumazet, Maciej Zenczykowski
From: Eric Dumazet
...
> > + if (!is_multicast_ether_addr(eth_data->h_dest)) {
> > + switch (ntohs(skb->protocol)) {
>
> Nit :
>
> You can avoid the ntohs(skb->protocol)
>
> > + case ETH_P_IP:
>
> by using " case htons(ETH_P_IP)
>
>
> > + case ETH_P_IPX: /* In case of IPX, it will falback to L2 hash */
> > + case ETH_P_IPV6:
> > + hash_index = bond_xmit_hash(bond, skb);
> > + tx_slave = tlb_choose_channel(bond, hash_index & 0xFF, skb->len);
> > + break;
> > + }
> > + }
Which may or not be faster...
As coded the case labels may be dense enough to generate a jump table.
If byte reversed the generated code will be a branch tree.
A deep branch tree will be worse, a match on the first item of a branch
tree will almost certainly be better.
I'm not going to guess where the breakeven point is for any modern cpus.
David
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH 4/5] bonding: Added bond_tlb_xmit() for tlb mode.
2014-03-31 16:00 ` David Laight
@ 2014-03-31 16:22 ` Eric Dumazet
2014-04-01 8:33 ` David Laight
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2014-03-31 16:22 UTC (permalink / raw)
To: David Laight
Cc: Mahesh Bandewar, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
David Miller, netdev, Eric Dumazet, Maciej Zenczykowski
On Mon, 2014-03-31 at 16:00 +0000, David Laight wrote:
> Which may or not be faster...
We don't care of this kind of assertions.
Bring us real data, or just say nothing. This is a waste of time.
> As coded the case labels may be dense enough to generate a jump table.
> If byte reversed the generated code will be a branch tree.
>
> A deep branch tree will be worse, a match on the first item of a branch
> tree will almost certainly be better.
> I'm not going to guess where the breakeven point is for any modern cpus.
This is ridiculous, there is no jumptable for ETH_P_IP, ETH_P_IPX,
ETH_P_IPV6.
Its not like their value will ever be 1 , 2 and 3
I suggest you take a look at the existing code in whole networking
stack. This is how we coded things, because there is no need to do
ntohs(variable) when we can simply write htons(constant) which even a
dumb compiler can understand. (We used to have __constant_htons() but it
looks its not needed these days.)
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 4/5] bonding: Added bond_tlb_xmit() for tlb mode.
2014-03-31 16:22 ` Eric Dumazet
@ 2014-04-01 8:33 ` David Laight
0 siblings, 0 replies; 6+ messages in thread
From: David Laight @ 2014-04-01 8:33 UTC (permalink / raw)
To: 'Eric Dumazet'
Cc: Mahesh Bandewar, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
David Miller, netdev, Eric Dumazet, Maciej Zenczykowski
From: Eric
> On Mon, 2014-03-31 at 16:00 +0000, David Laight wrote:
>
> > Which may or not be faster...
>
> We don't care of this kind of assertions.
>
> Bring us real data, or just say nothing. This is a waste of time.
>
> > As coded the case labels may be dense enough to generate a jump table.
> > If byte reversed the generated code will be a branch tree.
> >
> > A deep branch tree will be worse, a match on the first item of a branch
> > tree will almost certainly be better.
> > I'm not going to guess where the breakeven point is for any modern cpus.
>
> This is ridiculous, there is no jumptable for ETH_P_IP, ETH_P_IPX,
> ETH_P_IPV6.
>
> Its not like their value will ever be 1 , 2 and 3
>
> I suggest you take a look at the existing code in whole networking
> stack. This is how we coded things, because there is no need to do
> ntohs(variable) when we can simply write htons(constant) which even a
> dumb compiler can understand. (We used to have __constant_htons() but it
> looks its not needed these days.)
I am fully aware of the general advantage of doing htons(constant)
in a comparison, rather than converting the variable.
I am also aware of the different ways the compiler can compile
a switch statement, and many of the problems with each.
I'll admit to not looking at the values of those constants, so
may have assumed they were small integers (I was probably thinking
they were the values that appear in sockaddr).
However I stand by my assertion that given a switch statement that
is likely to have dense case labels you probably don't want to
byteswap the value so that the compiler generates a branch tree.
The 'problem' is that the jump table is likely to be a data cache
miss, but the branch tree might predict the correct outcome - so
the branch tree might be faster for some values of the expression.
David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 4/5] bonding: Added bond_tlb_xmit() for tlb mode.
2014-03-31 15:52 ` Eric Dumazet
2014-03-31 16:00 ` David Laight
@ 2014-03-31 22:16 ` Mahesh Bandewar
1 sibling, 0 replies; 6+ messages in thread
From: Mahesh Bandewar @ 2014-03-31 22:16 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David Miller,
netdev, Eric Dumazet, Maciej Zenczykowski
On Mon, Mar 31, 2014 at 8:52 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2014-03-28 at 22:29 -0700, Mahesh Bandewar wrote:
>> Re-organized the xmit function for the lb mode separating tlb xmit
>> from the alb mode. This will enable use of the hashing policies
>> like 802.3ad mode. Also extended use of xmit-hash-policy to tlb mode.
>>
>> Now the tlb-mode defaults to BOND_XMIT_POLICY_LAYER2 if the xmit policy
>> module parameter is not set (just like 802.3ad, or Xor mode).
>>
>> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>> ---
>> drivers/net/bonding/bond_alb.c | 25 +++++++++++++++++++++++++
>> drivers/net/bonding/bond_alb.h | 1 +
>> drivers/net/bonding/bond_main.c | 6 ++++--
>> drivers/net/bonding/bond_options.c | 2 +-
>> 4 files changed, 31 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>> index 5cd36016c393..8b7426ce6182 100644
>> --- a/drivers/net/bonding/bond_alb.c
>> +++ b/drivers/net/bonding/bond_alb.c
>> @@ -1381,6 +1381,31 @@ out:
>> return NETDEV_TX_OK;
>> }
>>
>> +int bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
>> +{
>> + struct bonding *bond = netdev_priv(bond_dev);
>> + struct ethhdr *eth_data;
>> + struct slave *tx_slave = NULL;
>> + u32 hash_index = 0;
>> +
>> + skb_reset_mac_header(skb);
>> + eth_data = eth_hdr(skb);
>> +
>> + /* Do not TX balance any multicast or broadcast */
>> + if (!is_multicast_ether_addr(eth_data->h_dest)) {
>> + switch (ntohs(skb->protocol)) {
>
> Nit :
>
> You can avoid the ntohs(skb->protocol)
>
>> + case ETH_P_IP:
>
> by using " case htons(ETH_P_IP)
>
Yes, will fix this too!
>
>> + case ETH_P_IPX: /* In case of IPX, it will falback to L2 hash */
>> + case ETH_P_IPV6:
>> + hash_index = bond_xmit_hash(bond, skb);
>> + tx_slave = tlb_choose_channel(bond, hash_index & 0xFF, skb->len);
>> + break;
>> + }
>> + }
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-04-01 8:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-29 5:29 [PATCH 4/5] bonding: Added bond_tlb_xmit() for tlb mode Mahesh Bandewar
2014-03-31 15:52 ` Eric Dumazet
2014-03-31 16:00 ` David Laight
2014-03-31 16:22 ` Eric Dumazet
2014-04-01 8:33 ` David Laight
2014-03-31 22:16 ` Mahesh Bandewar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).