Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH net-next v2 0/2] flow_dissector, cls_flower: Add support for multiple MPLS Label Stack Entries
@ 2020-05-21 17:47 Guillaume Nault
  2020-05-21 17:47 ` [PATCH net-next v2 1/2] flow_dissector: Parse " Guillaume Nault
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Guillaume Nault @ 2020-05-21 17:47 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: netdev, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
	Benjamin LaHaise, Tom Herbert, Pieter Jansen van Vuuren,
	Pablo Neira Ayuso, Liel Shoshan, Rony Efraim

Currently, the flow dissector and the Flower classifier can only handle
the first entry of an MPLS label stack. This patch series generalises
the code to allow parsing and matching the Label Stack Entries that
follow.

Patch 1 extends the flow dissector to parse MPLS LSEs until the Bottom
Of Stack bit is reached. The number of parsed LSEs is capped at
FLOW_DIS_MPLS_MAX (arbitrarily set to 7). Flower and the NFP driver
are updated to take into account the new layout of struct
flow_dissector_key_mpls.

Patch 2 extends Flower. It defines new netlink attributes, which are
independent from the previous MPLS ones. Mixing the old and the new
attributes in a same filter is not allowed. For backward compatibility,
the old attributes are used when dumping filters that don't require the
new ones.

Changes since v1:
  * Fix compilation of NFP driver (kbuild test robot).
  * Fix sparse warning with entropy label (kbuild test robot).

Guillaume Nault (2):
  flow_dissector: Parse multiple MPLS Label Stack Entries
  cls_flower: Support filtering on multiple MPLS Label Stack Entries

 .../net/ethernet/netronome/nfp/flower/match.c |  42 ++-
 include/net/flow_dissector.h                  |  14 +-
 include/uapi/linux/pkt_cls.h                  |  23 ++
 net/core/flow_dissector.c                     |  49 ++-
 net/sched/cls_flower.c                        | 295 +++++++++++++++++-
 5 files changed, 378 insertions(+), 45 deletions(-)

-- 
2.21.1

Note: the NFP udpate was only compile-tested as I don't have the
required hardware.


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

* [PATCH net-next v2 1/2] flow_dissector: Parse multiple MPLS Label Stack Entries
  2020-05-21 17:47 [PATCH net-next v2 0/2] flow_dissector, cls_flower: Add support for multiple MPLS Label Stack Entries Guillaume Nault
@ 2020-05-21 17:47 ` Guillaume Nault
  2020-05-23  9:55   ` Guillaume Nault
  2020-05-21 17:47 ` [PATCH net-next v2 2/2] cls_flower: Support filtering on " Guillaume Nault
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Guillaume Nault @ 2020-05-21 17:47 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: netdev, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
	Benjamin LaHaise, Tom Herbert, Pieter Jansen van Vuuren,
	Pablo Neira Ayuso, Liel Shoshan, Rony Efraim

The current MPLS dissector only parses the first MPLS Label Stack
Entry (second LSE can be parsed too, but only to set a key_id).

This patch adds the possibility to parse several LSEs by making
__skb_flow_dissect_mpls() return FLOW_DISSECT_RET_PROTO_AGAIN as long
as the Bottom Of Stack bit hasn't been seen, up to a maximum of
FLOW_DIS_MPLS_MAX entries.

FLOW_DIS_MPLS_MAX is arbitrarily set to 7. This should be enough for
many practical purposes, without wasting too much space.

To record the parsed values, flow_dissector_key_mpls is modified to
store an array of stack entries, instead of just the values of the
first one. A bit field, "used_lses", is also added to keep track of
the LSEs that have been set. The objective is to avoid defining a
new FLOW_DISSECTOR_KEY_MPLS_XX for each level of the MPLS stack.

TC flower is adapted for the new struct flow_dissector_key_mpls layout.
Matching on several MPLS Label Stack Entries will be added in the next
patch.

The NFP driver is also adapted: nfp_flower_compile_mac() now verifies
that the rule only uses the first LSE and fails if it doesn't.

Finally, the behaviour of the FLOW_DISSECTOR_KEY_MPLS_ENTROPY key is
slightly modified. Instead of recording the first Entropy Label, it
now records the last one. This shouldn't have any consequences since
there doesn't seem to have any user of FLOW_DISSECTOR_KEY_MPLS_ENTROPY
in the tree. We'd probably better do a hash of all parsed MPLS labels
instead (excluding reserved labels) anyway. That'd give better entropy
and would probably also simplify the code. But that's not the purpose
of this patch, so I'm keeping that as a future possible improvement.

Signed-off-by: Guillaume Nault <gnault@redhat.com>
---
 .../net/ethernet/netronome/nfp/flower/match.c | 42 +++++++++++----
 include/net/flow_dissector.h                  | 14 ++++-
 net/core/flow_dissector.c                     | 49 +++++++++++------
 net/sched/cls_flower.c                        | 52 +++++++++++++------
 4 files changed, 113 insertions(+), 44 deletions(-)

Note: the NFP driver update was only compile-tested. Review from
Netronome highly encouraged.

diff --git a/drivers/net/ethernet/netronome/nfp/flower/match.c b/drivers/net/ethernet/netronome/nfp/flower/match.c
index 546bc01d507d..f7f01e2e3dce 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/match.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/match.c
@@ -74,9 +74,10 @@ nfp_flower_compile_port(struct nfp_flower_in_port *frame, u32 cmsg_port,
 	return 0;
 }
 
-static void
+static int
 nfp_flower_compile_mac(struct nfp_flower_mac_mpls *ext,
-		       struct nfp_flower_mac_mpls *msk, struct flow_rule *rule)
+		       struct nfp_flower_mac_mpls *msk, struct flow_rule *rule,
+		       struct netlink_ext_ack *extack)
 {
 	memset(ext, 0, sizeof(struct nfp_flower_mac_mpls));
 	memset(msk, 0, sizeof(struct nfp_flower_mac_mpls));
@@ -97,14 +98,28 @@ nfp_flower_compile_mac(struct nfp_flower_mac_mpls *ext,
 		u32 t_mpls;
 
 		flow_rule_match_mpls(rule, &match);
-		t_mpls = FIELD_PREP(NFP_FLOWER_MASK_MPLS_LB, match.key->mpls_label) |
-			 FIELD_PREP(NFP_FLOWER_MASK_MPLS_TC, match.key->mpls_tc) |
-			 FIELD_PREP(NFP_FLOWER_MASK_MPLS_BOS, match.key->mpls_bos) |
+
+		/* Only support matching the first LSE */
+		if (match.mask->used_lses != 1) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "unsupported offload: invalid LSE depth for MPLS match offload");
+			return -EOPNOTSUPP;
+		}
+
+		t_mpls = FIELD_PREP(NFP_FLOWER_MASK_MPLS_LB,
+				    match.key->ls[0].mpls_label) |
+			 FIELD_PREP(NFP_FLOWER_MASK_MPLS_TC,
+				    match.key->ls[0].mpls_tc) |
+			 FIELD_PREP(NFP_FLOWER_MASK_MPLS_BOS,
+				    match.key->ls[0].mpls_bos) |
 			 NFP_FLOWER_MASK_MPLS_Q;
 		ext->mpls_lse = cpu_to_be32(t_mpls);
-		t_mpls = FIELD_PREP(NFP_FLOWER_MASK_MPLS_LB, match.mask->mpls_label) |
-			 FIELD_PREP(NFP_FLOWER_MASK_MPLS_TC, match.mask->mpls_tc) |
-			 FIELD_PREP(NFP_FLOWER_MASK_MPLS_BOS, match.mask->mpls_bos) |
+		t_mpls = FIELD_PREP(NFP_FLOWER_MASK_MPLS_LB,
+				    match.mask->ls[0].mpls_label) |
+			 FIELD_PREP(NFP_FLOWER_MASK_MPLS_TC,
+				    match.mask->ls[0].mpls_tc) |
+			 FIELD_PREP(NFP_FLOWER_MASK_MPLS_BOS,
+				    match.mask->ls[0].mpls_bos) |
 			 NFP_FLOWER_MASK_MPLS_Q;
 		msk->mpls_lse = cpu_to_be32(t_mpls);
 	} else if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_BASIC)) {
@@ -121,6 +136,8 @@ nfp_flower_compile_mac(struct nfp_flower_mac_mpls *ext,
 			msk->mpls_lse = cpu_to_be32(NFP_FLOWER_MASK_MPLS_Q);
 		}
 	}
+
+	return 0;
 }
 
 static void
