[PATCH RFC net-next 0/7] net/ipv6: Add support for path selection using hash of 5-tuple

[PATCH RFC net-next 0/7] net/ipv6: Add support for path selection using hash of 5-tuple

[David Ahern]

Hardware supports multipath selection using the standard L4 5-tuple
instead of just L3 and the flow label. In addition, some network
operators prefer IPv6 path selection to use the 5-tuple. 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.

A separate sysctl is added for IPv6, allowing IPv4 and IPv6 to use
different algorithms if desired.

The first 2 patches modify the IPv4 variant so that at the end of the
patch set the ipv4 and ipv6 implementations are direct parallels.

Patch 3 refactors the existing rt6_multipath_hash in preparation for
adding the policy option.

Patch 4 renames the existing netevent to have IPv4 in the name so ipv4
changes can be distinguished from IPv6 if the netevent handler cares.

Patch 5 adds the L4 hash support.

Patch 6 adds the hook for the netevent to the spectrum driver to update
the ASIC.

Patch 7 removes no longer used code.

David Ahern (7):
  net/ipv4: Pass net to fib_multipath_hash instead of fib_info
  net: Align ip_multipath_l3_keys and ip6_multipath_l3_keys
  net/ipv6: Make rt6_multipath_hash similar to fib_multipath_hash
  net: Rename NETEVENT_MULTIPATH_HASH_UPDATE
  net/ipv6: Add support for path selection using hash of 5-tuple
  mlxsw: spectrum_router: Add support for ipv6 hash policy update
  net: Remove unused get_hash_from_flow functions

 Documentation/networking/ip-sysctl.txt             |  7 +++
 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 13 ++++-
 include/net/flow.h                                 | 15 -----
 include/net/ip6_route.h                            |  3 +-
 include/net/ip_fib.h                               |  2 +-
 include/net/netevent.h                             |  3 +-
 include/net/netns/ipv6.h                           |  1 +
 net/core/flow_dissector.c                          | 16 ------
 net/ipv4/fib_semantics.c                           |  2 +-
 net/ipv4/route.c                                   | 25 +++++----
 net/ipv4/sysctl_net_ipv4.c                         |  2 +-
 net/ipv6/icmp.c                                    |  2 +-
 net/ipv6/route.c                                   | 64 ++++++++++++++++++----
 net/ipv6/sysctl_net_ipv6.c                         | 26 +++++++++
 14 files changed, 118 insertions(+), 63 deletions(-)

-- 
2.11.0

[PATCH RFC net-next 1/7] net/ipv4: Pass net to fib_multipath_hash instead of fib_info

[David Ahern]

