[net-next 1/2] ipv6: sr: add a per namespace sysctl to control seg6 flowlabel

[net-next 1/2] ipv6: sr: add a per namespace sysctl to control seg6 flowlabel

[Ahmed Abdelsalam]

This patch adds a per namespace sysctl, named 'seg6_flowlabel', to be used
by seg6_do_srh_encap() to control the behaviour of setting the flowlabel
value of outer IPv6.

The currently support behaviours are as follows:
-1 set flowlabel to zero.
 0 copy flowlabel from Inner paceket in case of Inner IPv6 (0 for IPv4/L2)
 1 Compute the flowlabel using seg6_make_flowlabel()

Signed-off-by: Ahmed Abdelsalam <amsalam20@gmail.com>
---
 include/net/netns/ipv6.h   | 1 +
 net/ipv6/sysctl_net_ipv6.c | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
index 97b3a54..c978a31 100644
--- a/include/net/netns/ipv6.h
+++ b/include/net/netns/ipv6.h
@@ -43,6 +43,7 @@ struct netns_sysctl_ipv6 {
 	int max_hbh_opts_cnt;
 	int max_dst_opts_len;
 	int max_hbh_opts_len;
+	int seg6_flowlabel;
 };
 
 struct netns_ipv6 {
diff --git a/net/ipv6/sysctl_net_ipv6.c b/net/ipv6/sysctl_net_ipv6.c
index 6fbdef6..e15cd37 100644
--- a/net/ipv6/sysctl_net_ipv6.c
+++ b/net/ipv6/sysctl_net_ipv6.c
@@ -152,6 +152,13 @@ static struct ctl_table ipv6_table_template[] = {
 		.extra1		= &zero,
 		.extra2		= &one,
 	},
+	{
+		.procname	= "seg6_flowlabel",
+		.data		= &init_net.ipv6.sysctl.seg6_flowlabel,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec
+	},
 	{ }
 };
 
@@ -217,6 +224,7 @@ static int __net_init ipv6_sysctl_net_init(struct net *net)
 	ipv6_table[12].data = &net->ipv6.sysctl.max_dst_opts_len;
 	ipv6_table[13].data = &net->ipv6.sysctl.max_hbh_opts_len;
 	ipv6_table[14].data = &net->ipv6.sysctl.multipath_hash_policy,
+	ipv6_table[15].data = &net->ipv6.sysctl.seg6_flowlabel;
 
 	ipv6_route_table = ipv6_route_sysctl_init(net);
 	if (!ipv6_route_table)
-- 
2.1.4

[net-next 2/2] ipv6: sr: Compute flowlabel of outer IPv6 header for seg6 encap mode

[Ahmed Abdelsalam]

ECMP (equal-cost multipath) hashes are typically computed on the
packets' 5-tuple(src IP, dst IP, src port, dst port, L4 proto).

For encapsulated packets, the L4 data is not readily available and
ECMP hashing will often revert to (src IP, dst IP). This will lead
to traffic polarization on a single ECMP path, causing congestion
and waste of network capacity.

In IPv6, the 20-bit flow label field is also used as part of the
ECMP hash. In the lack of L4 data, the hashing will be on (src IP,
dst IP, flow label).

Having a non-zero flow label is thus important for proper traffic
load balancing when L4 data is unavailable (i.e., when packets are
encapsulated)

Currently, the seg6_do_srh_encap() function extracts the original
packet's flow label and set it as the outer IPv6 flow label. There
are two issues with this behaviour:

a) There is no guarantee that the inner flow label will be set
by the source.

b) If the original packet is not IPv6, the flow label will be set
to zero (e.g., IPv4 or L2 encap).

This patch adds a function, named seg6_make_flowlabel(), that
computes a flow label from a given skb. It supports IPv6, IPv4
and L2 payloads, and leverages the per namespace "seg6_flowlabel"
sysctl value.

This patch has been tested for IPv6, IPv4, and L2 traffic.

Signed-off-by: Ahmed Abdelsalam <amsalam20@gmail.com>
---
 net/ipv6/seg6_iptunnel.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c
index 5fe1394..3d9cd86 100644
--- a/net/ipv6/seg6_iptunnel.c
+++ b/net/ipv6/seg6_iptunnel.c
@@ -91,6 +91,24 @@ static void set_tun_src(struct net *net, struct net_device *dev,
 	rcu_read_unlock();
 }
 
+/* Compute flowlabel for outer IPv6 header */
+__be32 seg6_make_flowlabel(struct net *net, struct sk_buff *skb,
+			   struct ipv6hdr *inner_hdr)
+{
+	int do_flowlabel = net->ipv6.sysctl.seg6_flowlabel;
+	__be32 flowlabel = 0;
+	u32 hash;
+
+	if (do_flowlabel > 0) {
+		hash = skb_get_hash(skb);
+		rol32(hash, 16);
+		flowlabel = (__force __be32)hash & IPV6_FLOWLABEL_MASK;
+	} else if (!do_flowlabel && skb->protocol == htons(ETH_P_IPV6)) {
+		flowlabel = ip6_flowlabel(inner_hdr);
+	}
+	return flowlabel;
+}
+
 /* encapsulate an IPv6 packet within an outer IPv6 header with a given SRH */
 int seg6_do_srh_encap(struct sk_buff *skb, struct ipv6_sr_hdr *osrh, int proto)
 {
@@ -99,6 +117,7 @@ int seg6_do_srh_encap(struct sk_buff *skb, struct ipv6_sr_hdr *osrh, int proto)
 	struct ipv6hdr *hdr, *inner_hdr;
 	struct ipv6_sr_hdr *isrh;
 	int hdrlen, tot_len, err;
+	__be32 flowlabel;
 
 	hdrlen = (osrh->hdrlen + 1) << 3;
 	tot_len = hdrlen + sizeof(*hdr);
@@ -119,12 +138,13 @@ int seg6_do_srh_encap(struct sk_buff *skb, struct ipv6_sr_hdr *osrh, int proto)
 	 * decapsulation will overwrite inner hlim with outer hlim
 	 */
 
+	flowlabel = seg6_make_flowlabel(net, skb, inner_hdr);
 	if (skb->protocol == htons(ETH_P_IPV6)) {
 		ip6_flow_hdr(hdr, ip6_tclass(ip6_flowinfo(inner_hdr)),
-			     ip6_flowlabel(inner_hdr));
+			     flowlabel);
 		hdr->hop_limit = inner_hdr->hop_limit;
 	} else {
-		ip6_flow_hdr(hdr, 0, 0);
+		ip6_flow_hdr(hdr, 0, flowlabel);
 		hdr->hop_limit = ip6_dst_hoplimit(skb_dst(skb));
 	}
 
-- 
2.1.4

[RFC PATCH] ipv6: sr: seg6_make_flowlabel() can be static

[kbuild test robot]

Fixes: 484c5b41c685 ("ipv6: sr: Compute flowlabel of outer IPv6 header for seg6 encap mode")
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
 seg6_iptunnel.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c
index bf76c59..d2ff788 100644
--- a/net/ipv6/seg6_iptunnel.c
+++ b/net/ipv6/seg6_iptunnel.c
@@ -92,8 +92,8 @@ static void set_tun_src(struct net *net, struct net_device *dev,
 }
 
 /* Compute flowlabel for outer IPv6 header */
-__be32 seg6_make_flowlabel(struct net *net, struct sk_buff *skb,
-			   struct ipv6hdr *inner_hdr)
+static __be32 seg6_make_flowlabel(struct net *net, struct sk_buff *skb,
+				  struct ipv6hdr *inner_hdr)
 {
 	int do_flowlabel = net->ipv6.sysctl.seg6_flowlabel;
 	__be32 flowlabel = 0;

Re: [net-next 2/2] ipv6: sr: Compute flowlabel of outer IPv6 header for seg6 encap mode

[kbuild test robot]

Hi Ahmed,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Ahmed-Abdelsalam/ipv6-sr-add-a-per-namespace-sysctl-to-control-seg6-flowlabel/20180424-142142
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> net/ipv6/seg6_iptunnel.c:95:8: sparse: symbol 'seg6_make_flowlabel' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [net-next 1/2] ipv6: sr: add a per namespace sysctl to control seg6 flowlabel

[David Miller]

From: Ahmed Abdelsalam <amsalam20@gmail.com>
Date: Mon, 23 Apr 2018 23:36:59 +0200

> This patch adds a per namespace sysctl, named 'seg6_flowlabel', to be used
> by seg6_do_srh_encap() to control the behaviour of setting the flowlabel
> value of outer IPv6.
> 
> The currently support behaviours are as follows:
> -1 set flowlabel to zero.
>  0 copy flowlabel from Inner paceket in case of Inner IPv6 (0 for IPv4/L2)
>  1 Compute the flowlabel using seg6_make_flowlabel()
> 
> Signed-off-by: Ahmed Abdelsalam <amsalam20@gmail.com>

There really isn't a reason to make this a separate patch.

Adding a sysctl that nothing refers to doesn't add much value.

So please combine patches #1 and #2.

Re: [net-next 1/2] ipv6: sr: add a per namespace sysctl to control seg6 flowlabel

[Ahmed Abdelsalam]

On Tue, 24 Apr 2018 13:16:50 -0400 (EDT)
David Miller <davem@davemloft.net> wrote:

> From: Ahmed Abdelsalam <amsalam20@gmail.com>
> Date: Mon, 23 Apr 2018 23:36:59 +0200
> 
> > This patch adds a per namespace sysctl, named 'seg6_flowlabel', to be used
> > by seg6_do_srh_encap() to control the behaviour of setting the flowlabel
> > value of outer IPv6.
> > 
> > The currently support behaviours are as follows:
> > -1 set flowlabel to zero.
> >  0 copy flowlabel from Inner paceket in case of Inner IPv6 (0 for IPv4/L2)
> >  1 Compute the flowlabel using seg6_make_flowlabel()
> > 
> > Signed-off-by: Ahmed Abdelsalam <amsalam20@gmail.com>
> 
> There really isn't a reason to make this a separate patch.
> 
> Adding a sysctl that nothing refers to doesn't add much value.
> 
> So please combine patches #1 and #2.

Ok, I will combine them and send you a new patch. 

Thanks

-- 
Ahmed Abdelsalam <amsalam20@gmail.com>