@@ -461,9 +478,12 @@ int nfp_flower_compile_flow_match(struct nfp_app *app,
 	msk += sizeof(struct nfp_flower_in_port);
 
 	if (NFP_FLOWER_LAYER_MAC & key_ls->key_layer) {
-		nfp_flower_compile_mac((struct nfp_flower_mac_mpls *)ext,
-				       (struct nfp_flower_mac_mpls *)msk,
-				       rule);
+		err = nfp_flower_compile_mac((struct nfp_flower_mac_mpls *)ext,
+					     (struct nfp_flower_mac_mpls *)msk,
+					     rule, extack);
+		if (err)
+			return err;
+
 		ext += sizeof(struct nfp_flower_mac_mpls);
 		msk += sizeof(struct nfp_flower_mac_mpls);
 	}
diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index 628383915827..4fb1a69c6ecf 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -59,13 +59,25 @@ struct flow_dissector_key_vlan {
 	__be16	vlan_tpid;
 };
 
-struct flow_dissector_key_mpls {
+struct flow_dissector_mpls_lse {
 	u32	mpls_ttl:8,
 		mpls_bos:1,
 		mpls_tc:3,
 		mpls_label:20;
 };
 
+#define FLOW_DIS_MPLS_MAX 7
+struct flow_dissector_key_mpls {
+	struct flow_dissector_mpls_lse ls[FLOW_DIS_MPLS_MAX]; /* Label Stack */
+	u8 used_lses; /* One bit set for each Label Stack Entry in use */
+};
+
+static inline void dissector_set_mpls_lse(struct flow_dissector_key_mpls *mpls,
+					  int lse_index)
+{
+	mpls->used_lses |= 1 << lse_index;
+}
+
 #define FLOW_DIS_TUN_OPTS_MAX 255
 /**
  * struct flow_dissector_key_enc_opts:
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 3eff84824c8b..e26b6ed22f6b 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -464,47 +464,59 @@ EXPORT_SYMBOL(skb_flow_dissect_tunnel_info);
 static enum flow_dissect_ret
 __skb_flow_dissect_mpls(const struct sk_buff *skb,
 			struct flow_dissector *flow_dissector,
-			void *target_container, void *data, int nhoff, int hlen)
+			void *target_container, void *data, int nhoff, int hlen,
+			int lse_index, bool *entropy_label)
 {
-	struct flow_dissector_key_keyid *key_keyid;
-	struct mpls_label *hdr, _hdr[2];
-	u32 entry, label;
+	struct mpls_label *hdr, _hdr;
+	u32 entry, label, bos;
 
 	if (!dissector_uses_key(flow_dissector,
 				FLOW_DISSECTOR_KEY_MPLS_ENTROPY) &&
 	    !dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_MPLS))
 		return FLOW_DISSECT_RET_OUT_GOOD;
 
+	if (lse_index >= FLOW_DIS_MPLS_MAX)
+		return FLOW_DISSECT_RET_OUT_GOOD;
+
 	hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data,
 				   hlen, &_hdr);
 	if (!hdr)
 		return FLOW_DISSECT_RET_OUT_BAD;
 
-	entry = ntohl(hdr[0].entry);
+	entry = ntohl(hdr->entry);
 	label = (entry & MPLS_LS_LABEL_MASK) >> MPLS_LS_LABEL_SHIFT;
+	bos = (entry & MPLS_LS_S_MASK) >> MPLS_LS_S_SHIFT;
 
 	if (dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_MPLS)) {
 		struct flow_dissector_key_mpls *key_mpls;
+		struct flow_dissector_mpls_lse *lse;
 
 		key_mpls = skb_flow_dissector_target(flow_dissector,
 						     FLOW_DISSECTOR_KEY_MPLS,
 						     target_container);
-		key_mpls->mpls_label = label;
-		key_mpls->mpls_ttl = (entry & MPLS_LS_TTL_MASK)
-					>> MPLS_LS_TTL_SHIFT;
-		key_mpls->mpls_tc = (entry & MPLS_LS_TC_MASK)
-					>> MPLS_LS_TC_SHIFT;
-		key_mpls->mpls_bos = (entry & MPLS_LS_S_MASK)
-					>> MPLS_LS_S_SHIFT;
+		lse = &key_mpls->ls[lse_index];
+
+		lse->mpls_ttl = (entry & MPLS_LS_TTL_MASK) >> MPLS_LS_TTL_SHIFT;
+		lse->mpls_bos = bos;
+		lse->mpls_tc = (entry & MPLS_LS_TC_MASK) >> MPLS_LS_TC_SHIFT;
+		lse->mpls_label = label;
+		dissector_set_mpls_lse(key_mpls, lse_index);
 	}
 
-	if (label == MPLS_LABEL_ENTROPY) {
+	if (*entropy_label &&
+	    dissector_uses_key(flow_dissector,
+			       FLOW_DISSECTOR_KEY_MPLS_ENTROPY)) {
+		struct flow_dissector_key_keyid *key_keyid;
+
 		key_keyid = skb_flow_dissector_target(flow_dissector,
 						      FLOW_DISSECTOR_KEY_MPLS_ENTROPY,
 						      target_container);
-		key_keyid->keyid = hdr[1].entry & htonl(MPLS_LS_LABEL_MASK);
+		key_keyid->keyid = cpu_to_be32(label);
 	}
-	return FLOW_DISSECT_RET_OUT_GOOD;
+
+	*entropy_label = label == MPLS_LABEL_ENTROPY;
+
+	return bos ? FLOW_DISSECT_RET_OUT_GOOD : FLOW_DISSECT_RET_PROTO_AGAIN;
 }
 
 static enum flow_dissect_ret
@@ -963,6 +975,8 @@ bool __skb_flow_dissect(const struct net *net,
 	struct bpf_prog *attached = NULL;
 	enum flow_dissect_ret fdret;
 	enum flow_dissector_key_id dissector_vlan = FLOW_DISSECTOR_KEY_MAX;
+	bool mpls_el = false;
+	int mpls_lse = 0;
 	int num_hdrs = 0;
 	u8 ip_proto = 0;
 	bool ret;
@@ -1262,7 +1276,10 @@ bool __skb_flow_dissect(const struct net *net,
 	case htons(ETH_P_MPLS_MC):
 		fdret = __skb_flow_dissect_mpls(skb, flow_dissector,
 						target_container, data,
-						nhoff, hlen);
+						nhoff, hlen, mpls_lse,
+						&mpls_el);
+		nhoff += sizeof(struct mpls_label);
+		mpls_lse++;
 		break;
 	case htons(ETH_P_FCOE):
 		if ((hlen - nhoff) < FCOE_HEADER_LEN) {
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 0c574700da75..f524afe0b7f5 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -781,9 +781,17 @@ static int fl_set_key_mpls(struct nlattr **tb,
 			   struct flow_dissector_key_mpls *key_mask,
 			   struct netlink_ext_ack *extack)
 {
+	struct flow_dissector_mpls_lse *lse_mask;
+	struct flow_dissector_mpls_lse *lse_val;
+
+	lse_val = &key_val->ls[0];
+	lse_mask = &key_mask->ls[0];
+
 	if (tb[TCA_FLOWER_KEY_MPLS_TTL]) {
-		key_val->mpls_ttl = nla_get_u8(tb[TCA_FLOWER_KEY_MPLS_TTL]);
-		key_mask->mpls_ttl = MPLS_TTL_MASK;
+		lse_val->mpls_ttl = nla_get_u8(tb[TCA_FLOWER_KEY_MPLS_TTL]);
+		lse_mask->mpls_ttl = MPLS_TTL_MASK;
+		dissector_set_mpls_lse(key_val, 0);
+		dissector_set_mpls_lse(key_mask, 0);
 	}
 	if (tb[TCA_FLOWER_KEY_MPLS_BOS]) {
 		u8 bos = nla_get_u8(tb[TCA_FLOWER_KEY_MPLS_BOS]);
@@ -794,8 +802,10 @@ static int fl_set_key_mpls(struct nlattr **tb,
 					    "Bottom Of Stack (BOS) must be 0 or 1");
 			return -EINVAL;
 		}
-		key_val->mpls_bos = bos;
-		key_mask->mpls_bos = MPLS_BOS_MASK;
+		lse_val->mpls_bos = bos;
+		lse_mask->mpls_bos = MPLS_BOS_MASK;
+		dissector_set_mpls_lse(key_val, 0);
+		dissector_set_mpls_lse(key_mask, 0);
 	}
 	if (tb[TCA_FLOWER_KEY_MPLS_TC]) {
 		u8 tc = nla_get_u8(tb[TCA_FLOWER_KEY_MPLS_TC]);
@@ -806,8 +816,10 @@ static int fl_set_key_mpls(struct nlattr **tb,
 					    "Traffic Class (TC) must be between 0 and 7");
 			return -EINVAL;
 		}
-		key_val->mpls_tc = tc;
-		key_mask->mpls_tc = MPLS_TC_MASK;
+		lse_val->mpls_tc = tc;
+		lse_mask->mpls_tc = MPLS_TC_MASK;
+		dissector_set_mpls_lse(key_val, 0);
+		dissector_set_mpls_lse(key_mask, 0);
 	}
 	if (tb[TCA_FLOWER_KEY_MPLS_LABEL]) {
 		u32 label = nla_get_u32(tb[TCA_FLOWER_KEY_MPLS_LABEL]);
@@ -818,8 +830,10 @@ static int fl_set_key_mpls(struct nlattr **tb,
 					    "Label must be between 0 and 1048575");
 			return -EINVAL;
 		}
-		key_val->mpls_label = label;
-		key_mask->mpls_label = MPLS_LABEL_MASK;
+		lse_val->mpls_label = label;
+		lse_mask->mpls_label = MPLS_LABEL_MASK;
+		dissector_set_mpls_lse(key_val, 0);
+		dissector_set_mpls_lse(key_mask, 0);
 	}
 	return 0;
 }
@@ -2222,31 +2236,37 @@ static int fl_dump_key_mpls(struct sk_buff *skb,
 			    struct flow_dissector_key_mpls *mpls_key,
 			    struct flow_dissector_key_mpls *mpls_mask)
 {
+	struct flow_dissector_mpls_lse *lse_mask;
+	struct flow_dissector_mpls_lse *lse_key;
 	int err;
 
 	if (!memchr_inv(mpls_mask, 0, sizeof(*mpls_mask)))
 		return 0;
-	if (mpls_mask->mpls_ttl) {
+
+	lse_mask = &mpls_mask->ls[0];
+	lse_key = &mpls_key->ls[0];
+
+	if (lse_mask->mpls_ttl) {
 		err = nla_put_u8(skb, TCA_FLOWER_KEY_MPLS_TTL,
-				 mpls_key->mpls_ttl);
+				 lse_key->mpls_ttl);
 		if (err)
 			return err;
 	}
-	if (mpls_mask->mpls_tc) {
+	if (lse_mask->mpls_tc) {
 		err = nla_put_u8(skb, TCA_FLOWER_KEY_MPLS_TC,
-				 mpls_key->mpls_tc);
+				 lse_key->mpls_tc);
 		if (err)
 			return err;
 	}
-	if (mpls_mask->mpls_label) {
+	if (lse_mask->mpls_label) {
 		err = nla_put_u32(skb, TCA_FLOWER_KEY_MPLS_LABEL,
-				  mpls_key->mpls_label);
+				  lse_key->mpls_label);
 		if (err)
 			return err;
 	}
-	if (mpls_mask->mpls_bos) {
+	if (lse_mask->mpls_bos) {
 		err = nla_put_u8(skb, TCA_FLOWER_KEY_MPLS_BOS,
-				 mpls_key->mpls_bos);
+				 lse_key->mpls_bos);
 		if (err)
 			return err;
 	}
-- 
2.21.1


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

* [PATCH net-next v2 2/2] cls_flower: Support filtering on multiple MPLS Label Stack Entries
  2020-05-21 17:47 [PATCH net-next v2 0/2] flow_dissector, cls_flower: Add support for multiple MPLS Label Stack Entries Guillaume Nault
  2020-05-21 17:47 ` [PATCH net-next v2 1/2] flow_dissector: Parse " Guillaume Nault
@ 2020-05-21 17:47 ` Guillaume Nault
  2020-05-23 16:35 ` [PATCH net-next v2 0/2] flow_dissector, cls_flower: Add support for " Guillaume Nault
  2020-05-26  0:38 ` David Miller
  3 siblings, 0 replies; 8+ messages in thread
From: Guillaume Nault @ 2020-05-21 17:47 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: netdev, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
	Benjamin LaHaise, Tom Herbert, Pieter Jansen van Vuuren,
	Pablo Neira Ayuso, Liel Shoshan, Rony Efraim

With struct flow_dissector_key_mpls now recording the first
FLOW_DIS_MPLS_MAX labels, we can extend Flower to filter on any of
these LSEs independently.

In order to avoid creating new netlink attributes for every possible
depth, let's define a new TCA_FLOWER_KEY_MPLS_OPTS nested attribute
that contains the list of LSEs to match. Each LSE is represented by
another attribute, TCA_FLOWER_KEY_MPLS_OPTS_LSE, which then contains
the attributes representing the depth and the MPLS fields to match at
this depth (label, TTL, etc.).

For each MPLS field, the mask is always set to all-ones, as this is
what the original API did. We could allow user configurable masks in
the future if there is demand for more flexibility.

The new API also allows to only specify an LSE depth. In that case,
Flower only verifies that the MPLS label stack depth is greater or
equal to the provided depth (that is, an LSE exists at this depth).

Filters that only match on one (or more) fields of the first LSE are
dumped using the old netlink attributes, to avoid confusing user space
programs that don't understand the new API.

Signed-off-by: Guillaume Nault <gnault@redhat.com>
---
 include/uapi/linux/pkt_cls.h |  23 ++++
 net/sched/cls_flower.c       | 243 ++++++++++++++++++++++++++++++++++-
 2 files changed, 265 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index fc672b232437..7576209d96f9 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -576,6 +576,8 @@ enum {
 	TCA_FLOWER_KEY_CT_LABELS,	/* u128 */
 	TCA_FLOWER_KEY_CT_LABELS_MASK,	/* u128 */
 
+	TCA_FLOWER_KEY_MPLS_OPTS,
+
 	__TCA_FLOWER_MAX,
 };
 
@@ -640,6 +642,27 @@ enum {
 #define TCA_FLOWER_KEY_ENC_OPT_ERSPAN_MAX \
 		(__TCA_FLOWER_KEY_ENC_OPT_ERSPAN_MAX - 1)
 
+enum {
+	TCA_FLOWER_KEY_MPLS_OPTS_UNSPEC,
+	TCA_FLOWER_KEY_MPLS_OPTS_LSE,
+	__TCA_FLOWER_KEY_MPLS_OPTS_MAX,
+};
+
+#define TCA_FLOWER_KEY_MPLS_OPTS_MAX (__TCA_FLOWER_KEY_MPLS_OPTS_MAX - 1)
+
+enum {
+	TCA_FLOWER_KEY_MPLS_OPT_LSE_UNSPEC,
+	TCA_FLOWER_KEY_MPLS_OPT_LSE_DEPTH,
+	TCA_FLOWER_KEY_MPLS_OPT_LSE_TTL,
+	TCA_FLOWER_KEY_MPLS_OPT_LSE_BOS,
+	TCA_FLOWER_KEY_MPLS_OPT_LSE_TC,
+	TCA_FLOWER_KEY_MPLS_OPT_LSE_LABEL,
+	__TCA_FLOWER_KEY_MPLS_OPT_LSE_MAX,
+};
+
+#define TCA_FLOWER_KEY_MPLS_OPT_LSE_MAX \
+		(__TCA_FLOWER_KEY_MPLS_OPT_LSE_MAX - 1)
+
 enum {
 	TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT = (1 << 0),
 	TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1),
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index f524afe0b7f5..96f5999281e0 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -668,6 +668,7 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
 	[TCA_FLOWER_KEY_MPLS_BOS]	= { .type = NLA_U8 },
 	[TCA_FLOWER_KEY_MPLS_TC]	= { .type = NLA_U8 },
 	[TCA_FLOWER_KEY_MPLS_LABEL]	= { .type = NLA_U32 },
+	[TCA_FLOWER_KEY_MPLS_OPTS]	= { .type = NLA_NESTED },
 	[TCA_FLOWER_KEY_TCP_FLAGS]	= { .type = NLA_U16 },
 	[TCA_FLOWER_KEY_TCP_FLAGS_MASK]	= { .type = NLA_U16 },
 	[TCA_FLOWER_KEY_IP_TOS]		= { .type = NLA_U8 },
@@ -726,6 +727,20 @@ erspan_opt_policy[TCA_FLOWER_KEY_ENC_OPT_ERSPAN_MAX + 1] = {
 	[TCA_FLOWER_KEY_ENC_OPT_ERSPAN_HWID]       = { .type = NLA_U8 },
 };
 
+static const struct nla_policy
+mpls_opts_policy[TCA_FLOWER_KEY_MPLS_OPTS_MAX + 1] = {
+	[TCA_FLOWER_KEY_MPLS_OPTS_LSE]    = { .type = NLA_NESTED },
+};
+
+static const struct nla_policy
+mpls_stack_entry_policy[TCA_FLOWER_KEY_MPLS_OPT_LSE_MAX + 1] = {
+	[TCA_FLOWER_KEY_MPLS_OPT_LSE_DEPTH]    = { .type = NLA_U8 },
+	[TCA_FLOWER_KEY_MPLS_OPT_LSE_TTL]      = { .type = NLA_U8 },
+	[TCA_FLOWER_KEY_MPLS_OPT_LSE_BOS]      = { .type = NLA_U8 },
+	[TCA_FLOWER_KEY_MPLS_OPT_LSE_TC]       = { .type = NLA_U8 },
+	[TCA_FLOWER_KEY_MPLS_OPT_LSE_LABEL]    = { .type = NLA_U32 },
+};
+
 static void fl_set_key_val(struct nlattr **tb,
 			   void *val, int val_type,
 			   void *mask, int mask_type, int len)
@@ -776,6 +791,126 @@ static int fl_set_key_port_range(struct nlattr **tb, struct fl_flow_key *key,
 	return 0;
 }
 
+static int fl_set_key_mpls_lse(const struct nlattr *nla_lse,
+			       struct flow_dissector_key_mpls *key_val,
+			       struct flow_dissector_key_mpls *key_mask,
+			       struct netlink_ext_ack *extack)
+{
+	struct nlattr *tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_MAX + 1];
+	struct flow_dissector_mpls_lse *lse_mask;
+	struct flow_dissector_mpls_lse *lse_val;
+	u8 lse_index;
+	u8 depth;
+	int err;
+
+	err = nla_parse_nested(tb, TCA_FLOWER_KEY_MPLS_OPT_LSE_MAX, nla_lse,
+			       mpls_stack_entry_policy, extack);
+	if (err < 0)
+		return err;
+
+	if (!tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_DEPTH]) {
+		NL_SET_ERR_MSG(extack, "Missing MPLS option \"depth\"");
+		return -EINVAL;
+	}
+
+	depth = nla_get_u8(tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_DEPTH]);
+
+	/* LSE depth starts at 1, for consistency with terminology used by
+	 * RFC 3031 (section 3.9), where depth 0 refers to unlabeled packets.
+	 */
+	if (depth < 1 || depth > FLOW_DIS_MPLS_MAX) {
+		NL_SET_ERR_MSG_ATTR(extack,
+				    tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_DEPTH],
+				    "Invalid MPLS depth");
+		return -EINVAL;
+	}
+	lse_index = depth - 1;
+
+	dissector_set_mpls_lse(key_val, lse_index);
+	dissector_set_mpls_lse(key_mask, lse_index);
+
+	lse_val = &key_val->ls[lse_index];
+	lse_mask = &key_mask->ls[lse_index];
+
+	if (tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_TTL]) {
+		lse_val->mpls_ttl = nla_get_u8(tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_TTL]);
+		lse_mask->mpls_ttl = MPLS_TTL_MASK;
+	}
+	if (tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_BOS]) {
+		u8 bos = nla_get_u8(tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_BOS]);
+
+		if (bos & ~MPLS_BOS_MASK) {
+			NL_SET_ERR_MSG_ATTR(extack,
+					    tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_BOS],
+					    "Bottom Of Stack (BOS) must be 0 or 1");
+			return -EINVAL;
+		}
+		lse_val->mpls_bos = bos;
+		lse_mask->mpls_bos = MPLS_BOS_MASK;
+	}
+	if (tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_TC]) {
+		u8 tc = nla_get_u8(tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_TC]);
+
+		if (tc & ~MPLS_TC_MASK) {
+			NL_SET_ERR_MSG_ATTR(extack,
+					    tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_TC],
+					    "Traffic Class (TC) must be between 0 and 7");
+			return -EINVAL;
+		}
+		lse_val->mpls_tc = tc;
+		lse_mask->mpls_tc = MPLS_TC_MASK;
+	}
+	if (tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_LABEL]) {
+		u32 label = nla_get_u32(tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_LABEL]);
+
+		if (label & ~MPLS_LABEL_MASK) {
+			NL_SET_ERR_MSG_ATTR(extack,
+					    tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_LABEL],
+					    "Label must be between 0 and 1048575");
+			return -EINVAL;
+		}
+		lse_val->mpls_label = label;
+		lse_mask->mpls_label = MPLS_LABEL_MASK;
+	}
+
+	return 0;
+}
+
+static int fl_set_key_mpls_opts(const struct nlattr *nla_mpls_opts,
+				struct flow_dissector_key_mpls *key_val,
+				struct flow_dissector_key_mpls *key_mask,
+				struct netlink_ext_ack *extack)
+{
+	struct nlattr *nla_lse;
+	int rem;
+	int err;
+
+	if (!(nla_mpls_opts->nla_type & NLA_F_NESTED)) {
+		NL_SET_ERR_MSG_ATTR(extack, nla_mpls_opts,
+				    "NLA_F_NESTED is missing");
+		return -EINVAL;
+	}
+
+	nla_for_each_nested(nla_lse, nla_mpls_opts, rem) {
+		if (nla_type(nla_lse) != TCA_FLOWER_KEY_MPLS_OPTS_LSE) {
+			NL_SET_ERR_MSG_ATTR(extack, nla_lse,
+					    "Invalid MPLS option type");
+			return -EINVAL;
+		}
+
+		err = fl_set_key_mpls_lse(nla_lse, key_val, key_mask, extack);
+		if (err < 0)
+			return err;
+	}
+	if (rem) {
+		NL_SET_ERR_MSG(extack,
+			       "Bytes leftover after parsing MPLS options");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int fl_set_key_mpls(struct nlattr **tb,
 			   struct flow_dissector_key_mpls *key_val,
 			   struct flow_dissector_key_mpls *key_mask,
@@ -784,6 +919,21 @@ static int fl_set_key_mpls(struct nlattr **tb,
 	struct flow_dissector_mpls_lse *lse_mask;
 	struct flow_dissector_mpls_lse *lse_val;
 
+	if (tb[TCA_FLOWER_KEY_MPLS_OPTS]) {
+		if (tb[TCA_FLOWER_KEY_MPLS_TTL] ||
+		    tb[TCA_FLOWER_KEY_MPLS_BOS] ||
+		    tb[TCA_FLOWER_KEY_MPLS_TC] ||
+		    tb[TCA_FLOWER_KEY_MPLS_LABEL]) {
+			NL_SET_ERR_MSG_ATTR(extack,
+					    tb[TCA_FLOWER_KEY_MPLS_OPTS],
+					    "MPLS label, Traffic Class, Bottom Of Stack and Time To Live must be encapsulated in the MPLS options attribute");
+			return -EBADMSG;
+		}
+
+		return fl_set_key_mpls_opts(tb[TCA_FLOWER_KEY_MPLS_OPTS],
+					    key_val, key_mask, extack);
+	}
+
 	lse_val = &key_val->ls[0];
 	lse_mask = &key_mask->ls[0];
 
@@ -2232,6 +2382,89 @@ static int fl_dump_key_port_range(struct sk_buff *skb, struct fl_flow_key *key,
 	return 0;
 }
 
+static int fl_dump_key_mpls_opt_lse(struct sk_buff *skb,
+				    struct flow_dissector_key_mpls *mpls_key,
+				    struct flow_dissector_key_mpls *mpls_mask,
+				    u8 lse_index)
+{
+	struct flow_dissector_mpls_lse *lse_mask = &mpls_mask->ls[lse_index];
+	struct flow_dissector_mpls_lse *lse_key = &mpls_key->ls[lse_index];
+	int err;
+
+	err = nla_put_u8(skb, TCA_FLOWER_KEY_MPLS_OPT_LSE_DEPTH,
+			 lse_index + 1);
+	if (err)
+		return err;
+
+	if (lse_mask->mpls_ttl) {
+		err = nla_put_u8(skb, TCA_FLOWER_KEY_MPLS_OPT_LSE_TTL,
+				 lse_key->mpls_ttl);
+		if (err)
+			return err;
+	}
+	if (lse_mask->mpls_bos) {
+		err = nla_put_u8(skb, TCA_FLOWER_KEY_MPLS_OPT_LSE_BOS,
+				 lse_key->mpls_bos);
+		if (err)
+			return err;
+	}
+	if (lse_mask->mpls_tc) {
+		err = nla_put_u8(skb, TCA_FLOWER_KEY_MPLS_OPT_LSE_TC,
+				 lse_key->mpls_tc);
+		if (err)
+			return err;
+	}
+	if (lse_mask->mpls_label) {
+		err = nla_put_u8(skb, TCA_FLOWER_KEY_MPLS_OPT_LSE_LABEL,
+				 lse_key->mpls_label);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static int fl_dump_key_mpls_opts(struct sk_buff *skb,
+				 struct flow_dissector_key_mpls *mpls_key,
+				 struct flow_dissector_key_mpls *mpls_mask)
+{
+	struct nlattr *opts;
+	struct nlattr *lse;
+	u8 lse_index;
+	int err;
+
+	opts = nla_nest_start(skb, TCA_FLOWER_KEY_MPLS_OPTS);
+	if (!opts)
+		return -EMSGSIZE;
+
+	for (lse_index = 0; lse_index < FLOW_DIS_MPLS_MAX; lse_index++) {
+		if (!(mpls_mask->used_lses & 1 << lse_index))
+			continue;
+
+		lse = nla_nest_start(skb, TCA_FLOWER_KEY_MPLS_OPTS_LSE);
+		if (!lse) {
+			err = -EMSGSIZE;
+			goto err_opts;
+		}
+
+		err = fl_dump_key_mpls_opt_lse(skb, mpls_key, mpls_mask,
+					       lse_index);
+		if (err)
+			goto err_opts_lse;
+		nla_nest_end(skb, lse);
+	}
+	nla_nest_end(skb, opts);
+
+	return 0;
+
+err_opts_lse:
+	nla_nest_cancel(skb, lse);
+err_opts:
+	nla_nest_cancel(skb, opts);
+
+	return err;
+}
+
 static int fl_dump_key_mpls(struct sk_buff *skb,
 			    struct flow_dissector_key_mpls *mpls_key,
 			    struct flow_dissector_key_mpls *mpls_mask)
@@ -2240,12 +2473,20 @@ static int fl_dump_key_mpls(struct sk_buff *skb,
 	struct flow_dissector_mpls_lse *lse_key;
 	int err;
 
-	if (!memchr_inv(mpls_mask, 0, sizeof(*mpls_mask)))
+	if (!mpls_mask->used_lses)
 		return 0;
 
 	lse_mask = &mpls_mask->ls[0];
 	lse_key = &mpls_key->ls[0];
 
+	/* For backward compatibility, don't use the MPLS nested attributes if
+	 * the rule can be expressed using the old attributes.
+	 */
+	if (mpls_mask->used_lses & ~1 ||
+	    (!lse_mask->mpls_ttl && !lse_mask->mpls_bos &&
+	     !lse_mask->mpls_tc && !lse_mask->mpls_label))
+		return fl_dump_key_mpls_opts(skb, mpls_key, mpls_mask);
+
 	if (lse_mask->mpls_ttl) {
 		err = nla_put_u8(skb, TCA_FLOWER_KEY_MPLS_TTL,
 				 lse_key->mpls_ttl);
-- 
2.21.1


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

* Re: [PATCH net-next v2 1/2] flow_dissector: Parse multiple MPLS Label Stack Entries
  2020-05-21 17:47 ` [PATCH net-next v2 1/2] flow_dissector: Parse " Guillaume Nault
@ 2020-05-23  9:55   ` Guillaume Nault
  0 siblings, 0 replies; 8+ messages in thread
From: Guillaume Nault @ 2020-05-23  9:55 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: John Hurley, netdev, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
	Tom Herbert, Pablo Neira Ayuso, Liel Shoshan, Rony Efraim

[Cc: John Hurley <john.hurley@netronome.com> for NFP driver review,
 as Pieter Jansen van Vuuren and Benjamin LaHaise emails bounce]

On Thu, May 21, 2020 at 07:47:17PM +0200, Guillaume Nault wrote:
> The current MPLS dissector only parses the first MPLS Label Stack
> Entry (second LSE can be parsed too, but only to set a key_id).
> 
> This patch adds the possibility to parse several LSEs by making
> __skb_flow_dissect_mpls() return FLOW_DISSECT_RET_PROTO_AGAIN as long
> as the Bottom Of Stack bit hasn't been seen, up to a maximum of
> FLOW_DIS_MPLS_MAX entries.
> 
> FLOW_DIS_MPLS_MAX is arbitrarily set to 7. This should be enough for
> many practical purposes, without wasting too much space.
> 
> To record the parsed values, flow_dissector_key_mpls is modified to
> store an array of stack entries, instead of just the values of the
> first one. A bit field, "used_lses", is also added to keep track of
> the LSEs that have been set. The objective is to avoid defining a
> new FLOW_DISSECTOR_KEY_MPLS_XX for each level of the MPLS stack.
> 
> TC flower is adapted for the new struct flow_dissector_key_mpls layout.
> Matching on several MPLS Label Stack Entries will be added in the next
> patch.
> 
> The NFP driver is also adapted: nfp_flower_compile_mac() now verifies
> that the rule only uses the first LSE and fails if it doesn't.
> 
> Finally, the behaviour of the FLOW_DISSECTOR_KEY_MPLS_ENTROPY key is
> slightly modified. Instead of recording the first Entropy Label, it
> now records the last one. This shouldn't have any consequences since
> there doesn't seem to have any user of FLOW_DISSECTOR_KEY_MPLS_ENTROPY
> in the tree. We'd probably better do a hash of all parsed MPLS labels
> instead (excluding reserved labels) anyway. That'd give better entropy
> and would probably also simplify the code. But that's not the purpose
> of this patch, so I'm keeping that as a future possible improvement.
> 
> Signed-off-by: Guillaume Nault <gnault@redhat.com>
> ---
>  .../net/ethernet/netronome/nfp/flower/match.c | 42 +++++++++++----
>  include/net/flow_dissector.h                  | 14 ++++-
>  net/core/flow_dissector.c                     | 49 +++++++++++------
>  net/sched/cls_flower.c                        | 52 +++++++++++++------
>  4 files changed, 113 insertions(+), 44 deletions(-)
> 
> Note: the NFP driver update was only compile-tested. Review from
> Netronome highly encouraged.
> 
> diff --git a/drivers/net/ethernet/netronome/nfp/flower/match.c b/drivers/net/ethernet/netronome/nfp/flower/match.c
> index 546bc01d507d..f7f01e2e3dce 100644
> --- a/drivers/net/ethernet/netronome/nfp/flower/match.c
> +++ b/drivers/net/ethernet/netronome/nfp/flower/match.c
> @@ -74,9 +74,10 @@ nfp_flower_compile_port(struct nfp_flower_in_port *frame, u32 cmsg_port,
>  	return 0;
>  }
>  
> -static void
> +static int
>  nfp_flower_compile_mac(struct nfp_flower_mac_mpls *ext,
> -		       struct nfp_flower_mac_mpls *msk, struct flow_rule *rule)
> +		       struct nfp_flower_mac_mpls *msk, struct flow_rule *rule,
> +		       struct netlink_ext_ack *extack)
>  {
>  	memset(ext, 0, sizeof(struct nfp_flower_mac_mpls));
>  	memset(msk, 0, sizeof(struct nfp_flower_mac_mpls));
> @@ -97,14 +98,28 @@ nfp_flower_compile_mac(struct nfp_flower_mac_mpls *ext,
>  		u32 t_mpls;
>  
>  		flow_rule_match_mpls(rule, &match);
> -		t_mpls = FIELD_PREP(NFP_FLOWER_MASK_MPLS_LB, match.key->mpls_label) |
> -			 FIELD_PREP(NFP_FLOWER_MASK_MPLS_TC, match.key->mpls_tc) |
> -			 FIELD_PREP(NFP_FLOWER_MASK_MPLS_BOS, match.key->mpls_bos) |
> +
> +		/* Only support matching the first LSE */
> +		if (match.mask->used_lses != 1) {
> +			NL_SET_ERR_MSG_MOD(extack,
> +					   "unsupported offload: invalid LSE depth for MPLS match offload");
> +			return -EOPNOTSUPP;
> +		}
> +
> +		t_mpls = FIELD_PREP(NFP_FLOWER_MASK_MPLS_LB,
> +				    match.key->ls[0].mpls_label) |
> +			 FIELD_PREP(NFP_FLOWER_MASK_MPLS_TC,
> +				    match.key->ls[0].mpls_tc) |
> +			 FIELD_PREP(NFP_FLOWER_MASK_MPLS_BOS,
> +				    match.key->ls[0].mpls_bos) |
>  			 NFP_FLOWER_MASK_MPLS_Q;
>  		ext->mpls_lse = cpu_to_be32(t_mpls);
> -		t_mpls = FIELD_PREP(NFP_FLOWER_MASK_MPLS_LB, match.mask->mpls_label) |
> -			 FIELD_PREP(NFP_FLOWER_MASK_MPLS_TC, match.mask->mpls_tc) |
> -			 FIELD_PREP(NFP_FLOWER_MASK_MPLS_BOS, match.mask->mpls_bos) |
> +		t_mpls = FIELD_PREP(NFP_FLOWER_MASK_MPLS_LB,
> +				    match.mask->ls[0].mpls_label) |
> +			 FIELD_PREP(NFP_FLOWER_MASK_MPLS_TC,
> +				    match.mask->ls[0].mpls_tc) |
> +			 FIELD_PREP(NFP_FLOWER_MASK_MPLS_BOS,
> +				    match.mask->ls[0].mpls_bos) |
>  			 NFP_FLOWER_MASK_MPLS_Q;
>  		msk->mpls_lse = cpu_to_be32(t_mpls);
>  	} else if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_BASIC)) {
> @@ -121,6 +136,8 @@ nfp_flower_compile_mac(struct nfp_flower_mac_mpls *ext,
>  			msk->mpls_lse = cpu_to_be32(NFP_FLOWER_MASK_MPLS_Q);
>  		}
>  	}
> +
> +	return 0;
>  }
>  
>  static void
> @@ -461,9 +478,12 @@ int nfp_flower_compile_flow_match(struct nfp_app *app,
>  	msk += sizeof(struct nfp_flower_in_port);
>  
>  	if (NFP_FLOWER_LAYER_MAC & key_ls->key_layer) {
> -		nfp_flower_compile_mac((struct nfp_flower_mac_mpls *)ext,
> -				       (struct nfp_flower_mac_mpls *)msk,
> -				       rule);
> +		err = nfp_flower_compile_mac((struct nfp_flower_mac_mpls *)ext,
> +					     (struct nfp_flower_mac_mpls *)msk,
> +					     rule, extack);
> +		if (err)
> +			return err;
> +
>  		ext += sizeof(struct nfp_flower_mac_mpls);
>  		msk += sizeof(struct nfp_flower_mac_mpls);
>  	}
> diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> index 628383915827..4fb1a69c6ecf 100644
> --- a/include/net/flow_dissector.h
> +++ b/include/net/flow_dissector.h
> @@ -59,13 +59,25 @@ struct flow_dissector_key_vlan {
>  	__be16	vlan_tpid;
>  };
>  
> -struct flow_dissector_key_mpls {
> +struct flow_dissector_mpls_lse {
>  	u32	mpls_ttl:8,
>  		mpls_bos:1,
>  		mpls_tc:3,
>  		mpls_label:20;
>  };
>  
> +#define FLOW_DIS_MPLS_MAX 7
> +struct flow_dissector_key_mpls {
> +	struct flow_dissector_mpls_lse ls[FLOW_DIS_MPLS_MAX]; /* Label Stack */
> +	u8 used_lses; /* One bit set for each Label Stack Entry in use */
> +};
> +
> +static inline void dissector_set_mpls_lse(struct flow_dissector_key_mpls *mpls,
> +					  int lse_index)
> +{
> +	mpls->used_lses |= 1 << lse_index;
> +}
> +
>  #define FLOW_DIS_TUN_OPTS_MAX 255
>  /**
>   * struct flow_dissector_key_enc_opts:
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 3eff84824c8b..e26b6ed22f6b 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -464,47 +464,59 @@ EXPORT_SYMBOL(skb_flow_dissect_tunnel_info);
>  static enum flow_dissect_ret
>  __skb_flow_dissect_mpls(const struct sk_buff *skb,
>  			struct flow_dissector *flow_dissector,
> -			void *target_container, void *data, int nhoff, int hlen)
> +			void *target_container, void *data, int nhoff, int hlen,
> +			int lse_index, bool *entropy_label)
>  {
> -	struct flow_dissector_key_keyid *key_keyid;
> -	struct mpls_label *hdr, _hdr[2];
> -	u32 entry, label;
> +	struct mpls_label *hdr, _hdr;
> +	u32 entry, label, bos;
>  
>  	if (!dissector_uses_key(flow_dissector,
>  				FLOW_DISSECTOR_KEY_MPLS_ENTROPY) &&
>  	    !dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_MPLS))
>  		return FLOW_DISSECT_RET_OUT_GOOD;
>  
> +	if (lse_index >= FLOW_DIS_MPLS_MAX)
> +		return FLOW_DISSECT_RET_OUT_GOOD;
> +
>  	hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data,
>  				   hlen, &_hdr);
>  	if (!hdr)
>  		return FLOW_DISSECT_RET_OUT_BAD;
>  
> -	entry = ntohl(hdr[0].entry);
> +	entry = ntohl(hdr->entry);
>  	label = (entry & MPLS_LS_LABEL_MASK) >> MPLS_LS_LABEL_SHIFT;
> +	bos = (entry & MPLS_LS_S_MASK) >> MPLS_LS_S_SHIFT;
>  
>  	if (dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_MPLS)) {
>  		struct flow_dissector_key_mpls *key_mpls;
> +		struct flow_dissector_mpls_lse *lse;
>  
>  		key_mpls = skb_flow_dissector_target(flow_dissector,
>  						     FLOW_DISSECTOR_KEY_MPLS,
>  						     target_container);
> -		key_mpls->mpls_label = label;
> -		key_mpls->mpls_ttl = (entry & MPLS_LS_TTL_MASK)
> -					>> MPLS_LS_TTL_SHIFT;
> -		key_mpls->mpls_tc = (entry & MPLS_LS_TC_MASK)
> -					>> MPLS_LS_TC_SHIFT;
> -		key_mpls->mpls_bos = (entry & MPLS_LS_S_MASK)
> -					>> MPLS_LS_S_SHIFT;
> +		lse = &key_mpls->ls[lse_index];
> +
> +		lse->mpls_ttl = (entry & MPLS_LS_TTL_MASK) >> MPLS_LS_TTL_SHIFT;
> +		lse->mpls_bos = bos;
> +		lse->mpls_tc = (entry & MPLS_LS_TC_MASK) >> MPLS_LS_TC_SHIFT;
> +		lse->mpls_label = label;
> +		dissector_set_mpls_lse(key_mpls, lse_index);
>  	}
>  
> -	if (label == MPLS_LABEL_ENTROPY) {
> +	if (*entropy_label &&
> +	    dissector_uses_key(flow_dissector,
> +			       FLOW_DISSECTOR_KEY_MPLS_ENTROPY)) {
> +		struct flow_dissector_key_keyid *key_keyid;
> +
>  		key_keyid = skb_flow_dissector_target(flow_dissector,
>  						      FLOW_DISSECTOR_KEY_MPLS_ENTROPY,
>  						      target_container);
> -		key_keyid->keyid = hdr[1].entry & htonl(MPLS_LS_LABEL_MASK);
> +		key_keyid->keyid = cpu_to_be32(label);
>  	}
> -	return FLOW_DISSECT_RET_OUT_GOOD;
> +
> +	*entropy_label = label == MPLS_LABEL_ENTROPY;
> +
> +	return bos ? FLOW_DISSECT_RET_OUT_GOOD : FLOW_DISSECT_RET_PROTO_AGAIN;
>  }
>  
>  static enum flow_dissect_ret
> @@ -963,6 +975,8 @@ bool __skb_flow_dissect(const struct net *net,
>  	struct bpf_prog *attached = NULL;
>  	enum flow_dissect_ret fdret;
>  	enum flow_dissector_key_id dissector_vlan = FLOW_DISSECTOR_KEY_MAX;
> +	bool mpls_el = false;
> +	int mpls_lse = 0;
>  	int num_hdrs = 0;
>  	u8 ip_proto = 0;
>  	bool ret;
> @@ -1262,7 +1276,10 @@ bool __skb_flow_dissect(const struct net *net,
>  	case htons(ETH_P_MPLS_MC):
>  		fdret = __skb_flow_dissect_mpls(skb, flow_dissector,
>  						target_container, data,
> -						nhoff, hlen);
> +						nhoff, hlen, mpls_lse,
> +						&mpls_el);
> +		nhoff += sizeof(struct mpls_label);
> +		mpls_lse++;
>  		break;
>  	case htons(ETH_P_FCOE):
>  		if ((hlen - nhoff) < FCOE_HEADER_LEN) {
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index 0c574700da75..f524afe0b7f5 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -781,9 +781,17 @@ static int fl_set_key_mpls(struct nlattr **tb,
>  			   struct flow_dissector_key_mpls *key_mask,
>  			   struct netlink_ext_ack *extack)
>  {
> +	struct flow_dissector_mpls_lse *lse_mask;
> +	struct flow_dissector_mpls_lse *lse_val;
> +
> +	lse_val = &key_val->ls[0];
> +	lse_mask = &key_mask->ls[0];
> +
>  	if (tb[TCA_FLOWER_KEY_MPLS_TTL]) {
> -		key_val->mpls_ttl = nla_get_u8(tb[TCA_FLOWER_KEY_MPLS_TTL]);
> -		key_mask->mpls_ttl = MPLS_TTL_MASK;
> +		lse_val->mpls_ttl = nla_get_u8(tb[TCA_FLOWER_KEY_MPLS_TTL]);
> +		lse_mask->mpls_ttl = MPLS_TTL_MASK;
> +		dissector_set_mpls_lse(key_val, 0);
> +		dissector_set_mpls_lse(key_mask, 0);
>  	}
>  	if (tb[TCA_FLOWER_KEY_MPLS_BOS]) {
>  		u8 bos = nla_get_u8(tb[TCA_FLOWER_KEY_MPLS_BOS]);
> @@ -794,8 +802,10 @@ static int fl_set_key_mpls(struct nlattr **tb,
>  					    "Bottom Of Stack (BOS) must be 0 or 1");
>  			return -EINVAL;
>  		}
> -		key_val->mpls_bos = bos;
> -		key_mask->mpls_bos = MPLS_BOS_MASK;
> +		lse_val->mpls_bos = bos;
> +		lse_mask->mpls_bos = MPLS_BOS_MASK;
> +		dissector_set_mpls_lse(key_val, 0);
> +		dissector_set_mpls_lse(key_mask, 0);
>  	}
>  	if (tb[TCA_FLOWER_KEY_MPLS_TC]) {
>  		u8 tc = nla_get_u8(tb[TCA_FLOWER_KEY_MPLS_TC]);
> @@ -806,8 +816,10 @@ static int fl_set_key_mpls(struct nlattr **tb,
>  					    "Traffic Class (TC) must be between 0 and 7");
>  			return -EINVAL;
>  		}
> -		key_val->mpls_tc = tc;
> -		key_mask->mpls_tc = MPLS_TC_MASK;
> +		lse_val->mpls_tc = tc;
> +		lse_mask->mpls_tc = MPLS_TC_MASK;
> +		dissector_set_mpls_lse(key_val, 0);
> +		dissector_set_mpls_lse(key_mask, 0);
>  	}
>  	if (tb[TCA_FLOWER_KEY_MPLS_LABEL]) {
>  		u32 label = nla_get_u32(tb[TCA_FLOWER_KEY_MPLS_LABEL]);
> @@ -818,8 +830,10 @@ static int fl_set_key_mpls(struct nlattr **tb,
>  					    "Label must be between 0 and 1048575");
>  			return -EINVAL;
>  		}
> -		key_val->mpls_label = label;
> -		key_mask->mpls_label = MPLS_LABEL_MASK;
> +		lse_val->mpls_label = label;
> +		lse_mask->mpls_label = MPLS_LABEL_MASK;
> +		dissector_set_mpls_lse(key_val, 0);
> +		dissector_set_mpls_lse(key_mask, 0);
>  	}
>  	return 0;
>  }
> @@ -2222,31 +2236,37 @@ static int fl_dump_key_mpls(struct sk_buff *skb,
>  			    struct flow_dissector_key_mpls *mpls_key,
>  			    struct flow_dissector_key_mpls *mpls_mask)
>  {
> +	struct flow_dissector_mpls_lse *lse_mask;
> +	struct flow_dissector_mpls_lse *lse_key;
>  	int err;
>  
>  	if (!memchr_inv(mpls_mask, 0, sizeof(*mpls_mask)))
>  		return 0;
> -	if (mpls_mask->mpls_ttl) {
> +
> +	lse_mask = &mpls_mask->ls[0];
> +	lse_key = &mpls_key->ls[0];
> +
> +	if (lse_mask->mpls_ttl) {
>  		err = nla_put_u8(skb, TCA_FLOWER_KEY_MPLS_TTL,
> -				 mpls_key->mpls_ttl);
> +				 lse_key->mpls_ttl);
>  		if (err)
>  			return err;
>  	}
> -	if (mpls_mask->mpls_tc) {
> +	if (lse_mask->mpls_tc) {
>  		err = nla_put_u8(skb, TCA_FLOWER_KEY_MPLS_TC,
> -				 mpls_key->mpls_tc);
> +				 lse_key->mpls_tc);
>  		if (err)
>  			return err;
>  	}
> -	if (mpls_mask->mpls_label) {
> +	if (lse_mask->mpls_label) {
>  		err = nla_put_u32(skb, TCA_FLOWER_KEY_MPLS_LABEL,
> -				  mpls_key->mpls_label);
> +				  lse_key->mpls_label);
>  		if (err)
>  			return err;
>  	}
> -	if (mpls_mask->mpls_bos) {
> +	if (lse_mask->mpls_bos) {
>  		err = nla_put_u8(skb, TCA_FLOWER_KEY_MPLS_BOS,
> -				 mpls_key->mpls_bos);
> +				 lse_key->mpls_bos);
>  		if (err)
>  			return err;
>  	}
> -- 
> 2.21.1
> 


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

* Re: [PATCH net-next v2 0/2] flow_dissector, cls_flower: Add support for multiple MPLS Label Stack Entries
  2020-05-21 17:47 [PATCH net-next v2 0/2] flow_dissector, cls_flower: Add support for multiple MPLS Label Stack Entries Guillaume Nault
  2020-05-21 17:47 ` [PATCH net-next v2 1/2] flow_dissector: Parse " Guillaume Nault
  2020-05-21 17:47 ` [PATCH net-next v2 2/2] cls_flower: Support filtering on " Guillaume Nault
@ 2020-05-23 16:35 ` Guillaume Nault
  2020-05-26  0:38 ` David Miller
  3 siblings, 0 replies; 8+ messages in thread
From: Guillaume Nault @ 2020-05-23 16:35 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: netdev, Jamal Hadi Salim, Cong Wang, Jiri Pirko, Tom Herbert,
	Pablo Neira Ayuso, Liel Shoshan, Rony Efraim

[ Dropping the @netronome.com addresses that bounce. ]

On Thu, May 21, 2020 at 07:47:12PM +0200, Guillaume Nault wrote:
> Currently, the flow dissector and the Flower classifier can only handle
> the first entry of an MPLS label stack. This patch series generalises
> the code to allow parsing and matching the Label Stack Entries that
> follow.
> 

Here's a draft patch, adding support for iproute2. I'll polish it and
submit it formally once the kernel part is merged.

It adds the new "mpls" keyword that can be used to match MPLS fields in
arbitrary Label Stack Entries.
LSEs are introduced by the "lse" keyword and followed by LSE options:
"depth", "label", "tc", "bos" and "ttl". The depth is manadtory but
other options are optionals.

For example, the following filter drops MPLS packets having two labels,
where the first label is 20 and has TTL 64 and the second label is 21:

$ tc filter add dev ethX ingress proto mpls_uc flower mpls \
    lse depth 1 label 20 ttl 64 \
    lse depth 2 label 21 bos 1 \
    action drop

Signed-off-by: Guillaume Nault <gnault@redhat.com>
---
 include/uapi/linux/pkt_cls.h |  23 ++++
 man/man8/tc-flower.8         |  55 +++++++-
 tc/f_flower.c                | 244 ++++++++++++++++++++++++++++++++++-
 3 files changed, 317 insertions(+), 5 deletions(-)

diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index fc672b23..7576209d 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -576,6 +576,8 @@ enum {
 	TCA_FLOWER_KEY_CT_LABELS,	/* u128 */
 	TCA_FLOWER_KEY_CT_LABELS_MASK,	/* u128 */
 
+	TCA_FLOWER_KEY_MPLS_OPTS,
+
 	__TCA_FLOWER_MAX,
 };
 
@@ -640,6 +642,27 @@ enum {
 #define TCA_FLOWER_KEY_ENC_OPT_ERSPAN_MAX \
 		(__TCA_FLOWER_KEY_ENC_OPT_ERSPAN_MAX - 1)
 
+enum {
+	TCA_FLOWER_KEY_MPLS_OPTS_UNSPEC,
+	TCA_FLOWER_KEY_MPLS_OPTS_LSE,
+	__TCA_FLOWER_KEY_MPLS_OPTS_MAX,
+};
+
+#define TCA_FLOWER_KEY_MPLS_OPTS_MAX (__TCA_FLOWER_KEY_MPLS_OPTS_MAX - 1)
+
+enum {
+	TCA_FLOWER_KEY_MPLS_OPT_LSE_UNSPEC,
+	TCA_FLOWER_KEY_MPLS_OPT_LSE_DEPTH,
+	TCA_FLOWER_KEY_MPLS_OPT_LSE_TTL,
+	TCA_FLOWER_KEY_MPLS_OPT_LSE_BOS,
+	TCA_FLOWER_KEY_MPLS_OPT_LSE_TC,
+	TCA_FLOWER_KEY_MPLS_OPT_LSE_LABEL,
+	__TCA_FLOWER_KEY_MPLS_OPT_LSE_MAX,
+};
+
+#define TCA_FLOWER_KEY_MPLS_OPT_LSE_MAX \
+		(__TCA_FLOWER_KEY_MPLS_OPT_LSE_MAX - 1)
+
 enum {
 	TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT = (1 << 0),
 	TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1),
diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index b3dfcf68..a168fbfb 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -40,6 +40,8 @@ flower \- flow based traffic control filter
 .IR PRIORITY " | "
 .BR cvlan_ethtype " { " ipv4 " | " ipv6 " | "
 .IR ETH_TYPE " } | "
+.B mpls
+.IR LSE_LIST " | "
 .B mpls_label
 .IR LABEL " | "
 .B mpls_tc
@@ -90,7 +92,24 @@ flower \- flow based traffic control filter
 }
 .IR OPTIONS " | "
 .BR ip_flags
-.IR IP_FLAGS
+.IR IP_FLAGS " } "
+
+.ti -8
+.IR LSE_LIST " := [ " LSE_LIST " ] " LSE
+
+.ti -8
+.IR LSE " := "
+.B lse depth
+.IR DEPTH " { "
+.B label
+.IR LABEL " | "
+.B tc
+.IR TC " | "
+.B bos
+.IR BOS " | "
+.B ttl
+.IR TTL " }"
+
 .SH DESCRIPTION
 The
 .B flower
@@ -176,6 +195,40 @@ Match on QinQ layer three protocol.
 may be either
 .BR ipv4 ", " ipv6
 or an unsigned 16bit value in hexadecimal format.
+
+.TP
+.BI mpls " LSE_LIST"
+Match an MPLS label stack.
+.I LSE_LIST
+is a list of Label Stack Entries, each introduced by the
+.BR lse " keyword."
+This is incompatible with the standalone
+.BR mpls_label ", " mpls_tc ", " mpls_bos " and " mpls_ttl " options."
+.RS
+.TP
+.BI lse " LSE_OPTIONS"
+Match an MPLS Label Stack Entry.
+.RS
+.TP
+.BI depth " DEPTH"
+The depth of the Label Stack Entry to consider. Depth starts at 1 (the
+outermost Label Stack Entry). The maximum usable depth may be limitted by the
+kernel. This option is mandatory.
+.TP
+.BI label " LABEL"
+Match the label id.
+.TP
+.BI tc " TC"
+Match the Traffic Class.
+.TP
+.BI bos " BOS"
+Match the Bottom Of Stack.
+.TP
+.BI ttl " TTL"
+Match the Time To Live.
+.RE
+.RE
+
 .TP
 .BI mpls_label " LABEL"
 Match the label id in the outermost MPLS label stack entry.
diff --git a/tc/f_flower.c b/tc/f_flower.c
index fc136911..1707812e 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -59,6 +59,7 @@ static void explain(void)
 		"			ip_proto [tcp | udp | sctp | icmp | icmpv6 | IP-PROTO ] |\n"
 		"			ip_tos MASKED-IP_TOS |\n"
 		"			ip_ttl MASKED-IP_TTL |\n"
+		"			mpls LSE-LIST |\n"
 		"			mpls_label LABEL |\n"
 		"			mpls_tc TC |\n"
 		"			mpls_bos BOS |\n"
@@ -89,6 +90,8 @@ static void explain(void)
 		"			ct_label MASKED_CT_LABEL |\n"
 		"			ct_mark MASKED_CT_MARK |\n"
 		"			ct_zone MASKED_CT_ZONE }\n"
+		"	LSE-LIST := [ LSE-LIST ] LSE\n"
+		"	LSE := lse depth DEPTH { label LABEL | tc TC | bos BOS | ttl TTL }\n"
 		"	FILTERID := X:Y:Z\n"
 		"	MASKED_LLADDR := { LLADDR | LLADDR/MASK | LLADDR/BITS }\n"
 		"	MASKED_CT_STATE := combination of {+|-} and flags trk,est,new\n"
@@ -1199,11 +1202,139 @@ static int flower_parse_enc_opts_erspan(char *str, struct nlmsghdr *n)
 	return 0;
 }
 
+static int flower_parse_mpls_lse(int *argc_p, char ***argv_p,
+				 struct nlmsghdr *nlh)
+{
+	struct rtattr *lse_attr;
+	char **argv = *argv_p;
+	int argc = *argc_p;
+	__u8 depth = 0;
+	int ret;
+
+	if (argc <= 0) {
+		fprintf(stderr, "Not enough information: \"label\", \"tc\", \"bos\" or \"ttl\" argument is required.\n");
+		return -1;
+	}
+
+	lse_attr = addattr_nest(nlh, MAX_MSG, TCA_FLOWER_KEY_MPLS_OPTS_LSE | NLA_F_NESTED);
+
+	while (argc > 0) {
+		if (strcmp(*argv, "depth") == 0) {
+			NEXT_ARG();
+			ret = get_u8(&depth, *argv, 0);
+			if (ret < 0 || depth < 1) {
+				fprintf(stderr, "Illegal \"depth\"\n");
+				return -1;
+			}
+			addattr8(nlh, MAX_MSG,
+				 TCA_FLOWER_KEY_MPLS_OPT_LSE_DEPTH, depth);
+		} else if (strcmp(*argv, "label") == 0) {
+			__u32 label;
+
+			NEXT_ARG();
+			ret = get_u32(&label, *argv, 0);
+			if (ret < 0 ||
+			    label & ~(MPLS_LS_LABEL_MASK >> MPLS_LS_LABEL_SHIFT)) {
+				fprintf(stderr, "Illegal \"label\"\n");
+				return -1;
+			}
+			addattr32(nlh, MAX_MSG,
+				  TCA_FLOWER_KEY_MPLS_OPT_LSE_LABEL, label);
+		} else if (strcmp(*argv, "tc") == 0) {
+			__u8 tc;
+
+			NEXT_ARG();
+			ret = get_u8(&tc, *argv, 0);
+			if (ret < 0 ||
+			    tc & ~(MPLS_LS_TC_MASK >> MPLS_LS_TC_SHIFT)) {
+				fprintf(stderr, "Illegal \"tc\"\n");
+				return -1;
+			}
+			addattr8(nlh, MAX_MSG, TCA_FLOWER_KEY_MPLS_OPT_LSE_TC,
+				 tc);
+		} else if (strcmp(*argv, "bos") == 0) {
+			__u8 bos;
+
+			NEXT_ARG();
+			ret = get_u8(&bos, *argv, 0);
+			if (ret < 0 || bos & ~(MPLS_LS_S_MASK >> MPLS_LS_S_SHIFT)) {
+				fprintf(stderr, "Illegal \"bos\"\n");
+				return -1;
+			}
+			addattr8(nlh, MAX_MSG, TCA_FLOWER_KEY_MPLS_OPT_LSE_BOS,
+				 bos);
+		} else if (strcmp(*argv, "ttl") == 0) {
+			__u8 ttl;
+
+			NEXT_ARG();
+
+			ret = get_u8(&ttl, *argv, 0);
+			if (ret < 0 || ttl & ~(MPLS_LS_TTL_MASK >> MPLS_LS_TTL_SHIFT)) {
+				fprintf(stderr, "Illegal \"ttl\"\n");
+				return -1;
+			}
+			addattr8(nlh, MAX_MSG, TCA_FLOWER_KEY_MPLS_OPT_LSE_TTL,
+				 ttl);
+		} else {
+			break;
+		}
+		argc--; argv++;
+	}
+
+	if (!depth) {
+		missarg("depth");
+		return -1;
+	}
+
+	addattr_nest_end(nlh, lse_attr);
+
+	*argc_p = argc;
+	*argv_p = argv;
+
+	return 0;
+}
+
+static int flower_parse_mpls(int *argc_p, char ***argv_p, struct nlmsghdr *nlh)
+{
+	struct rtattr *mpls_attr;
+	char **argv = *argv_p;
+	int argc = *argc_p;
+
+	if (argc <= 0) {
+		fprintf(stderr, "Not enough information: \"depth\" argument is required.\n");
+		return -1;
+	}
+
+	mpls_attr = addattr_nest(nlh, MAX_MSG, TCA_FLOWER_KEY_MPLS_OPTS | NLA_F_NESTED);
+
+	while (argc > 0) {
+		if (strcmp(*argv, "lse") == 0) {
+
+			NEXT_ARG();
+			if (flower_parse_mpls_lse(&argc, &argv, nlh) < 0)
+				return -1;
+			continue;
+		} else {
+			break;
+		}
+		argc--; argv++;
+	}
+
+	addattr_nest_end(nlh, mpls_attr);
+
+	*argc_p = argc;
+	*argv_p = argv;
+
+	return 0;
+}
+
 static int flower_parse_opt(struct filter_util *qu, char *handle,
 			    int argc, char **argv, struct nlmsghdr *n)
 {
 	int ret;
 	struct tcmsg *t = NLMSG_DATA(n);
+	bool mpls_format_old = false;
+	bool mpls_format_new = false;
 	struct rtattr *tail;
 	__be16 eth_type = TC_H_MIN(t->tcm_info);
 	__be16 vlan_ethtype = 0;
@@ -1381,6 +1512,23 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
 						 &cvlan_ethtype, n);
 			if (ret < 0)
 				return -1;
+		} else if (matches(*argv, "mpls") == 0) {
+			NEXT_ARG();
+			if (eth_type != htons(ETH_P_MPLS_UC) &&
+			    eth_type != htons(ETH_P_MPLS_MC)) {
+				fprintf(stderr,
+					"Can't set \"mpls\" if ethertype isn't MPLS\n");
+				return -1;
+			}
+			if (mpls_format_old) {
+				fprintf(stderr,
+					"Can't set \"mpls\" if \"mpls_label\", \"mpls_tc\", \"mpls_bos\" or \"mpls_ttl\" is set\n");
+				return -1;
+			}
+			mpls_format_new = true;
+			if (flower_parse_mpls(&argc, &argv, n) < 0)
+				return -1;
+			continue;
 		} else if (matches(*argv, "mpls_label") == 0) {
 			__u32 label;
 
@@ -1391,7 +1539,13 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
 					"Can't set \"mpls_label\" if ethertype isn't MPLS\n");
 				return -1;
 			}
-			ret = get_u32(&label, *argv, 10);
+			if (mpls_format_new) {
+				fprintf(stderr,
+					"Can't set \"mpls_label\" if \"mpls\" is set\n");
+				return -1;
+			}
+			mpls_format_old = true;
+			ret = get_u32(&label, *argv, 0);
 			if (ret < 0 || label & ~(MPLS_LS_LABEL_MASK >> MPLS_LS_LABEL_SHIFT)) {
 				fprintf(stderr, "Illegal \"mpls_label\"\n");
 				return -1;
@@ -1407,7 +1561,13 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
 					"Can't set \"mpls_tc\" if ethertype isn't MPLS\n");
 				return -1;
 			}
-			ret = get_u8(&tc, *argv, 10);
+			if (mpls_format_new) {
+				fprintf(stderr,
+					"Can't set \"mpls_tc\" if \"mpls\" is set\n");
+				return -1;
+			}
+			mpls_format_old = true;
+			ret = get_u8(&tc, *argv, 0);
 			if (ret < 0 || tc & ~(MPLS_LS_TC_MASK >> MPLS_LS_TC_SHIFT)) {
 				fprintf(stderr, "Illegal \"mpls_tc\"\n");
 				return -1;
@@ -1423,7 +1583,13 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
 					"Can't set \"mpls_bos\" if ethertype isn't MPLS\n");
 				return -1;
 			}
-			ret = get_u8(&bos, *argv, 10);
+			if (mpls_format_new) {
+				fprintf(stderr,
+					"Can't set \"mpls_bos\" if \"mpls\" is set\n");
+				return -1;
+			}
+			mpls_format_old = true;
+			ret = get_u8(&bos, *argv, 0);
 			if (ret < 0 || bos & ~(MPLS_LS_S_MASK >> MPLS_LS_S_SHIFT)) {
 				fprintf(stderr, "Illegal \"mpls_bos\"\n");
 				return -1;
@@ -1439,7 +1605,13 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
 					"Can't set \"mpls_ttl\" if ethertype isn't MPLS\n");
 				return -1;
 			}
