[PATCH v4 net-next 00/11] net: Increase inputs to flow_keys hashing

[PATCH v4 net-next 00/11] net: Increase inputs to flow_keys hashing

[Tom Herbert]

This patch set adds new fields to the flow_keys structure and hashes
over these fields to get a better flow hash. In particular, these
patches now include hashing over the full IPv6 addresses in order
to defend against address spoofing that always results in the
same hash. The new input also includes the Ethertype, L4 protocol,
VLAN, flow label, GRE keyid, and MPLS entropy label.

In order to increase hash inputs, we switch to using jhash2
which operates an an array of u32's. jhash2 operates on multiples of
three words. The data in the hash is constructed for that, and there
are are two variants for IPv4 and Ipv6 addressing. For IPv4 addresses,
jhash is performed over six u32's and for IPv6 it is done over twelve.

flow_keys can store either IPv4 or IPv6 addresses (addr_proto field
is a selector). ipv6_addr_hash is no longer used to convert addresses
for setting in flow table. For legacy uses of flow keys outside of
flow_dissector the flow_get_u32_src and flow_get_u32_dst functions
have been added to get u32 representation representations of addresses
in flow_keys.

For flow labels we also eliminate the short circuit in flow_dissector
for non-zero flow label. The flow label is now considered additional
input to ports.

Testing: Ran netperf TCP_RR for 200 flows using IPv4 and IPv6 comparing
before the patches and with the patches. Did not detect any performance
degradation.

v2:
  - Took out MPLS entropy label. Will add this later.
v3:
  - Ensure hash start offset is a four byte boundary. Add BUG_BUILD_ON
    to check for this.
  - Fixes sparse error in GRE to get entropy from keyid.
v4:
  - Rebase to Jiri changes to generalize flow dissection
  - Support TIPC as its own address
  - Bring back MPLS entropy label dissection
  - Remove FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS


Tom Herbert (11):
  net: Simplify GRE case in flow_dissector
  mpls: Add definition for IPPROTO_MPLS
  net: Remove superfluous setting of key_basic
  net: Get skb hash over flow_keys structure
  net: Add full IPv6 addresses to flow_keys
  net: Add keys for TIPC address
  net: Get rid of IPv6 hash addresses flow keys
  net: Add VLAN ID to flow_keys
  net: Add IPv6 flow label to flow_keys
  net: Add GRE keyid in flow_keys
  mpls: Add MPLS entropy label in flow_keys

 drivers/net/bonding/bond_main.c                |   9 +-
 drivers/net/ethernet/cisco/enic/enic_clsf.c    |   8 +-
 drivers/net/ethernet/cisco/enic/enic_ethtool.c |   4 +-
 include/linux/skbuff.h                         |   2 +-
 include/net/flow_dissector.h                   |  97 +++++--
 include/net/ip.h                               |  21 +-
 include/net/ipv6.h                             |  23 +-
 include/uapi/linux/in.h                        |   2 +
 net/core/flow_dissector.c                      | 336 ++++++++++++++++++-------
 net/ethernet/eth.c                             |   2 +-
 net/sched/cls_flow.c                           |  14 +-
 net/sched/cls_flower.c                         |  13 +-
 12 files changed, 392 insertions(+), 139 deletions(-)

-- 
1.8.1

[PATCH v4 net-next 01/11] net: Simplify GRE case in flow_dissector

[Tom Herbert]

Do break when we see routing flag or a non-zero version number in GRE
header.

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 net/core/flow_dissector.c | 44 ++++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 703d059..4a2cb93 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -308,30 +308,30 @@ flow_label:
 		 * Only look inside GRE if version zero and no
 		 * routing
 		 */
-		if (!(hdr->flags & (GRE_VERSION|GRE_ROUTING))) {
-			proto = hdr->proto;
+		if (hdr->flags & (GRE_VERSION | GRE_ROUTING))
+			break;
+
+		proto = hdr->proto;
+		nhoff += 4;
+		if (hdr->flags & GRE_CSUM)
+			nhoff += 4;
+		if (hdr->flags & GRE_KEY)
 			nhoff += 4;
-			if (hdr->flags & GRE_CSUM)
-				nhoff += 4;
-			if (hdr->flags & GRE_KEY)
-				nhoff += 4;
-			if (hdr->flags & GRE_SEQ)
-				nhoff += 4;
-			if (proto == htons(ETH_P_TEB)) {
-				const struct ethhdr *eth;
-				struct ethhdr _eth;
-
-				eth = __skb_header_pointer(skb, nhoff,
-							   sizeof(_eth),
-							   data, hlen, &_eth);
-				if (!eth)
-					return false;
-				proto = eth->h_proto;
-				nhoff += sizeof(*eth);
-			}
-			goto again;
+		if (hdr->flags & GRE_SEQ)
+			nhoff += 4;
+		if (proto == htons(ETH_P_TEB)) {
+			const struct ethhdr *eth;
+			struct ethhdr _eth;
+
+			eth = __skb_header_pointer(skb, nhoff,
+						   sizeof(_eth),
+						   data, hlen, &_eth);
+			if (!eth)
+				return false;
+			proto = eth->h_proto;
+			nhoff += sizeof(*eth);
 		}
-		break;
+		goto again;
 	}
 	case IPPROTO_IPIP:
 		proto = htons(ETH_P_IP);
-- 
1.8.1

[PATCH v4 net-next 02/11] mpls: Add definition for IPPROTO_MPLS

[Tom Herbert]

Add uapi define for MPLS over IP.

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/uapi/linux/in.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/in.h b/include/uapi/linux/in.h
index 589ced0..641338b 100644
--- a/include/uapi/linux/in.h
+++ b/include/uapi/linux/in.h
@@ -69,6 +69,8 @@ enum {
 #define IPPROTO_SCTP		IPPROTO_SCTP
   IPPROTO_UDPLITE = 136,	/* UDP-Lite (RFC 3828)			*/
 #define IPPROTO_UDPLITE		IPPROTO_UDPLITE
+  IPPROTO_MPLS = 137,		/* MPLS in IP (RFC 4023)		*/
+#define IPPROTO_MPLS		IPPROTO_MPLS
   IPPROTO_RAW = 255,		/* Raw IP packets			*/
 #define IPPROTO_RAW		IPPROTO_RAW
   IPPROTO_MAX
-- 
1.8.1

[PATCH v4 net-next 03/11] net: Remove superfluous setting of key_basic

[Tom Herbert]

key_basic is set twice in __skb_flow_dissect which seems unnecessary.
Remove second one.

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 net/core/flow_dissector.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 4a2cb93..6aebe99 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -343,12 +343,6 @@ flow_label:
 		break;
 	}
 
-	/* It is ensured by skb_flow_dissector_init() that basic key will
-	 * be always present.
-	 */
-	key_basic = skb_flow_dissector_target(flow_dissector,
-					      FLOW_DISSECTOR_KEY_BASIC,
-					      target_container);
 	key_basic->n_proto = proto;
 	key_basic->ip_proto = ip_proto;
 	key_basic->thoff = (u16) nhoff;
-- 
1.8.1

[PATCH v4 net-next 04/11] net: Get skb hash over flow_keys structure

[Tom Herbert]

This patch changes flow hashing to use jhash2 over the flow_keys
structure instead just doing jhash_3words over src, dst, and ports.
This method will allow us take more input into the hashing function
so that we can include full IPv6 addresses, VLAN, flow labels etc.
without needing to resort to xor'ing which makes for a poor hash.

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/linux/skbuff.h       |  2 +-
 include/net/flow_dissector.h | 21 ++++++++++++++---
 include/net/ip.h             |  2 ++
 include/net/ipv6.h           |  2 ++
 net/core/flow_dissector.c    | 54 +++++++++++++++++++++++++++++++++-----------
 net/sched/cls_flower.c       |  2 ++
 6 files changed, 66 insertions(+), 17 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 40960fe..79362f6 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1938,7 +1938,7 @@ static inline void skb_probe_transport_header(struct sk_buff *skb,
 	if (skb_transport_header_was_set(skb))
 		return;
 	else if (skb_flow_dissect_flow_keys(skb, &keys))
-		skb_set_transport_header(skb, keys.basic.thoff);
+		skb_set_transport_header(skb, keys.control.thoff);
 	else
 		skb_set_transport_header(skb, offset_hint);
 }
diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index bac9c14..cba6a10 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -7,15 +7,24 @@
 #include <uapi/linux/if_ether.h>
 
 /**
+ * struct flow_dissector_key_control:
+ * @thoff: Transport header offset
+ */
+struct flow_dissector_key_control {
+	u16	thoff;
+	u16	padding;
+};
+
+/**
  * struct flow_dissector_key_basic:
  * @thoff: Transport header offset
  * @n_proto: Network header protocol (eg. IPv4/IPv6)
  * @ip_proto: Transport header protocol (eg. TCP/UDP)
  */
 struct flow_dissector_key_basic {
-	u16	thoff;
 	__be16	n_proto;
 	u8	ip_proto;
+	u8	padding;
 };
 
 /**
@@ -70,6 +79,7 @@ struct flow_dissector_key_eth_addrs {
 };
 
 enum flow_dissector_key_id {
+	FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */
 	FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */
 	FLOW_DISSECTOR_KEY_IPV4_ADDRS, /* struct flow_dissector_key_addrs */
 	FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS, /* struct flow_dissector_key_addrs */
@@ -109,11 +119,16 @@ static inline bool skb_flow_dissect(const struct sk_buff *skb,
 }
 
 struct flow_keys {
-	struct flow_dissector_key_addrs addrs;
-	struct flow_dissector_key_ports ports;
+	struct flow_dissector_key_control control;
+#define FLOW_KEYS_HASH_START_FIELD basic
 	struct flow_dissector_key_basic basic;
+	struct flow_dissector_key_ports ports;
+	struct flow_dissector_key_addrs addrs;
 };
 
+#define FLOW_KEYS_HASH_OFFSET		\
+	offsetof(struct flow_keys, FLOW_KEYS_HASH_START_FIELD)
+
 extern struct flow_dissector flow_keys_dissector;
 extern struct flow_dissector flow_keys_buf_dissector;
 
diff --git a/include/net/ip.h b/include/net/ip.h
index 7921a36..00d5993 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -359,6 +359,8 @@ static inline void inet_set_txhash(struct sock *sk)
 	struct inet_sock *inet = inet_sk(sk);
 	struct flow_keys keys;
 
+	memset(&keys, 0, sizeof(keys));
+
 	keys.addrs.src = inet->inet_saddr;
 	keys.addrs.dst = inet->inet_daddr;
 	keys.ports.src = inet->inet_sport;
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index aab8190..de1cde0 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -698,6 +698,8 @@ static inline void ip6_set_txhash(struct sock *sk)
 	struct ipv6_pinfo *np = inet6_sk(sk);
 	struct flow_keys keys;
 
+	memset(&keys, 0, sizeof(keys));
+
 	keys.addrs.src = (__force __be32)ipv6_addr_hash(&np->saddr);
 	keys.addrs.dst = (__force __be32)ipv6_addr_hash(&sk->sk_v6_daddr);
 	keys.ports.src = inet->inet_sport;
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 6aebe99..017fb84 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -57,10 +57,12 @@ void skb_flow_dissector_init(struct flow_dissector *flow_dissector,
 		flow_dissector->offset[key->key_id] = key->offset;
 	}
 
-	/* Ensure that the dissector always includes basic key. That way
-	 * we are able to avoid handling lack of it in fast path.
+	/* Ensure that the dissector always includes control and basic key.
+	 * That way we are able to avoid handling lack of these in fast path.
 	 */
 	BUG_ON(!skb_flow_dissector_uses_key(flow_dissector,
+					    FLOW_DISSECTOR_KEY_CONTROL));
+	BUG_ON(!skb_flow_dissector_uses_key(flow_dissector,
 					    FLOW_DISSECTOR_KEY_BASIC));
 }
 EXPORT_SYMBOL(skb_flow_dissector_init);
