netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next V2 0/5] net_sched, flow_dissector, flower: Introduce vlan tag support
@ 2016-08-17 10:36 Hadar Hen Zion
  2016-08-17 10:36 ` [PATCH net-next V2 1/5] flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci Hadar Hen Zion
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Hadar Hen Zion @ 2016-08-17 10:36 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Jiri Pirko, Amir Vadai, Or Gerlitz, Tom Herbert, Hadar Hen Zion

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 flow-dissector:
 - The first patch is a fix, in case the vlan was already stripped from the
   skb, take it from skb->vlan_tci.
 - The second patch adds support for vlan priority.

The next 2 patches are dealing with flower:
 - The first patch is a fix, sets flow dissector 'used_keys' according to the
   mask value of each key.
 - The secound 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.

Changes from V1:
 - A new patch was added to this series "net_sched: flower: Avoid dissection of unmasked keys"
 - Adding u16 padding to struct flow_dissector_key_vlan
 - change flow_label field in struct flow_dissector_key_tags form 20 bits field to u32
 - Remove 'if (v->tcfv_push_prio)' check from tcf_vlan_dump function
 - Add support to un-stripped vlan skb and skb with multipale vlans in __skb_flow_dissect


Hadar Hen Zion (5):
  flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci
  flow_dissector: Get vlan priority in addition to vlan id
  net_sched: flower: Avoid dissection of unmasked keys
  net_sched: flower: Add vlan support
  net_sched: act_vlan: Add priority option

 include/linux/if_vlan.h             |  1 +
 include/net/flow_dissector.h        | 12 +++--
 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           | 51 ++++++++++++++-----
 net/sched/act_vlan.c                | 13 ++++-
 net/sched/cls_flower.c              | 98 ++++++++++++++++++++++++++++++-------
 8 files changed, 144 insertions(+), 36 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH net-next V2 1/5] flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci
  2016-08-17 10:36 [PATCH net-next V2 0/5] net_sched, flow_dissector, flower: Introduce vlan tag support Hadar Hen Zion
@ 2016-08-17 10:36 ` Hadar Hen Zion
  2016-08-17 10:46   ` Jiri Pirko
  2016-08-18  7:24   ` Jiri Pirko
  2016-08-17 10:36 ` [PATCH net-next V2 2/5] flow_dissector: Get vlan priority in addition to vlan id Hadar Hen Zion
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 14+ messages in thread
From: Hadar Hen Zion @ 2016-08-17 10:36 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Jiri Pirko, Amir Vadai, Or Gerlitz, Tom Herbert, Hadar Hen Zion

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 stripped vlan packets correctly.
In some flows, vlan doesn't exist in skb->data anymore when applying
flow dissection on the skb, fix that.

In case vlan info wasn't stripped before applying flow_dissector (RPS
flow for example), or in case of skb with multiple vlans (e.g. 802.1ad),
get the vlan info from skb->data. The flow_dissector correctly skips
any number of vlans and stores only the first level vlan.

Fixes: 0744dd00c1b1 ('net: introduce skb_flow_dissect()')
Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
---
 net/core/flow_dissector.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 91028ae..362d693 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -119,12 +119,14 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 	struct flow_dissector_key_ports *key_ports;
 	struct flow_dissector_key_tags *key_tags;
 	struct flow_dissector_key_keyid *key_keyid;
+	bool skip_vlan = false;
 	u8 ip_proto = 0;
 	bool ret = false;
 
 	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);
 	}
@@ -243,23 +245,39 @@ 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 (skb_vlan_tag_present(skb))
+			proto = skb->protocol;
+
+		if (!skb_vlan_tag_present(skb) ||
+		    proto == cpu_to_be16(ETH_P_8021Q) ||
+		    proto == cpu_to_be16(ETH_P_8021AD)) {
+			struct vlan_hdr _vlan;
+
+			vlan = __skb_header_pointer(skb, nhoff, sizeof(_vlan),
+						    data, hlen, &_vlan);
+			if (!vlan)
+				goto out_bad;
+			proto = vlan->h_vlan_encapsulated_proto;
+			nhoff += sizeof(*vlan);
+			if (skip_vlan)
+				goto again;
+		}
 
+		skip_vlan = true;
 		if (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);
+			if (skb_vlan_tag_present(skb))
+				key_tags->vlan_id = skb_vlan_tag_get_id(skb);
+			else
+				key_tags->vlan_id = ntohs(vlan->h_vlan_TCI) &
+					VLAN_VID_MASK;
 		}
 
-		proto = vlan->h_vlan_encapsulated_proto;
-		nhoff += sizeof(*vlan);
 		goto again;
 	}
 	case htons(ETH_P_PPP_SES): {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next V2 2/5] flow_dissector: Get vlan priority in addition to vlan id
  2016-08-17 10:36 [PATCH net-next V2 0/5] net_sched, flow_dissector, flower: Introduce vlan tag support Hadar Hen Zion
  2016-08-17 10:36 ` [PATCH net-next V2 1/5] flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci Hadar Hen Zion