-			ret = get_u8(&ttl, *argv, 10);
+			if (mpls_format_new) {
+				fprintf(stderr,
+					"Can't set \"mpls_ttl\" if \"mpls\" is set\n");
+				return -1;
+			}
+			mpls_format_old = true;
+			ret = get_u8(&ttl, *argv, 0);
 			if (ret < 0 || ttl & ~(MPLS_LS_TTL_MASK >> MPLS_LS_TTL_SHIFT)) {
 				fprintf(stderr, "Illegal \"mpls_ttl\"\n");
 				return -1;
@@ -2316,6 +2488,69 @@ static void flower_print_u32(const char *name, struct rtattr *attr)
 	print_uint(PRINT_ANY, name, namefrm, rta_getattr_u32(attr));
 }
 
+static void flower_print_mpls_opt_lse(const char *name, struct rtattr *lse)
+{
+	struct rtattr *tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_MAX + 1];
+	struct rtattr *attr;
+
+	if (lse->rta_type != (TCA_FLOWER_KEY_MPLS_OPTS_LSE | NLA_F_NESTED)) {
+		printf("rta_type 0x%x, expeting 0x%x (0x%x & 0x%x)\n",
+		       lse->rta_type,
+		       TCA_FLOWER_KEY_MPLS_OPTS_LSE & NLA_F_NESTED,
+		       TCA_FLOWER_KEY_MPLS_OPTS_LSE, NLA_F_NESTED);
+		return;
+	}
+
+	parse_rtattr(tb, TCA_FLOWER_KEY_MPLS_OPT_LSE_MAX, RTA_DATA(lse),
+		     RTA_PAYLOAD(lse));
+
+	print_nl();
+	open_json_array(PRINT_ANY, name);
+	attr = tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_DEPTH];
+	if (attr)
+		print_hhu(PRINT_ANY, "depth", " depth %u",
+			  rta_getattr_u8(attr));
+	attr = tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_LABEL];
+	if (attr)
+		print_uint(PRINT_ANY, "label", " label %u",
+			   rta_getattr_u32(attr));
+	attr = tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_TC];
+	if (attr)
+		print_hhu(PRINT_ANY, "tc", " tc %u", rta_getattr_u8(attr));
+	attr = tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_BOS];
+	if (attr)
+		print_hhu(PRINT_ANY, "bos", " bos %u", rta_getattr_u8(attr));
+	attr = tb[TCA_FLOWER_KEY_MPLS_OPT_LSE_TTL];
+	if (attr)
+		print_hhu(PRINT_ANY, "ttl", " ttl %u", rta_getattr_u8(attr));
+	close_json_array(PRINT_JSON, NULL);
+}
+
+static void flower_print_mpls_opts(const char *name, struct rtattr *attr)
+{
+	struct rtattr *lse;
+	int rem;
+
+	if (!attr || !(attr->rta_type & NLA_F_NESTED))
+		return;
+
+	print_nl();
+	open_json_array(PRINT_ANY, name);
+	rem = RTA_PAYLOAD(attr);
+	lse = RTA_DATA(attr);
+	while (RTA_OK(lse, rem)) {
+		if (lse->rta_type & NLA_F_NESTED)
+			flower_print_mpls_opt_lse("    lse", lse);
+		else
+			printf("rta_type 0x%x (no NLA_F_NESTED)\n", lse->rta_type);
+		lse = RTA_NEXT(lse, rem);
+	};
+	if (rem)
+		fprintf(stderr, "!!!Deficit %d, rta_len=%d\n",
+			rem, lse->rta_len);
+	close_json_array(PRINT_JSON, NULL);
+}
+
 static void flower_print_arp_op(const char *name,
 				struct rtattr *op_attr,
 				struct rtattr *mask_attr)