@@ -120,6 +122,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 			void *target_container,
 			void *data, __be16 proto, int nhoff, int hlen)
 {
+	struct flow_dissector_key_control *key_control;
 	struct flow_dissector_key_basic *key_basic;
 	struct flow_dissector_key_addrs *key_addrs;
 	struct flow_dissector_key_ports *key_ports;
@@ -132,6 +135,13 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 		hlen = skb_headlen(skb);
 	}
 
+	/* It is ensured by skb_flow_dissector_init() that control key will
+	 * be always present.
+	 */
+	key_control = skb_flow_dissector_target(flow_dissector,
+						FLOW_DISSECTOR_KEY_CONTROL,
+						target_container);
+
 	/* It is ensured by skb_flow_dissector_init() that basic key will
 	 * be always present.
 	 */
@@ -219,7 +229,7 @@ flow_label:
 
 			key_basic->n_proto = proto;
 			key_basic->ip_proto = ip_proto;
-			key_basic->thoff = (u16)nhoff;
+			key_control->thoff = (u16)nhoff;
 
 			if (!skb_flow_dissector_uses_key(flow_dissector,
 							 FLOW_DISSECTOR_KEY_PORTS))
@@ -275,7 +285,7 @@ flow_label:
 		if (!hdr)
 			return false;
 		key_basic->n_proto = proto;
-		key_basic->thoff = (u16)nhoff;
+		key_control->thoff = (u16)nhoff;
 
 		if (skb_flow_dissector_uses_key(flow_dissector,
 						FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS)) {
@@ -288,7 +298,7 @@ flow_label:
 		return true;
 	}
 	case htons(ETH_P_FCOE):
-		key_basic->thoff = (u16)(nhoff + FCOE_HEADER_LEN);
+		key_control->thoff = (u16)(nhoff + FCOE_HEADER_LEN);
 		/* fall through */
 	default:
 		return false;
@@ -345,7 +355,7 @@ flow_label:
 
 	key_basic->n_proto = proto;
 	key_basic->ip_proto = ip_proto;
-	key_basic->thoff = (u16) nhoff;
+	key_control->thoff = (u16)nhoff;
 
 	if (skb_flow_dissector_uses_key(flow_dissector,
 					FLOW_DISSECTOR_KEY_PORTS)) {
@@ -366,9 +376,21 @@ static __always_inline void __flow_hash_secret_init(void)
 	net_get_random_once(&hashrnd, sizeof(hashrnd));
 }
 
-static __always_inline u32 __flow_hash_3words(u32 a, u32 b, u32 c, u32 keyval)
+static __always_inline u32 __flow_hash_words(u32 *words, u32 length, u32 keyval)
+{
+	return jhash2(words, length, keyval);
+}
+
+static inline void *flow_keys_hash_start(struct flow_keys *flow)
 {
-	return jhash_3words(a, b, c, keyval);
+	BUILD_BUG_ON(FLOW_KEYS_HASH_OFFSET % sizeof(u32));
+	return (void *)flow + FLOW_KEYS_HASH_OFFSET;
+}
+
+static inline size_t flow_keys_hash_length(struct flow_keys *flow)
+{
+	BUILD_BUG_ON((sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) % sizeof(u32));
+	return (sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) / sizeof(u32);
 }
 
 static inline u32 __flow_hash_from_keys(struct flow_keys *keys, u32 keyval)
@@ -383,10 +405,8 @@ static inline u32 __flow_hash_from_keys(struct flow_keys *keys, u32 keyval)
 		swap(keys->ports.src, keys->ports.dst);
 	}
 
-	hash = __flow_hash_3words((__force u32)keys->addrs.dst,
-				  (__force u32)keys->addrs.src,
-				  (__force u32)keys->ports.ports,
-				  keyval);
+	hash = __flow_hash_words((u32 *)flow_keys_hash_start(keys),
+				 flow_keys_hash_length(keys), keyval);
 	if (!hash)
 		hash = 1;
 