@ 2016-08-17 10:36 ` Hadar Hen Zion
  2016-08-18  7:25   ` Jiri Pirko
  2016-08-17 10:36 ` [PATCH net-next V2 3/5] net_sched: flower: Avoid dissection of unmasked keys Hadar Hen Zion
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Hadar Hen Zion @ 2016-08-17 10:36 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Jiri Pirko, Amir Vadai, Or Gerlitz, Tom Herbert, Hadar Hen Zion

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 <hadarh@mellanox.com>
---
 include/linux/if_vlan.h      |  1 +
 include/net/flow_dissector.h | 12 +++++++++---
 net/core/flow_dissector.c    | 25 ++++++++++++++++---------
 3 files changed, 26 insertions(+), 12 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..f266b51 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -32,8 +32,13 @@ struct flow_dissector_key_basic {
 };
 
 struct flow_dissector_key_tags {
-	u32	vlan_id:12,
-		flow_label:20;
+	u32	flow_label;
+};
+
+struct flow_dissector_key_vlan {
+	u16	vlan_id:12,
+		vlan_priority:3;
+	u16	padding;
 };
 
 struct flow_dissector_key_keyid {
@@ -119,7 +124,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 +153,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 362d693..a2879c0 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -118,6 +118,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;
 	bool skip_vlan = false;
 	u8 ip_proto = 0;
@@ -266,16 +267,22 @@ ipv6:
 
 		skip_vlan = true;
 		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);
 
-			if (skb_vlan_tag_present(skb))
-				key_tags->vlan_id = skb_vlan_tag_get_id(skb);
-			else
-				key_tags->vlan_id = ntohs(vlan->h_vlan_TCI) &
+			if (skb_vlan_tag_present(skb)) {
+				key_vlan->vlan_id = skb_vlan_tag_get_id(skb);
+				key_vlan->vlan_priority =
+					(skb_vlan_tag_get_prio(skb) >> VLAN_PRIO_SHIFT);
+			} else {
+				key_vlan->vlan_id = ntohs(vlan->h_vlan_TCI) &
 					VLAN_VID_MASK;
+				key_vlan->vlan_priority =
+					(ntohs(vlan->h_vlan_TCI) &
+					 VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
+			}
 		}
 
 		goto again;
@@ -935,8 +942,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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next V2 3/5] net_sched: flower: Avoid dissection of unmasked keys
  2016-08-17 10:36 [PATCH net-next V2 0/5] net_sched, flow_dissector, flower: Introduce vlan tag support Hadar Hen Zion
  2016-08-17 10:36 ` [PATCH net-next V2 1/5] flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci Hadar Hen Zion
  2016-08-17 10:36 ` [PATCH net-next V2 2/5] flow_dissector: Get vlan priority in addition to vlan id Hadar Hen Zion
@ 2016-08-17 10:36 ` Hadar Hen Zion
  2016-08-17 10:36 ` [PATCH net-next V2 4/5] net_sched: flower: Add vlan support Hadar Hen Zion
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Hadar Hen Zion @ 2016-08-17 10:36 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Jiri Pirko, Amir Vadai, Or Gerlitz, Tom Herbert, Hadar Hen Zion

The current flower implementation checks the mask range and set all the
keys included in that range as "used_keys", even if a specific key in
the range has a zero mask.

This behavior can cause a false positive return value of
dissector_uses_key function and unnecessary dissection in
__skb_flow_dissect.

This patch checks explicitly the mask of each key and "used_keys" will
be set accordingly.

Fixes: 77b9900ef53a ('tc: introduce Flower classifier')
Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_flower.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 5060801..0080fc0 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -404,12 +404,10 @@ static int fl_init_hashtable(struct cls_fl_head *head,
 
 #define FL_KEY_MEMBER_OFFSET(member) offsetof(struct fl_flow_key, member)
 #define FL_KEY_MEMBER_SIZE(member) (sizeof(((struct fl_flow_key *) 0)->member))
-#define FL_KEY_MEMBER_END_OFFSET(member)					\
-	(FL_KEY_MEMBER_OFFSET(member) + FL_KEY_MEMBER_SIZE(member))
 
-#define FL_KEY_IN_RANGE(mask, member)						\
-        (FL_KEY_MEMBER_OFFSET(member) <= (mask)->range.end &&			\
-         FL_KEY_MEMBER_END_OFFSET(member) >= (mask)->range.start)
+#define FL_KEY_IS_MASKED(mask, member)						\
+	memchr_inv(((char *)mask) + FL_KEY_MEMBER_OFFSET(member),		\
+		   0, FL_KEY_MEMBER_SIZE(member))				\
 
 #define FL_KEY_SET(keys, cnt, id, member)					\
 	do {									\
@@ -418,9 +416,9 @@ static int fl_init_hashtable(struct cls_fl_head *head,
 		cnt++;								\
 	} while(0);
 
-#define FL_KEY_SET_IF_IN_RANGE(mask, keys, cnt, id, member)			\
+#define FL_KEY_SET_IF_MASKED(mask, keys, cnt, id, member)			\
 	do {									\
-		if (FL_KEY_IN_RANGE(mask, member))				\
+		if (FL_KEY_IS_MASKED(mask, member))				\
 			FL_KEY_SET(keys, cnt, id, member);			\
 	} while(0);
 
@@ -432,14 +430,14 @@ static void fl_init_dissector(struct cls_fl_head *head,
 
 	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);
-	FL_KEY_SET_IF_IN_RANGE(mask, keys, cnt,
-			       FLOW_DISSECTOR_KEY_IPV4_ADDRS, ipv4);
-	FL_KEY_SET_IF_IN_RANGE(mask, keys, cnt,
-			       FLOW_DISSECTOR_KEY_IPV6_ADDRS, ipv6);
-	FL_KEY_SET_IF_IN_RANGE(mask, keys, cnt,
-			       FLOW_DISSECTOR_KEY_PORTS, tp);
+	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+			     FLOW_DISSECTOR_KEY_ETH_ADDRS, eth);
+	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+			     FLOW_DISSECTOR_KEY_IPV4_ADDRS, ipv4);
+	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+			     FLOW_DISSECTOR_KEY_IPV6_ADDRS, ipv6);
+	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+			     FLOW_DISSECTOR_KEY_PORTS, tp);
 
 	skb_flow_dissector_init(&head->dissector, keys, cnt);
 }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next V2 4/5] net_sched: flower: Add vlan support
  2016-08-17 10:36 [PATCH net-next V2 0/5] net_sched, flow_dissector, flower: Introduce vlan tag support Hadar Hen Zion
                   ` (2 preceding siblings ...)
  2016-08-17 10:36 ` [PATCH net-next V2 3/5] net_sched: flower: Avoid dissection of unmasked keys Hadar Hen Zion
@ 2016-08-17 10:36 ` Hadar Hen Zion
  2016-08-17 10:36 ` [PATCH net-next V2 5/5] net_sched: act_vlan: Add priority option Hadar Hen Zion
  2016-08-19  6:14 ` [PATCH net-next V2 0/5] net_sched, flow_dissector, flower: Introduce vlan tag support David Miller
  5 siblings, 0 replies; 14+ messages in thread