@@ -2430,6 +2665,7 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
 	flower_print_ip_attr("ip_ttl", tb[TCA_FLOWER_KEY_IP_TTL],
 			    tb[TCA_FLOWER_KEY_IP_TTL_MASK]);
 
+	flower_print_mpls_opts("  mpls", tb[TCA_FLOWER_KEY_MPLS_OPTS]);
 	flower_print_u32("mpls_label", tb[TCA_FLOWER_KEY_MPLS_LABEL]);
 	flower_print_u8("mpls_tc", tb[TCA_FLOWER_KEY_MPLS_TC]);
 	flower_print_u8("mpls_bos", tb[TCA_FLOWER_KEY_MPLS_BOS]);
-- 
2.21.1


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

* Re: [PATCH net-next v2 0/2] flow_dissector, cls_flower: Add support for multiple MPLS Label Stack Entries
  2020-05-21 17:47 [PATCH net-next v2 0/2] flow_dissector, cls_flower: Add support for multiple MPLS Label Stack Entries Guillaume Nault
                   ` (2 preceding siblings ...)
  2020-05-23 16:35 ` [PATCH net-next v2 0/2] flow_dissector, cls_flower: Add support for " Guillaume Nault
@ 2020-05-26  0:38 ` David Miller
  2020-05-26  0:49   ` David Miller
  3 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2020-05-26  0:38 UTC (permalink / raw)
  To: gnault
  Cc: kuba, netdev, jhs, xiyou.wangcong, jiri, benjamin.lahaise, tom,
	pieter.jansenvanvuuren, pablo, liels, ronye