@@ -473,7 +493,7 @@ EXPORT_SYMBOL(skb_get_hash_perturb);
 u32 __skb_get_poff(const struct sk_buff *skb, void *data,
 		   const struct flow_keys *keys, int hlen)
 {
-	u32 poff = keys->basic.thoff;
+	u32 poff = keys->control.thoff;
 
 	switch (keys->basic.ip_proto) {
 	case IPPROTO_TCP: {
@@ -537,6 +557,10 @@ u32 skb_get_poff(const struct sk_buff *skb)
 
 static const struct flow_dissector_key flow_keys_dissector_keys[] = {
 	{
+		.key_id = FLOW_DISSECTOR_KEY_CONTROL,
+		.offset = offsetof(struct flow_keys, control),
+	},
+	{
 		.key_id = FLOW_DISSECTOR_KEY_BASIC,
 		.offset = offsetof(struct flow_keys, basic),
 	},
@@ -556,6 +580,10 @@ static const struct flow_dissector_key flow_keys_dissector_keys[] = {
 
 static const struct flow_dissector_key flow_keys_buf_dissector_keys[] = {
 	{
+		.key_id = FLOW_DISSECTOR_KEY_CONTROL,
+		.offset = offsetof(struct flow_keys, control),
+	},
+	{
 		.key_id = FLOW_DISSECTOR_KEY_BASIC,
 		.offset = offsetof(struct flow_keys, basic),
 	},
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 8c8f34e..5a7d66c 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -25,6 +25,7 @@
 
 struct fl_flow_key {
 	int	indev_ifindex;
+	struct flow_dissector_key_control control;
 	struct flow_dissector_key_basic basic;
 	struct flow_dissector_key_eth_addrs eth;
 	union {
@@ -347,6 +348,7 @@ static void fl_init_dissector(struct cls_fl_head *head,
 	struct flow_dissector_key keys[FLOW_DISSECTOR_KEY_MAX];
 	size_t cnt = 0;
 
+	FL_KEY_SET(keys, cnt, FLOW_DISSECTOR_KEY_CONTROL, control);
 	FL_KEY_SET(keys, cnt, FLOW_DISSECTOR_KEY_BASIC, basic);
 	FL_KEY_SET_IF_IN_RANGE(mask, keys, cnt,
 			       FLOW_DISSECTOR_KEY_ETH_ADDRS, eth);
-- 
1.8.1

[PATCH v4 net-next 05/11] net: Add full IPv6 addresses to flow_keys

[Tom Herbert]

This patch adds full IPv6 addresses into flow_keys and uses them as
input to the flow hash function. The implementation supports either
IPv4 or IPv6 addresses in a union, and selector is used to determine
how may words to input to jhash2.

We also add flow_get_u32_dst and flow_get_u32_src functions which are
used to get a u32 representation of the source and destination
addresses. For IPv6, ipv6_addr_hash is called. These functions retain
getting the legacy values of src and dst in flow_keys.

With this patch, Ethertype and IP protocol are now included in the
flow hash input.

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 drivers/net/bonding/bond_main.c                |   9 +-
 drivers/net/ethernet/cisco/enic/enic_clsf.c    |   8 +-
 drivers/net/ethernet/cisco/enic/enic_ethtool.c |   4 +-
 include/net/flow_dissector.h                   |  52 +++++++----
 include/net/ip.h                               |  19 +++-
 include/net/ipv6.h                             |  21 ++++-
 net/core/flow_dissector.c                      | 116 +++++++++++++++++++++----
 net/ethernet/eth.c                             |   2 +-
 net/sched/cls_flow.c                           |  14 ++-
 net/sched/cls_flower.c                         |  11 +--
 10 files changed, 193 insertions(+), 63 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 2268438..19eb990 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3059,8 +3059,7 @@ static bool bond_flow_dissect(struct bonding *bond, struct sk_buff *skb,
 		if (unlikely(!pskb_may_pull(skb, noff + sizeof(*iph))))
 			return false;
 		iph = ip_hdr(skb);
-		fk->addrs.src = iph->saddr;
-		fk->addrs.dst = iph->daddr;
+		iph_to_flow_copy_v4addrs(fk, iph);
 		noff += iph->ihl << 2;
 		if (!ip_is_fragment(iph))
 			proto = iph->protocol;
@@ -3068,8 +3067,7 @@ static bool bond_flow_dissect(struct bonding *bond, struct sk_buff *skb,
 		if (unlikely(!pskb_may_pull(skb, noff + sizeof(*iph6))))
 			return false;
 		iph6 = ipv6_hdr(skb);
-		fk->addrs.src = (__force __be32)ipv6_addr_hash(&iph6->saddr);
-		fk->addrs.dst = (__force __be32)ipv6_addr_hash(&iph6->daddr);
+		iph_to_flow_copy_v6addrs(fk, iph6);
 		noff += sizeof(*iph6);
 		proto = iph6->nexthdr;
 	} else {
@@ -3103,7 +3101,8 @@ u32 bond_xmit_hash(struct bonding *bond, struct sk_buff *skb)
 		hash = bond_eth_hash(skb);
 	else
 		hash = (__force u32)flow.ports.ports;
-	hash ^= (__force u32)flow.addrs.dst ^ (__force u32)flow.addrs.src;
+	hash ^= (__force u32)flow_get_u32_dst(&flow) ^
+		(__force u32)flow_get_u32_src(&flow);
 	hash ^= (hash >> 16);
 	hash ^= (hash >> 8);
 
diff --git a/drivers/net/ethernet/cisco/enic/enic_clsf.c b/drivers/net/ethernet/cisco/enic/enic_clsf.c
index 6739ebc..9afaa67 100644
--- a/drivers/net/ethernet/cisco/enic/enic_clsf.c
+++ b/drivers/net/ethernet/cisco/enic/enic_clsf.c
@@ -33,8 +33,8 @@ int enic_addfltr_5t(struct enic *enic, struct flow_keys *keys, u16 rq)
 		return -EPROTONOSUPPORT;
 	};
 	data.type = FILTER_IPV4_5TUPLE;
-	data.u.ipv4.src_addr = ntohl(keys->addrs.src);
-	data.u.ipv4.dst_addr = ntohl(keys->addrs.dst);
+	data.u.ipv4.src_addr = ntohl(keys->addrs.v4addrs.src);
+	data.u.ipv4.dst_addr = ntohl(keys->addrs.v4addrs.dst);
 	data.u.ipv4.src_port = ntohs(keys->ports.src);
 	data.u.ipv4.dst_port = ntohs(keys->ports.dst);
 	data.u.ipv4.flags = FILTER_FIELDS_IPV4_5TUPLE;
@@ -158,8 +158,8 @@ static struct enic_rfs_fltr_node *htbl_key_search(struct hlist_head *h,
 	struct enic_rfs_fltr_node *tpos;
 
 	hlist_for_each_entry(tpos, h, node)
-		if (tpos->keys.addrs.src == k->addrs.src &&
-		    tpos->keys.addrs.dst == k->addrs.dst &&
+		if (tpos->keys.addrs.v4addrs.src == k->addrs.v4addrs.src &&
+		    tpos->keys.addrs.v4addrs.dst == k->addrs.v4addrs.dst &&
 		    tpos->keys.ports.ports == k->ports.ports &&
 		    tpos->keys.basic.ip_proto == k->basic.ip_proto &&
 		    tpos->keys.basic.n_proto == k->basic.n_proto)
diff --git a/drivers/net/ethernet/cisco/enic/enic_ethtool.c b/drivers/net/ethernet/cisco/enic/enic_ethtool.c
index 117c096..73874b2 100644
--- a/drivers/net/ethernet/cisco/enic/enic_ethtool.c
+++ b/drivers/net/ethernet/cisco/enic/enic_ethtool.c
@@ -346,10 +346,10 @@ static int enic_grxclsrule(struct enic *enic, struct ethtool_rxnfc *cmd)
 		break;
 	}
 
-	fsp->h_u.tcp_ip4_spec.ip4src = n->keys.addrs.src;
+	fsp->h_u.tcp_ip4_spec.ip4src = flow_get_u32_src(&n->keys);
 	fsp->m_u.tcp_ip4_spec.ip4src = (__u32)~0;
 
-	fsp->h_u.tcp_ip4_spec.ip4dst = n->keys.addrs.dst;
+	fsp->h_u.tcp_ip4_spec.ip4dst = flow_get_u32_dst(&n->keys);
 	fsp->m_u.tcp_ip4_spec.ip4dst = (__u32)~0;
 
 	fsp->h_u.tcp_ip4_spec.psrc = n->keys.ports.src;
diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index cba6a10..306d461 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -12,7 +12,7 @@
  */
 struct flow_dissector_key_control {
 	u16	thoff;
-	u16	padding;
+	u16	addr_type;
 };
 
 /**
@@ -28,19 +28,40 @@ struct flow_dissector_key_basic {
 };
 
 /**
- * struct flow_dissector_key_addrs:
- * @src: source ip address in case of IPv4
- *	 For IPv6 it contains 32bit hash of src address
- * @dst: destination ip address in case of IPv4
- *	 For IPv6 it contains 32bit hash of dst address
+ * struct flow_dissector_key_ipv4_addrs:
+ * @src: source ip address
+ * @dst: destination ip address
  */
-struct flow_dissector_key_addrs {
+struct flow_dissector_key_ipv4_addrs {
 	/* (src,dst) must be grouped, in the same way than in IP header */
 	__be32 src;
 	__be32 dst;
 };
 
 /**
+ * struct flow_dissector_key_ipv6_addrs:
+ * @src: source ip address
+ * @dst: destination ip address
+ */
+struct flow_dissector_key_ipv6_addrs {
+	/* (src,dst) must be grouped, in the same way than in IP header */
+	struct in6_addr src;
+	struct in6_addr dst;
+};
+
+/**
+ * struct flow_dissector_key_addrs:
+ * @v4addrs: IPv4 addresses
+ * @v6addrs: IPv6 addresses
+ */
+struct flow_dissector_key_addrs {
+	union {
+		struct flow_dissector_key_ipv4_addrs v4addrs;
+		struct flow_dissector_key_ipv6_addrs v6addrs;
+	};
+};
+
+/**
  * flow_dissector_key_tp_ports:
  *	@ports: port numbers of Transport header
  *		src: source port number
@@ -56,16 +77,6 @@ struct flow_dissector_key_ports {
 	};
 };
 
-/**
- * struct flow_dissector_key_ipv6_addrs:
- * @src: source ip address
- * @dst: destination ip address
- */
-struct flow_dissector_key_ipv6_addrs {
-	/* (src,dst) must be grouped, in the same way than in IP header */
-	struct in6_addr src;
-	struct in6_addr dst;
-};
 
 /**
  * struct flow_dissector_key_eth_addrs:
@@ -81,10 +92,10 @@ struct flow_dissector_key_eth_addrs {
 enum flow_dissector_key_id {
 	FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */
 	FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */
-	FLOW_DISSECTOR_KEY_IPV4_ADDRS, /* struct flow_dissector_key_addrs */
+	FLOW_DISSECTOR_KEY_IPV4_ADDRS, /* struct flow_dissector_key_ipv4_addrs */
+	FLOW_DISSECTOR_KEY_IPV6_ADDRS, /* struct flow_dissector_key_ipv6_addrs */
 	FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS, /* struct flow_dissector_key_addrs */
 	FLOW_DISSECTOR_KEY_PORTS, /* struct flow_dissector_key_ports */
-	FLOW_DISSECTOR_KEY_IPV6_ADDRS, /* struct flow_dissector_key_ipv6_addrs */
 	FLOW_DISSECTOR_KEY_ETH_ADDRS, /* struct flow_dissector_key_eth_addrs */
 
 	FLOW_DISSECTOR_KEY_MAX,
@@ -129,6 +140,9 @@ struct flow_keys {
 #define FLOW_KEYS_HASH_OFFSET		\
 	offsetof(struct flow_keys, FLOW_KEYS_HASH_START_FIELD)
 
+__be32 flow_get_u32_src(const struct flow_keys *flow);
+__be32 flow_get_u32_dst(const struct flow_keys *flow);
+
 extern struct flow_dissector flow_keys_dissector;
 extern struct flow_dissector flow_keys_buf_dissector;
 
diff --git a/include/net/ip.h b/include/net/ip.h
index 00d5993..947b59e 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -354,6 +354,20 @@ static inline __wsum inet_compute_pseudo(struct sk_buff *skb, int proto)
 				  skb->len, proto, 0);
 }
 
+/* copy IPv4 saddr & daddr to flow_keys, possibly using 64bit load/store
+ * Equivalent to :	flow->v4addrs.src = iph->saddr;
+ *			flow->v4addrs.dst = iph->daddr;
+ */
+static inline void iph_to_flow_copy_v4addrs(struct flow_keys *flow,
+					    const struct iphdr *iph)
+{
+	BUILD_BUG_ON(offsetof(typeof(flow->addrs), v4addrs.dst) !=
+		     offsetof(typeof(flow->addrs), v4addrs.src) +
+			      sizeof(flow->addrs.v4addrs.src));
+	memcpy(&flow->addrs.v4addrs, &iph->saddr, sizeof(flow->addrs.v4addrs));
+	flow->control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
+}
+
 static inline void inet_set_txhash(struct sock *sk)
 {
 	struct inet_sock *inet = inet_sk(sk);
@@ -361,8 +375,9 @@ static inline void inet_set_txhash(struct sock *sk)
 
 	memset(&keys, 0, sizeof(keys));
 
-	keys.addrs.src = inet->inet_saddr;
-	keys.addrs.dst = inet->inet_daddr;
+	keys.addrs.v4addrs.src = inet->inet_saddr;
+	keys.addrs.v4addrs.dst = inet->inet_daddr;
+	keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
 	keys.ports.src = inet->inet_sport;
 	keys.ports.dst = inet->inet_dport;
 
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index de1cde0..eb03ae8 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -691,6 +691,20 @@ static inline int ip6_sk_dst_hoplimit(struct ipv6_pinfo *np, struct flowi6 *fl6,
 	return hlimit;
 }
 
+/* copy IPv6 saddr & daddr to flow_keys, possibly using 64bit load/store
+ * Equivalent to :	flow->v6addrs.src = iph->saddr;
+ *			flow->v6addrs.dst = iph->daddr;
+ */
+static inline void iph_to_flow_copy_v6addrs(struct flow_keys *flow,
+					    const struct ipv6hdr *iph)
+{
+	BUILD_BUG_ON(offsetof(typeof(flow->addrs), v6addrs.dst) !=
+		     offsetof(typeof(flow->addrs), v6addrs.src) +
+		     sizeof(flow->addrs.v6addrs.src));
+	memcpy(&flow->addrs.v6addrs, &iph->saddr, sizeof(flow->addrs.v6addrs));
+	flow->control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
+}
+
 #if IS_ENABLED(CONFIG_IPV6)
 static inline void ip6_set_txhash(struct sock *sk)
 {
@@ -700,8 +714,11 @@ static inline void ip6_set_txhash(struct sock *sk)
 
 	memset(&keys, 0, sizeof(keys));
 
-	keys.addrs.src = (__force __be32)ipv6_addr_hash(&np->saddr);
-	keys.addrs.dst = (__force __be32)ipv6_addr_hash(&sk->sk_v6_daddr);
+	memcpy(&keys.addrs.v6addrs.src, &np->saddr,
+	       sizeof(keys.addrs.v6addrs.src));
+	memcpy(&keys.addrs.v6addrs.dst, &sk->sk_v6_daddr,
+	       sizeof(keys.addrs.v6addrs.dst));
+	keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
 	keys.ports.src = inet->inet_sport;
 	keys.ports.dst = inet->inet_dport;
 
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 017fb84..d77619f 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -178,10 +178,12 @@ ip:
 		if (!skb_flow_dissector_uses_key(flow_dissector,
 						 FLOW_DISSECTOR_KEY_IPV4_ADDRS))
 			break;
+
 		key_addrs = skb_flow_dissector_target(flow_dissector,
-						      FLOW_DISSECTOR_KEY_IPV4_ADDRS,
-						      target_container);
-		memcpy(key_addrs, &iph->saddr, sizeof(*key_addrs));
+			      FLOW_DISSECTOR_KEY_IPV4_ADDRS, target_container);
+		memcpy(&key_addrs->v4addrs, &iph->saddr,
+		       sizeof(key_addrs->v4addrs));
+		key_control->addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
 		break;
 	}
 	case htons(ETH_P_IPV6): {
@@ -203,8 +205,11 @@ ipv6:
 							      FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS,
 							      target_container);
 
-			key_addrs->src = (__force __be32)ipv6_addr_hash(&iph->saddr);
-			key_addrs->dst = (__force __be32)ipv6_addr_hash(&iph->daddr);
+			key_addrs->v4addrs.src =
+				(__force __be32)ipv6_addr_hash(&iph->saddr);
+			key_addrs->v4addrs.dst =
+				(__force __be32)ipv6_addr_hash(&iph->daddr);
+			key_control->addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
 			goto flow_label;
 		}
 		if (skb_flow_dissector_uses_key(flow_dissector,
@@ -216,6 +221,7 @@ ipv6:
 								   target_container);
 
 			memcpy(key_ipv6_addrs, &iph->saddr, sizeof(*key_ipv6_addrs));
+			key_control->addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
 			goto flow_label;
 		}
 		break;
@@ -292,8 +298,9 @@ flow_label:
 			key_addrs = skb_flow_dissector_target(flow_dissector,
 							      FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS,
 							      target_container);
-			key_addrs->src = hdr->srcnode;
-			key_addrs->dst = 0;
+			key_addrs->v4addrs.src = hdr->srcnode;
+			key_addrs->v4addrs.dst = 0;
+			key_control->addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
 		}
 		return true;
 	}
@@ -389,21 +396,88 @@ static inline void *flow_keys_hash_start(struct flow_keys *flow)
 
 static inline size_t flow_keys_hash_length(struct flow_keys *flow)
 {
+	size_t diff = FLOW_KEYS_HASH_OFFSET + sizeof(flow->addrs);
 	BUILD_BUG_ON((sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) % sizeof(u32));
-	return (sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) / sizeof(u32);
+	BUILD_BUG_ON(offsetof(typeof(*flow), addrs) !=
+		     sizeof(*flow) - sizeof(flow->addrs));
+
+	switch (flow->control.addr_type) {
+	case FLOW_DISSECTOR_KEY_IPV4_ADDRS:
+		diff -= sizeof(flow->addrs.v4addrs);
+		break;
+	case FLOW_DISSECTOR_KEY_IPV6_ADDRS:
+		diff -= sizeof(flow->addrs.v6addrs);
+		break;
+	}
+	return (sizeof(*flow) - diff) / sizeof(u32);
+}
+
+__be32 flow_get_u32_src(const struct flow_keys *flow)
+{
+	switch (flow->control.addr_type) {
+	case FLOW_DISSECTOR_KEY_IPV4_ADDRS:
+		return flow->addrs.v4addrs.src;
+	case FLOW_DISSECTOR_KEY_IPV6_ADDRS:
+		return (__force __be32)ipv6_addr_hash(
+			&flow->addrs.v6addrs.src);
+	default:
+		return 0;
+	}
+}
+EXPORT_SYMBOL(flow_get_u32_src);
+
+__be32 flow_get_u32_dst(const struct flow_keys *flow)
+{
+	switch (flow->control.addr_type) {
+	case FLOW_DISSECTOR_KEY_IPV4_ADDRS:
+		return flow->addrs.v4addrs.dst;
+	case FLOW_DISSECTOR_KEY_IPV6_ADDRS:
+		return (__force __be32)ipv6_addr_hash(
+			&flow->addrs.v6addrs.dst);
+	default:
+		return 0;
+	}
+}
+EXPORT_SYMBOL(flow_get_u32_dst);
+
+static inline void __flow_hash_consistentify(struct flow_keys *keys)
+{
+	int addr_diff, i;
+
+	switch (keys->control.addr_type) {
+	case FLOW_DISSECTOR_KEY_IPV4_ADDRS:
+		addr_diff = (__force u32)keys->addrs.v4addrs.dst -
+			    (__force u32)keys->addrs.v4addrs.src;
+		if ((addr_diff < 0) ||
+		    (addr_diff == 0 &&
+		     ((__force u16)keys->ports.dst <
+		      (__force u16)keys->ports.src))) {
+			swap(keys->addrs.v4addrs.src, keys->addrs.v4addrs.dst);
+			swap(keys->ports.src, keys->ports.dst);
+		}
+		break;
+	case FLOW_DISSECTOR_KEY_IPV6_ADDRS:
+		addr_diff = memcmp(&keys->addrs.v6addrs.dst,
+				   &keys->addrs.v6addrs.src,
+				   sizeof(keys->addrs.v6addrs.dst));
+		if ((addr_diff < 0) ||
+		    (addr_diff == 0 &&
+		     ((__force u16)keys->ports.dst <
+		      (__force u16)keys->ports.src))) {
+			for (i = 0; i < 4; i++)
+				swap(keys->addrs.v6addrs.src.s6_addr32[i],
+				     keys->addrs.v6addrs.dst.s6_addr32[i]);
+			swap(keys->ports.src, keys->ports.dst);
+		}
+		break;
+	}
 }
 
 static inline u32 __flow_hash_from_keys(struct flow_keys *keys, u32 keyval)
 {
 	u32 hash;
 
-	/* get a consistent hash (same value on both flow directions) */
-	if (((__force u32)keys->addrs.dst < (__force u32)keys->addrs.src) ||
-	    (((__force u32)keys->addrs.dst == (__force u32)keys->addrs.src) &&
-	     ((__force u16)keys->ports.dst < (__force u16)keys->ports.src))) {
-		swap(keys->addrs.dst, keys->addrs.src);
-		swap(keys->ports.src, keys->ports.dst);
-	}
+	__flow_hash_consistentify(keys);
 
 	hash = __flow_hash_words((u32 *)flow_keys_hash_start(keys),
 				 flow_keys_hash_length(keys), keyval);
@@ -451,8 +525,8 @@ void make_flow_keys_digest(struct flow_keys_digest *digest,
 	data->n_proto = flow->basic.n_proto;
 	data->ip_proto = flow->basic.ip_proto;
 	data->ports = flow->ports.ports;
-	data->src = flow->addrs.src;
-	data->dst = flow->addrs.dst;
+	data->src = flow->addrs.v4addrs.src;
+	data->dst = flow->addrs.v4addrs.dst;
 }
 EXPORT_SYMBOL(make_flow_keys_digest);
 
@@ -566,11 +640,15 @@ static const struct flow_dissector_key flow_keys_dissector_keys[] = {
 	},
 	{
 		.key_id = FLOW_DISSECTOR_KEY_IPV4_ADDRS,
-		.offset = offsetof(struct flow_keys, addrs),
+		.offset = offsetof(struct flow_keys, addrs.v4addrs),
+	},
+	{
+		.key_id = FLOW_DISSECTOR_KEY_IPV6_ADDRS,
+		.offset = offsetof(struct flow_keys, addrs.v6addrs),
 	},
 	{
 		.key_id = FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS,
-		.offset = offsetof(struct flow_keys, addrs),
+		.offset = offsetof(struct flow_keys, addrs.v4addrs),
 	},
 	{
 		.key_id = FLOW_DISSECTOR_KEY_PORTS,
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index c3325bd..b8b277e 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -133,7 +133,7 @@ u32 eth_get_headlen(void *data, unsigned int len)
 	/* parse any remaining L2/L3 headers, check for L4 */
 	if (!skb_flow_dissect_flow_keys_buf(&keys, data, eth->h_proto,
 					    sizeof(*eth), len))
-		return max_t(u32, keys.basic.thoff, sizeof(*eth));
+		return max_t(u32, keys.control.thoff, sizeof(*eth));
 
 	/* parse for any L4 headers */
 	return min_t(u32, __skb_get_poff(NULL, data, &keys, len), len);
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index b435992..76bc3a2 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -68,15 +68,21 @@ static inline u32 addr_fold(void *addr)
 
 static u32 flow_get_src(const struct sk_buff *skb, const struct flow_keys *flow)
 {
-	if (flow->addrs.src)
-		return ntohl(flow->addrs.src);
+	__be32 src = flow_get_u32_src(flow);
+
+	if (src)
+		return ntohl(src);
+
 	return addr_fold(skb->sk);
 }
 
 static u32 flow_get_dst(const struct sk_buff *skb, const struct flow_keys *flow)
 {
-	if (flow->addrs.dst)
-		return ntohl(flow->addrs.dst);
+	__be32 dst = flow_get_u32_dst(flow);
+
+	if (dst)
+		return ntohl(dst);
+
 	return addr_fold(skb_dst(skb)) ^ (__force u16) tc_skb_protocol(skb);
 }
 
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 5a7d66c..b92d3f4 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -28,8 +28,9 @@ struct fl_flow_key {
 	struct flow_dissector_key_control control;
 	struct flow_dissector_key_basic basic;
 	struct flow_dissector_key_eth_addrs eth;
+	struct flow_dissector_key_addrs ipaddrs;
 	union {
-		struct flow_dissector_key_addrs ipv4;
+		struct flow_dissector_key_ipv4_addrs ipv4;
 		struct flow_dissector_key_ipv6_addrs ipv6;
 	};
 	struct flow_dissector_key_ports tp;
@@ -260,14 +261,14 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
 			       &mask->basic.ip_proto, TCA_FLOWER_UNSPEC,
 			       sizeof(key->basic.ip_proto));
 	}
-	if (key->basic.n_proto == htons(ETH_P_IP)) {
+	if (key->control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS) {
 		fl_set_key_val(tb, &key->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC,
 			       &mask->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC_MASK,
 			       sizeof(key->ipv4.src));
 		fl_set_key_val(tb, &key->ipv4.dst, TCA_FLOWER_KEY_IPV4_DST,
 			       &mask->ipv4.dst, TCA_FLOWER_KEY_IPV4_DST_MASK,
 			       sizeof(key->ipv4.dst));
-	} else if (key->basic.n_proto == htons(ETH_P_IPV6)) {
+	} else if (key->control.addr_type == FLOW_DISSECTOR_KEY_IPV6_ADDRS) {
 		fl_set_key_val(tb, &key->ipv6.src, TCA_FLOWER_KEY_IPV6_SRC,
 			       &mask->ipv6.src, TCA_FLOWER_KEY_IPV6_SRC_MASK,
 			       sizeof(key->ipv6.src));
@@ -610,7 +611,7 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 			    sizeof(key->basic.ip_proto)))
 		goto nla_put_failure;
 
-	if (key->basic.n_proto == htons(ETH_P_IP) &&
+	if (key->control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS &&
 	    (fl_dump_key_val(skb, &key->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC,
 			     &mask->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC_MASK,
 			     sizeof(key->ipv4.src)) ||
@@ -618,7 +619,7 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 			     &mask->ipv4.dst, TCA_FLOWER_KEY_IPV4_DST_MASK,
 			     sizeof(key->ipv4.dst))))
 		goto nla_put_failure;
-	else if (key->basic.n_proto == htons(ETH_P_IPV6) &&
+	else if (key->control.addr_type == FLOW_DISSECTOR_KEY_IPV6_ADDRS &&
 		 (fl_dump_key_val(skb, &key->ipv6.src, TCA_FLOWER_KEY_IPV6_SRC,
 				  &mask->ipv6.src, TCA_FLOWER_KEY_IPV6_SRC_MASK,
 				  sizeof(key->ipv6.src)) ||
-- 
1.8.1

[PATCH v4 net-next 06/11] net: Add keys for TIPC address

[Tom Herbert]

Add a new flow key for TIPC addresses.

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/net/flow_dissector.h | 10 ++++++++++
 net/core/flow_dissector.c    | 20 ++++++++++++++------
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index 306d461..3ee606a 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -50,6 +50,14 @@ struct flow_dissector_key_ipv6_addrs {
 };
 
 /**
+ * struct flow_dissector_key_tipc_addrs:
+ * @srcnode: source node address
+ */
+struct flow_dissector_key_tipc_addrs {
+	__be32 srcnode;
+};
+
+/**
  * struct flow_dissector_key_addrs:
  * @v4addrs: IPv4 addresses
  * @v6addrs: IPv6 addresses
@@ -58,6 +66,7 @@ struct flow_dissector_key_addrs {
 	union {
 		struct flow_dissector_key_ipv4_addrs v4addrs;
 		struct flow_dissector_key_ipv6_addrs v6addrs;
+		struct flow_dissector_key_tipc_addrs tipcaddrs;
 	};
 };
 
@@ -97,6 +106,7 @@ enum flow_dissector_key_id {
 	FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS, /* struct flow_dissector_key_addrs */
 	FLOW_DISSECTOR_KEY_PORTS, /* struct flow_dissector_key_ports */
 	FLOW_DISSECTOR_KEY_ETH_ADDRS, /* struct flow_dissector_key_eth_addrs */
+	FLOW_DISSECTOR_KEY_TIPC_ADDRS, /* struct flow_dissector_key_tipc_addrs */
 
 	FLOW_DISSECTOR_KEY_MAX,
 };
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index d77619f..8d6f089 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -294,13 +294,12 @@ flow_label:
 		key_control->thoff = (u16)nhoff;
 
 		if (skb_flow_dissector_uses_key(flow_dissector,
-						FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS)) {
+				FLOW_DISSECTOR_KEY_TIPC_ADDRS)) {
 			key_addrs = skb_flow_dissector_target(flow_dissector,
-							      FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS,
-							      target_container);
-			key_addrs->v4addrs.src = hdr->srcnode;
-			key_addrs->v4addrs.dst = 0;
-			key_control->addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
+				FLOW_DISSECTOR_KEY_TIPC_ADDRS,
+				target_container);
+			key_addrs->tipcaddrs.srcnode = hdr->srcnode;
+			key_control->addr_type = FLOW_DISSECTOR_KEY_TIPC_ADDRS;
 		}
 		return true;
 	}
@@ -408,6 +407,9 @@ static inline size_t flow_keys_hash_length(struct flow_keys *flow)
 	case FLOW_DISSECTOR_KEY_IPV6_ADDRS:
 		diff -= sizeof(flow->addrs.v6addrs);
 		break;
+	case FLOW_DISSECTOR_KEY_TIPC_ADDRS:
+		diff -= sizeof(flow->addrs.tipcaddrs);
+		break;
 	}
 	return (sizeof(*flow) - diff) / sizeof(u32);
 }
@@ -420,6 +422,8 @@ __be32 flow_get_u32_src(const struct flow_keys *flow)
 	case FLOW_DISSECTOR_KEY_IPV6_ADDRS:
 		return (__force __be32)ipv6_addr_hash(
 			&flow->addrs.v6addrs.src);
+	case FLOW_DISSECTOR_KEY_TIPC_ADDRS:
+		return flow->addrs.tipcaddrs.srcnode;
 	default:
 		return 0;
 	}
@@ -651,6 +655,10 @@ static const struct flow_dissector_key flow_keys_dissector_keys[] = {
 		.offset = offsetof(struct flow_keys, addrs.v4addrs),
 	},
 	{
+		.key_id = FLOW_DISSECTOR_KEY_TIPC_ADDRS,
+		.offset = offsetof(struct flow_keys, addrs.tipcaddrs),
+	},
+	{
 		.key_id = FLOW_DISSECTOR_KEY_PORTS,
 		.offset = offsetof(struct flow_keys, ports),
 	},
-- 
1.8.1

[PATCH v4 net-next 07/11] net: Get rid of IPv6 hash addresses flow keys

[Tom Herbert]

We don't need to return the IPv6 address hash as part of flow keys.
In general, using the IPv6 address hash is risky in a hash value
since the underlying use of xor provides no entropy. If someone
really needs the hash value they can get it from the full IPv6
addresses in flow keys (e.g. from flow_get_u32_src).

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/net/flow_dissector.h |  1 -
 net/core/flow_dissector.c    | 17 -----------------
 2 files changed, 18 deletions(-)

diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index 3ee606a..59f00f9 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -103,7 +103,6 @@ enum flow_dissector_key_id {
 	FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */
 	FLOW_DISSECTOR_KEY_IPV4_ADDRS, /* struct flow_dissector_key_ipv4_addrs */
 	FLOW_DISSECTOR_KEY_IPV6_ADDRS, /* struct flow_dissector_key_ipv6_addrs */
-	FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS, /* struct flow_dissector_key_addrs */
 	FLOW_DISSECTOR_KEY_PORTS, /* struct flow_dissector_key_ports */
 	FLOW_DISSECTOR_KEY_ETH_ADDRS, /* struct flow_dissector_key_eth_addrs */
 	FLOW_DISSECTOR_KEY_TIPC_ADDRS, /* struct flow_dissector_key_tipc_addrs */
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 8d6f089..44e47c5 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -200,19 +200,6 @@ ipv6:
 		nhoff += sizeof(struct ipv6hdr);
 
 		if (skb_flow_dissector_uses_key(flow_dissector,
-						FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS)) {
-			key_addrs = skb_flow_dissector_target(flow_dissector,
-							      FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS,
-							      target_container);
-
-			key_addrs->v4addrs.src =
-				(__force __be32)ipv6_addr_hash(&iph->saddr);
-			key_addrs->v4addrs.dst =
-				(__force __be32)ipv6_addr_hash(&iph->daddr);
-			key_control->addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
-			goto flow_label;
-		}
-		if (skb_flow_dissector_uses_key(flow_dissector,
 						FLOW_DISSECTOR_KEY_IPV6_ADDRS)) {
 			struct flow_dissector_key_ipv6_addrs *key_ipv6_addrs;
 
@@ -651,10 +638,6 @@ static const struct flow_dissector_key flow_keys_dissector_keys[] = {
 		.offset = offsetof(struct flow_keys, addrs.v6addrs),
 	},
 	{
-		.key_id = FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS,
-		.offset = offsetof(struct flow_keys, addrs.v4addrs),
-	},
-	{
 		.key_id = FLOW_DISSECTOR_KEY_TIPC_ADDRS,
 		.offset = offsetof(struct flow_keys, addrs.tipcaddrs),
 	},
-- 
1.8.1

[PATCH v4 net-next 08/11] net: Add VLAN ID to flow_keys

[Tom Herbert]

In flow_dissector set vlan_id in flow_keys when VLAN is found.

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/net/flow_dissector.h |  6 ++++++
 net/core/flow_dissector.c    | 14 ++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index 59f00f9..08480fb 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -27,6 +27,10 @@ struct flow_dissector_key_basic {
 	u8	padding;
 };
 
+struct flow_dissector_key_tags {
+	u32	vlan_id:12;
+};
+
 /**
  * struct flow_dissector_key_ipv4_addrs:
  * @src: source ip address
@@ -106,6 +110,7 @@ enum flow_dissector_key_id {
 	FLOW_DISSECTOR_KEY_PORTS, /* struct flow_dissector_key_ports */
 	FLOW_DISSECTOR_KEY_ETH_ADDRS, /* struct flow_dissector_key_eth_addrs */
 	FLOW_DISSECTOR_KEY_TIPC_ADDRS, /* struct flow_dissector_key_tipc_addrs */
+	FLOW_DISSECTOR_KEY_VLANID, /* struct flow_dissector_key_flow_tags */
 
 	FLOW_DISSECTOR_KEY_MAX,
 };
@@ -142,6 +147,7 @@ struct flow_keys {
 	struct flow_dissector_key_control control;
 #define FLOW_KEYS_HASH_START_FIELD basic
 	struct flow_dissector_key_basic basic;
+	struct flow_dissector_key_tags tags;
 	struct flow_dissector_key_ports ports;
 	struct flow_dissector_key_addrs addrs;
 };
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 44e47c5..3512366 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -126,6 +126,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 	struct flow_dissector_key_basic *key_basic;
 	struct flow_dissector_key_addrs *key_addrs;
 	struct flow_dissector_key_ports *key_ports;
+	struct flow_dissector_key_tags *key_tags;
 	u8 ip_proto;
 
 	if (!data) {
@@ -246,6 +247,15 @@ flow_label:
 		if (!vlan)
 			return false;
 
+		if (skb_flow_dissector_uses_key(flow_dissector,
+						FLOW_DISSECTOR_KEY_VLANID)) {
+			key_tags = skb_flow_dissector_target(
+				flow_dissector, FLOW_DISSECTOR_KEY_VLANID,
+				target_container);
+
+			key_tags->vlan_id = skb_vlan_tag_get_id(skb);
+		}
+
 		proto = vlan->h_vlan_encapsulated_proto;
 		nhoff += sizeof(*vlan);
 		goto again;
@@ -645,6 +655,10 @@ static const struct flow_dissector_key flow_keys_dissector_keys[] = {
 		.key_id = FLOW_DISSECTOR_KEY_PORTS,
 		.offset = offsetof(struct flow_keys, ports),
 	},
+	{
+		.key_id = FLOW_DISSECTOR_KEY_VLANID,
+		.offset = offsetof(struct flow_keys, tags),
+	},
 };
 
 static const struct flow_dissector_key flow_keys_buf_dissector_keys[] = {
-- 
1.8.1

[PATCH v4 net-next 09/11] net: Add IPv6 flow label to flow_keys

[Tom Herbert]

In flow_dissector set the flow label in flow_keys for IPv6. This also
removes the shortcircuiting of flow dissection when a non-zero label
is present, the flow label can be considered to provide additional
entropy for a hash.

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/net/flow_dissector.h |  4 +++-
 net/core/flow_dissector.c    | 37 +++++++++++++------------------------
 2 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index 08480fb..effe607 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -28,7 +28,8 @@ struct flow_dissector_key_basic {
 };
 
 struct flow_dissector_key_tags {
-	u32	vlan_id:12;
+	u32	vlan_id:12,
+		flow_label:20;
 };
 
 /**
@@ -111,6 +112,7 @@ enum flow_dissector_key_id {
 	FLOW_DISSECTOR_KEY_ETH_ADDRS, /* struct flow_dissector_key_eth_addrs */
 	FLOW_DISSECTOR_KEY_TIPC_ADDRS, /* struct flow_dissector_key_tipc_addrs */
 	FLOW_DISSECTOR_KEY_VLANID, /* struct flow_dissector_key_flow_tags */
+	FLOW_DISSECTOR_KEY_FLOW_LABEL, /* struct flow_dissector_key_flow_label */
 
 	FLOW_DISSECTOR_KEY_MAX,
 };
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 3512366..2fba492 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -190,7 +190,7 @@ ip:
 	case htons(ETH_P_IPV6): {
 		const struct ipv6hdr *iph;
 		struct ipv6hdr _iph;
-		__be32 flow_label;
+		u32 flow_label;
 
 ipv6:
 		iph = __skb_header_pointer(skb, nhoff, sizeof(_iph), data, hlen, &_iph);
@@ -210,30 +210,15 @@ ipv6:
 
 			memcpy(key_ipv6_addrs, &iph->saddr, sizeof(*key_ipv6_addrs));
 			key_control->addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
-			goto flow_label;
 		}
-		break;
-flow_label:
-		flow_label = ip6_flowlabel(iph);
-		if (flow_label) {
-			/* Awesome, IPv6 packet has a flow label so we can
-			 * use that to represent the ports without any
-			 * further dissection.
-			 */
-
-			key_basic->n_proto = proto;
-			key_basic->ip_proto = ip_proto;
-			key_control->thoff = (u16)nhoff;
-
-			if (!skb_flow_dissector_uses_key(flow_dissector,
-							 FLOW_DISSECTOR_KEY_PORTS))
-				break;
-			key_ports = skb_flow_dissector_target(flow_dissector,
-							      FLOW_DISSECTOR_KEY_PORTS,
-							      target_container);
-			key_ports->ports = flow_label;
-
-			return true;
+
+		flow_label = ntohl(ip6_flowlabel(iph));
+		if (flow_label && skb_flow_dissector_uses_key(flow_dissector,
+				FLOW_DISSECTOR_KEY_FLOW_LABEL)) {
+			key_tags = skb_flow_dissector_target(flow_dissector,
+			    FLOW_DISSECTOR_KEY_FLOW_LABEL, target_container);
+
+			key_tags->flow_label = flow_label;
 		}
 
 		break;
@@ -659,6 +644,10 @@ static const struct flow_dissector_key flow_keys_dissector_keys[] = {
 		.key_id = FLOW_DISSECTOR_KEY_VLANID,
 		.offset = offsetof(struct flow_keys, tags),
 	},
+	{
+		.key_id = FLOW_DISSECTOR_KEY_FLOW_LABEL,
+		.offset = offsetof(struct flow_keys, tags),
+	},
 };
 
 static const struct flow_dissector_key flow_keys_buf_dissector_keys[] = {
-- 
1.8.1

[PATCH v4 net-next 10/11] net: Add GRE keyid in flow_keys

[Tom Herbert]

In flow dissector if a GRE header contains a keyid this is saved in the
new keyid field of flow_keys. The GRE keyid is then represented
in the flow hash function input.

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/net/flow_dissector.h |  6 ++++++
 net/core/flow_dissector.c    | 25 ++++++++++++++++++++++++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index effe607..f9a3084 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -32,6 +32,10 @@ struct flow_dissector_key_tags {
 		flow_label:20;
 };
 
+struct flow_dissector_key_keyid {
+	u32	keyid;
+};
+
 /**
  * struct flow_dissector_key_ipv4_addrs:
  * @src: source ip address
@@ -113,6 +117,7 @@ enum flow_dissector_key_id {
 	FLOW_DISSECTOR_KEY_TIPC_ADDRS, /* struct flow_dissector_key_tipc_addrs */
 	FLOW_DISSECTOR_KEY_VLANID, /* struct flow_dissector_key_flow_tags */
 	FLOW_DISSECTOR_KEY_FLOW_LABEL, /* struct flow_dissector_key_flow_label */
+	FLOW_DISSECTOR_KEY_GRE_KEYID, /* struct flow_dissector_key_keyid */
 
 	FLOW_DISSECTOR_KEY_MAX,
 };
@@ -150,6 +155,7 @@ struct flow_keys {
 #define FLOW_KEYS_HASH_START_FIELD basic
 	struct flow_dissector_key_basic basic;
 	struct flow_dissector_key_tags tags;
+	struct flow_dissector_key_keyid keyid;
 	struct flow_dissector_key_ports ports;
 	struct flow_dissector_key_addrs addrs;
 };
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 2fba492..20ce55d 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -127,6 +127,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 	struct flow_dissector_key_addrs *key_addrs;
 	struct flow_dissector_key_ports *key_ports;
 	struct flow_dissector_key_tags *key_tags;
+	struct flow_dissector_key_keyid *key_keyid;
 	u8 ip_proto;
 
 	if (!data) {
@@ -313,8 +314,26 @@ ipv6:
 		nhoff += 4;
 		if (hdr->flags & GRE_CSUM)
 			nhoff += 4;
-		if (hdr->flags & GRE_KEY)
+		if (hdr->flags & GRE_KEY) {
+			const __be32 *keyid;
+			__be32 _keyid;
+
+			keyid = __skb_header_pointer(skb, nhoff, sizeof(_keyid),
+						     data, hlen, &_keyid);
+
+			if (!keyid)
+				return false;
+
+			if (skb_flow_dissector_uses_key(flow_dissector,
+					FLOW_DISSECTOR_KEY_GRE_KEYID)) {
+				key_keyid = skb_flow_dissector_target(
+						flow_dissector,
+						FLOW_DISSECTOR_KEY_GRE_KEYID,
+						target_container);
+				key_keyid->keyid = (__force u32)*keyid;
+			}
 			nhoff += 4;
+		}
 		if (hdr->flags & GRE_SEQ)
 			nhoff += 4;
 		if (proto == htons(ETH_P_TEB)) {
@@ -648,6 +667,10 @@ static const struct flow_dissector_key flow_keys_dissector_keys[] = {
 		.key_id = FLOW_DISSECTOR_KEY_FLOW_LABEL,
 		.offset = offsetof(struct flow_keys, tags),
 	},
+	{
+		.key_id = FLOW_DISSECTOR_KEY_GRE_KEYID,
+		.offset = offsetof(struct flow_keys, keyid),
+	},
 };
 
 static const struct flow_dissector_key flow_keys_buf_dissector_keys[] = {
-- 
1.8.1

[PATCH v4 net-next 11/11] mpls: Add MPLS entropy label in flow_keys

[Tom Herbert]

In flow dissector if an MPLS header contains an entropy label this is
saved in the new keyid field of flow_keys. The entropy label is
then represented in the flow hash function input.

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/net/flow_dissector.h |  1 +
 net/core/flow_dissector.c    | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index f9a3084..65db4eb 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -118,6 +118,7 @@ enum flow_dissector_key_id {
 	FLOW_DISSECTOR_KEY_VLANID, /* struct flow_dissector_key_flow_tags */
 	FLOW_DISSECTOR_KEY_FLOW_LABEL, /* struct flow_dissector_key_flow_label */
 	FLOW_DISSECTOR_KEY_GRE_KEYID, /* struct flow_dissector_key_keyid */
+	FLOW_DISSECTOR_KEY_MPLS_ENTROPY, /* struct flow_dissector_key_keyid */
 
 	FLOW_DISSECTOR_KEY_MAX,
 };
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 20ce55d..da4e895 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -15,6 +15,7 @@
 #include <linux/ppp_defs.h>
 #include <linux/stddef.h>
 #include <linux/if_ether.h>
+#include <linux/mpls.h>
 #include <net/flow_dissector.h>
 #include <scsi/fc/fc_fcoe.h>
 
@@ -286,6 +287,39 @@ ipv6:
 		}
 		return true;
 	}
+
+	case htons(ETH_P_MPLS_UC):
+	case htons(ETH_P_MPLS_MC): {
+		struct mpls_label *hdr, _hdr[2];
+mpls:
+		hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data,
+					   hlen, &_hdr);
+		if (!hdr)
+			return false;
+
+		if ((ntohl(hdr[0].entry) & MPLS_LS_LABEL_MASK) ==
+		     MPLS_LABEL_ENTROPY) {
+			if (skb_flow_dissector_uses_key(
+					flow_dissector,
+					FLOW_DISSECTOR_KEY_MPLS_ENTROPY)) {
+				key_keyid = skb_flow_dissector_target(
+					flow_dissector,
+					FLOW_DISSECTOR_KEY_MPLS_ENTROPY,
+					target_container);
+				key_keyid->keyid = ntohl(hdr[1].entry) &
+					MPLS_LS_LABEL_MASK;
+			}
+
+			key_basic->n_proto = proto;
+			key_basic->ip_proto = ip_proto;
+			key_control->thoff = (u16)nhoff;
+
+			return true;
+		}
+
+		return true;
+	}
+
 	case htons(ETH_P_FCOE):
 		key_control->thoff = (u16)(nhoff + FCOE_HEADER_LEN);
 		/* fall through */
@@ -356,6 +390,9 @@ ipv6:
 	case IPPROTO_IPV6:
 		proto = htons(ETH_P_IPV6);
 		goto ipv6;
+	case IPPROTO_MPLS:
+		proto = htons(ETH_P_MPLS_UC);
+		goto mpls;
 	default:
 		break;
 	}
-- 
1.8.1

Re: [PATCH v4 net-next 05/11] net: Add full IPv6 addresses to flow_keys

[Cong Wang]

On Thu, May 21, 2015 at 5:11 PM, Tom Herbert <tom@herbertland.com> wrote:
>  static inline void ip6_set_txhash(struct sock *sk)
>  {
> @@ -700,8 +714,11 @@ static inline void ip6_set_txhash(struct sock *sk)
>
>         memset(&keys, 0, sizeof(keys));
>
> -       keys.addrs.src = (__force __be32)ipv6_addr_hash(&np->saddr);
> -       keys.addrs.dst = (__force __be32)ipv6_addr_hash(&sk->sk_v6_daddr);
> +       memcpy(&keys.addrs.v6addrs.src, &np->saddr,
> +              sizeof(keys.addrs.v6addrs.src));
> +       memcpy(&keys.addrs.v6addrs.dst, &sk->sk_v6_daddr,
> +              sizeof(keys.addrs.v6addrs.dst));
> +       keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
>         keys.ports.src = inet->inet_sport;
>         keys.ports.dst = inet->inet_dport;
>
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 017fb84..d77619f 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -178,10 +178,12 @@ ip:
>                 if (!skb_flow_dissector_uses_key(flow_dissector,
>                                                  FLOW_DISSECTOR_KEY_IPV4_ADDRS))
>                         break;
> +
>                 key_addrs = skb_flow_dissector_target(flow_dissector,
> -                                                     FLOW_DISSECTOR_KEY_IPV4_ADDRS,
> -                                                     target_container);
> -               memcpy(key_addrs, &iph->saddr, sizeof(*key_addrs));
> +                             FLOW_DISSECTOR_KEY_IPV4_ADDRS, target_container);
> +               memcpy(&key_addrs->v4addrs, &iph->saddr,
> +                      sizeof(key_addrs->v4addrs));
> +               key_control->addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
>                 break;
>         }
>         case htons(ETH_P_IPV6): {
> @@ -203,8 +205,11 @@ ipv6:
>                                                               FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS,
>                                                               target_container);
>
> -                       key_addrs->src = (__force __be32)ipv6_addr_hash(&iph->saddr);
> -                       key_addrs->dst = (__force __be32)ipv6_addr_hash(&iph->daddr);
> +                       key_addrs->v4addrs.src =
> +                               (__force __be32)ipv6_addr_hash(&iph->saddr);
> +                       key_addrs->v4addrs.dst =
> +                               (__force __be32)ipv6_addr_hash(&iph->daddr);
> +                       key_control->addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;


Comparing with the change in ip6_set_txhash(), this one reads odd, not sure
if you do this intentionally?

Re: [PATCH v4 net-next 01/11] net: Simplify GRE case in flow_dissector

[Jiri Pirko]

Fri, May 22, 2015 at 02:11:36AM CEST, tom@herbertland.com wrote:
>Do break when we see routing flag or a non-zero version number in GRE
>header.
>
>Signed-off-by: Tom Herbert <tom@herbertland.com>

Acked-by: Jiri Pirko <jiri@resnulli.us>

Re: [PATCH v4 net-next 02/11] mpls: Add definition for IPPROTO_MPLS

[Jiri Pirko]

Fri, May 22, 2015 at 02:11:37AM CEST, tom@herbertland.com wrote:
>Add uapi define for MPLS over IP.
>
>Signed-off-by: Tom Herbert <tom@herbertland.com>

Acked-by: Jiri Pirko <jiri@resnulli.us>

Re: [PATCH v4 net-next 03/11] net: Remove superfluous setting of key_basic

[Jiri Pirko]

Fri, May 22, 2015 at 02:11:38AM CEST, tom@herbertland.com wrote:
>key_basic is set twice in __skb_flow_dissect which seems unnecessary.
>Remove second one.
>
>Signed-off-by: Tom Herbert <tom@herbertland.com>

I have the same patch in my queue :)

Acked-by: Jiri Pirko <jiri@resnulli.us>

Re: [PATCH v4 net-next 04/11] net: Get skb hash over flow_keys structure

[Jiri Pirko]

Fri, May 22, 2015 at 02:11:39AM CEST, tom@herbertland.com wrote:
>This patch changes flow hashing to use jhash2 over the flow_keys
>structure instead just doing jhash_3words over src, dst, and ports.
>This method will allow us take more input into the hashing function
>so that we can include full IPv6 addresses, VLAN, flow labels etc.
>without needing to resort to xor'ing which makes for a poor hash.
>
>Signed-off-by: Tom Herbert <tom@herbertland.com>

Acked-by: Jiri Pirko <jiri@resnulli.us>

Re: [PATCH v4 net-next 05/11] net: Add full IPv6 addresses to flow_keys

[Jiri Pirko]

Fri, May 22, 2015 at 02:11:40AM CEST, tom@herbertland.com wrote:
>This patch adds full IPv6 addresses into flow_keys and uses them as
>input to the flow hash function. The implementation supports either
>IPv4 or IPv6 addresses in a union, and selector is used to determine
>how may words to input to jhash2.
>
>We also add flow_get_u32_dst and flow_get_u32_src functions which are
>used to get a u32 representation of the source and destination
>addresses. For IPv6, ipv6_addr_hash is called. These functions retain
>getting the legacy values of src and dst in flow_keys.
>
>With this patch, Ethertype and IP protocol are now included in the
>flow hash input.
>
>Signed-off-by: Tom Herbert <tom@herbertland.com>

...

>diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
>index cba6a10..306d461 100644
>--- a/include/net/flow_dissector.h
>+++ b/include/net/flow_dissector.h
>@@ -12,7 +12,7 @@
>  */
> struct flow_dissector_key_control {
> 	u16	thoff;
>-	u16	padding;
>+	u16	addr_type;


I don't understand why this is needed. I don't like this. You assign enum
flow_dissector_key_id to this. Why not just continue to use
key->basic.n_proto?

Re: [PATCH v4 net-next 06/11] net: Add keys for TIPC address

[Jiri Pirko]

Fri, May 22, 2015 at 02:11:41AM CEST, tom@herbertland.com wrote:
>Add a new flow key for TIPC addresses.
>
>Signed-off-by: Tom Herbert <tom@herbertland.com>
>---
> include/net/flow_dissector.h | 10 ++++++++++
> net/core/flow_dissector.c    | 20 ++++++++++++++------
> 2 files changed, 24 insertions(+), 6 deletions(-)
>
>diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
>index 306d461..3ee606a 100644
>--- a/include/net/flow_dissector.h
>+++ b/include/net/flow_dissector.h
>@@ -50,6 +50,14 @@ struct flow_dissector_key_ipv6_addrs {
> };
> 
> /**
>+ * struct flow_dissector_key_tipc_addrs:
>+ * @srcnode: source node address
>+ */
>+struct flow_dissector_key_tipc_addrs {
>+	__be32 srcnode;
>+};
>+
>+/**
>  * struct flow_dissector_key_addrs:
>  * @v4addrs: IPv4 addresses
>  * @v6addrs: IPv6 addresses
>@@ -58,6 +66,7 @@ struct flow_dissector_key_addrs {
> 	union {
> 		struct flow_dissector_key_ipv4_addrs v4addrs;
> 		struct flow_dissector_key_ipv6_addrs v6addrs;
>+		struct flow_dissector_key_tipc_addrs tipcaddrs;
> 	};
> };
> 
>@@ -97,6 +106,7 @@ enum flow_dissector_key_id {
> 	FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS, /* struct flow_dissector_key_addrs */
> 	FLOW_DISSECTOR_KEY_PORTS, /* struct flow_dissector_key_ports */
> 	FLOW_DISSECTOR_KEY_ETH_ADDRS, /* struct flow_dissector_key_eth_addrs */
>+	FLOW_DISSECTOR_KEY_TIPC_ADDRS, /* struct flow_dissector_key_tipc_addrs */
> 
> 	FLOW_DISSECTOR_KEY_MAX,
> };
>diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>index d77619f..8d6f089 100644
>--- a/net/core/flow_dissector.c
>+++ b/net/core/flow_dissector.c
>@@ -294,13 +294,12 @@ flow_label:
> 		key_control->thoff = (u16)nhoff;
> 
> 		if (skb_flow_dissector_uses_key(flow_dissector,
>-						FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS)) {
>+				FLOW_DISSECTOR_KEY_TIPC_ADDRS)) {
> 			key_addrs = skb_flow_dissector_target(flow_dissector,
>-							      FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS,
>-							      target_container);
>-			key_addrs->v4addrs.src = hdr->srcnode;
>-			key_addrs->v4addrs.dst = 0;
>-			key_control->addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
>+				FLOW_DISSECTOR_KEY_TIPC_ADDRS,
>+				target_container);
>+			key_addrs->tipcaddrs.srcnode = hdr->srcnode;
>+			key_control->addr_type = FLOW_DISSECTOR_KEY_TIPC_ADDRS;


This patch looks good to me, leaving the addr_type part I mentioned in my
previous reply aside.

Re: [PATCH v4 net-next 07/11] net: Get rid of IPv6 hash addresses flow keys

[Jiri Pirko]

Fri, May 22, 2015 at 02:11:42AM CEST, tom@herbertland.com wrote:
>We don't need to return the IPv6 address hash as part of flow keys.
>In general, using the IPv6 address hash is risky in a hash value
>since the underlying use of xor provides no entropy. If someone
>really needs the hash value they can get it from the full IPv6
>addresses in flow keys (e.g. from flow_get_u32_src).
>
>Signed-off-by: Tom Herbert <tom@herbertland.com>
>---
> include/net/flow_dissector.h |  1 -
> net/core/flow_dissector.c    | 17 -----------------
> 2 files changed, 18 deletions(-)
>
>diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
>index 3ee606a..59f00f9 100644
>--- a/include/net/flow_dissector.h
>+++ b/include/net/flow_dissector.h
>@@ -103,7 +103,6 @@ enum flow_dissector_key_id {
> 	FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */
> 	FLOW_DISSECTOR_KEY_IPV4_ADDRS, /* struct flow_dissector_key_ipv4_addrs */
> 	FLOW_DISSECTOR_KEY_IPV6_ADDRS, /* struct flow_dissector_key_ipv6_addrs */
>-	FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS, /* struct flow_dissector_key_addrs */
> 	FLOW_DISSECTOR_KEY_PORTS, /* struct flow_dissector_key_ports */
> 	FLOW_DISSECTOR_KEY_ETH_ADDRS, /* struct flow_dissector_key_eth_addrs */
> 	FLOW_DISSECTOR_KEY_TIPC_ADDRS, /* struct flow_dissector_key_tipc_addrs */
>diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>index 8d6f089..44e47c5 100644
>--- a/net/core/flow_dissector.c
>+++ b/net/core/flow_dissector.c
>@@ -200,19 +200,6 @@ ipv6:
> 		nhoff += sizeof(struct ipv6hdr);
> 
> 		if (skb_flow_dissector_uses_key(flow_dissector,
>-						FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS)) {
>-			key_addrs = skb_flow_dissector_target(flow_dissector,
>-							      FLOW_DISSECTOR_KEY_IPV6_HASH_ADDRS,
>-							      target_container);
>-
>-			key_addrs->v4addrs.src =
>-				(__force __be32)ipv6_addr_hash(&iph->saddr);
>-			key_addrs->v4addrs.dst =
>-				(__force __be32)ipv6_addr_hash(&iph->daddr);
>-			key_control->addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
>-			goto flow_label;
>-		}
>-		if (skb_flow_dissector_uses_key(flow_dissector,
> 						FLOW_DISSECTOR_KEY_IPV6_ADDRS)) {
> 			struct flow_dissector_key_ipv6_addrs *key_ipv6_addrs;
> 


You can change the code flow now to pre-b924933cbbfbdcaa2831a39 state:

		if (!skb_flow_dissector_uses_key(flow_dissector,
						 FLOW_DISSECTOR_KEY_IPV6_ADDRS))
			break;
		....
		flow_label = .....

killing flow_label label.

Other than that,

Acked-by: Jiri Pirko <jiri@resnulli.us>

Re: [PATCH v4 net-next 09/11] net: Add IPv6 flow label to flow_keys

[Jiri Pirko]

Fri, May 22, 2015 at 02:11:44AM CEST, tom@herbertland.com wrote:
>In flow_dissector set the flow label in flow_keys for IPv6. This also
>removes the shortcircuiting of flow dissection when a non-zero label
>is present, the flow label can be considered to provide additional
>entropy for a hash.
>
>Signed-off-by: Tom Herbert <tom@herbertland.com>
>---
> include/net/flow_dissector.h |  4 +++-
> net/core/flow_dissector.c    | 37 +++++++++++++------------------------
> 2 files changed, 16 insertions(+), 25 deletions(-)
>
>diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
>index 08480fb..effe607 100644
>--- a/include/net/flow_dissector.h
>+++ b/include/net/flow_dissector.h
>@@ -28,7 +28,8 @@ struct flow_dissector_key_basic {
> };
> 
> struct flow_dissector_key_tags {
>-	u32	vlan_id:12;
>+	u32	vlan_id:12,
>+		flow_label:20;
> };
> 
> /**
>@@ -111,6 +112,7 @@ enum flow_dissector_key_id {
> 	FLOW_DISSECTOR_KEY_ETH_ADDRS, /* struct flow_dissector_key_eth_addrs */
> 	FLOW_DISSECTOR_KEY_TIPC_ADDRS, /* struct flow_dissector_key_tipc_addrs */
> 	FLOW_DISSECTOR_KEY_VLANID, /* struct flow_dissector_key_flow_tags */
>+	FLOW_DISSECTOR_KEY_FLOW_LABEL, /* struct flow_dissector_key_flow_label */


I think it makes sense to pair FLOW_DISSECTOR_KEY_* with
struct flow_dissector_key_*

How about to have FLOW_DISSECTOR_KEY_TAGS istead of VLANID and FLOW_LABEL?

Re: [PATCH v4 net-next 11/11] mpls: Add MPLS entropy label in flow_keys

[Jiri Pirko]

Fri, May 22, 2015 at 02:11:46AM CEST, tom@herbertland.com wrote:
>In flow dissector if an MPLS header contains an entropy label this is
>saved in the new keyid field of flow_keys. The entropy label is
>then represented in the flow hash function input.
>
>Signed-off-by: Tom Herbert <tom@herbertland.com>
>---
> include/net/flow_dissector.h |  1 +
> net/core/flow_dissector.c    | 37 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 38 insertions(+)
>
>diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
>index f9a3084..65db4eb 100644
>--- a/include/net/flow_dissector.h
>+++ b/include/net/flow_dissector.h
>@@ -118,6 +118,7 @@ enum flow_dissector_key_id {
> 	FLOW_DISSECTOR_KEY_VLANID, /* struct flow_dissector_key_flow_tags */
> 	FLOW_DISSECTOR_KEY_FLOW_LABEL, /* struct flow_dissector_key_flow_label */
> 	FLOW_DISSECTOR_KEY_GRE_KEYID, /* struct flow_dissector_key_keyid */
>+	FLOW_DISSECTOR_KEY_MPLS_ENTROPY, /* struct flow_dissector_key_keyid */
> 
> 	FLOW_DISSECTOR_KEY_MAX,
> };
>diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>index 20ce55d..da4e895 100644
>--- a/net/core/flow_dissector.c
>+++ b/net/core/flow_dissector.c
>@@ -15,6 +15,7 @@
> #include <linux/ppp_defs.h>
> #include <linux/stddef.h>
> #include <linux/if_ether.h>
>+#include <linux/mpls.h>
> #include <net/flow_dissector.h>
> #include <scsi/fc/fc_fcoe.h>
> 
>@@ -286,6 +287,39 @@ ipv6:
> 		}
> 		return true;
> 	}
>+
>+	case htons(ETH_P_MPLS_UC):
>+	case htons(ETH_P_MPLS_MC): {
>+		struct mpls_label *hdr, _hdr[2];
>+mpls:
>+		hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data,
>+					   hlen, &_hdr);
>+		if (!hdr)
>+			return false;
>+
>+		if ((ntohl(hdr[0].entry) & MPLS_LS_LABEL_MASK) ==
>+		     MPLS_LABEL_ENTROPY) {
>+			if (skb_flow_dissector_uses_key(
>+					flow_dissector,
>+					FLOW_DISSECTOR_KEY_MPLS_ENTROPY)) {

You have this in your other patches as well. I believe that it not alway
make sense to be strict with 80char/line. In this case, I think that
following format is much better:

			if (skb_flow_dissector_uses_key(flow_dissector,
							FLOW_DISSECTOR_KEY_MPLS_ENTROPY)) {

>+				key_keyid = skb_flow_dissector_target(
>+					flow_dissector,
>+					FLOW_DISSECTOR_KEY_MPLS_ENTROPY,
>+					target_container);

				key_keyid = skb_flow_dissector_target(flow_dissector,
								      FLOW_DISSECTOR_KEY_MPLS_ENTROPY,
								      target_container);


Another thing is that it might make sense to split __skb_flow_dissect
into multiple (inline) functions.

Re: [PATCH v4 net-next 05/11] net: Add full IPv6 addresses to flow_keys

[Tom Herbert]

On Fri, May 22, 2015 at 12:57 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Fri, May 22, 2015 at 02:11:40AM CEST, tom@herbertland.com wrote:
>>This patch adds full IPv6 addresses into flow_keys and uses them as
>>input to the flow hash function. The implementation supports either
>>IPv4 or IPv6 addresses in a union, and selector is used to determine
>>how may words to input to jhash2.
>>
>>We also add flow_get_u32_dst and flow_get_u32_src functions which are
>>used to get a u32 representation of the source and destination
>>addresses. For IPv6, ipv6_addr_hash is called. These functions retain
>>getting the legacy values of src and dst in flow_keys.
>>
>>With this patch, Ethertype and IP protocol are now included in the
>>flow hash input.
>>
>>Signed-off-by: Tom Herbert <tom@herbertland.com>
>
> ...
>
>>diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
>>index cba6a10..306d461 100644
>>--- a/include/net/flow_dissector.h
>>+++ b/include/net/flow_dissector.h
>>@@ -12,7 +12,7 @@
>>  */
>> struct flow_dissector_key_control {
>>       u16     thoff;
>>-      u16     padding;
>>+      u16     addr_type;
>
>
> I don't understand why this is needed. I don't like this. You assign enum
> flow_dissector_key_id to this. Why not just continue to use
> key->basic.n_proto?

The reason we need the address type is in the case that the caller
provides a single union space for the addresses. Caller is just
interested in the last address type written to that space. If we
overload n_proto to be both the protocol and address type there is a
loss of information. For instance, with MPLS we should be able to
return the value (e.g. MPLS) but also return Ethernet addresses-- this
can be done if n_proto is set to ETH_P_MPLS_UC and addr_type is set to
FLOW_DISSECTOR_KEY_ETH_ADDRS (need to add latter). Another point is
that not all address types we could conceive will have an Ethertype,
but all would have a FLOW_DISSECTOR_KEY_*_ADDRS definition.

Re: [PATCH v4 net-next 09/11] net: Add IPv6 flow label to flow_keys

[Tom Herbert]

On Fri, May 22, 2015 at 1:14 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Fri, May 22, 2015 at 02:11:44AM CEST, tom@herbertland.com wrote:
>>In flow_dissector set the flow label in flow_keys for IPv6. This also
>>removes the shortcircuiting of flow dissection when a non-zero label
>>is present, the flow label can be considered to provide additional
>>entropy for a hash.
>>
>>Signed-off-by: Tom Herbert <tom@herbertland.com>
>>---
>> include/net/flow_dissector.h |  4 +++-
>> net/core/flow_dissector.c    | 37 +++++++++++++------------------------
>> 2 files changed, 16 insertions(+), 25 deletions(-)
>>
>>diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
>>index 08480fb..effe607 100644
>>--- a/include/net/flow_dissector.h
>>+++ b/include/net/flow_dissector.h
>>@@ -28,7 +28,8 @@ struct flow_dissector_key_basic {
>> };
>>
>> struct flow_dissector_key_tags {
>>-      u32     vlan_id:12;
>>+      u32     vlan_id:12,
>>+              flow_label:20;
>> };
>>
>> /**
>>@@ -111,6 +112,7 @@ enum flow_dissector_key_id {
>>       FLOW_DISSECTOR_KEY_ETH_ADDRS, /* struct flow_dissector_key_eth_addrs */
>>       FLOW_DISSECTOR_KEY_TIPC_ADDRS, /* struct flow_dissector_key_tipc_addrs */
>>       FLOW_DISSECTOR_KEY_VLANID, /* struct flow_dissector_key_flow_tags */
>>+      FLOW_DISSECTOR_KEY_FLOW_LABEL, /* struct flow_dissector_key_flow_label */
>
>
> I think it makes sense to pair FLOW_DISSECTOR_KEY_* with
> struct flow_dissector_key_*
>
> How about to have FLOW_DISSECTOR_KEY_TAGS istead of VLANID and FLOW_LABEL?
>

I thought about that, but it doesn't really save anything to be less
explicit as we still need a conditional at each occurrence. If someone
is only looking for IPv6 flow label and nothing else they are able to
do that.

Re: [PATCH v4 net-next 09/11] net: Add IPv6 flow label to flow_keys

[Jiri Pirko]

Fri, May 22, 2015 at 05:14:21PM CEST, tom@herbertland.com wrote:
>On Fri, May 22, 2015 at 1:14 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Fri, May 22, 2015 at 02:11:44AM CEST, tom@herbertland.com wrote:
>>>In flow_dissector set the flow label in flow_keys for IPv6. This also
>>>removes the shortcircuiting of flow dissection when a non-zero label
>>>is present, the flow label can be considered to provide additional
>>>entropy for a hash.
>>>
>>>Signed-off-by: Tom Herbert <tom@herbertland.com>
>>>---
>>> include/net/flow_dissector.h |  4 +++-
>>> net/core/flow_dissector.c    | 37 +++++++++++++------------------------
>>> 2 files changed, 16 insertions(+), 25 deletions(-)
>>>
>>>diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
>>>index 08480fb..effe607 100644
>>>--- a/include/net/flow_dissector.h
>>>+++ b/include/net/flow_dissector.h
>>>@@ -28,7 +28,8 @@ struct flow_dissector_key_basic {
>>> };
>>>
>>> struct flow_dissector_key_tags {
>>>-      u32     vlan_id:12;
>>>+      u32     vlan_id:12,
>>>+              flow_label:20;
>>> };
>>>
>>> /**
>>>@@ -111,6 +112,7 @@ enum flow_dissector_key_id {
>>>       FLOW_DISSECTOR_KEY_ETH_ADDRS, /* struct flow_dissector_key_eth_addrs */
>>>       FLOW_DISSECTOR_KEY_TIPC_ADDRS, /* struct flow_dissector_key_tipc_addrs */
>>>       FLOW_DISSECTOR_KEY_VLANID, /* struct flow_dissector_key_flow_tags */
>>>+      FLOW_DISSECTOR_KEY_FLOW_LABEL, /* struct flow_dissector_key_flow_label */
>>
>>
>> I think it makes sense to pair FLOW_DISSECTOR_KEY_* with
>> struct flow_dissector_key_*
>>
>> How about to have FLOW_DISSECTOR_KEY_TAGS istead of VLANID and FLOW_LABEL?
>>
>
>I thought about that, but it doesn't really save anything to be less
>explicit as we still need a conditional at each occurrence. If someone
>is only looking for IPv6 flow label and nothing else they are able to
>do that.

So have 2 structs then.

Re: [PATCH v4 net-next 09/11] net: Add IPv6 flow label to flow_keys

[Tom Herbert]

On Fri, May 22, 2015 at 8:22 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Fri, May 22, 2015 at 05:14:21PM CEST, tom@herbertland.com wrote:
>>On Fri, May 22, 2015 at 1:14 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>>> Fri, May 22, 2015 at 02:11:44AM CEST, tom@herbertland.com wrote:
>>>>In flow_dissector set the flow label in flow_keys for IPv6. This also
>>>>removes the shortcircuiting of flow dissection when a non-zero label
>>>>is present, the flow label can be considered to provide additional
>>>>entropy for a hash.
>>>>
>>>>Signed-off-by: Tom Herbert <tom@herbertland.com>
>>>>---
>>>> include/net/flow_dissector.h |  4 +++-
>>>> net/core/flow_dissector.c    | 37 +++++++++++++------------------------
>>>> 2 files changed, 16 insertions(+), 25 deletions(-)
>>>>
>>>>diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
>>>>index 08480fb..effe607 100644
>>>>--- a/include/net/flow_dissector.h
>>>>+++ b/include/net/flow_dissector.h
>>>>@@ -28,7 +28,8 @@ struct flow_dissector_key_basic {
>>>> };
>>>>
>>>> struct flow_dissector_key_tags {
>>>>-      u32     vlan_id:12;
>>>>+      u32     vlan_id:12,
>>>>+              flow_label:20;
>>>> };
>>>>
>>>> /**
>>>>@@ -111,6 +112,7 @@ enum flow_dissector_key_id {
>>>>       FLOW_DISSECTOR_KEY_ETH_ADDRS, /* struct flow_dissector_key_eth_addrs */
>>>>       FLOW_DISSECTOR_KEY_TIPC_ADDRS, /* struct flow_dissector_key_tipc_addrs */
>>>>       FLOW_DISSECTOR_KEY_VLANID, /* struct flow_dissector_key_flow_tags */
>>>>+      FLOW_DISSECTOR_KEY_FLOW_LABEL, /* struct flow_dissector_key_flow_label */
>>>
>>>
>>> I think it makes sense to pair FLOW_DISSECTOR_KEY_* with
>>> struct flow_dissector_key_*
>>>
>>> How about to have FLOW_DISSECTOR_KEY_TAGS istead of VLANID and FLOW_LABEL?
>>>
>>
>>I thought about that, but it doesn't really save anything to be less
>>explicit as we still need a conditional at each occurrence. If someone
>>is only looking for IPv6 flow label and nothing else they are able to
>>do that.
>
> So have 2 structs then.

We also use this structure in flow_keys and it's convenient to packet
both VLAN and and flow label into single structure in that case. So we
get precisely 24 or 48 bytes in flow_keys from skb_flow_dissector with
the trick os setting keys to same field. The comment for
FLOW_DISSECTOR_KEY_FLOW_LABEL does need to be fixed.

[PATCH v4 net-next 08/11] net: Add VLAN ID to flow_keys

[Tom Herbert]

In flow_dissector set vlan_id in flow_keys when VLAN is found.

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/net/flow_dissector.h |  6 ++++++
 net/core/flow_dissector.c    | 14 ++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index 59f00f9..08480fb 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -27,6 +27,10 @@ struct flow_dissector_key_basic {
 	u8	padding;
 };
 
+struct flow_dissector_key_tags {
+	u32	vlan_id:12;
+};
+
 /**
  * struct flow_dissector_key_ipv4_addrs:
  * @src: source ip address
@@ -106,6 +110,7 @@ enum flow_dissector_key_id {
 	FLOW_DISSECTOR_KEY_PORTS, /* struct flow_dissector_key_ports */
 	FLOW_DISSECTOR_KEY_ETH_ADDRS, /* struct flow_dissector_key_eth_addrs */
 	FLOW_DISSECTOR_KEY_TIPC_ADDRS, /* struct flow_dissector_key_tipc_addrs */
+	FLOW_DISSECTOR_KEY_VLANID, /* struct flow_dissector_key_flow_tags */
 
 	FLOW_DISSECTOR_KEY_MAX,
 };
@@ -142,6 +147,7 @@ struct flow_keys {
 	struct flow_dissector_key_control control;
 #define FLOW_KEYS_HASH_START_FIELD basic
 	struct flow_dissector_key_basic basic;
+	struct flow_dissector_key_tags tags;
 	struct flow_dissector_key_ports ports;
 	struct flow_dissector_key_addrs addrs;
 };
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 5348a46..5c66cb2 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -126,6 +126,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 	struct flow_dissector_key_basic *key_basic;
 	struct flow_dissector_key_addrs *key_addrs;
 	struct flow_dissector_key_ports *key_ports;
+	struct flow_dissector_key_tags *key_tags;
 	u8 ip_proto;
 
 	if (!data) {
@@ -246,6 +247,15 @@ flow_label:
 		if (!vlan)
 			return false;
 
+		if (skb_flow_dissector_uses_key(flow_dissector,
+						FLOW_DISSECTOR_KEY_VLANID)) {
+			key_tags = skb_flow_dissector_target(flow_dissector,
+							     FLOW_DISSECTOR_KEY_VLANID,
+							     target_container);
+
+			key_tags->vlan_id = skb_vlan_tag_get_id(skb);
+		}
+
 		proto = vlan->h_vlan_encapsulated_proto;
 		nhoff += sizeof(*vlan);
 		goto again;
@@ -645,6 +655,10 @@ static const struct flow_dissector_key flow_keys_dissector_keys[] = {
 		.key_id = FLOW_DISSECTOR_KEY_PORTS,
 		.offset = offsetof(struct flow_keys, ports),
 	},
+	{
+		.key_id = FLOW_DISSECTOR_KEY_VLANID,
+		.offset = offsetof(struct flow_keys, tags),
+	},
 };
 
 static const struct flow_dissector_key flow_keys_buf_dissector_keys[] = {
-- 
1.8.1