From: Hadar Hen Zion @ 2016-08-17 10:36 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Jiri Pirko, Amir Vadai, Or Gerlitz, Tom Herbert, Hadar Hen Zion

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 <hadarh@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 include/uapi/linux/pkt_cls.h |  3 ++
 net/sched/cls_flower.c       | 70 ++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 70 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 0080fc0..1e11e57 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,29 @@ 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 +353,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)) {
@@ -438,6 +473,8 @@ static void fl_init_dissector(struct cls_fl_head *head,
 			     FLOW_DISSECTOR_KEY_IPV6_ADDRS, ipv6);
 	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
 			     FLOW_DISSECTOR_KEY_PORTS, tp);
+	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+			     FLOW_DISSECTOR_KEY_VLAN, vlan);
 
 	skb_flow_dissector_init(&head->dissector, keys, cnt);
 }
@@ -666,6 +703,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)
 {
@@ -710,6 +770,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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next V2 5/5] net_sched: act_vlan: Add priority option
  2016-08-17 10:36 [PATCH net-next V2 0/5] net_sched, flow_dissector, flower: Introduce vlan tag support Hadar Hen Zion
                   ` (3 preceding siblings ...)
  2016-08-17 10:36 ` [PATCH net-next V2 4/5] net_sched: flower: Add vlan support Hadar Hen Zion