From: Guillaume Nault <gnault@redhat.com>
Date: Thu, 21 May 2020 19:47:08 +0200

> Currently, the flow dissector and the Flower classifier can only handle
> the first entry of an MPLS label stack. This patch series generalises
> the code to allow parsing and matching the Label Stack Entries that
> follow.
> 
> Patch 1 extends the flow dissector to parse MPLS LSEs until the Bottom
> Of Stack bit is reached. The number of parsed LSEs is capped at
> FLOW_DIS_MPLS_MAX (arbitrarily set to 7). Flower and the NFP driver
> are updated to take into account the new layout of struct
> flow_dissector_key_mpls.
> 
> Patch 2 extends Flower. It defines new netlink attributes, which are
> independent from the previous MPLS ones. Mixing the old and the new
> attributes in a same filter is not allowed. For backward compatibility,
> the old attributes are used when dumping filters that don't require the
> new ones.
> 
> Changes since v1:
>   * Fix compilation of NFP driver (kbuild test robot).
>   * Fix sparse warning with entropy label (kbuild test robot).

Series applied, thanks.

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

* Re: [PATCH net-next v2 0/2] flow_dissector, cls_flower: Add support for multiple MPLS Label Stack Entries
  2020-05-26  0:38 ` David Miller
@ 2020-05-26  0:49   ` David Miller
  2020-05-26  7:54     ` Guillaume Nault
  0 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2020-05-26  0:49 UTC (permalink / raw)
  To: gnault
  Cc: kuba, netdev, jhs, xiyou.wangcong, jiri, benjamin.lahaise, tom,
	pieter.jansenvanvuuren, pablo, liels, ronye

From: David Miller <davem@davemloft.net>
Date: Mon, 25 May 2020 17:38:18 -0700 (PDT)

> Series applied, thanks.

Reverted, this doesn't even build with the one of the most popular drivers
in the tree, mlx5.

drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_mplsoudp.c: In function ‘parse_tunnel’:
drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_mplsoudp.c:105:52: error: ‘struct flow_dissector_key_mpls’ has no member named ‘mpls_label’
  105 |    outer_first_mpls_over_udp.mpls_label, match.mask->mpls_label);
      |                                                    ^~
./include/linux/mlx5/device.h:74:11: note: in definition of macro ‘MLX5_SET’
   74 |  u32 _v = v; \
      |           ^
drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_mplsoudp.c:107:51: error: ‘struct flow_dissector_key_mpls’ has no member named ‘mpls_label’
  107 |    outer_first_mpls_over_udp.mpls_label, match.key->mpls_label);
      |                                                   ^~
./include/linux/mlx5/device.h:74:11: note: in definition of macro ‘MLX5_SET’
   74 |  u32 _v = v; \
      |           ^

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

* Re: [PATCH net-next v2 0/2] flow_dissector, cls_flower: Add support for multiple MPLS Label Stack Entries
  2020-05-26  0:49   ` David Miller
@ 2020-05-26  7:54     ` Guillaume Nault
  0 siblings, 0 replies; 8+ messages in thread
From: Guillaume Nault @ 2020-05-26  7:54 UTC (permalink / raw)
  To: David Miller
  Cc: kuba, netdev, jhs, xiyou.wangcong, jiri, benjamin.lahaise, tom,
	pieter.jansenvanvuuren, pablo, liels, ronye

On Mon, May 25, 2020 at 05:49:51PM -0700, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Mon, 25 May 2020 17:38:18 -0700 (PDT)
> 
> > Series applied, thanks.
> 
> Reverted, this doesn't even build with the one of the most popular drivers
> in the tree, mlx5.
> 
Well, this comes from the latest mlx5 pull request, which was posted
_after_ this patch set.

> drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_mplsoudp.c: In function ‘parse_tunnel’:
> drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_mplsoudp.c:105:52: error: ‘struct flow_dissector_key_mpls’ has no member named ‘mpls_label’
>   105 |    outer_first_mpls_over_udp.mpls_label, match.mask->mpls_label);
>       |                                                    ^~
> ./include/linux/mlx5/device.h:74:11: note: in definition of macro ‘MLX5_SET’
>    74 |  u32 _v = v; \
>       |           ^
> drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_mplsoudp.c:107:51: error: ‘struct flow_dissector_key_mpls’ has no member named ‘mpls_label’
>   107 |    outer_first_mpls_over_udp.mpls_label, match.key->mpls_label);
>       |                                                   ^~
> ./include/linux/mlx5/device.h:74:11: note: in definition of macro ‘MLX5_SET’
>    74 |  u32 _v = v; \
>       |           ^

Anyway, I'll respin.


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

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-21 17:47 [PATCH net-next v2 0/2] flow_dissector, cls_flower: Add support for multiple MPLS Label Stack Entries Guillaume Nault
2020-05-21 17:47 ` [PATCH net-next v2 1/2] flow_dissector: Parse " Guillaume Nault
2020-05-23  9:55   ` Guillaume Nault
2020-05-21 17:47 ` [PATCH net-next v2 2/2] cls_flower: Support filtering on " Guillaume Nault
2020-05-23 16:35 ` [PATCH net-next v2 0/2] flow_dissector, cls_flower: Add support for " Guillaume Nault
2020-05-26  0:38 ` David Miller
2020-05-26  0:49   ` David Miller
2020-05-26  7:54     ` Guillaume Nault

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git