From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [PATCH net] bonding: discard lowest hash bit for 802.3ad layer3+4 Date: Sun, 5 Nov 2017 13:38:47 -0800 Message-ID: References: <1509893773-28453-1-git-send-email-liuhangbin@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: netdev , Paolo Abeni , "David S. Miller" To: Hangbin Liu Return-path: Received: from mail-wr0-f195.google.com ([209.85.128.195]:53405 "EHLO mail-wr0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751140AbdKEVit (ORCPT ); Sun, 5 Nov 2017 16:38:49 -0500 Received: by mail-wr0-f195.google.com with SMTP id u40so6854612wrf.10 for ; Sun, 05 Nov 2017 13:38:49 -0800 (PST) In-Reply-To: <1509893773-28453-1-git-send-email-liuhangbin@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: On Sun, Nov 5, 2017 at 6:56 AM, Hangbin Liu wrote: > After commit 07f4c90062f8 ("tcp/dccp: try to not exhaust ip_local_port_range > in connect()"), we will try to use even ports for connect(). Then If an > application (seen clearly with iperf) opens multiple streams to the same > destination IP and port, each stream will be given an even source port. > > So the bonding driver's simple xmit_hash_policy based on layer3+4 addressing > will always hash all these streams to the same interface. And the total > throughput will limited to a single slave. > > Change the tcp code will impact the whole tcp behavior, only for bonding > usage. Paolo Abeni suggested fix this by changing the bonding code only, > which should be more reasonable, and less impact. > > Fix this by discarding the lowest hash bit because it contains little entropy. > After the fix we can re-balance between slaves. > > Signed-off-by: Paolo Abeni > Signed-off-by: Hangbin Liu > --- > drivers/net/bonding/bond_main.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c > index c99dc59..728fa08 100644 > --- a/drivers/net/bonding/bond_main.c > +++ b/drivers/net/bonding/bond_main.c > @@ -3237,7 +3237,7 @@ u32 bond_xmit_hash(struct bonding *bond, struct sk_buff *skb) > > if (bond->params.xmit_policy == BOND_XMIT_POLICY_ENCAP34 && > skb->l4_hash) > - return skb->hash; > + return skb->hash >> 1; Why are you changing this part ? The l4 hash provided by local TCP stack does not use a pathological XOR based on ports/addresses, but a random value with pretty good entropy. No need to try do 'enhance' it by actually being slightly worse. > > if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER2 || > !bond_flow_dissect(bond, skb, &flow)) > @@ -3253,7 +3253,7 @@ u32 bond_xmit_hash(struct bonding *bond, struct sk_buff *skb) > hash ^= (hash >> 16); > hash ^= (hash >> 8); > > - return hash; > + return hash >> 1; > } > > /*-------------------------- Device entry points ----------------------------*/ > -- > 2.5.5 >