@ 2016-08-17 10:36 ` Hadar Hen Zion
  2016-08-19  6:14 ` [PATCH net-next V2 0/5] net_sched, flow_dissector, flower: Introduce vlan tag support David Miller
  5 siblings, 0 replies; 14+ messages in thread
From: Hadar Hen Zion @ 2016-08-17 10:36 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Jiri Pirko, Amir Vadai, Or Gerlitz, Tom Herbert, Hadar Hen Zion

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 <hadarh@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 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..59a8d31 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) ||
+	     (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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next V2 1/5] flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci
  2016-08-17 10:36 ` [PATCH net-next V2 1/5] flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci Hadar Hen Zion
@ 2016-08-17 10:46   ` Jiri Pirko
  2016-08-17 11:05     ` Hadar Hen Zion
  2016-08-18  7:24   ` Jiri Pirko
  1 sibling, 1 reply; 14+ messages in thread
From: Jiri Pirko @ 2016-08-17 10:46 UTC (permalink / raw)
  To: Hadar Hen Zion
  Cc: David S. Miller, netdev, Jiri Pirko, Amir Vadai, Or Gerlitz, Tom Herbert

Wed, Aug 17, 2016 at 12:36:10PM 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 stripped vlan packets correctly.
>In some flows, vlan doesn't exist in skb->data anymore when applying
>flow dissection on the skb, fix that.
>
>In case vlan info wasn't stripped before applying flow_dissector (RPS
>flow for example), or in case of skb with multiple vlans (e.g. 802.1ad),
>get the vlan info from skb->data. The flow_dissector correctly skips
>any number of vlans and stores only the first level vlan.
>
>Fixes: 0744dd00c1b1 ('net: introduce skb_flow_dissect()')
>Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
>---
> net/core/flow_dissector.c | 34 ++++++++++++++++++++++++++--------
> 1 file changed, 26 insertions(+), 8 deletions(-)
>
>diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>index 91028ae..362d693 100644
>--- a/net/core/flow_dissector.c
>+++ b/net/core/flow_dissector.c
>@@ -119,12 +119,14 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
> 	struct flow_dissector_key_ports *key_ports;
> 	struct flow_dissector_key_tags *key_tags;
> 	struct flow_dissector_key_keyid *key_keyid;
>+	bool skip_vlan = false;
> 	u8 ip_proto = 0;
> 	bool ret = false;
> 
> 	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);
> 	}
>@@ -243,23 +245,39 @@ 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 (skb_vlan_tag_present(skb))
>+			proto = skb->protocol;
>+
>+		if (!skb_vlan_tag_present(skb) ||
>+		    proto == cpu_to_be16(ETH_P_8021Q) ||
>+		    proto == cpu_to_be16(ETH_P_8021AD)) {

How this can happen? Could you give me an example?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next V2 1/5] flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci
  2016-08-17 10:46   ` Jiri Pirko
@ 2016-08-17 11:05     ` Hadar Hen Zion
  2016-08-17 13:02       ` Jiri Pirko
  0 siblings, 1 reply; 14+ messages in thread
From: Hadar Hen Zion @ 2016-08-17 11:05 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Hadar Hen Zion, David S. Miller, netdev, Jiri Pirko, Amir Vadai,
	Or Gerlitz, Tom Herbert

On Wed, Aug 17, 2016 at 1:46 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Wed, Aug 17, 2016 at 12:36:10PM 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 stripped vlan packets correctly.
>>In some flows, vlan doesn't exist in skb->data anymore when applying
>>flow dissection on the skb, fix that.
>>
>>In case vlan info wasn't stripped before applying flow_dissector (RPS
>>flow for example), or in case of skb with multiple vlans (e.g. 802.1ad),
>>get the vlan info from skb->data. The flow_dissector correctly skips
>>any number of vlans and stores only the first level vlan.
>>
>>Fixes: 0744dd00c1b1 ('net: introduce skb_flow_dissect()')
>>Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
>>---
>> net/core/flow_dissector.c | 34 ++++++++++++++++++++++++++--------
>> 1 file changed, 26 insertions(+), 8 deletions(-)
>>
>>diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>>index 91028ae..362d693 100644
>>--- a/net/core/flow_dissector.c
>>+++ b/net/core/flow_dissector.c
>>@@ -119,12 +119,14 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>>       struct flow_dissector_key_ports *key_ports;
>>       struct flow_dissector_key_tags *key_tags;
>>       struct flow_dissector_key_keyid *key_keyid;
>>+      bool skip_vlan = false;
>>       u8 ip_proto = 0;
>>       bool ret = false;
>>
>>       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);
>>       }
>>@@ -243,23 +245,39 @@ 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 (skb_vlan_tag_present(skb))
>>+                      proto = skb->protocol;
>>+
>>+              if (!skb_vlan_tag_present(skb) ||
>>+                  proto == cpu_to_be16(ETH_P_8021Q) ||
>>+                  proto == cpu_to_be16(ETH_P_8021AD)) {
>
> How this can happen? Could you give me an example?
>

This can happen in 2 cases:

1. vlan wasn't stripped yet from the skb.
In RPS flow for example, get_rps_cpu function is using flow-dissector
before vlan_untag is called by __netif_receive_skb_core.

2. skb with multiple vlan tags.
Only the first vlan is stripped while the inner vlans are still in skb->data.
In this case skb->vlan_proto is 802.1AD and skb->protocol is 802.1Q
(for example) so I have to take the next header from skb->data.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next V2 1/5] flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci
  2016-08-17 11:05     ` Hadar Hen Zion
