Re: [PATCH net] bonding: discard lowest hash bit for 802.3ad layer3+4

[Eric Dumazet <edumazet@google.com>]

On Sun, Nov 5, 2017 at 6:56 AM, Hangbin Liu <liuhangbin@gmail.com> 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 <pabeni@redhat.com>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  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
>