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 <dsah...@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));
> }