@ 2016-08-17 13:02       ` Jiri Pirko
  2016-08-18  6:22         ` Hadar Hen Zion
  0 siblings, 1 reply; 14+ messages in thread
From: Jiri Pirko @ 2016-08-17 13:02 UTC (permalink / raw)
  To: Hadar Hen Zion
  Cc: Hadar Hen Zion, David S. Miller, netdev, Jiri Pirko, Amir Vadai,
	Or Gerlitz, Tom Herbert

Wed, Aug 17, 2016 at 01:05:33PM CEST, hadarh@dev.mellanox.co.il wrote:
>On Wed, Aug 17, 2016 at 1:46 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Wed, Aug 17, 2016 at 12:36:10PM 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 stripped vlan packets correctly.
>>>In some flows, vlan doesn't exist in skb->data anymore when applying
>>>flow dissection on the skb, fix that.
>>>
>>>In case vlan info wasn't stripped before applying flow_dissector (RPS
>>>flow for example), or in case of skb with multiple vlans (e.g. 802.1ad),
>>>get the vlan info from skb->data. The flow_dissector correctly skips
>>>any number of vlans and stores only the first level vlan.
>>>
>>>Fixes: 0744dd00c1b1 ('net: introduce skb_flow_dissect()')
>>>Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
>>>---
>>> net/core/flow_dissector.c | 34 ++++++++++++++++++++++++++--------
>>> 1 file changed, 26 insertions(+), 8 deletions(-)
>>>
>>>diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>>>index 91028ae..362d693 100644
>>>--- a/net/core/flow_dissector.c
>>>+++ b/net/core/flow_dissector.c
>>>@@ -119,12 +119,14 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>>>       struct flow_dissector_key_ports *key_ports;
>>>       struct flow_dissector_key_tags *key_tags;
>>>       struct flow_dissector_key_keyid *key_keyid;
>>>+      bool skip_vlan = false;
>>>       u8 ip_proto = 0;
>>>       bool ret = false;
>>>
>>>       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);
>>>       }
>>>@@ -243,23 +245,39 @@ 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 (skb_vlan_tag_present(skb))
>>>+                      proto = skb->protocol;
>>>+
>>>+              if (!skb_vlan_tag_present(skb) ||
>>>+                  proto == cpu_to_be16(ETH_P_8021Q) ||
>>>+                  proto == cpu_to_be16(ETH_P_8021AD)) {
>>
>> How this can happen? Could you give me an example?
>>
>
>This can happen in 2 cases:
>
>1. vlan wasn't stripped yet from the skb.
>In RPS flow for example, get_rps_cpu function is using flow-dissector
>before vlan_untag is called by __netif_receive_skb_core.

right, sigh...


>
>2. skb with multiple vlan tags.
>Only the first vlan is stripped while the inner vlans are still in skb->data.
>In this case skb->vlan_proto is 802.1AD and skb->protocol is 802.1Q
>(for example) so I have to take the next header from skb->data.

Hmm I think that whoever removes the outermost vlan from skb->vlan_*
should strip the next header into skb->vlan_*

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next V2 1/5] flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci
  2016-08-17 13:02       ` Jiri Pirko
@ 2016-08-18  6:22         ` Hadar Hen Zion
  2016-08-18  6:46           ` Jiri Pirko
  0 siblings, 1 reply; 14+ messages in thread
From: Hadar Hen Zion @ 2016-08-18  6:22 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Hadar Hen Zion, David S. Miller, netdev, Jiri Pirko, Amir Vadai,
	Or Gerlitz, Tom Herbert

On Wed, Aug 17, 2016 at 4:02 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Wed, Aug 17, 2016 at 01:05:33PM CEST, hadarh@dev.mellanox.co.il wrote:
>>On Wed, Aug 17, 2016 at 1:46 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>>> Wed, Aug 17, 2016 at 12:36:10PM 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 stripped vlan packets correctly.
>>>>In some flows, vlan doesn't exist in skb->data anymore when applying
>>>>flow dissection on the skb, fix that.
>>>>
>>>>In case vlan info wasn't stripped before applying flow_dissector (RPS
>>>>flow for example), or in case of skb with multiple vlans (e.g. 802.1ad),
>>>>get the vlan info from skb->data. The flow_dissector correctly skips
>>>>any number of vlans and stores only the first level vlan.
>>>>
>>>>Fixes: 0744dd00c1b1 ('net: introduce skb_flow_dissect()')
>>>>Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
>>>>---
>>>> net/core/flow_dissector.c | 34 ++++++++++++++++++++++++++--------
>>>> 1 file changed, 26 insertions(+), 8 deletions(-)
>>>>
>>>>diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>>>>index 91028ae..362d693 100644
>>>>--- a/net/core/flow_dissector.c
>>>>+++ b/net/core/flow_dissector.c
>>>>@@ -119,12 +119,14 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>>>>       struct flow_dissector_key_ports *key_ports;
>>>>       struct flow_dissector_key_tags *key_tags;
>>>>       struct flow_dissector_key_keyid *key_keyid;
>>>>+      bool skip_vlan = false;
>>>>       u8 ip_proto = 0;
>>>>       bool ret = false;
>>>>
>>>>       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);
>>>>       }
>>>>@@ -243,23 +245,39 @@ 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 (skb_vlan_tag_present(skb))
>>>>+                      proto = skb->protocol;
>>>>+
>>>>+              if (!skb_vlan_tag_present(skb) ||
>>>>+                  proto == cpu_to_be16(ETH_P_8021Q) ||
>>>>+                  proto == cpu_to_be16(ETH_P_8021AD)) {
>>>
>>> How this can happen? Could you give me an example?
>>>
>>
>>This can happen in 2 cases:
>>
>>1. vlan wasn't stripped yet from the skb.
>>In RPS flow for example, get_rps_cpu function is using flow-dissector
>>before vlan_untag is called by __netif_receive_skb_core.
>
> right, sigh...
>
>
>>
>>2. skb with multiple vlan tags.
>>Only the first vlan is stripped while the inner vlans are still in skb->data.
>>In this case skb->vlan_proto is 802.1AD and skb->protocol is 802.1Q
>>(for example) so I have to take the next header from skb->data.
>
> Hmm I think that whoever removes the outermost vlan from skb->vlan_*
> should strip the next header into skb->vlan_*

The outermost vlan isn't removed from skb->vlan_* it stays there.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next V2 1/5] flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci
  2016-08-18  6:22         ` Hadar Hen Zion
@ 2016-08-18  6:46           ` Jiri Pirko
  0 siblings, 0 replies; 14+ messages in thread
From: Jiri Pirko @ 2016-08-18  6:46 UTC (permalink / raw)
  To: Hadar Hen Zion
  Cc: Hadar Hen Zion, David S. Miller, netdev, Jiri Pirko, Amir Vadai,
	Or Gerlitz, Tom Herbert

Thu, Aug 18, 2016 at 08:22:49AM CEST, hadarh@dev.mellanox.co.il wrote:
>On Wed, Aug 17, 2016 at 4:02 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Wed, Aug 17, 2016 at 01:05:33PM CEST, hadarh@dev.mellanox.co.il wrote:
>>>On Wed, Aug 17, 2016 at 1:46 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>>>> Wed, Aug 17, 2016 at 12:36:10PM 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 stripped vlan packets correctly.
>>>>>In some flows, vlan doesn't exist in skb->data anymore when applying
>>>>>flow dissection on the skb, fix that.
>>>>>
>>>>>In case vlan info wasn't stripped before applying flow_dissector (RPS
>>>>>flow for example), or in case of skb with multiple vlans (e.g. 802.1ad),
>>>>>get the vlan info from skb->data. The flow_dissector correctly skips
>>>>>any number of vlans and stores only the first level vlan.
>>>>>
>>>>>Fixes: 0744dd00c1b1 ('net: introduce skb_flow_dissect()')
>>>>>Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
>>>>>---
>>>>> net/core/flow_dissector.c | 34 ++++++++++++++++++++++++++--------
>>>>> 1 file changed, 26 insertions(+), 8 deletions(-)
>>>>>
>>>>>diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>>>>>index 91028ae..362d693 100644
>>>>>--- a/net/core/flow_dissector.c
>>>>>+++ b/net/core/flow_dissector.c
>>>>>@@ -119,12 +119,14 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>>>>>       struct flow_dissector_key_ports *key_ports;
>>>>>       struct flow_dissector_key_tags *key_tags;
>>>>>       struct flow_dissector_key_keyid *key_keyid;
>>>>>+      bool skip_vlan = false;
>>>>>       u8 ip_proto = 0;
>>>>>       bool ret = false;
>>>>>
>>>>>       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);
>>>>>       }
>>>>>@@ -243,23 +245,39 @@ 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 (skb_vlan_tag_present(skb))
>>>>>+                      proto = skb->protocol;
>>>>>+
>>>>>+              if (!skb_vlan_tag_present(skb) ||
>>>>>+                  proto == cpu_to_be16(ETH_P_8021Q) ||
>>>>>+                  proto == cpu_to_be16(ETH_P_8021AD)) {
>>>>
>>>> How this can happen? Could you give me an example?
>>>>
>>>
>>>This can happen in 2 cases:
>>>
>>>1. vlan wasn't stripped yet from the skb.
>>>In RPS flow for example, get_rps_cpu function is using flow-dissector
>>>before vlan_untag is called by __netif_receive_skb_core.
>>
>> right, sigh...
>>
>>
>>>
>>>2. skb with multiple vlan tags.
>>>Only the first vlan is stripped while the inner vlans are still in skb->data.
>>>In this case skb->vlan_proto is 802.1AD and skb->protocol is 802.1Q
>>>(for example) so I have to take the next header from skb->data.
>>
>> Hmm I think that whoever removes the outermost vlan from skb->vlan_*
>> should strip the next header into skb->vlan_*
>
>The outermost vlan isn't removed from skb->vlan_* it stays there.

No I didn't mean in your code. But other code that removes it should
ensure that next vlan header is stripped so the rest of the code (like
yours) can count with it.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next V2 1/5] flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci
  2016-08-17 10:36 ` [PATCH net-next V2 1/5] flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci Hadar Hen Zion
  2016-08-17 10:46   ` Jiri Pirko
@ 2016-08-18  7:24   ` Jiri Pirko
  1 sibling, 0 replies; 14+ messages in thread