fib_multipath_hash only needs net struct to check a sysctl. Make it
clear by passing net instead of fib_info. In the need this allows
alignment between the ipv4 and ipv6 versions.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/net/ip_fib.h     | 2 +-
 net/ipv4/fib_semantics.c | 2 +-
 net/ipv4/route.c         | 5 ++---
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index f80524396c06..0472bf774a5a 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -370,7 +370,7 @@ int fib_sync_down_addr(struct net_device *dev, __be32 local);
 int fib_sync_up(struct net_device *dev, unsigned int nh_flags);
 
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
-int fib_multipath_hash(const struct fib_info *fi, const struct flowi4 *fl4,
+int fib_multipath_hash(const struct net *net, const struct flowi4 *fl4,
 		       const struct sk_buff *skb);
 #endif
 void fib_select_multipath(struct fib_result *res, int hash);
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index c586597da20d..e22443244190 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -1767,7 +1767,7 @@ void fib_select_path(struct net *net, struct fib_result *res,
 
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
 	if (res->fi->fib_nhs > 1 && oif_check) {
-		int h = fib_multipath_hash(res->fi, fl4, skb);
+		int h = fib_multipath_hash(res->fi->fib_net, fl4, skb);
 
 		fib_select_multipath(res, h);
 	}
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 49cc1c1df1ba..975c4b7cff07 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1797,10 +1797,9 @@ static void ip_multipath_l3_keys(const struct sk_buff *skb,
 }
 
 /* if skb is set it will be used and fl4 can be NULL */
-int fib_multipath_hash(const struct fib_info *fi, const struct flowi4 *fl4,
+int fib_multipath_hash(const struct net *net, const struct flowi4 *fl4,
 		       const struct sk_buff *skb)
 {
-	struct net *net = fi->fib_net;
 	struct flow_keys hash_keys;
 	u32 mhash;
 
@@ -1856,7 +1855,7 @@ static int ip_mkroute_input(struct sk_buff *skb,
 {
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
 	if (res->fi && res->fi->fib_nhs > 1) {
-		int h = fib_multipath_hash(res->fi, NULL, skb);
+		int h = fib_multipath_hash(res->fi->fib_net, NULL, skb);
 
 		fib_select_multipath(res, h);
 	}
-- 
2.11.0

[PATCH RFC net-next 2/7] net: Align ip_multipath_l3_keys and ip6_multipath_l3_keys

[David Ahern]

Symmetry is good and allows easy comparison that ipv4 and ipv6 are
doing the same thing. To that end, change ip_multipath_l3_keys to
set addresses at the end after the icmp compares, and move the
initialization of ipv6 flow keys to rt6_multipath_hash.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/ipv4/route.c | 20 +++++++++++---------
 net/ipv6/route.c |  4 ++--
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 975c4b7cff07..228af793d154 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1763,37 +1763,39 @@ static void ip_multipath_l3_keys(const struct sk_buff *skb,
 				 struct flow_keys *hash_keys)
 {
 	const struct iphdr *outer_iph = ip_hdr(skb);
+	const struct iphdr *key_iph = outer_iph;
 	const struct iphdr *inner_iph;
 	const struct icmphdr *icmph;
 	struct iphdr _inner_iph;
 	struct icmphdr _icmph;
 
-	hash_keys->addrs.v4addrs.src = outer_iph->saddr;
-	hash_keys->addrs.v4addrs.dst = outer_iph->daddr;
 	if (likely(outer_iph->protocol != IPPROTO_ICMP))
-		return;
+		goto out;
 
 	if (unlikely((outer_iph->frag_off & htons(IP_OFFSET)) != 0))
-		return;
+		goto out;
 
 	icmph = skb_header_pointer(skb, outer_iph->ihl * 4, sizeof(_icmph),
 				   &_icmph);
 	if (!icmph)
-		return;
+		goto out;
 
 	if (icmph->type != ICMP_DEST_UNREACH &&
 	    icmph->type != ICMP_REDIRECT &&
 	    icmph->type != ICMP_TIME_EXCEEDED &&
 	    icmph->type != ICMP_PARAMETERPROB)
-		return;
+		goto out;
 
 	inner_iph = skb_header_pointer(skb,
 				       outer_iph->ihl * 4 + sizeof(_icmph),
 				       sizeof(_inner_iph), &_inner_iph);
 	if (!inner_iph)
-		return;
-	hash_keys->addrs.v4addrs.src = inner_iph->saddr;
-	hash_keys->addrs.v4addrs.dst = inner_iph->daddr;
+		goto out;
+
+	key_iph = inner_iph;
+out:
+	hash_keys->addrs.v4addrs.src = key_iph->saddr;
+	hash_keys->addrs.v4addrs.dst = key_iph->daddr;
 }
 
 /* if skb is set it will be used and fl4 can be NULL */
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 9dcfadddd800..baef18635f96 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1812,8 +1812,6 @@ static void ip6_multipath_l3_keys(const struct sk_buff *skb,
 
 	key_iph = inner_iph;
 out:
-	memset(keys, 0, sizeof(*keys));
-	keys->control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
 	keys->addrs.v6addrs.src = key_iph->saddr;
 	keys->addrs.v6addrs.dst = key_iph->daddr;
 	keys->tags.flow_label = ip6_flowinfo(key_iph);
@@ -1826,6 +1824,8 @@ u32 rt6_multipath_hash(const struct flowi6 *fl6, const struct sk_buff *skb)
 	struct flow_keys hash_keys;
 
 	if (skb) {
+		memset(&hash_keys, 0, sizeof(hash_keys));
+		hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
 		ip6_multipath_l3_keys(skb, &hash_keys);
 		return flow_hash_from_keys(&hash_keys) >> 1;
 	}
-- 
2.11.0

[PATCH RFC net-next 3/7] net/ipv6: Make rt6_multipath_hash similar to fib_multipath_hash

[David Ahern]

Make rt6_multipath_hash more of a direct parallel to fib_multipath_hash
and reduce stack and overhead in the process: get_hash_from_flowi6 is
just a wrapper around __get_hash_from_flowi6 with another stack
allocation for flow_keys. Move setting the addresses, protocol and
label into rt6_multipath_hash and allow it to make the call to
flow_hash_from_keys.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/ipv6/route.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index baef18635f96..8d3a95eed662 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1822,15 +1822,21 @@ static void ip6_multipath_l3_keys(const struct sk_buff *skb,
 u32 rt6_multipath_hash(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) {
-		memset(&hash_keys, 0, sizeof(hash_keys));
-		hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
 		ip6_multipath_l3_keys(skb, &hash_keys);
-		return flow_hash_from_keys(&hash_keys) >> 1;
+	} 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;
 	}
+	mhash = flow_hash_from_keys(&hash_keys);
 
-	return get_hash_from_flowi6(fl6) >> 1;
+	return mhash >> 1;
 }
 
 void ip6_route_input(struct sk_buff *skb)
-- 
2.11.0

[PATCH RFC net-next 4/7] net: Rename NETEVENT_MULTIPATH_HASH_UPDATE

[David Ahern]

Rename NETEVENT_MULTIPATH_HASH_UPDATE to
NETEVENT_IPV4_MPATH_HASH_UPDATE to denote it relates to a change
in the IPv4 hash policy.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 2 +-
 include/net/netevent.h                                | 2 +-
 net/ipv4/sysctl_net_ipv4.c                            | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index f0b25baba09a..3071b75af6e0 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -2413,7 +2413,7 @@ static int mlxsw_sp_router_netevent_event(struct notifier_block *nb,
 		mlxsw_core_schedule_work(&net_work->work);
 		mlxsw_sp_port_dev_put(mlxsw_sp_port);
 		break;
-	case NETEVENT_MULTIPATH_HASH_UPDATE:
+	case NETEVENT_IPV4_MPATH_HASH_UPDATE:
 		net = ptr;
 
 		if (!net_eq(net, &init_net))
diff --git a/include/net/netevent.h b/include/net/netevent.h
index 40e7bab68490..baee605a94ab 100644
--- a/include/net/netevent.h
+++ b/include/net/netevent.h
@@ -26,7 +26,7 @@ enum netevent_notif_type {
 	NETEVENT_NEIGH_UPDATE = 1, /* arg is struct neighbour ptr */
 	NETEVENT_REDIRECT,	   /* arg is struct netevent_redirect ptr */
 	NETEVENT_DELAY_PROBE_TIME_UPDATE, /* arg is struct neigh_parms ptr */
-	NETEVENT_MULTIPATH_HASH_UPDATE, /* arg is struct net ptr */
+	NETEVENT_IPV4_MPATH_HASH_UPDATE, /* arg is struct net ptr */
 };
 
 int register_netevent_notifier(struct notifier_block *nb);
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 93e172118a94..52d869d17554 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -400,7 +400,7 @@ static int proc_fib_multipath_hash_policy(struct ctl_table *table, int write,
 
 	ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
 	if (write && ret == 0)
-		call_netevent_notifiers(NETEVENT_MULTIPATH_HASH_UPDATE, net);
+		call_netevent_notifiers(NETEVENT_IPV4_MPATH_HASH_UPDATE, net);
 
 	return ret;
 }
-- 
2.11.0

[PATCH RFC net-next 5/7] net/ipv6: Add support for path selection using hash of 5-tuple

[David Ahern]

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>
---
 Documentation/networking/ip-sysctl.txt |  7 ++++
 include/net/ip6_route.h                |  3 +-
 include/net/netevent.h                 |  1 +
 include/net/netns/ipv6.h               |  1 +
 net/ipv6/icmp.c                        |  2 +-
 net/ipv6/route.c                       | 64 ++++++++++++++++++++++++++--------
 net/ipv6/sysctl_net_ipv6.c             | 26 ++++++++++++++
 7 files changed, 87 insertions(+), 17 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index a553d4e4a0fb..783675a730e5 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1363,6 +1363,13 @@ flowlabel_reflect - BOOLEAN
 	FALSE: disabled
 	Default: FALSE
 
+fib_multipath_hash_policy - INTEGER
+	Controls which hash policy to use for multipath routes.
+	Default: 0 (Layer 3)
+	Possible values:
+	0 - Layer 3 (source and destination addresses plus flow label)
+	1 - Layer 4 (standard 5-tuple)
+
 anycast_src_echo_reply - BOOLEAN
 	Controls the use of anycast addresses as source addresses for ICMPv6
 	echo reply
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 27d23a65f3cd..f13657a62e5c 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -127,7 +127,8 @@ static inline int ip6_route_get_saddr(struct net *net, struct rt6_info *rt,
 
 struct rt6_info *rt6_lookup(struct net *net, const struct in6_addr *daddr,
 			    const struct in6_addr *saddr, int oif, int flags);
-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 dst_entry *icmp6_dst_alloc(struct net_device *dev, struct flowi6 *fl6);
 
diff --git a/include/net/netevent.h b/include/net/netevent.h
index baee605a94ab..d9918261701c 100644
--- a/include/net/netevent.h
+++ b/include/net/netevent.h
@@ -27,6 +27,7 @@ enum netevent_notif_type {
 	NETEVENT_REDIRECT,	   /* arg is struct netevent_redirect ptr */
 	NETEVENT_DELAY_PROBE_TIME_UPDATE, /* arg is struct neigh_parms ptr */
 	NETEVENT_IPV4_MPATH_HASH_UPDATE, /* arg is struct net ptr */
+	NETEVENT_IPV6_MPATH_HASH_UPDATE, /* arg is struct net ptr */
 };
 
 int register_netevent_notifier(struct notifier_block *nb);
diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
index 987cc4569cb8..6b0de3b71bbf 100644
--- a/include/net/netns/ipv6.h
+++ b/include/net/netns/ipv6.h
@@ -28,6 +28,7 @@ struct netns_sysctl_ipv6 {
 	int ip6_rt_gc_elasticity;
 	int ip6_rt_mtu_expires;
 	int ip6_rt_min_advmss;
+	int multipath_hash_policy;
 	int flowlabel_consistency;
 	int auto_flowlabels;
 	int icmpv6_time;
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 6ae5dd3f4d0d..e255c9dd9b57 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -522,7 +522,7 @@ static void icmp6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info,
 	fl6.fl6_icmp_type = type;
 	fl6.fl6_icmp_code = code;
 	fl6.flowi6_uid = sock_net_uid(net, NULL);
-	fl6.mp_hash = rt6_multipath_hash(&fl6, skb);
+	fl6.mp_hash = rt6_multipath_hash(net, &fl6, skb);
 	security_skb_classify_flow(skb, flowi6_to_flowi(&fl6));
 
 	sk = icmpv6_xmit_lock(net);
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 8d3a95eed662..1b559f78a896 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -450,7 +450,8 @@ static bool rt6_check_expired(const struct rt6_info *rt)
 	return false;
 }
 
-static struct rt6_info *rt6_multipath_select(struct rt6_info *match,
+static struct rt6_info *rt6_multipath_select(const struct net *net,
+					     struct rt6_info *match,
 					     struct flowi6 *fl6, int oif,
 					     int strict)
 {
@@ -460,7 +461,7 @@ static struct rt6_info *rt6_multipath_select(struct rt6_info *match,
 	 * case it will always be non-zero. Otherwise now is the time to do it.
 	 */
 	if (!fl6->mp_hash)
-		fl6->mp_hash = rt6_multipath_hash(fl6, NULL);
+		fl6->mp_hash = rt6_multipath_hash(net, fl6, NULL);
 
 	if (fl6->mp_hash <= atomic_read(&match->rt6i_nh_upper_bound))
 		return match;
@@ -929,7 +930,7 @@ static struct rt6_info *ip6_pol_route_lookup(struct net *net,
 		rt = rt6_device_match(net, rt, &fl6->saddr,
 				      fl6->flowi6_oif, flags);
 		if (rt->rt6i_nsiblings && fl6->flowi6_oif == 0)
-			rt = rt6_multipath_select(rt, fl6,
+			rt = rt6_multipath_select(net, rt, fl6,
 						  fl6->flowi6_oif, flags);
 	}
 	if (rt == net->ipv6.ip6_null_entry) {
@@ -1669,7 +1670,7 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
 redo_rt6_select:
 	rt = rt6_select(net, fn, oif, strict);
 	if (rt->rt6i_nsiblings)
-		rt = rt6_multipath_select(rt, fl6, oif, strict);
+		rt = rt6_multipath_select(net, rt, fl6, oif, strict);
 	if (rt == net->ipv6.ip6_null_entry) {
 		fn = fib6_backtrack(fn, &fl6->saddr);
 		if (fn)
@@ -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;
+			hash_keys.ports.src = keys.ports.src;
+			hash_keys.ports.dst = keys.ports.dst;
+			hash_keys.tags.flow_label = keys.tags.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));
 }
diff --git a/net/ipv6/sysctl_net_ipv6.c b/net/ipv6/sysctl_net_ipv6.c
index a789a8ac6a64..ca4696b46483 100644
--- a/net/ipv6/sysctl_net_ipv6.c
+++ b/net/ipv6/sysctl_net_ipv6.c
@@ -16,14 +16,31 @@
 #include <net/ipv6.h>
 #include <net/addrconf.h>
 #include <net/inet_frag.h>
+#include <net/netevent.h>
 #ifdef CONFIG_NETLABEL
 #include <net/calipso.h>
 #endif
 
+static int zero;
 static int one = 1;
 static int auto_flowlabels_min;
 static int auto_flowlabels_max = IP6_AUTO_FLOW_LABEL_MAX;
 
+static int proc_rt6_multipath_hash_policy(struct ctl_table *table, int write,
+					  void __user *buffer, size_t *lenp,
+					  loff_t *ppos)
+{
+	struct net *net;
+	int ret;
+
+	net = container_of(table->data, struct net,
+			   ipv6.sysctl.multipath_hash_policy);
+	ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
+	if (write && ret == 0)
+		call_netevent_notifiers(NETEVENT_IPV6_MPATH_HASH_UPDATE, net);
+
+	return ret;
+}
 
 static struct ctl_table ipv6_table_template[] = {
 	{
@@ -126,6 +143,15 @@ static struct ctl_table ipv6_table_template[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec
 	},
+	{
+		.procname	= "fib_multipath_hash_policy",
+		.data		= &init_net.ipv6.sysctl.multipath_hash_policy,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler   = proc_rt6_multipath_hash_policy,
+		.extra1		= &zero,
+		.extra2		= &one,
+	},
 	{ }
 };
 
-- 
2.11.0

[PATCH RFC net-next 6/7] mlxsw: spectrum_router: Add support for ipv6 hash policy update

[David Ahern]

Similar to 28678f07f127d ("mlxsw: spectrum_router: Update multipath hash
parameters upon netevents") for IPv4, make sure the kernel and asic are
using the same hash algorithm for path selection.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 3071b75af6e0..d23ed0002a4a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -2414,6 +2414,7 @@ static int mlxsw_sp_router_netevent_event(struct notifier_block *nb,
 		mlxsw_sp_port_dev_put(mlxsw_sp_port);
 		break;
 	case NETEVENT_IPV4_MPATH_HASH_UPDATE:
+	case NETEVENT_IPV6_MPATH_HASH_UPDATE:
 		net = ptr;
 
 		if (!net_eq(net, &init_net))
@@ -7006,13 +7007,21 @@ static void mlxsw_sp_mp4_hash_init(char *recr2_pl)
 
 static void mlxsw_sp_mp6_hash_init(char *recr2_pl)
 {
+	bool only_l3 = !init_net.ipv6.sysctl.multipath_hash_policy;
+
 	mlxsw_sp_mp_hash_header_set(recr2_pl,
 				    MLXSW_REG_RECR2_IPV6_EN_NOT_TCP_NOT_UDP);
 	mlxsw_sp_mp_hash_header_set(recr2_pl, MLXSW_REG_RECR2_IPV6_EN_TCP_UDP);
 	mlxsw_reg_recr2_ipv6_sip_enable(recr2_pl);
 	mlxsw_reg_recr2_ipv6_dip_enable(recr2_pl);
-	mlxsw_sp_mp_hash_field_set(recr2_pl, MLXSW_REG_RECR2_IPV6_FLOW_LABEL);
 	mlxsw_sp_mp_hash_field_set(recr2_pl, MLXSW_REG_RECR2_IPV6_NEXT_HEADER);
+	if (only_l3) {
+		mlxsw_sp_mp_hash_field_set(recr2_pl, MLXSW_REG_RECR2_IPV6_FLOW_LABEL);
+	} else {
+		mlxsw_sp_mp_hash_header_set(recr2_pl, MLXSW_REG_RECR2_TCP_UDP_EN_IPV6);
+		mlxsw_sp_mp_hash_field_set(recr2_pl, MLXSW_REG_RECR2_TCP_UDP_SPORT);
+		mlxsw_sp_mp_hash_field_set(recr2_pl, MLXSW_REG_RECR2_TCP_UDP_DPORT);
+	}
 }
 
 static int mlxsw_sp_mp_hash_init(struct mlxsw_sp *mlxsw_sp)
-- 
2.11.0

[PATCH RFC net-next 7/7] net: Remove unused get_hash_from_flow functions

[David Ahern]

__get_hash_from_flowi6 is still used for flowlabels, but the IPv4
variant and the wrappers to both are not used. Remove them.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/net/flow.h        | 15 ---------------
 net/core/flow_dissector.c | 16 ----------------
 2 files changed, 31 deletions(-)

diff --git a/include/net/flow.h b/include/net/flow.h
index f1624fd5b1d0..871c27af6577 100644
--- a/include/net/flow.h
+++ b/include/net/flow.h
@@ -221,21 +221,6 @@ static inline unsigned int flow_key_size(u16 family)
 }
 
 __u32 __get_hash_from_flowi6(const struct flowi6 *fl6, struct flow_keys *keys);
-
-static inline __u32 get_hash_from_flowi6(const struct flowi6 *fl6)
-{
-	struct flow_keys keys;
-
-	return __get_hash_from_flowi6(fl6, &keys);
-}
-
 __u32 __get_hash_from_flowi4(const struct flowi4 *fl4, struct flow_keys *keys);
 
-static inline __u32 get_hash_from_flowi4(const struct flowi4 *fl4)
-{
-	struct flow_keys keys;
-
-	return __get_hash_from_flowi4(fl4, &keys);
-}
-
 #endif
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 559db9ea8d86..d29f09bc5ff9 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -1341,22 +1341,6 @@ __u32 __get_hash_from_flowi6(const struct flowi6 *fl6, struct flow_keys *keys)
 }
 EXPORT_SYMBOL(__get_hash_from_flowi6);
 
-__u32 __get_hash_from_flowi4(const struct flowi4 *fl4, struct flow_keys *keys)
-{
-	memset(keys, 0, sizeof(*keys));
-
-	keys->addrs.v4addrs.src = fl4->saddr;
-	keys->addrs.v4addrs.dst = fl4->daddr;
-	keys->control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
-	keys->ports.src = fl4->fl4_sport;
-	keys->ports.dst = fl4->fl4_dport;
-	keys->keyid.keyid = fl4->fl4_gre_key;
-	keys->basic.ip_proto = fl4->flowi4_proto;
-
-	return flow_hash_from_keys(keys);
-}
-EXPORT_SYMBOL(__get_hash_from_flowi4);
-
 static const struct flow_dissector_key flow_keys_dissector_keys[] = {
 	{
 		.key_id = FLOW_DISSECTOR_KEY_CONTROL,
-- 
2.11.0

Re: [PATCH RFC net-next 0/7] net/ipv6: Add support for path selection using hash of 5-tuple

[Or Gerlitz]

On Tue, Feb 13, 2018 at 2:05 AM, David Ahern <dsahern@gmail.com> wrote:
> Hardware supports multipath selection using the standard L4 5-tuple
> instead of just L3 and the flow label. In addition, some network
> operators prefer IPv6 path selection to use the 5-tuple.

The HW supports using flow label and AFAIK that is the preferred approach
by the community (?)

> To that end, add support to IPv6 for multipath hash policy

so a question comes up if/what are the disadvantaged
to support 5-tuple. E.g Tom was commenting that such DPI is problematic
when multiple IPv6 header extensions are used.

> 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.
>
> A separate sysctl is added for IPv6, allowing IPv4 and IPv6 to use
> different algorithms if desired.

Re: [PATCH RFC net-next 0/7] net/ipv6: Add support for path selection using hash of 5-tuple

[Ido Schimmel]

Hi David,

On Mon, Feb 12, 2018 at 04:05:55PM -0800, David Ahern wrote:
> Hardware supports multipath selection using the standard L4 5-tuple
> instead of just L3 and the flow label. In addition, some network
> operators prefer IPv6 path selection to use the 5-tuple. 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.
> 
> A separate sysctl is added for IPv6, allowing IPv4 and IPv6 to use
> different algorithms if desired.
> 
> The first 2 patches modify the IPv4 variant so that at the end of the
> patch set the ipv4 and ipv6 implementations are direct parallels.
> 
> Patch 3 refactors the existing rt6_multipath_hash in preparation for
> adding the policy option.
> 
> Patch 4 renames the existing netevent to have IPv4 in the name so ipv4
> changes can be distinguished from IPv6 if the netevent handler cares.
> 
> Patch 5 adds the L4 hash support.
> 
> Patch 6 adds the hook for the netevent to the spectrum driver to update
> the ASIC.
> 
> Patch 7 removes no longer used code.

Thanks for working on this.

I've yet to review the patches, but I did test them. The good news is
that the offloaded path works as expected. Different flows (random UDP
destination port, zero flowlabel) are hashed to different nexthops. With
the default policy all the flows are hashed to the same nexthop.

However, it seems that the kernel isn't spreading the traffic as
expected and the same nexthop is always hit. I'll look into it later
this week when I have some more time.

Thanks!

Re: [PATCH RFC net-next 0/7] net/ipv6: Add support for path selection using hash of 5-tuple

[Ido Schimmel]

On Tue, Feb 13, 2018 at 01:03:14PM +0200, Or Gerlitz wrote:
> On Tue, Feb 13, 2018 at 2:05 AM, David Ahern <dsahern@gmail.com> wrote:
> > Hardware supports multipath selection using the standard L4 5-tuple
> > instead of just L3 and the flow label. In addition, some network
> > operators prefer IPv6 path selection to use the 5-tuple.
> 
> The HW supports using flow label and AFAIK that is the preferred approach
> by the community (?)
> 
> > To that end, add support to IPv6 for multipath hash policy
> 
> so a question comes up if/what are the disadvantaged
> to support 5-tuple. E.g Tom was commenting that such DPI is problematic
> when multiple IPv6 header extensions are used.

Tom is much more qualified to answer this, but I think the problem is
that the flow label isn't always set. Also, apparently some devices
change the flow label mid flow. See:

"At Fastly, this hashing is performed by an Ethernet switch ASIC, and to
avoid breakage, the IPv6 hashing function must not include the flow
label. As in IPv4, the hash function includes the source and destination
information in the L3 and L4 headers."
https://blog.apnic.net/2018/01/11/ipv6-flow-label-misuse-hashing/
https://www.youtube.com/watch?v=b0CRjOpnT7w
https://pc.nanog.org/static/published/meetings/NANOG71/1531/20171003_Jaeggli_Lightning_Talk_Ipv6_v1.pdf

Re: [PATCH RFC net-next 0/7] net/ipv6: Add support for path selection using hash of 5-tuple

[Or Gerlitz]

On Tue, Feb 13, 2018 at 2:42 PM, Ido Schimmel <idosch@idosch.org> wrote:
> On Tue, Feb 13, 2018 at 01:03:14PM +0200, Or Gerlitz wrote:
>> On Tue, Feb 13, 2018 at 2:05 AM, David Ahern <dsahern@gmail.com> wrote:
>> > Hardware supports multipath selection using the standard L4 5-tuple
>> > instead of just L3 and the flow label. In addition, some network
>> > operators prefer IPv6 path selection to use the 5-tuple.
>>
>> The HW supports using flow label and AFAIK that is the preferred approach
>> by the community (?)
>>
>> > To that end, add support to IPv6 for multipath hash policy
>>
>> so a question comes up if/what are the disadvantaged
>> to support 5-tuple. E.g Tom was commenting that such DPI is problematic
>> when multiple IPv6 header extensions are used.
>
> Tom is much more qualified to answer this, but I think the problem is
> that the flow label isn't always set. Also, apparently some devices
> change the flow label mid flow. See:

OK. But note we have two ends to deal with here (1) generation (2) usage

E.g if the kernel generates flow label but uses source port we have inconsistent
environment. Problem is that the generation and usage typically don't happen
on the same network point.

Re: [PATCH RFC net-next 0/7] net/ipv6: Add support for path selection using hash of 5-tuple

[David Ahern]

On 2/13/18 5:42 AM, Ido Schimmel wrote:
> On Tue, Feb 13, 2018 at 01:03:14PM +0200, Or Gerlitz wrote:
>> On Tue, Feb 13, 2018 at 2:05 AM, David Ahern <dsahern@gmail.com> wrote:
>>> Hardware supports multipath selection using the standard L4 5-tuple
>>> instead of just L3 and the flow label. In addition, some network
>>> operators prefer IPv6 path selection to use the 5-tuple.
>>
>> The HW supports using flow label and AFAIK that is the preferred approach
>> by the community (?)
>>
>>> To that end, add support to IPv6 for multipath hash policy
>>
>> so a question comes up if/what are the disadvantaged
>> to support 5-tuple. E.g Tom was commenting that such DPI is problematic
>> when multiple IPv6 header extensions are used.

Pros and cons to both approaches (L3 only or L4). We (Cumulus Networks)
use L4 5-tuple hash for both IPv4 and IPv6. When I asked around various
experts all of them gave me a puzzled look as to why I was asking the
question. Basically, the unanimous response was of course it is an L4 hash.


> 
> Tom is much more qualified to answer this, but I think the problem is
> that the flow label isn't always set. Also, apparently some devices
> change the flow label mid flow. See:
> 
> "At Fastly, this hashing is performed by an Ethernet switch ASIC, and to
> avoid breakage, the IPv6 hashing function must not include the flow
> label. As in IPv4, the hash function includes the source and destination
> information in the L3 and L4 headers."
> https://blog.apnic.net/2018/01/11/ipv6-flow-label-misuse-hashing/
> https://www.youtube.com/watch?v=b0CRjOpnT7w
> https://pc.nanog.org/static/published/meetings/NANOG71/1531/20171003_Jaeggli_Lightning_Talk_Ipv6_v1.pdf
> 

Re: [PATCH RFC net-next 5/7] net/ipv6: Add support for path selection using hash of 5-tuple

[Nicolas Dichtel]

Le 13/02/2018 à 01:06, David Ahern a écrit :
> 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>
> ---
Do you plan to add the nl NETCONF support for this new sysctl?


Regards,
Nicolas

Re: [PATCH RFC net-next 5/7] net/ipv6: Add support for path selection using hash of 5-tuple

[David Ahern]

On 2/13/18 1:31 PM, Nicolas Dichtel wrote:
> Le 13/02/2018 à 01:06, David Ahern a écrit :
>> 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>
>> ---
> Do you plan to add the nl NETCONF support for this new sysctl?
> 

hmmm.... doesn't exist for the current IPv4 version, but in-kernel
drivers are notified. I wasn't planning on it, but it can be added.

Re: [PATCH RFC net-next 5/7] net/ipv6: Add support for path selection using hash of 5-tuple

[Nicolas Dichtel]

Le 13/02/2018 à 21:35, David Ahern a écrit :
> On 2/13/18 1:31 PM, Nicolas Dichtel wrote:
>> Le 13/02/2018 à 01:06, David Ahern a écrit :
>>> 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>
>>> ---
>> Do you plan to add the nl NETCONF support for this new sysctl?
>>
> 
> hmmm.... doesn't exist for the current IPv4 version, but in-kernel
> drivers are notified. I wasn't planning on it, but it can be added.
> 
Right for ipv4, it was on my todo list ;-)

Re: [PATCH RFC net-next 5/7] net/ipv6: Add support for path selection using hash of 5-tuple

[David Ahern]

On 2/13/18 1:59 PM, Nicolas Dichtel wrote:
> Le 13/02/2018 à 21:35, David Ahern a écrit :
>> On 2/13/18 1:31 PM, Nicolas Dichtel wrote:
>>> Le 13/02/2018 à 01:06, David Ahern a écrit :
>>>> 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>
>>>> ---
>>> Do you plan to add the nl NETCONF support for this new sysctl?
>>>
>>
>> hmmm.... doesn't exist for the current IPv4 version, but in-kernel
>> drivers are notified. I wasn't planning on it, but it can be added.
>>
> Right for ipv4, it was on my todo list ;-)
> 

Awesome, then ipv6 is a 1-liner after you add ipv4 support. :-)

Re: [PATCH RFC net-next 5/7] net/ipv6: Add support for path selection using hash of 5-tuple

[Nicolas Dichtel]

Le 13/02/2018 à 22:02, David Ahern a écrit :
> On 2/13/18 1:59 PM, Nicolas Dichtel wrote:
>> Le 13/02/2018 à 21:35, David Ahern a écrit :
>>> On 2/13/18 1:31 PM, Nicolas Dichtel wrote:
>>>> Le 13/02/2018 à 01:06, David Ahern a écrit :
>>>>> 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>
>>>>> ---
>>>> Do you plan to add the nl NETCONF support for this new sysctl?
>>>>
>>>
>>> hmmm.... doesn't exist for the current IPv4 version, but in-kernel
>>> drivers are notified. I wasn't planning on it, but it can be added.
>>>
>> Right for ipv4, it was on my todo list ;-)
>>
> 
> Awesome, then ipv6 is a 1-liner after you add ipv4 support. :-)
> 
Sure :)

Re: [PATCH RFC net-next 0/7] net/ipv6: Add support for path selection using hash of 5-tuple

[Or Gerlitz]

On Tue, Feb 13, 2018 at 5:21 PM, David Ahern <dsahern@gmail.com> wrote:
> On 2/13/18 5:42 AM, Ido Schimmel wrote:
>> On Tue, Feb 13, 2018 at 01:03:14PM +0200, Or Gerlitz wrote:
>>> On Tue, Feb 13, 2018 at 2:05 AM, David Ahern <dsahern@gmail.com> wrote:
>>>> Hardware supports multipath selection using the standard L4 5-tuple
>>>> instead of just L3 and the flow label. In addition, some network
>>>> operators prefer IPv6 path selection to use the 5-tuple.
>>>
>>> The HW supports using flow label and AFAIK that is the preferred approach
>>> by the community (?)
>>>
>>>> To that end, add support to IPv6 for multipath hash policy
>>>
>>> so a question comes up if/what are the disadvantaged
>>> to support 5-tuple. E.g Tom was commenting that such DPI is problematic
>>> when multiple IPv6 header extensions are used.
>
> Pros and cons to both approaches (L3 only or L4). We (Cumulus Networks)
> use L4 5-tuple hash for both IPv4 and IPv6. When I asked around various
> experts all of them gave me a puzzled look as to why I was asking the
> question. Basically, the unanimous response was of course it is an L4 hash.

how the various systems you are dealing with do with traffic that involves
ipv6 extension headers? what about environments with GRE? in ipv4 GRE
fabrics are just broken for ECMP, in ipv6 they can fly with flow label but
will crash again with L4 hash.

Re: [PATCH RFC net-next 0/7] net/ipv6: Add support for path selection using hash of 5-tuple

[Or Gerlitz]

On Tue, Feb 13, 2018 at 3:16 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:

> [...] note we have two ends to deal with here (1) generation (2) usage
>
> E.g if the kernel generates flow label but uses source port we have inconsistent
> environment. Problem is that the generation and usage typically don't happen
> on the same network point.

David, what's your thinking here? e.g for overly networks environments

Re: [PATCH RFC net-next 0/7] net/ipv6: Add support for path selection using hash of 5-tuple

[David Ahern]

On 2/14/18 3:45 PM, Or Gerlitz wrote:
> On Tue, Feb 13, 2018 at 5:21 PM, David Ahern <dsahern@gmail.com> wrote:
>> On 2/13/18 5:42 AM, Ido Schimmel wrote:
>>> On Tue, Feb 13, 2018 at 01:03:14PM +0200, Or Gerlitz wrote:
>>>> On Tue, Feb 13, 2018 at 2:05 AM, David Ahern <dsahern@gmail.com> wrote:
>>>>> Hardware supports multipath selection using the standard L4 5-tuple
>>>>> instead of just L3 and the flow label. In addition, some network
>>>>> operators prefer IPv6 path selection to use the 5-tuple.
>>>>
>>>> The HW supports using flow label and AFAIK that is the preferred approach
>>>> by the community (?)
>>>>
>>>>> To that end, add support to IPv6 for multipath hash policy
>>>>
>>>> so a question comes up if/what are the disadvantaged
>>>> to support 5-tuple. E.g Tom was commenting that such DPI is problematic
>>>> when multiple IPv6 header extensions are used.
>>
>> Pros and cons to both approaches (L3 only or L4). We (Cumulus Networks)
>> use L4 5-tuple hash for both IPv4 and IPv6. When I asked around various
>> experts all of them gave me a puzzled look as to why I was asking the
>> question. Basically, the unanimous response was of course it is an L4 hash.
> 
> how the various systems you are dealing with do with traffic that involves
> ipv6 extension headers? what about environments with GRE? in ipv4 GRE
> fabrics are just broken for ECMP, in ipv6 they can fly with flow label but
> will crash again with L4 hash.
> 

If you like your ecmp hash algorithm, you can keep your ecmp hash algorithm.

This gives users a choice; it is not a requirement to move from L3 only
to L4. Further, this makes IPv6 on par with IPv4 with a choice between
L3 and L4 and allows users to decide what works best for them.

Re: [PATCH RFC net-next 0/7] net/ipv6: Add support for path selection using hash of 5-tuple

[Or Gerlitz]

On Thu, Feb 15, 2018 at 12:56 AM, David Ahern <dsahern@gmail.com> wrote:
> On 2/14/18 3:45 PM, Or Gerlitz wrote:

>> how the various systems you are dealing with do with traffic that involves
>> ipv6 extension headers? what about environments with GRE? in ipv4 GRE
>> fabrics are just broken for ECMP, in ipv6 they can fly with flow label but
>> will crash again with L4 hash.

> If you like your ecmp hash algorithm, you can keep your ecmp hash algorithm.

> This gives users a choice; it is not a requirement to move from L3 only
> to L4. Further, this makes IPv6 on par with IPv4 with a choice between
> L3 and L4 and allows users to decide what works best for them.

Looking in the code for tunnels e.g in the vxlan xmit path (but I
believe this is the case for
other UDP tunnels as well), a call is made to generate the source udp
port as the hash of
the overlay tuple regardless if we are on v4/v6. Next, when it comes
to ipv6, the kernel
computes the  flow label which effectively takes into account the inner header.

Re: [PATCH RFC net-next 5/7] net/ipv6: Add support for path selection using hash of 5-tuple

[Ido Schimmel]

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));
>  }

Re: [PATCH RFC net-next 5/7] net/ipv6: Add support for path selection using hash of 5-tuple

[David Ahern]

On 2/21/18 9:22 AM, Ido Schimmel wrote:

>> +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.

Yes, I think you are correct. The oversight here is based on the missing
set in the ipv4 variant.

> 
>> +			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?

will remove. It is supposed to be an L4 hash.