[PATCH net-next 0/4] net_sched, flow_dissector, flower: Introduce vlan tag support

This patchset introduce vlan tag support to the flower classifier and the flow
dissector. In addition to adding vlan priority to act vlan.

The first 2 patches are dealing with the flow dissector:
 - The first patch is a fix, vlan id value should be taken from skb->vlan_tci
   and not from skb->data.
 - The second patch adds support for vlan priority.

The third patch adds vlan tag support to the flower classifier, user space
patches will be sent later to complete it.
The last patch adds vlan priority to act vlan since only vlan id is currently supported.

Hadar Hen Zion (4):
  flow_dissector: Get vlan info from skb->vlan_tci instead of skb->data
  flow_dissector: Get vlan priority in addition to vlan id
  net_sched: flower: Add vlan support
  net_sched: act_vlan: Add priority option

 include/linux/if_vlan.h             |  1 +
 include/net/flow_dissector.h        | 11 ++++--
 include/net/tc_act/tc_vlan.h        |  1 +
 include/uapi/linux/pkt_cls.h        |  3 ++
 include/uapi/linux/tc_act/tc_vlan.h |  1 +
 net/core/flow_dissector.c           | 28 +++++++--------
 net/sched/act_vlan.c                | 13 +++++--
 net/sched/cls_flower.c              | 69 +++++++++++++++++++++++++++++++++++--
 8 files changed, 103 insertions(+), 24 deletions(-)

-- 
1.8.3.1

Early in the datapath skb_vlan_untag function is called, stripped
the vlan from the skb and set skb->vlan_tci and skb->vlan_proto fields.

The current dissection doesn't handle vlan packets correctly.  Vlan
doesn't exist in skb->data anymore when applying flow dissection on the
skb, fix that.

Fixes: 0744dd00c1b1 ('net: introduce skb_flow_dissect()')
Signed-off-by: Hadar Hen Zion <redacted>
---
 net/core/flow_dissector.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 61ad43f..6060fc2 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c

@@ -122,7 +122,8 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 
 	if (!data) {
 		data = skb->data;
-		proto = skb->protocol;
+		proto = skb_vlan_tag_present(skb) ?
+			 skb->vlan_proto : skb->protocol;
 		nhoff = skb_network_offset(skb);
 		hlen = skb_headlen(skb);
 	}