From: Jiri Pirko @ 2016-08-18  7:24 UTC (permalink / raw)
  To: Hadar Hen Zion
  Cc: David S. Miller, netdev, Jiri Pirko, Amir Vadai, Or Gerlitz, Tom Herbert

Wed, Aug 17, 2016 at 12:36:10PM 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 stripped vlan packets correctly.
>In some flows, vlan doesn't exist in skb->data anymore when applying
>flow dissection on the skb, fix that.
>
>In case vlan info wasn't stripped before applying flow_dissector (RPS
>flow for example), or in case of skb with multiple vlans (e.g. 802.1ad),
>get the vlan info from skb->data. The flow_dissector correctly skips
>any number of vlans and stores only the first level vlan.
>
>Fixes: 0744dd00c1b1 ('net: introduce skb_flow_dissect()')
>Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next V2 2/5] flow_dissector: Get vlan priority in addition to vlan id
  2016-08-17 10:36 ` [PATCH net-next V2 2/5] flow_dissector: Get vlan priority in addition to vlan id Hadar Hen Zion
@ 2016-08-18  7:25   ` Jiri Pirko
  0 siblings, 0 replies; 14+ messages in thread
From: Jiri Pirko @ 2016-08-18  7:25 UTC (permalink / raw)
  To: Hadar Hen Zion
  Cc: David S. Miller, netdev, Jiri Pirko, Amir Vadai, Or Gerlitz, Tom Herbert

Wed, Aug 17, 2016 at 12:36:11PM CEST, hadarh@mellanox.com wrote:
>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 <hadarh@mellanox.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next V2 0/5] net_sched, flow_dissector, flower: Introduce vlan tag support
  2016-08-17 10:36 [PATCH net-next V2 0/5] net_sched, flow_dissector, flower: Introduce vlan tag support Hadar Hen Zion
                   ` (4 preceding siblings ...)
  2016-08-17 10:36 ` [PATCH net-next V2 5/5] net_sched: act_vlan: Add priority option Hadar Hen Zion
@ 2016-08-19  6:14 ` David Miller
  5 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2016-08-19  6:14 UTC (permalink / raw)
  To: hadarh; +Cc: netdev, jiri, amirva, ogerlitz, tom

From: Hadar Hen Zion <hadarh@mellanox.com>
Date: Wed, 17 Aug 2016 13:36:09 +0300

> 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 flow-dissector:
>  - The first patch is a fix, in case the vlan was already stripped from the
>    skb, take it from skb->vlan_tci.
>  - The second patch adds support for vlan priority.
> 
> The next 2 patches are dealing with flower:
>  - The first patch is a fix, sets flow dissector 'used_keys' according to the
>    mask value of each key.
>  - The secound 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.
> 
> Changes from V1:
>  - A new patch was added to this series "net_sched: flower: Avoid dissection of unmasked keys"
>  - Adding u16 padding to struct flow_dissector_key_vlan
>  - change flow_label field in struct flow_dissector_key_tags form 20 bits field to u32
>  - Remove 'if (v->tcfv_push_prio)' check from tcf_vlan_dump function
>  - Add support to un-stripped vlan skb and skb with multipale vlans in __skb_flow_dissect

Series applied, thanks.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-08-19  6:14 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-17 10:36 [PATCH net-next V2 0/5] net_sched, flow_dissector, flower: Introduce vlan tag support Hadar Hen Zion
2016-08-17 10:36 ` [PATCH net-next V2 1/5] flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci Hadar Hen Zion
2016-08-17 10:46   ` Jiri Pirko
2016-08-17 11:05     ` Hadar Hen Zion
2016-08-17 13:02       ` Jiri Pirko
2016-08-18  6:22         ` Hadar Hen Zion
2016-08-18  6:46           ` Jiri Pirko
2016-08-18  7:24   ` Jiri Pirko
2016-08-17 10:36 ` [PATCH net-next V2 2/5] flow_dissector: Get vlan priority in addition to vlan id Hadar Hen Zion
2016-08-18  7:25   ` Jiri Pirko
2016-08-17 10:36 ` [PATCH net-next V2 3/5] net_sched: flower: Avoid dissection of unmasked keys Hadar Hen Zion
2016-08-17 10:36 ` [PATCH net-next V2 4/5] net_sched: flower: Add vlan support Hadar Hen Zion
2016-08-17 10:36 ` [PATCH net-next V2 5/5] net_sched: act_vlan: Add priority option Hadar Hen Zion
2016-08-19  6:14 ` [PATCH net-next V2 0/5] net_sched, flow_dissector, flower: Introduce vlan tag support David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).