public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Ido Schimmel <idosch@idosch•org>
To: David Ahern <dsahern@gmail•com>
Cc: netdev@vger•kernel.org, roopa@cumulusnetworks•com,
	nikolay@cumulusnetworks•com, idosch@mellanox•com
Subject: Re: [PATCH RFC net-next 5/7] net/ipv6: Add support for path selection using hash of 5-tuple
Date: Wed, 21 Feb 2018 18:22:59 +0200	[thread overview]
Message-ID: <20180221162259.GA6597@splinter.mtl.com> (raw)
In-Reply-To: <20180213000602.12150-6-dsahern@gmail.com>

On Mon, Feb 12, 2018 at 04:06:00PM -0800, David Ahern wrote:
> Some operators prefer IPv6 path selection to use a standard 5-tuple
> hash rather than just an L3 hash with the flow the label. To that end
> add support to IPv6 for multipath hash policy similar to bf4e0a3db97eb
> ("net: ipv4: add support for ECMP hash policy choice"). The default
> is still L3 which covers source and destination addresses along with
> flow label and IPv6 protocol.
> 
> Signed-off-by: David Ahern <dsahern@gmail•com>

[...]

> @@ -1819,20 +1820,53 @@ static void ip6_multipath_l3_keys(const struct sk_buff *skb,
>  }
>  
>  /* if skb is set it will be used and fl6 can be NULL */
> -u32 rt6_multipath_hash(const struct flowi6 *fl6, const struct sk_buff *skb)
> +u32 rt6_multipath_hash(const struct net *net, const struct flowi6 *fl6,
> +		       const struct sk_buff *skb)
>  {
>  	struct flow_keys hash_keys;
>  	u32 mhash;
>  
> -	memset(&hash_keys, 0, sizeof(hash_keys));
> -	hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
> -	if (skb) {
> -		ip6_multipath_l3_keys(skb, &hash_keys);
> -	} else {
> -		hash_keys.addrs.v6addrs.src = fl6->saddr;
> -		hash_keys.addrs.v6addrs.dst = fl6->daddr;
> -		hash_keys.tags.flow_label = (__force u32)fl6->flowlabel;
> -		hash_keys.basic.ip_proto = fl6->flowi6_proto;
> +	switch (net->ipv6.sysctl.multipath_hash_policy) {
> +	case 0:
> +		memset(&hash_keys, 0, sizeof(hash_keys));
> +		hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
> +		if (skb) {
> +			ip6_multipath_l3_keys(skb, &hash_keys);
> +		} else {
> +			hash_keys.addrs.v6addrs.src = fl6->saddr;
> +			hash_keys.addrs.v6addrs.dst = fl6->daddr;
> +			hash_keys.tags.flow_label = (__force u32)fl6->flowlabel;
> +			hash_keys.basic.ip_proto = fl6->flowi6_proto;
> +		}
> +		break;
> +	case 1:
> +		if (skb) {
> +			unsigned int flag = FLOW_DISSECTOR_F_STOP_AT_ENCAP;
> +			struct flow_keys keys;
> +
> +			/* short-circuit if we already have L4 hash present */
> +			if (skb->l4_hash)
> +				return skb_get_hash_raw(skb) >> 1;
> +
> +			memset(&hash_keys, 0, sizeof(hash_keys));
> +
> +			skb_flow_dissect_flow_keys(skb, &keys, flag);
> +
> +			hash_keys.addrs.v6addrs.src = keys.addrs.v6addrs.src;
> +			hash_keys.addrs.v6addrs.dst = keys.addrs.v6addrs.dst;

Shouldn't you add:

hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;

?

Otherwise flow_hash_from_keys() will not consistentify the ports and the
addresses and it will also not use the addresses for the hash
computation.

It's missing from fib_multipath_hash() as well, so I might be missing
something.

> +			hash_keys.ports.src = keys.ports.src;
> +			hash_keys.ports.dst = keys.ports.dst;
> +			hash_keys.tags.flow_label = keys.tags.flow_label;

Why are you using the flow label?

> +			hash_keys.basic.ip_proto = keys.basic.ip_proto;
> +		} else {
> +			memset(&hash_keys, 0, sizeof(hash_keys));
> +			hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
> +			hash_keys.addrs.v6addrs.src = fl6->saddr;
> +			hash_keys.addrs.v6addrs.dst = fl6->daddr;
> +			hash_keys.ports.src = fl6->fl6_sport;
> +			hash_keys.ports.dst = fl6->fl6_dport;
> +			hash_keys.basic.ip_proto = fl6->flowi6_proto;
> +		}
>  	}
>  	mhash = flow_hash_from_keys(&hash_keys);
>  
> @@ -1858,7 +1892,7 @@ void ip6_route_input(struct sk_buff *skb)
>  	if (tun_info && !(tun_info->mode & IP_TUNNEL_INFO_TX))
>  		fl6.flowi6_tun_key.tun_id = tun_info->key.tun_id;
>  	if (unlikely(fl6.flowi6_proto == IPPROTO_ICMPV6))
> -		fl6.mp_hash = rt6_multipath_hash(&fl6, skb);
> +		fl6.mp_hash = rt6_multipath_hash(net, &fl6, skb);
>  	skb_dst_drop(skb);
>  	skb_dst_set(skb, ip6_route_input_lookup(net, skb->dev, &fl6, flags));
>  }

  parent reply	other threads:[~2018-02-21 16:23 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-13  0:05 [PATCH RFC net-next 0/7] net/ipv6: Add support for path selection using hash of 5-tuple David Ahern
2018-02-13  0:05 ` [PATCH RFC net-next 1/7] net/ipv4: Pass net to fib_multipath_hash instead of fib_info David Ahern
2018-02-13  0:05 ` [PATCH RFC net-next 2/7] net: Align ip_multipath_l3_keys and ip6_multipath_l3_keys David Ahern
2018-02-13  0:05 ` [PATCH RFC net-next 3/7] net/ipv6: Make rt6_multipath_hash similar to fib_multipath_hash David Ahern
2018-02-13  0:05 ` [PATCH RFC net-next 4/7] net: Rename NETEVENT_MULTIPATH_HASH_UPDATE David Ahern
2018-02-13  0:06 ` [PATCH RFC net-next 5/7] net/ipv6: Add support for path selection using hash of 5-tuple David Ahern
2018-02-13 20:31   ` Nicolas Dichtel
2018-02-13 20:35     ` David Ahern
2018-02-13 20:59       ` Nicolas Dichtel
2018-02-13 21:02         ` David Ahern
2018-02-13 21:21           ` Nicolas Dichtel
2018-02-21 16:22   ` Ido Schimmel [this message]
2018-02-21 17:54     ` David Ahern
2018-02-13  0:06 ` [PATCH RFC net-next 6/7] mlxsw: spectrum_router: Add support for ipv6 hash policy update David Ahern
2018-02-13  0:06 ` [PATCH RFC net-next 7/7] net: Remove unused get_hash_from_flow functions David Ahern
2018-02-13 11:03 ` [PATCH RFC net-next 0/7] net/ipv6: Add support for path selection using hash of 5-tuple Or Gerlitz
2018-02-13 12:42   ` Ido Schimmel
2018-02-13 13:16     ` Or Gerlitz
2018-02-14 22:45       ` Or Gerlitz
2018-02-13 15:21     ` David Ahern
2018-02-14 22:45       ` Or Gerlitz
2018-02-14 22:56         ` David Ahern
2018-02-18 10:40           ` Or Gerlitz
2018-02-13 12:32 ` Ido Schimmel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180221162259.GA6597@splinter.mtl.com \
    --to=idosch@idosch$(echo .)org \
    --cc=dsahern@gmail$(echo .)com \
    --cc=idosch@mellanox$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=nikolay@cumulusnetworks$(echo .)com \
    --cc=roopa@cumulusnetworks$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox