[PATCH net 0/3] flowi6_tos fixes

[PATCH net 0/3] flowi6_tos fixes

[Daniel Borkmann]

This set is a follow-up to address Jiri's recent feedback [1] on
the flowi6_tos issue, that it is not used for IPv6 route lookups.
The three patches fix all current users of flowi6_tos and remove
the define to avoid any future confusion on this. Tested the vxlan
and geneve ones with IPv6 routing rules. For details, please see
individual patches.

[ As fixes are currently applied against net-next tree, I've rebased
  it against that. ]

Thanks!

  [1] http://patchwork.ozlabs.org/patch/592055/

Daniel Borkmann (3):
  vxlan: fix populating tclass in vxlan6_get_route
  geneve: fix populating tclass in geneve_get_v6_dst
  ipv6, trace: fix tos reporting on fib6_table_lookup

 drivers/net/geneve.c        | 11 ++++++-----
 drivers/net/vxlan.c         |  3 +--
 include/net/flow.h          |  2 +-
 include/net/ipv6.h          |  6 ++++++
 include/trace/events/fib6.h |  2 +-
 5 files changed, 15 insertions(+), 9 deletions(-)

-- 
1.9.3

[PATCH net 1/3] vxlan: fix populating tclass in vxlan6_get_route

[Daniel Borkmann]

Jiri mentioned that flowi6_tos of struct flowi6 is never used/read
anywhere. In fact, rest of the kernel uses the flowi6's flowlabel,
where the traffic class _and_ the flowlabel (aka flowinfo) is encoded.

For example, for policy routing, fib6_rule_match() uses ip6_tclass()
that is applied on the flowlabel member for matching on tclass. Similar
fix is needed for geneve, where flowi6_tos is set as well. Installing
a v6 blackhole rule that f.e. matches on tos is now working with vxlan.

Fixes: 1400615d64cf ("vxlan: allow setting ipv6 traffic class")
Reported-by: Jiri Benc <jbenc@redhat.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 drivers/net/vxlan.c | 3 +--
 include/net/ipv6.h  | 6 ++++++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 800106a7..7bfcb9a62a 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1810,10 +1810,9 @@ static struct dst_entry *vxlan6_get_route(struct vxlan_dev *vxlan,
 
 	memset(&fl6, 0, sizeof(fl6));
 	fl6.flowi6_oif = oif;
-	fl6.flowi6_tos = RT_TOS(tos);
 	fl6.daddr = *daddr;
 	fl6.saddr = vxlan->cfg.saddr.sin6.sin6_addr;
-	fl6.flowlabel = label;
+	fl6.flowlabel = ip6_make_flowinfo(RT_TOS(tos), label);
 	fl6.flowi6_mark = skb->mark;
 	fl6.flowi6_proto = IPPROTO_UDP;
 
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index f3c9857..d0aeb97 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -835,6 +835,12 @@ static inline u8 ip6_tclass(__be32 flowinfo)
 {
 	return ntohl(flowinfo & IPV6_TCLASS_MASK) >> IPV6_TCLASS_SHIFT;
 }
+
+static inline __be32 ip6_make_flowinfo(unsigned int tclass, __be32 flowlabel)
+{
+	return htonl(tclass << IPV6_TCLASS_SHIFT) | flowlabel;
+}
+
 /*
  *	Prototypes exported by ipv6
  */
-- 
1.9.3

[PATCH net 2/3] geneve: fix populating tclass in geneve_get_v6_dst

[Daniel Borkmann]

The struct flowi6's flowi6_tos is not used in IPv6 route lookup, the
traffic class information is handled in the flowi6's flowlabel member
instead. For example, for policy routing, fib6_rule_match() uses
ip6_tclass() that is applied on the flowlabel for matching on tclass,
which would currently not work as expected.

Fixes: 3a56f86f1be6 ("geneve: handle ipv6 priority like ipv4 tos")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 drivers/net/geneve.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 192631a..bc16889 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -843,8 +843,8 @@ static struct dst_entry *geneve_get_v6_dst(struct sk_buff *skb,
 	if (info) {
 		fl6->daddr = info->key.u.ipv6.dst;
 		fl6->saddr = info->key.u.ipv6.src;
-		fl6->flowi6_tos = RT_TOS(info->key.tos);
-		fl6->flowlabel = info->key.label;
+		fl6->flowlabel = ip6_make_flowinfo(RT_TOS(info->key.tos),
+						   info->key.label);
 		dst_cache = &info->dst_cache;
 	} else {
 		prio = geneve->tos;
@@ -855,8 +855,8 @@ static struct dst_entry *geneve_get_v6_dst(struct sk_buff *skb,
 			use_cache = false;
 		}
 
-		fl6->flowi6_tos = RT_TOS(prio);
-		fl6->flowlabel = geneve->label;
+		fl6->flowlabel = ip6_make_flowinfo(RT_TOS(prio),
+						   geneve->label);
 		fl6->daddr = geneve->remote.sin6.sin6_addr;
 		dst_cache = &geneve->dst_cache;
 	}
@@ -1049,7 +1049,8 @@ static netdev_tx_t geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
 		if (unlikely(err))
 			goto err;
 
-		prio = ip_tunnel_ecn_encap(fl6.flowi6_tos, iip, skb);
+		prio = ip_tunnel_ecn_encap(ip6_tclass(fl6.flowlabel),
+					   iip, skb);
 		ttl = geneve->ttl;
 		if (!ttl && ipv6_addr_is_multicast(&fl6.daddr))
 			ttl = 1;
-- 
1.9.3

[PATCH net 3/3] ipv6, trace: fix tos reporting on fib6_table_lookup

[Daniel Borkmann]

flowi6_tos of struct flowi6 is unused in IPv6, therefore dumping tos on
that tracepoint will also give incorrect information wrt traffic class.

If we want to fix it, we need to extract it via ip6_tclass(flp->flowlabel).
While for the same test case I get a count of 0 non-zero tos values before
the change, they now start to show up after the change:

  # ./perf record -e fib6:fib6_table_lookup -a sleep 10
  # ./perf script | grep -v "tos 0" | wc -l
  60

Since there's no user in the kernel tree anymore of flowi6_tos, remove the
define to avoid any future confusion on this.

Fixes: b811580d91e9 ("net: IPv6 fib lookup tracepoint")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 include/net/flow.h          | 2 +-
 include/trace/events/fib6.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/net/flow.h b/include/net/flow.h
index 83969ee..d47ef4b 100644
--- a/include/net/flow.h
+++ b/include/net/flow.h
@@ -127,7 +127,6 @@ struct flowi6 {
 #define flowi6_oif		__fl_common.flowic_oif
 #define flowi6_iif		__fl_common.flowic_iif
 #define flowi6_mark		__fl_common.flowic_mark
-#define flowi6_tos		__fl_common.flowic_tos
 #define flowi6_scope		__fl_common.flowic_scope
 #define flowi6_proto		__fl_common.flowic_proto
 #define flowi6_flags		__fl_common.flowic_flags
@@ -135,6 +134,7 @@ struct flowi6 {
 #define flowi6_tun_key		__fl_common.flowic_tun_key
 	struct in6_addr		daddr;
 	struct in6_addr		saddr;
+	/* Note: flowi6_tos is encoded in flowlabel, too. */
 	__be32			flowlabel;
 	union flowi_uli		uli;
 #define fl6_sport		uli.ports.sport
diff --git a/include/trace/events/fib6.h b/include/trace/events/fib6.h
index 4cf6bac..d60096c 100644
--- a/include/trace/events/fib6.h
+++ b/include/trace/events/fib6.h
@@ -37,7 +37,7 @@ TRACE_EVENT(fib6_table_lookup,
 		__entry->tb_id = tb_id;
 		__entry->oif = flp->flowi6_oif;
 		__entry->iif = flp->flowi6_iif;
-		__entry->tos = flp->flowi6_tos;
+		__entry->tos = ip6_tclass(flp->flowlabel);
 		__entry->scope = flp->flowi6_scope;
 		__entry->flags = flp->flowi6_flags;
 
-- 
1.9.3

Re: [PATCH net 0/3] flowi6_tos fixes

[David Miller]

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Fri, 18 Mar 2016 18:37:56 +0100

> This set is a follow-up to address Jiri's recent feedback [1] on
> the flowi6_tos issue, that it is not used for IPv6 route lookups.
> The three patches fix all current users of flowi6_tos and remove
> the define to avoid any future confusion on this. Tested the vxlan
> and geneve ones with IPv6 routing rules. For details, please see
> individual patches.
> 
> [ As fixes are currently applied against net-next tree, I've rebased
>   it against that. ]
> 
> Thanks!
> 
>   [1] http://patchwork.ozlabs.org/patch/592055/

Series applied, thanks Daniel.