@@ -240,13 +241,6 @@ ipv6:
 	}
 	case htons(ETH_P_8021AD):
 	case htons(ETH_P_8021Q): {
-		const struct vlan_hdr *vlan;
-		struct vlan_hdr _vlan;
-
-		vlan = __skb_header_pointer(skb, nhoff, sizeof(_vlan), data, hlen, &_vlan);
-		if (!vlan)
-			goto out_bad;
-
 		if (dissector_uses_key(flow_dissector,
 				       FLOW_DISSECTOR_KEY_VLANID)) {
 			key_tags = skb_flow_dissector_target(flow_dissector,

@@ -256,8 +250,7 @@ ipv6:
 			key_tags->vlan_id = skb_vlan_tag_get_id(skb);
 		}
 
-		proto = vlan->h_vlan_encapsulated_proto;
-		nhoff += sizeof(*vlan);
+		proto = skb->protocol;
 		goto again;
 	}
 	case htons(ETH_P_PPP_SES): {

-- 
1.8.3.1

Add vlan priority check to the flow dissector by adding new flow
dissector struct, flow_dissector_key_vlan which includes vlan tag
fields.

vlan_id and flow_label fields were under the same struct
(flow_dissector_key_tags). It was a convenient setting since struct
flow_dissector_key_tags is used by struct flow_keys and by setting
vlan_id and flow_label under the same struct, we get precisely 24 or 48
bytes in flow_keys from flow_dissector_key_basic.

Now, when adding vlan priority support, the code will be cleaner if
flow_label and vlan tag won't be under the same struct anymore.

Signed-off-by: Hadar Hen Zion <redacted>
---
 include/linux/if_vlan.h      |  1 +
 include/net/flow_dissector.h | 11 ++++++++---
 net/core/flow_dissector.c    | 15 +++++++++------
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index a5f6ce6..49d4aef 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h

@@ -81,6 +81,7 @@ static inline bool is_vlan_dev(const struct net_device *dev)
 #define skb_vlan_tag_present(__skb)	((__skb)->vlan_tci & VLAN_TAG_PRESENT)
 #define skb_vlan_tag_get(__skb)		((__skb)->vlan_tci & ~VLAN_TAG_PRESENT)
 #define skb_vlan_tag_get_id(__skb)	((__skb)->vlan_tci & VLAN_VID_MASK)
+#define skb_vlan_tag_get_prio(__skb)	((__skb)->vlan_tci & VLAN_PRIO_MASK)
 
 /**
  *	struct vlan_pcpu_stats - VLAN percpu rx/tx stats

diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index d3d60dc..3781f18 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h

@@ -32,8 +32,12 @@ struct flow_dissector_key_basic {
 };
 
 struct flow_dissector_key_tags {
-	u32	vlan_id:12,
-		flow_label:20;
+	u32	flow_label:20;
+};
+
+struct flow_dissector_key_vlan {
+	u16	vlan_id:12,
+		vlan_priority:3;
 };
 
 struct flow_dissector_key_keyid {

@@ -119,7 +123,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_VLAN, /* struct flow_dissector_key_flow_vlan */
 	FLOW_DISSECTOR_KEY_FLOW_LABEL, /* struct flow_dissector_key_flow_tags */
 	FLOW_DISSECTOR_KEY_GRE_KEYID, /* struct flow_dissector_key_keyid */
 	FLOW_DISSECTOR_KEY_MPLS_ENTROPY, /* struct flow_dissector_key_keyid */

@@ -148,6 +152,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_vlan vlan;
 	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 6060fc2..6dfcb10 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c

@@ -116,6 +116,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_vlan *key_vlan;
 	struct flow_dissector_key_keyid *key_keyid;
 	u8 ip_proto = 0;
 	bool ret = false;

@@ -242,12 +243,14 @@ ipv6:
 	case htons(ETH_P_8021AD):
 	case htons(ETH_P_8021Q): {
 		if (dissector_uses_key(flow_dissector,
-				       FLOW_DISSECTOR_KEY_VLANID)) {
-			key_tags = skb_flow_dissector_target(flow_dissector,
-							     FLOW_DISSECTOR_KEY_VLANID,
+				       FLOW_DISSECTOR_KEY_VLAN)) {
+			key_vlan = skb_flow_dissector_target(flow_dissector,
+							     FLOW_DISSECTOR_KEY_VLAN,
 							     target_container);
 
-			key_tags->vlan_id = skb_vlan_tag_get_id(skb);
+			key_vlan->vlan_id = skb_vlan_tag_get_id(skb);
+			key_vlan->vlan_priority =
+				(skb_vlan_tag_get_prio(skb) >> VLAN_PRIO_SHIFT);
 		}
 
 		proto = skb->protocol;

@@ -865,8 +868,8 @@ static const struct flow_dissector_key flow_keys_dissector_keys[] = {
 		.offset = offsetof(struct flow_keys, ports),
 	},
 	{
-		.key_id = FLOW_DISSECTOR_KEY_VLANID,
-		.offset = offsetof(struct flow_keys, tags),
+		.key_id = FLOW_DISSECTOR_KEY_VLAN,
+		.offset = offsetof(struct flow_keys, vlan),
 	},
 	{
 		.key_id = FLOW_DISSECTOR_KEY_FLOW_LABEL,

-- 
1.8.3.1

Enhance flower to support 802.1Q vlan protocol classification.
Currently, the supported fields are vlan_id and vlan_priority.

Example:

	# add a flower filter with vlan id and priority classification
	tc filter add dev ens4f0 protocol 802.1Q parent ffff: \
		flower \
		indev ens4f0 \
		vlan_ethtype ipv4 \
		vlan_id 100 \
		vlan_prio 3 \
	action vlan pop

Signed-off-by: Hadar Hen Zion <redacted>
---
 include/uapi/linux/pkt_cls.h |  3 ++
 net/sched/cls_flower.c       | 69 ++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index d1c1cca..51b5b24 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h

@@ -428,6 +428,9 @@ enum {
 	TCA_FLOWER_KEY_UDP_DST,		/* be16 */
 
 	TCA_FLOWER_FLAGS,
+	TCA_FLOWER_KEY_VLAN_ID,
+	TCA_FLOWER_KEY_VLAN_PRIO,
+	TCA_FLOWER_KEY_VLAN_ETH_TYPE,
 	__TCA_FLOWER_MAX,
 };
 

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 5060801..4e249be 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c

@@ -28,6 +28,7 @@ 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_vlan vlan;
 	struct flow_dissector_key_addrs ipaddrs;
 	union {
 		struct flow_dissector_key_ipv4_addrs ipv4;

@@ -293,6 +294,10 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
 	[TCA_FLOWER_KEY_TCP_DST]	= { .type = NLA_U16 },
 	[TCA_FLOWER_KEY_UDP_SRC]	= { .type = NLA_U16 },
 	[TCA_FLOWER_KEY_UDP_DST]	= { .type = NLA_U16 },
+	[TCA_FLOWER_KEY_VLAN_ID]	= { .type = NLA_U16 },
+	[TCA_FLOWER_KEY_VLAN_PRIO]	= { .type = NLA_U8 },
+	[TCA_FLOWER_KEY_VLAN_ETH_TYPE]	= { .type = NLA_U16 },
+
 };
 
 static void fl_set_key_val(struct nlattr **tb,

@@ -308,9 +313,28 @@ static void fl_set_key_val(struct nlattr **tb,
 		memcpy(mask, nla_data(tb[mask_type]), len);
 }
 
+static void fl_set_key_vlan(struct nlattr **tb,
+			    struct flow_dissector_key_vlan *key_val,
+			    struct flow_dissector_key_vlan *key_mask)
+{
+#define VLAN_PRIORITY_MASK	0x7
+
+	if (tb[TCA_FLOWER_KEY_VLAN_ID]) {
+		key_val->vlan_id =
+			nla_get_u16(tb[TCA_FLOWER_KEY_VLAN_ID]) & VLAN_VID_MASK;
+		key_mask->vlan_id = VLAN_VID_MASK;
+	}
+	if (tb[TCA_FLOWER_KEY_VLAN_PRIO]) {
+		key_val->vlan_priority =
+			nla_get_u8(tb[TCA_FLOWER_KEY_VLAN_PRIO]) & VLAN_PRIORITY_MASK;
+		key_mask->vlan_priority = VLAN_PRIORITY_MASK;
+	}
+}
+
 static int fl_set_key(struct net *net, struct nlattr **tb,
 		      struct fl_flow_key *key, struct fl_flow_key *mask)
 {
+	__be16 ethertype;
 #ifdef CONFIG_NET_CLS_IND
 	if (tb[TCA_FLOWER_INDEV]) {
 		int err = tcf_change_indev(net, tb[TCA_FLOWER_INDEV]);

@@ -328,9 +352,19 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
 		       mask->eth.src, TCA_FLOWER_KEY_ETH_SRC_MASK,
 		       sizeof(key->eth.src));
 
-	fl_set_key_val(tb, &key->basic.n_proto, TCA_FLOWER_KEY_ETH_TYPE,
-		       &mask->basic.n_proto, TCA_FLOWER_UNSPEC,
-		       sizeof(key->basic.n_proto));
+	if (tb[TCA_FLOWER_KEY_ETH_TYPE])
+		ethertype = nla_get_be16(tb[TCA_FLOWER_KEY_ETH_TYPE]);
+
+	if (ethertype == htons(ETH_P_8021Q)) {
+		fl_set_key_vlan(tb, &key->vlan, &mask->vlan);
+		fl_set_key_val(tb, &key->basic.n_proto,
+			       TCA_FLOWER_KEY_VLAN_ETH_TYPE,
+			       &mask->basic.n_proto, TCA_FLOWER_UNSPEC,
+			       sizeof(key->basic.n_proto));
+	} else {
+		key->basic.n_proto = ethertype;
+		mask->basic.n_proto = cpu_to_be16(~0);
+	}
 
 	if (key->basic.n_proto == htons(ETH_P_IP) ||
 	    key->basic.n_proto == htons(ETH_P_IPV6)) {

@@ -440,6 +474,8 @@ static void fl_init_dissector(struct cls_fl_head *head,
 			       FLOW_DISSECTOR_KEY_IPV6_ADDRS, ipv6);
 	FL_KEY_SET_IF_IN_RANGE(mask, keys, cnt,
 			       FLOW_DISSECTOR_KEY_PORTS, tp);
+	FL_KEY_SET_IF_IN_RANGE(mask, keys, cnt,
+			       FLOW_DISSECTOR_KEY_VLAN, vlan);
 
 	skb_flow_dissector_init(&head->dissector, keys, cnt);
 }

@@ -668,6 +704,29 @@ static int fl_dump_key_val(struct sk_buff *skb,
 	return 0;
 }
 
+static int fl_dump_key_vlan(struct sk_buff *skb,
+			    struct flow_dissector_key_vlan *vlan_key,
+			    struct flow_dissector_key_vlan *vlan_mask)
+{
+	int err;
+
+	if (!memchr_inv(vlan_mask, 0, sizeof(*vlan_mask)))
+		return 0;
+	if (vlan_mask->vlan_id) {
+		err = nla_put_u16(skb, TCA_FLOWER_KEY_VLAN_ID,
+				  vlan_key->vlan_id);
+		if (err)
+			return err;
+	}
+	if (vlan_mask->vlan_priority) {
+		err = nla_put_u8(skb, TCA_FLOWER_KEY_VLAN_PRIO,
+				 vlan_key->vlan_priority);
+		if (err)
+			return err;
+	}
+	return 0;
+}
+
 static int fl_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 		   struct sk_buff *skb, struct tcmsg *t)
 {

@@ -712,6 +771,10 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 			    &mask->basic.n_proto, TCA_FLOWER_UNSPEC,
 			    sizeof(key->basic.n_proto)))
 		goto nla_put_failure;
+
+	if (fl_dump_key_vlan(skb, &key->vlan, &mask->vlan))
+		goto nla_put_failure;
+
 	if ((key->basic.n_proto == htons(ETH_P_IP) ||
 	     key->basic.n_proto == htons(ETH_P_IPV6)) &&
 	    fl_dump_key_val(skb, &key->basic.ip_proto, TCA_FLOWER_KEY_IP_PROTO,

-- 
1.8.3.1

The current vlan push action supports only vid and protocol options.
Add priority option.

Example script that adds vlan push action with vid and
priority:

tc filter add dev veth0 protocol ip parent ffff: \
	   flower \
	   	indev veth0 \
	   action vlan push id 100 priority 5

Signed-off-by: Hadar Hen Zion <redacted>
---
 include/net/tc_act/tc_vlan.h        |  1 +
 include/uapi/linux/tc_act/tc_vlan.h |  1 +
 net/sched/act_vlan.c                | 13 +++++++++++--
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/include/net/tc_act/tc_vlan.h b/include/net/tc_act/tc_vlan.h
index e29f52e..6b83588 100644
--- a/include/net/tc_act/tc_vlan.h
+++ b/include/net/tc_act/tc_vlan.h

@@ -20,6 +20,7 @@ struct tcf_vlan {
 	int			tcfv_action;
 	u16			tcfv_push_vid;
 	__be16			tcfv_push_proto;
+	u8			tcfv_push_prio;
 };
 #define to_vlan(a) ((struct tcf_vlan *)a)
 

diff --git a/include/uapi/linux/tc_act/tc_vlan.h b/include/uapi/linux/tc_act/tc_vlan.h
index 31151ff62..be72b6e 100644
--- a/include/uapi/linux/tc_act/tc_vlan.h
+++ b/include/uapi/linux/tc_act/tc_vlan.h

@@ -29,6 +29,7 @@ enum {
 	TCA_VLAN_PUSH_VLAN_ID,
 	TCA_VLAN_PUSH_VLAN_PROTOCOL,
 	TCA_VLAN_PAD,
+	TCA_VLAN_PUSH_VLAN_PRIORITY,
 	__TCA_VLAN_MAX,
 };
 #define TCA_VLAN_MAX (__TCA_VLAN_MAX - 1)

diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 691409d..d2fcf7cd 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c

@@ -43,7 +43,8 @@ static int tcf_vlan(struct sk_buff *skb, const struct tc_action *a,
 			goto drop;
 		break;
 	case TCA_VLAN_ACT_PUSH:
-		err = skb_vlan_push(skb, v->tcfv_push_proto, v->tcfv_push_vid);
+		err = skb_vlan_push(skb, v->tcfv_push_proto, v->tcfv_push_vid |
+				    (v->tcfv_push_prio << VLAN_PRIO_SHIFT));
 		if (err)
 			goto drop;
 		break;

@@ -65,6 +66,7 @@ static const struct nla_policy vlan_policy[TCA_VLAN_MAX + 1] = {
 	[TCA_VLAN_PARMS]		= { .len = sizeof(struct tc_vlan) },
 	[TCA_VLAN_PUSH_VLAN_ID]		= { .type = NLA_U16 },
 	[TCA_VLAN_PUSH_VLAN_PROTOCOL]	= { .type = NLA_U16 },
+	[TCA_VLAN_PUSH_VLAN_PRIORITY]	= { .type = NLA_U8 },
 };
 
 static int tcf_vlan_init(struct net *net, struct nlattr *nla,

@@ -78,6 +80,7 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
 	int action;
 	__be16 push_vid = 0;
 	__be16 push_proto = 0;
+	u8 push_prio = 0;
 	bool exists = false;
 	int ret = 0, err;
 

@@ -123,6 +126,9 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
 		} else {
 			push_proto = htons(ETH_P_8021Q);
 		}
+
+		if (tb[TCA_VLAN_PUSH_VLAN_PRIORITY])
+			push_prio = nla_get_u8(tb[TCA_VLAN_PUSH_VLAN_PRIORITY]);
 		break;
 	default:
 		if (exists)

@@ -150,6 +156,7 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
 
 	v->tcfv_action = action;
 	v->tcfv_push_vid = push_vid;
+	v->tcfv_push_prio = push_prio;
 	v->tcfv_push_proto = push_proto;
 
 	v->tcf_action = parm->action;

@@ -181,7 +188,9 @@ static int tcf_vlan_dump(struct sk_buff *skb, struct tc_action *a,
 	if (v->tcfv_action == TCA_VLAN_ACT_PUSH &&
 	    (nla_put_u16(skb, TCA_VLAN_PUSH_VLAN_ID, v->tcfv_push_vid) ||
 	     nla_put_be16(skb, TCA_VLAN_PUSH_VLAN_PROTOCOL,
-			  v->tcfv_push_proto)))
+			  v->tcfv_push_proto) ||
+	     (v->tcfv_push_prio && nla_put_u8(skb, TCA_VLAN_PUSH_VLAN_PRIORITY,
+					      v->tcfv_push_prio))))
 		goto nla_put_failure;
 
 	tcf_tm_dump(&t, &v->tcf_tm);

-- 
1.8.3.1

Hi Hadar,

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

url:    https://github.com/0day-ci/linux/commits/Hadar-Hen-Zion/flow_dissector-Get-vlan-info-from-skb-vlan_tci-instead-of-skb-data/20160811-042500
config: m68k-sun3_defconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 4.9.0
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=m68k 

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/linkage.h:4:0,
                    from include/linux/kernel.h:6,
                    from net/core/flow_dissector.c:1:
   In function 'flow_keys_hash_length.isra.3',
       inlined from 'flow_hash_from_keys' at net/core/flow_dissector.c:599:9:

quoted

include/linux/compiler.h:491:38: error: call to '__compiletime_assert_512' declared with attribute error: BUILD_BUG_ON failed: (sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) % sizeof(u32)

     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
                                         ^
   include/linux/compiler.h:474:4: note: in definition of macro '__compiletime_assert'
       prefix ## suffix();    \
       ^
   include/linux/compiler.h:491:2: note: in expansion of macro '_compiletime_assert'
     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
     ^
   include/linux/bug.h:51:37: note: in expansion of macro 'compiletime_assert'
    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                        ^
   include/linux/bug.h:75:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
     ^

quoted

net/core/flow_dissector.c:512:2: note: in expansion of macro 'BUILD_BUG_ON'

     BUILD_BUG_ON((sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) % sizeof(u32));
     ^
   In function 'flow_keys_hash_length.isra.3',
       inlined from '__skb_get_hash' at net/core/flow_dissector.c:599:9:

quoted

include/linux/compiler.h:491:38: error: call to '__compiletime_assert_512' declared with attribute error: BUILD_BUG_ON failed: (sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) % sizeof(u32)

     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
                                         ^
   include/linux/compiler.h:474:4: note: in definition of macro '__compiletime_assert'
       prefix ## suffix();    \
       ^
   include/linux/compiler.h:491:2: note: in expansion of macro '_compiletime_assert'
     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
     ^
   include/linux/bug.h:51:37: note: in expansion of macro 'compiletime_assert'
    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                        ^
   include/linux/bug.h:75:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
     ^

quoted

net/core/flow_dissector.c:512:2: note: in expansion of macro 'BUILD_BUG_ON'

     BUILD_BUG_ON((sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) % sizeof(u32));
     ^
   In function 'flow_keys_hash_length.isra.3',
       inlined from 'skb_get_hash_perturb' at net/core/flow_dissector.c:599:9:

quoted

include/linux/compiler.h:491:38: error: call to '__compiletime_assert_512' declared with attribute error: BUILD_BUG_ON failed: (sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) % sizeof(u32)

     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
                                         ^
   include/linux/compiler.h:474:4: note: in definition of macro '__compiletime_assert'
       prefix ## suffix();    \
       ^
   include/linux/compiler.h:491:2: note: in expansion of macro '_compiletime_assert'
     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
     ^
   include/linux/bug.h:51:37: note: in expansion of macro 'compiletime_assert'
    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                        ^
   include/linux/bug.h:75:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
     ^

quoted

net/core/flow_dissector.c:512:2: note: in expansion of macro 'BUILD_BUG_ON'

     BUILD_BUG_ON((sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) % sizeof(u32));
     ^
   In function 'flow_keys_hash_length.isra.3',
       inlined from '__skb_get_hash_symmetric' at net/core/flow_dissector.c:599:9:

quoted

include/linux/compiler.h:491:38: error: call to '__compiletime_assert_512' declared with attribute error: BUILD_BUG_ON failed: (sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) % sizeof(u32)

     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
                                         ^
   include/linux/compiler.h:474:4: note: in definition of macro '__compiletime_assert'
       prefix ## suffix();    \
       ^
   include/linux/compiler.h:491:2: note: in expansion of macro '_compiletime_assert'
     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
     ^
   include/linux/bug.h:51:37: note: in expansion of macro 'compiletime_assert'
    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                        ^
   include/linux/bug.h:75:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
     ^

quoted

net/core/flow_dissector.c:512:2: note: in expansion of macro 'BUILD_BUG_ON'

     BUILD_BUG_ON((sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) % sizeof(u32));
     ^

vim +/BUILD_BUG_ON +512 net/core/flow_dissector.c

20a17bf6 David S. Miller      2015-09-01  496  					     u32 keyval)
42aecaa9 Tom Herbert          2015-06-04  497  {
42aecaa9 Tom Herbert          2015-06-04  498  	return jhash2(words, length, keyval);
42aecaa9 Tom Herbert          2015-06-04  499  }
42aecaa9 Tom Herbert          2015-06-04  500  
20a17bf6 David S. Miller      2015-09-01  501  static inline const u32 *flow_keys_hash_start(const struct flow_keys *flow)
66415cf8 Hannes Frederic Sowa 2013-10-23  502  {
20a17bf6 David S. Miller      2015-09-01  503  	const void *p = flow;
20a17bf6 David S. Miller      2015-09-01  504  
42aecaa9 Tom Herbert          2015-06-04  505  	BUILD_BUG_ON(FLOW_KEYS_HASH_OFFSET % sizeof(u32));
20a17bf6 David S. Miller      2015-09-01  506  	return (const u32 *)(p + FLOW_KEYS_HASH_OFFSET);
42aecaa9 Tom Herbert          2015-06-04  507  }
42aecaa9 Tom Herbert          2015-06-04  508  
20a17bf6 David S. Miller      2015-09-01  509  static inline size_t flow_keys_hash_length(const struct flow_keys *flow)
42aecaa9 Tom Herbert          2015-06-04  510  {
c3f83241 Tom Herbert          2015-06-04  511  	size_t diff = FLOW_KEYS_HASH_OFFSET + sizeof(flow->addrs);
42aecaa9 Tom Herbert          2015-06-04 @512  	BUILD_BUG_ON((sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) % sizeof(u32));
c3f83241 Tom Herbert          2015-06-04  513  	BUILD_BUG_ON(offsetof(typeof(*flow), addrs) !=
c3f83241 Tom Herbert          2015-06-04  514  		     sizeof(*flow) - sizeof(flow->addrs));
c3f83241 Tom Herbert          2015-06-04  515  
c3f83241 Tom Herbert          2015-06-04  516  	switch (flow->control.addr_type) {
c3f83241 Tom Herbert          2015-06-04  517  	case FLOW_DISSECTOR_KEY_IPV4_ADDRS:
c3f83241 Tom Herbert          2015-06-04  518  		diff -= sizeof(flow->addrs.v4addrs);
c3f83241 Tom Herbert          2015-06-04  519  		break;
c3f83241 Tom Herbert          2015-06-04  520  	case FLOW_DISSECTOR_KEY_IPV6_ADDRS:

:::::: The code at line 512 was first introduced by commit
:::::: 42aecaa9bb2bd57eb8d61b4565cee5d3640863fb net: Get skb hash over flow_keys structure

:::::: TO: Tom Herbert [off-list ref]
:::::: CC: David S. Miller [off-list ref]

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

Hi Hadar,

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

url:    https://github.com/0day-ci/linux/commits/Hadar-Hen-Zion/flow_dissector-Get-vlan-info-from-skb-vlan_tci-instead-of-skb-data/20160811-042500
config: cris-etrax-100lx_v2_defconfig (attached as .config)
compiler: cris-linux-gcc (GCC) 4.6.3
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=cris 

All errors (new ones prefixed by >>):

   In function 'flow_keys_hash_length.isra.6',
       inlined from '__flow_hash_from_keys' at net/core/flow_dissector.c:599:26,
       inlined from 'flow_hash_from_keys' at net/core/flow_dissector.c:610:2:

quoted

net/core/flow_dissector.c:512:2: error: call to '__compiletime_assert_512' declared with attribute error: BUILD_BUG_ON failed: (sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) % sizeof(u32)

   In function 'flow_keys_hash_length.isra.6',
       inlined from '__flow_hash_from_keys' at net/core/flow_dissector.c:599:26,
       inlined from '__skb_get_hash_symmetric' at net/core/flow_dissector.c:663:2:

quoted

net/core/flow_dissector.c:512:2: error: call to '__compiletime_assert_512' declared with attribute error: BUILD_BUG_ON failed: (sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) % sizeof(u32)

   In function 'flow_keys_hash_length.isra.6',
       inlined from '__flow_hash_from_keys' at net/core/flow_dissector.c:599:26,
       inlined from '__skb_get_hash' at net/core/flow_dissector.c:620:2:

quoted

net/core/flow_dissector.c:512:2: error: call to '__compiletime_assert_512' declared with attribute error: BUILD_BUG_ON failed: (sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) % sizeof(u32)

   In function 'flow_keys_hash_length.isra.6',
       inlined from '__flow_hash_from_keys' at net/core/flow_dissector.c:599:26,
       inlined from 'skb_get_hash_perturb' at net/core/flow_dissector.c:620:2:

quoted

net/core/flow_dissector.c:512:2: error: call to '__compiletime_assert_512' declared with attribute error: BUILD_BUG_ON failed: (sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) % sizeof(u32)

vim +/__compiletime_assert_512 +512 net/core/flow_dissector.c

20a17bf6 David S. Miller      2015-09-01  506  	return (const u32 *)(p + FLOW_KEYS_HASH_OFFSET);
42aecaa9 Tom Herbert          2015-06-04  507  }
42aecaa9 Tom Herbert          2015-06-04  508  
20a17bf6 David S. Miller      2015-09-01  509  static inline size_t flow_keys_hash_length(const struct flow_keys *flow)
42aecaa9 Tom Herbert          2015-06-04  510  {
c3f83241 Tom Herbert          2015-06-04  511  	size_t diff = FLOW_KEYS_HASH_OFFSET + sizeof(flow->addrs);
42aecaa9 Tom Herbert          2015-06-04 @512  	BUILD_BUG_ON((sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) % sizeof(u32));
c3f83241 Tom Herbert          2015-06-04  513  	BUILD_BUG_ON(offsetof(typeof(*flow), addrs) !=
c3f83241 Tom Herbert          2015-06-04  514  		     sizeof(*flow) - sizeof(flow->addrs));
c3f83241 Tom Herbert          2015-06-04  515  
c3f83241 Tom Herbert          2015-06-04  516  	switch (flow->control.addr_type) {
c3f83241 Tom Herbert          2015-06-04  517  	case FLOW_DISSECTOR_KEY_IPV4_ADDRS:
c3f83241 Tom Herbert          2015-06-04  518  		diff -= sizeof(flow->addrs.v4addrs);
c3f83241 Tom Herbert          2015-06-04  519  		break;
c3f83241 Tom Herbert          2015-06-04  520  	case FLOW_DISSECTOR_KEY_IPV6_ADDRS:
c3f83241 Tom Herbert          2015-06-04  521  		diff -= sizeof(flow->addrs.v6addrs);
c3f83241 Tom Herbert          2015-06-04  522  		break;
9f249089 Tom Herbert          2015-06-04  523  	case FLOW_DISSECTOR_KEY_TIPC_ADDRS:
9f249089 Tom Herbert          2015-06-04  524  		diff -= sizeof(flow->addrs.tipcaddrs);
9f249089 Tom Herbert          2015-06-04  525  		break;
c3f83241 Tom Herbert          2015-06-04  526  	}
c3f83241 Tom Herbert          2015-06-04  527  	return (sizeof(*flow) - diff) / sizeof(u32);
66415cf8 Hannes Frederic Sowa 2013-10-23  528  }
66415cf8 Hannes Frederic Sowa 2013-10-23  529  
c3f83241 Tom Herbert          2015-06-04  530  __be32 flow_get_u32_src(const struct flow_keys *flow)
5ed20a68 Tom Herbert          2014-07-01  531  {
c3f83241 Tom Herbert          2015-06-04  532  	switch (flow->control.addr_type) {
c3f83241 Tom Herbert          2015-06-04  533  	case FLOW_DISSECTOR_KEY_IPV4_ADDRS:
c3f83241 Tom Herbert          2015-06-04  534  		return flow->addrs.v4addrs.src;
c3f83241 Tom Herbert          2015-06-04  535  	case FLOW_DISSECTOR_KEY_IPV6_ADDRS:
c3f83241 Tom Herbert          2015-06-04  536  		return (__force __be32)ipv6_addr_hash(
c3f83241 Tom Herbert          2015-06-04  537  			&flow->addrs.v6addrs.src);
9f249089 Tom Herbert          2015-06-04  538  	case FLOW_DISSECTOR_KEY_TIPC_ADDRS:
9f249089 Tom Herbert          2015-06-04  539  		return flow->addrs.tipcaddrs.srcnode;
c3f83241 Tom Herbert          2015-06-04  540  	default:
c3f83241 Tom Herbert          2015-06-04  541  		return 0;
c3f83241 Tom Herbert          2015-06-04  542  	}
c3f83241 Tom Herbert          2015-06-04  543  }
c3f83241 Tom Herbert          2015-06-04  544  EXPORT_SYMBOL(flow_get_u32_src);
c3f83241 Tom Herbert          2015-06-04  545  
c3f83241 Tom Herbert          2015-06-04  546  __be32 flow_get_u32_dst(const struct flow_keys *flow)
c3f83241 Tom Herbert          2015-06-04  547  {
c3f83241 Tom Herbert          2015-06-04  548  	switch (flow->control.addr_type) {
c3f83241 Tom Herbert          2015-06-04  549  	case FLOW_DISSECTOR_KEY_IPV4_ADDRS:
c3f83241 Tom Herbert          2015-06-04  550  		return flow->addrs.v4addrs.dst;
c3f83241 Tom Herbert          2015-06-04  551  	case FLOW_DISSECTOR_KEY_IPV6_ADDRS:
c3f83241 Tom Herbert          2015-06-04  552  		return (__force __be32)ipv6_addr_hash(
c3f83241 Tom Herbert          2015-06-04  553  			&flow->addrs.v6addrs.dst);
c3f83241 Tom Herbert          2015-06-04  554  	default:
c3f83241 Tom Herbert          2015-06-04  555  		return 0;
c3f83241 Tom Herbert          2015-06-04  556  	}
c3f83241 Tom Herbert          2015-06-04  557  }
c3f83241 Tom Herbert          2015-06-04  558  EXPORT_SYMBOL(flow_get_u32_dst);
5ed20a68 Tom Herbert          2014-07-01  559  
c3f83241 Tom Herbert          2015-06-04  560  static inline void __flow_hash_consistentify(struct flow_keys *keys)
c3f83241 Tom Herbert          2015-06-04  561  {
c3f83241 Tom Herbert          2015-06-04  562  	int addr_diff, i;
c3f83241 Tom Herbert          2015-06-04  563  
c3f83241 Tom Herbert          2015-06-04  564  	switch (keys->control.addr_type) {
c3f83241 Tom Herbert          2015-06-04  565  	case FLOW_DISSECTOR_KEY_IPV4_ADDRS:
c3f83241 Tom Herbert          2015-06-04  566  		addr_diff = (__force u32)keys->addrs.v4addrs.dst -
c3f83241 Tom Herbert          2015-06-04  567  			    (__force u32)keys->addrs.v4addrs.src;
c3f83241 Tom Herbert          2015-06-04  568  		if ((addr_diff < 0) ||
c3f83241 Tom Herbert          2015-06-04  569  		    (addr_diff == 0 &&
c3f83241 Tom Herbert          2015-06-04  570  		     ((__force u16)keys->ports.dst <
c3f83241 Tom Herbert          2015-06-04  571  		      (__force u16)keys->ports.src))) {
c3f83241 Tom Herbert          2015-06-04  572  			swap(keys->addrs.v4addrs.src, keys->addrs.v4addrs.dst);
c3f83241 Tom Herbert          2015-06-04  573  			swap(keys->ports.src, keys->ports.dst);
c3f83241 Tom Herbert          2015-06-04  574  		}
c3f83241 Tom Herbert          2015-06-04  575  		break;
c3f83241 Tom Herbert          2015-06-04  576  	case FLOW_DISSECTOR_KEY_IPV6_ADDRS:
c3f83241 Tom Herbert          2015-06-04  577  		addr_diff = memcmp(&keys->addrs.v6addrs.dst,
c3f83241 Tom Herbert          2015-06-04  578  				   &keys->addrs.v6addrs.src,
c3f83241 Tom Herbert          2015-06-04  579  				   sizeof(keys->addrs.v6addrs.dst));
c3f83241 Tom Herbert          2015-06-04  580  		if ((addr_diff < 0) ||
c3f83241 Tom Herbert          2015-06-04  581  		    (addr_diff == 0 &&
c3f83241 Tom Herbert          2015-06-04  582  		     ((__force u16)keys->ports.dst <
c3f83241 Tom Herbert          2015-06-04  583  		      (__force u16)keys->ports.src))) {
c3f83241 Tom Herbert          2015-06-04  584  			for (i = 0; i < 4; i++)
c3f83241 Tom Herbert          2015-06-04  585  				swap(keys->addrs.v6addrs.src.s6_addr32[i],
c3f83241 Tom Herbert          2015-06-04  586  				     keys->addrs.v6addrs.dst.s6_addr32[i]);
59346afe Jiri Pirko           2015-05-12  587  			swap(keys->ports.src, keys->ports.dst);
5ed20a68 Tom Herbert          2014-07-01  588  		}
c3f83241 Tom Herbert          2015-06-04  589  		break;
c3f83241 Tom Herbert          2015-06-04  590  	}
c3f83241 Tom Herbert          2015-06-04  591  }
c3f83241 Tom Herbert          2015-06-04  592  
c3f83241 Tom Herbert          2015-06-04  593  static inline u32 __flow_hash_from_keys(struct flow_keys *keys, u32 keyval)
c3f83241 Tom Herbert          2015-06-04  594  {
c3f83241 Tom Herbert          2015-06-04  595  	u32 hash;
c3f83241 Tom Herbert          2015-06-04  596  
c3f83241 Tom Herbert          2015-06-04  597  	__flow_hash_consistentify(keys);
5ed20a68 Tom Herbert          2014-07-01  598  
20a17bf6 David S. Miller      2015-09-01 @599  	hash = __flow_hash_words(flow_keys_hash_start(keys),
42aecaa9 Tom Herbert          2015-06-04  600  				 flow_keys_hash_length(keys), keyval);
5ed20a68 Tom Herbert          2014-07-01  601  	if (!hash)
5ed20a68 Tom Herbert          2014-07-01  602  		hash = 1;

:::::: The code at line 512 was first introduced by commit
:::::: 42aecaa9bb2bd57eb8d61b4565cee5d3640863fb net: Get skb hash over flow_keys structure

:::::: TO: Tom Herbert [off-list ref]
:::::: CC: David S. Miller [off-list ref]

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

On Thu, Aug 11, 2016 at 12:58 AM, kbuild test robot [off-list ref] wrote:

Hi Hadar,

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

url:    https://github.com/0day-ci/linux/commits/Hadar-Hen-Zion/flow_dissector-Get-vlan-info-from-skb-vlan_tci-instead-of-skb-data/20160811-042500
config: cris-etrax-100lx_v2_defconfig (attached as .config)
compiler: cris-linux-gcc (GCC) 4.6.3
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=cris

All errors (new ones prefixed by >>):

   In function 'flow_keys_hash_length.isra.6',
       inlined from '__flow_hash_from_keys' at net/core/flow_dissector.c:599:26,
       inlined from 'flow_hash_from_keys' at net/core/flow_dissector.c:610:2:

quoted

quoted

net/core/flow_dissector.c:512:2: error: call to '__compiletime_assert_512' declared with attribute error: BUILD_BUG_ON failed: (sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) % sizeof(u32)

[...]


I'm working on a fix, will send it soon.

I'll be happy to get your review and comments on my flow_dissector and
flower patches.

Thank you,

Hadar

[...]

Wed, Aug 10, 2016 at 03:32:22PM CEST, hadarh@mellanox.com wrote:

Enhance flower to support 802.1Q vlan protocol classification.
Currently, the supported fields are vlan_id and vlan_priority.

Example:

# add a flower filter with vlan id and priority classification
tc filter add dev ens4f0 protocol 802.1Q parent ffff: \
	flower \
	indev ens4f0 \
	vlan_ethtype ipv4 \
	vlan_id 100 \
	vlan_prio 3 \
action vlan pop

Signed-off-by: Hadar Hen Zion <redacted>

Acked-by: Jiri Pirko <redacted>

Wed, Aug 10, 2016 at 03:32:23PM CEST, hadarh@mellanox.com wrote:

The current vlan push action supports only vid and protocol options.
Add priority option.

Example script that adds vlan push action with vid and
priority:

tc filter add dev veth0 protocol ip parent ffff: \
   flower \
   	indev veth0 \
   action vlan push id 100 priority 5

Signed-off-by: Hadar Hen Zion <redacted>

Acked-by: Jiri Pirko <redacted>

Wed, Aug 10, 2016 at 03:32:20PM CEST, hadarh@mellanox.com wrote:

Early in the datapath skb_vlan_untag function is called, stripped
the vlan from the skb and set skb->vlan_tci and skb->vlan_proto fields.

The current dissection doesn't handle vlan packets correctly.  Vlan
doesn't exist in skb->data anymore when applying flow dissection on the
skb, fix that.

Fixes: 0744dd00c1b1 ('net: introduce skb_flow_dissect()')
Signed-off-by: Hadar Hen Zion <redacted>

Acked-by: Jiri Pirko <redacted>

Thu, Aug 11, 2016 at 05:23:00PM CEST, hadarh@dev.mellanox.co.il wrote:

On Thu, Aug 11, 2016 at 12:58 AM, kbuild test robot [off-list ref] wrote:

quoted

Hi Hadar,

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

url:    https://github.com/0day-ci/linux/commits/Hadar-Hen-Zion/flow_dissector-Get-vlan-info-from-skb-vlan_tci-instead-of-skb-data/20160811-042500
config: cris-etrax-100lx_v2_defconfig (attached as .config)
compiler: cris-linux-gcc (GCC) 4.6.3
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=cris

All errors (new ones prefixed by >>):

   In function 'flow_keys_hash_length.isra.6',
       inlined from '__flow_hash_from_keys' at net/core/flow_dissector.c:599:26,
       inlined from 'flow_hash_from_keys' at net/core/flow_dissector.c:610:2:

quoted

quoted

net/core/flow_dissector.c:512:2: error: call to '__compiletime_assert_512' declared with attribute error: BUILD_BUG_ON failed: (sizeof(*flow) - FLOW_KEYS_HASH_OFFSET) % sizeof(u32)

[...]


I'm working on a fix, will send it soon.

I'll be happy to get your review and comments on my flow_dissector and
flower patches.

This patch looks good to me. I'll ack the fixed version. Thanks.

On 2016/08/10 22:32, Hadar Hen Zion wrote:

Early in the datapath skb_vlan_untag function is called, stripped
the vlan from the skb and set skb->vlan_tci and skb->vlan_proto fields.

The current dissection doesn't handle vlan packets correctly.  Vlan
doesn't exist in skb->data anymore when applying flow dissection on the
skb, fix that.

RPS (and flow-dissector called in RPS) is performed before vlan-strip in
__netif_receive_skb_core().
Also, in cases skb is tagged with multiple vlan headers (typical when
using 802.1ad), the second level vlan tag is in skb->data.
So I think you should handle both of skb->vlan_tci and skb->data cases.

Thanks,
Toshiaki Makita

Hi,

On Wed, 10 Aug 2016 16:32:23 +0300 Hadar Hen Zion [off-list ref] wrote:

nit: We could avoid the 'v->tcfv_push_prio' test, so user can explicitly
observe the associated priority, even if it is set to zero.

Reviewed-by: Shmulik Ladkani <redacted>

On Fri, Aug 12, 2016 at 9:36 AM, Toshiaki Makita
[off-list ref] wrote:

On 2016/08/10 22:32, Hadar Hen Zion wrote:

quoted

Early in the datapath skb_vlan_untag function is called, stripped
the vlan from the skb and set skb->vlan_tci and skb->vlan_proto fields.

The current dissection doesn't handle vlan packets correctly.  Vlan
doesn't exist in skb->data anymore when applying flow dissection on the
skb, fix that.

RPS (and flow-dissector called in RPS) is performed before vlan-strip in
__netif_receive_skb_core().

right, I'll fix it to v2.

Also, in cases skb is tagged with multiple vlan headers (typical when
using 802.1ad), the second level vlan tag is in skb->data.

Currently, flow_dissector doesn't support multiple vlan headers, only
one vlan_id field is present.
There aren't any flow_dissector "customers" yet for multiple vlan support.

So I think you should handle both of skb->vlan_tci and skb->data cases.

Sure, will do it.

Thanks,
Toshiaki Makita

On 16/08/14 (日) 23:58, Hadar Hen Zion wrote:

On Fri, Aug 12, 2016 at 9:36 AM, Toshiaki Makita
[off-list ref] wrote:

quoted

On 2016/08/10 22:32, Hadar Hen Zion wrote:

quoted

Early in the datapath skb_vlan_untag function is called, stripped
the vlan from the skb and set skb->vlan_tci and skb->vlan_proto fields.

The current dissection doesn't handle vlan packets correctly.  Vlan
doesn't exist in skb->data anymore when applying flow dissection on the
skb, fix that.

RPS (and flow-dissector called in RPS) is performed before vlan-strip in
__netif_receive_skb_core().

right, I'll fix it to v2.

quoted

Also, in cases skb is tagged with multiple vlan headers (typical when
using 802.1ad), the second level vlan tag is in skb->data.

Currently, flow_dissector doesn't support multiple vlan headers, only
one vlan_id field is present.
There aren't any flow_dissector "customers" yet for multiple vlan support.

Sure, no need to store second level vlan tag information for now.
The point is that current flow-dissector correctly skips any number of 
vlan tags and get hash value from IP/TCP/UDP headers, so RPS works for 
multiple vlan tagged packets.

Thanks,
Toshiaki Makita

On Mon, Aug 15, 2016 at 5:38 AM, Toshiaki Makita
[off-list ref] wrote:

On 16/08/14 (日) 23:58, Hadar Hen Zion wrote:

quoted

On Fri, Aug 12, 2016 at 9:36 AM, Toshiaki Makita
[off-list ref] wrote:

quoted

On 2016/08/10 22:32, Hadar Hen Zion wrote:

quoted

Early in the datapath skb_vlan_untag function is called, stripped
the vlan from the skb and set skb->vlan_tci and skb->vlan_proto fields.

The current dissection doesn't handle vlan packets correctly.  Vlan
doesn't exist in skb->data anymore when applying flow dissection on the
skb, fix that.

RPS (and flow-dissector called in RPS) is performed before vlan-strip in
__netif_receive_skb_core().

right, I'll fix it to v2.

quoted

Also, in cases skb is tagged with multiple vlan headers (typical when
using 802.1ad), the second level vlan tag is in skb->data.

Currently, flow_dissector doesn't support multiple vlan headers, only
one vlan_id field is present.
There aren't any flow_dissector "customers" yet for multiple vlan support.

Sure, no need to store second level vlan tag information for now.
The point is that current flow-dissector correctly skips any number of vlan
tags and get hash value from IP/TCP/UDP headers, so RPS works for multiple
vlan tagged packets.

Thanks,
Toshiaki Makita

ok, so we are on the same page.
The flow dissector will correctly skip any number of vlans regardless
if the first vlan is stripped or not.

I also found a dependency between my vlan addition to flower and mlx5
tc offload support so I'm working to fix it for V2.

Thanks,
Hadar

Mon, Aug 15, 2016 at 05:51:38PM CEST, hadarh@dev.mellanox.co.il wrote:

On Mon, Aug 15, 2016 at 5:38 AM, Toshiaki Makita
[off-list ref] wrote:

quoted

On 16/08/14 (日) 23:58, Hadar Hen Zion wrote:

quoted

On Fri, Aug 12, 2016 at 9:36 AM, Toshiaki Makita
[off-list ref] wrote:

quoted

On 2016/08/10 22:32, Hadar Hen Zion wrote:

quoted

Early in the datapath skb_vlan_untag function is called, stripped
the vlan from the skb and set skb->vlan_tci and skb->vlan_proto fields.

The current dissection doesn't handle vlan packets correctly.  Vlan
doesn't exist in skb->data anymore when applying flow dissection on the
skb, fix that.

RPS (and flow-dissector called in RPS) is performed before vlan-strip in
__netif_receive_skb_core().

right, I'll fix it to v2.

quoted

Also, in cases skb is tagged with multiple vlan headers (typical when
using 802.1ad), the second level vlan tag is in skb->data.

Currently, flow_dissector doesn't support multiple vlan headers, only
one vlan_id field is present.
There aren't any flow_dissector "customers" yet for multiple vlan support.

Sure, no need to store second level vlan tag information for now.
The point is that current flow-dissector correctly skips any number of vlan
tags and get hash value from IP/TCP/UDP headers, so RPS works for multiple
vlan tagged packets.

Thanks,
Toshiaki Makita

ok, so we are on the same page.
The flow dissector will correctly skip any number of vlans regardless
if the first vlan is stripped or not.

On RX the first vlan is always stripped either by hw or by skb_vlan_untag.
On TX the first vlan is also stripped as validate_xmit_skb_list which
pushes vlan header is called just before dev_hard_start_xmit.

So I believe you can safely work just with skb->vlan_*