netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v3 0/2] Add MPLS actions to TC
@ 2019-06-14 14:58 John Hurley
  2019-06-14 14:58 ` [PATCH net-next v3 1/2] net: sched: add mpls manipulation " John Hurley
  2019-06-14 14:58 ` [PATCH net-next v3 2/2] selftests: tc-tests: actions: add MPLS tests John Hurley
  0 siblings, 2 replies; 6+ messages in thread
From: John Hurley @ 2019-06-14 14:58 UTC (permalink / raw)
  To: netdev
  Cc: davem, jiri, xiyou.wangcong, dcaratti, simon.horman,
	jakub.kicinski, oss-drivers, John Hurley

This patchset introduces a new TC action module that allows the
manipulation of the MPLS headers of packets. The code impliments
functionality including push, pop, and modify.

Also included are tests for the new funtionality. Note that these will
require iproute2 changes to be submitted soon.

v2-v3:
- remove a few unnecessary line breaks (Jiri Pirko)
- retract hw offload patch from set (resubmit with driver changes) (Jiri)
v1->v2:
- ensure TCA_ID_MPLS does not conflict with TCA_ID_CTINFO (Davide Caratti)

John Hurley (2):
  net: sched: add mpls manipulation actions to TC
  selftests: tc-tests: actions: add MPLS tests

 include/net/tc_act/tc_mpls.h                       |  27 +
 include/uapi/linux/pkt_cls.h                       |   3 +-
 include/uapi/linux/tc_act/tc_mpls.h                |  32 +
 net/sched/Kconfig                                  |  11 +
 net/sched/Makefile                                 |   1 +
 net/sched/act_mpls.c                               | 447 +++++++++++++
 .../tc-testing/tc-tests/actions/mpls.json          | 744 +++++++++++++++++++++
 7 files changed, 1264 insertions(+), 1 deletion(-)
 create mode 100644 include/net/tc_act/tc_mpls.h
 create mode 100644 include/uapi/linux/tc_act/tc_mpls.h
 create mode 100644 net/sched/act_mpls.c
 create mode 100644 tools/testing/selftests/tc-testing/tc-tests/actions/mpls.json

-- 
2.7.4


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

* [PATCH net-next v3 1/2] net: sched: add mpls manipulation actions to TC
  2019-06-14 14:58 [PATCH net-next v3 0/2] Add MPLS actions to TC John Hurley
@ 2019-06-14 14:58 ` John Hurley
  2019-06-14 17:22   ` David Ahern
  2019-06-14 14:58 ` [PATCH net-next v3 2/2] selftests: tc-tests: actions: add MPLS tests John Hurley
  1 sibling, 1 reply; 6+ messages in thread
From: John Hurley @ 2019-06-14 14:58 UTC (permalink / raw)
  To: netdev
  Cc: davem, jiri, xiyou.wangcong, dcaratti, simon.horman,
	jakub.kicinski, oss-drivers, John Hurley

Currently, TC offers the ability to match on the MPLS fields of a packet
through the use of the flow_dissector_key_mpls struct. However, as yet, TC
actions do not allow the modification or manipulation of such fields.

Add a new module that registers TC action ops to allow manipulation of
MPLS. This includes the ability to push and pop headers as well as modify
the contents of new or existing headers. A further action to decrement the
TTL field of an MPLS header is also provided.

Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/tc_act/tc_mpls.h        |  27 +++
 include/uapi/linux/pkt_cls.h        |   3 +-
 include/uapi/linux/tc_act/tc_mpls.h |  32 +++
 net/sched/Kconfig                   |  11 +
 net/sched/Makefile                  |   1 +
 net/sched/act_mpls.c                | 447 ++++++++++++++++++++++++++++++++++++
 6 files changed, 520 insertions(+), 1 deletion(-)
 create mode 100644 include/net/tc_act/tc_mpls.h
 create mode 100644 include/uapi/linux/tc_act/tc_mpls.h
 create mode 100644 net/sched/act_mpls.c

diff --git a/include/net/tc_act/tc_mpls.h b/include/net/tc_act/tc_mpls.h
new file mode 100644
index 0000000..ca7393a
--- /dev/null
+++ b/include/net/tc_act/tc_mpls.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/* Copyright (C) 2019 Netronome Systems, Inc. */
+
+#ifndef __NET_TC_MPLS_H
+#define __NET_TC_MPLS_H
+
+#include <linux/tc_act/tc_mpls.h>
+#include <net/act_api.h>
+
+struct tcf_mpls_params {
+	int tcfm_action;
+	u32 tcfm_label;
+	u8 tcfm_tc;
+	u8 tcfm_ttl;
+	__be16 tcfm_proto;
+	struct rcu_head	rcu;
+};
+
+#define ACT_MPLS_TC_NOT_SET	0xff
+
+struct tcf_mpls {
+	struct tc_action common;
+	struct tcf_mpls_params __rcu *mpls_p;
+};
+#define to_mpls(a) ((struct tcf_mpls *)a)
+
+#endif /* __NET_TC_MPLS_H */
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index a93680f..fb9804a 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -104,8 +104,9 @@ enum tca_id {
 	TCA_ID_SIMP = TCA_ACT_SIMP,
 	TCA_ID_IFE = TCA_ACT_IFE,
 	TCA_ID_SAMPLE = TCA_ACT_SAMPLE,
-	/* other actions go here */
 	TCA_ID_CTINFO,
+	TCA_ID_MPLS,
+	/* other actions go here */
 	__TCA_ID_MAX = 255
 };
 
diff --git a/include/uapi/linux/tc_act/tc_mpls.h b/include/uapi/linux/tc_act/tc_mpls.h
new file mode 100644
index 0000000..6e8907b
--- /dev/null
+++ b/include/uapi/linux/tc_act/tc_mpls.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/* Copyright (C) 2019 Netronome Systems, Inc. */
+
+#ifndef __LINUX_TC_MPLS_H
+#define __LINUX_TC_MPLS_H
+
+#include <linux/pkt_cls.h>
+
+#define TCA_MPLS_ACT_POP	1
+#define TCA_MPLS_ACT_PUSH	2
+#define TCA_MPLS_ACT_MODIFY	3
+#define TCA_MPLS_ACT_DEC_TTL	4
+
+struct tc_mpls {
+	tc_gen;
+	int m_action;
+};
+
+enum {
+	TCA_MPLS_UNSPEC,
+	TCA_MPLS_TM,
+	TCA_MPLS_PARMS,
+	TCA_MPLS_PAD,
+	TCA_MPLS_PROTO,
+	TCA_MPLS_LABEL,
+	TCA_MPLS_TC,
+	TCA_MPLS_TTL,
+	__TCA_MPLS_MAX,
+};
+#define TCA_MPLS_MAX (__TCA_MPLS_MAX - 1)
+
+#endif
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index d104f7e..a34dcd3 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -842,6 +842,17 @@ config NET_ACT_CSUM
 	  To compile this code as a module, choose M here: the
 	  module will be called act_csum.
 
+config NET_ACT_MPLS
+	tristate "MPLS manipulation"
+	depends on NET_CLS_ACT
+	help
+	  Say Y here to push or pop MPLS headers.
+
+	  If unsure, say N.
+
+	  To compile this code as a module, choose M here: the
+	  module will be called act_mpls.
+
 config NET_ACT_VLAN
         tristate "Vlan manipulation"
         depends on NET_CLS_ACT
diff --git a/net/sched/Makefile b/net/sched/Makefile
index d54bfcb..c266036 100644
--- a/net/sched/Makefile
+++ b/net/sched/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_NET_ACT_PEDIT)	+= act_pedit.o
 obj-$(CONFIG_NET_ACT_SIMP)	+= act_simple.o
 obj-$(CONFIG_NET_ACT_SKBEDIT)	+= act_skbedit.o
 obj-$(CONFIG_NET_ACT_CSUM)	+= act_csum.o
+obj-$(CONFIG_NET_ACT_MPLS)	+= act_mpls.o
 obj-$(CONFIG_NET_ACT_VLAN)	+= act_vlan.o
 obj-$(CONFIG_NET_ACT_BPF)	+= act_bpf.o
 obj-$(CONFIG_NET_ACT_CONNMARK)	+= act_connmark.o
diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c
new file mode 100644
index 0000000..828a8d9
--- /dev/null
+++ b/net/sched/act_mpls.c
@@ -0,0 +1,447 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/* Copyright (C) 2019 Netronome Systems, Inc. */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mpls.h>
+#include <linux/rtnetlink.h>
+#include <linux/skbuff.h>
+#include <linux/tc_act/tc_mpls.h>
+#include <net/mpls.h>
+#include <net/netlink.h>
+#include <net/pkt_sched.h>
+#include <net/pkt_cls.h>
+#include <net/tc_act/tc_mpls.h>
+
+static unsigned int mpls_net_id;
+static struct tc_action_ops act_mpls_ops;
+
+#define ACT_MPLS_TTL_DEFAULT	255
+
+static void tcf_mpls_mod_lse(struct mpls_shim_hdr *lse,
+			     struct tcf_mpls_params *p, bool set_bos)
+{
+	u32 new_lse = be32_to_cpu(lse->label_stack_entry);
+
+	if (p->tcfm_label) {
+		new_lse &= ~MPLS_LS_LABEL_MASK;
+		new_lse |= p->tcfm_label << MPLS_LS_LABEL_SHIFT;
+	}
+	if (p->tcfm_ttl) {
+		new_lse &= ~MPLS_LS_TTL_MASK;
+		new_lse |= p->tcfm_ttl << MPLS_LS_TTL_SHIFT;
+	}
+	if (p->tcfm_tc != ACT_MPLS_TC_NOT_SET) {
+		new_lse &= ~MPLS_LS_TC_MASK;
+		new_lse |= p->tcfm_tc << MPLS_LS_TC_SHIFT;
+	}
+	if (set_bos)
+		new_lse |= 1 << MPLS_LS_S_SHIFT;
+
+	lse->label_stack_entry = cpu_to_be32(new_lse);
+}
+
+static inline void tcf_mpls_set_eth_type(struct sk_buff *skb, __be16 ethertype)
+{
+	struct ethhdr *hdr = eth_hdr(skb);
+
+	skb_postpull_rcsum(skb, &hdr->h_proto, ETH_TLEN);
+	hdr->h_proto = ethertype;
+	skb_postpush_rcsum(skb, &hdr->h_proto, ETH_TLEN);
+}
+
+static int tcf_mpls_act(struct sk_buff *skb, const struct tc_action *a,
+			struct tcf_result *res)
+{
+	struct tcf_mpls *m = to_mpls(a);
+	struct mpls_shim_hdr *lse;
+	struct tcf_mpls_params *p;
+	u32 temp_lse;
+	int ret;
+	u8 ttl;
+
+	tcf_lastuse_update(&m->tcf_tm);
+	bstats_cpu_update(this_cpu_ptr(m->common.cpu_bstats), skb);
+
+	/* Ensure 'data' points at mac_header prior calling mpls manipulating
+	 * functions.
+	 */
+	if (skb_at_tc_ingress(skb))
+		skb_push_rcsum(skb, skb->mac_len);
+
+	ret = READ_ONCE(m->tcf_action);
+
+	p = rcu_dereference_bh(m->mpls_p);
+
+	switch (p->tcfm_action) {
+	case TCA_MPLS_ACT_POP:
+		if (unlikely(!eth_p_mpls(skb->protocol)))
+			goto out;
+
+		if (unlikely(skb_ensure_writable(skb, ETH_HLEN + MPLS_HLEN)))
+			goto drop;
+
+		skb_postpull_rcsum(skb, mpls_hdr(skb), MPLS_HLEN);
+		memmove(skb->data + MPLS_HLEN, skb->data, ETH_HLEN);
+
+		__skb_pull(skb, MPLS_HLEN);
+		skb_reset_mac_header(skb);
+		skb_set_network_header(skb, ETH_HLEN);
+
+		tcf_mpls_set_eth_type(skb, p->tcfm_proto);
+		skb->protocol = p->tcfm_proto;
+		break;
+	case TCA_MPLS_ACT_PUSH:
+		if (unlikely(skb_cow_head(skb, MPLS_HLEN)))
+			goto drop;
+
+		skb_push(skb, MPLS_HLEN);
+		memmove(skb->data, skb->data + MPLS_HLEN, ETH_HLEN);
+		skb_reset_mac_header(skb);
+		skb_set_network_header(skb, ETH_HLEN);
+
+		lse = mpls_hdr(skb);
+		lse->label_stack_entry = 0;
+		tcf_mpls_mod_lse(lse, p, !eth_p_mpls(skb->protocol));
+		skb_postpush_rcsum(skb, lse, MPLS_HLEN);
+
+		tcf_mpls_set_eth_type(skb, p->tcfm_proto);
+		skb->protocol = p->tcfm_proto;
+		break;
+	case TCA_MPLS_ACT_MODIFY:
+		if (unlikely(!eth_p_mpls(skb->protocol)))
+			goto out;
+
+		if (unlikely(skb_ensure_writable(skb, ETH_HLEN + MPLS_HLEN)))
+			goto drop;
+
+		lse = mpls_hdr(skb);
+		skb_postpull_rcsum(skb, lse, MPLS_HLEN);
+		tcf_mpls_mod_lse(lse, p, false);
+		skb_postpush_rcsum(skb, lse, MPLS_HLEN);
+		break;
+	case TCA_MPLS_ACT_DEC_TTL:
+		if (unlikely(!eth_p_mpls(skb->protocol)))
+			goto out;
+
+		if (unlikely(skb_ensure_writable(skb, ETH_HLEN + MPLS_HLEN)))
+			goto drop;
+
+		lse = mpls_hdr(skb);
+		temp_lse = be32_to_cpu(lse->label_stack_entry);
+		ttl = (temp_lse & MPLS_LS_TTL_MASK) >> MPLS_LS_TTL_SHIFT;
+		if (!--ttl)
+			goto drop;
+
+		temp_lse &= ~MPLS_LS_TTL_MASK;
+		temp_lse |= ttl << MPLS_LS_TTL_SHIFT;
+		skb_postpull_rcsum(skb, lse, MPLS_HLEN);
+		lse->label_stack_entry = cpu_to_be32(temp_lse);
+		skb_postpush_rcsum(skb, lse, MPLS_HLEN);
+		break;
+	default:
+		WARN_ONCE(1, "Invalid MPLS action\n");
+	}
+
+out:
+	if (skb_at_tc_ingress(skb))
+		skb_pull_rcsum(skb, skb->mac_len);
+
+	return ret;
+
+drop:
+	qstats_drop_inc(this_cpu_ptr(m->common.cpu_qstats));
+	return TC_ACT_SHOT;
+}
+
+static int valid_label(const struct nlattr *attr,
+		       struct netlink_ext_ack *extack)
+{
+	const u32 *label = nla_data(attr);
+
+	if (!*label || *label & ~MPLS_LABEL_MASK) {
+		NL_SET_ERR_MSG_MOD(extack, "MPLS label out of range");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct nla_policy mpls_policy[TCA_MPLS_MAX + 1] = {
+	[TCA_MPLS_PARMS]	= NLA_POLICY_EXACT_LEN(sizeof(struct tc_mpls)),
+	[TCA_MPLS_PROTO]	= { .type = NLA_U16 },
+	[TCA_MPLS_LABEL]	= NLA_POLICY_VALIDATE_FN(NLA_U32, valid_label),
+	[TCA_MPLS_TC]		= NLA_POLICY_RANGE(NLA_U8, 0, 7),
+	[TCA_MPLS_TTL]		= NLA_POLICY_MIN(NLA_U8, 1),
+};
+
+static int tcf_mpls_init(struct net *net, struct nlattr *nla,
+			 struct nlattr *est, struct tc_action **a,
+			 int ovr, int bind, bool rtnl_held,
+			 struct tcf_proto *tp, struct netlink_ext_ack *extack)
+{
+	struct tc_action_net *tn = net_generic(net, mpls_net_id);
+	struct nlattr *tb[TCA_MPLS_MAX + 1];
+	struct tcf_chain *goto_ch = NULL;
+	struct tcf_mpls_params *p;
+	struct tc_mpls *parm;
+	bool exists = false;
+	struct tcf_mpls *m;
+	int ret = 0, err;
+	u8 mpls_ttl = 0;
+
+	if (!nla) {
+		NL_SET_ERR_MSG_MOD(extack, "missing netlink attributes");
+		return -EINVAL;
+	}
+
+	err = nla_parse_nested(tb, TCA_MPLS_MAX, nla, mpls_policy, extack);
+	if (err < 0)
+		return err;
+
+	if (!tb[TCA_MPLS_PARMS]) {
+		NL_SET_ERR_MSG_MOD(extack, "no MPLS params");
+		return -EINVAL;
+	}
+	parm = nla_data(tb[TCA_MPLS_PARMS]);
+
+	/* Verify parameters against action type. */
+	switch (parm->m_action) {
+	case TCA_MPLS_ACT_POP:
+		if (!tb[TCA_MPLS_PROTO] ||
+		    !eth_proto_is_802_3(nla_get_be16(tb[TCA_MPLS_PROTO]))) {
+			NL_SET_ERR_MSG_MOD(extack, "MPLS POP: invalid proto");
+			return -EINVAL;
+		}
+		if (tb[TCA_MPLS_LABEL] || tb[TCA_MPLS_TTL] || tb[TCA_MPLS_TC]) {
+			NL_SET_ERR_MSG_MOD(extack, "MPLS POP: unsupported attrs");
+			return -EINVAL;
+		}
+		break;
+	case TCA_MPLS_ACT_DEC_TTL:
+		if (tb[TCA_MPLS_PROTO] || tb[TCA_MPLS_LABEL] ||
+		    tb[TCA_MPLS_TTL] || tb[TCA_MPLS_TC]) {
+			NL_SET_ERR_MSG_MOD(extack, "MPLS DEC TTL: unsupported attrs");
+			return -EINVAL;
+		}
+		break;
+	case TCA_MPLS_ACT_PUSH:
+		if (!tb[TCA_MPLS_LABEL]) {
+			NL_SET_ERR_MSG_MOD(extack, "MPLS PUSH: missing label");
+			return -EINVAL;
+		}
+		if (tb[TCA_MPLS_PROTO] &&
+		    !eth_p_mpls(nla_get_be16(tb[TCA_MPLS_PROTO]))) {
+			NL_SET_ERR_MSG_MOD(extack, "MPLS PUSH: invalid proto");
+			return -EPROTONOSUPPORT;
+		}
+		/* Push needs a TTL - if not specified, set a default value. */
+		if (!tb[TCA_MPLS_TTL]) {
+#if IS_ENABLED(CONFIG_MPLS)
+			mpls_ttl = net->mpls.default_ttl ?
+				   net->mpls.default_ttl : ACT_MPLS_TTL_DEFAULT;
+#else
+			mpls_ttl = ACT_MPLS_TTL_DEFAULT;
+#endif
+		}
+		break;
+	case TCA_MPLS_ACT_MODIFY:
+		if (tb[TCA_MPLS_PROTO]) {
+			NL_SET_ERR_MSG_MOD(extack, "MPLS MOD: unsupported attrs");
+			return -EINVAL;
+		}
+		break;
+	default:
+		NL_SET_ERR_MSG_MOD(extack, "MPLS: unknown action");
+		return -EINVAL;
+	}
+
+	err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
+	if (err < 0)
+		return err;
+	exists = err;
+	if (exists && bind)
+		return 0;
+
+	if (!exists) {
+		ret = tcf_idr_create(tn, parm->index, est, a,
+				     &act_mpls_ops, bind, true);
+		if (ret) {
+			tcf_idr_cleanup(tn, parm->index);
+			return ret;
+		}
+
+		ret = ACT_P_CREATED;
+	} else if (!ovr) {
+		tcf_idr_release(*a, bind);
+		return -EEXIST;
+	}
+
+	err = tcf_action_check_ctrlact(parm->action, tp, &goto_ch, extack);
+	if (err < 0)
+		goto release_idr;
+
+	m = to_mpls(*a);
+
+	p = kzalloc(sizeof(*p), GFP_KERNEL);
+	if (!p) {
+		err = -ENOMEM;
+		goto put_chain;
+	}
+
+	p->tcfm_action = parm->m_action;
+	p->tcfm_label = tb[TCA_MPLS_LABEL] ? nla_get_u32(tb[TCA_MPLS_LABEL]) :
+					     0;
+	p->tcfm_tc = tb[TCA_MPLS_TC] ? nla_get_u8(tb[TCA_MPLS_TC]) :
+				       ACT_MPLS_TC_NOT_SET;
+	p->tcfm_ttl = tb[TCA_MPLS_TTL] ? nla_get_u8(tb[TCA_MPLS_TTL]) :
+					 mpls_ttl;
+	p->tcfm_proto = tb[TCA_MPLS_PROTO] ? nla_get_be16(tb[TCA_MPLS_PROTO]) :
+					     htons(ETH_P_MPLS_UC);
+
+	spin_lock_bh(&m->tcf_lock);
+	goto_ch = tcf_action_set_ctrlact(*a, parm->action, goto_ch);
+	rcu_swap_protected(m->mpls_p, p, lockdep_is_held(&m->tcf_lock));
+	spin_unlock_bh(&m->tcf_lock);
+
+	if (goto_ch)
+		tcf_chain_put_by_act(goto_ch);
+	if (p)
+		kfree_rcu(p, rcu);
+
+	if (ret == ACT_P_CREATED)
+		tcf_idr_insert(tn, *a);
+	return ret;
+put_chain:
+	if (goto_ch)
+		tcf_chain_put_by_act(goto_ch);
+release_idr:
+	tcf_idr_release(*a, bind);
+	return err;
+}
+
+static void tcf_mpls_cleanup(struct tc_action *a)
+{
+	struct tcf_mpls *m = to_mpls(a);
+	struct tcf_mpls_params *p;
+
+	p = rcu_dereference_protected(m->mpls_p, 1);
+	if (p)
+		kfree_rcu(p, rcu);
+}
+
+static int tcf_mpls_dump(struct sk_buff *skb, struct tc_action *a,
+			 int bind, int ref)
+{
+	unsigned char *b = skb_tail_pointer(skb);
+	struct tcf_mpls *m = to_mpls(a);
+	struct tcf_mpls_params *p;
+	struct tc_mpls opt = {
+		.index    = m->tcf_index,
+		.refcnt   = refcount_read(&m->tcf_refcnt) - ref,
+		.bindcnt  = atomic_read(&m->tcf_bindcnt) - bind,
+	};
+	struct tcf_t t;
+
+	spin_lock_bh(&m->tcf_lock);
+	opt.action = m->tcf_action;
+	p = rcu_dereference_protected(m->mpls_p, lockdep_is_held(&m->tcf_lock));
+	opt.m_action = p->tcfm_action;
+
+	if (nla_put(skb, TCA_MPLS_PARMS, sizeof(opt), &opt))
+		goto nla_put_failure;
+
+	if (p->tcfm_label && nla_put_u32(skb, TCA_MPLS_LABEL, p->tcfm_label))
+		goto nla_put_failure;
+
+	if (p->tcfm_tc != ACT_MPLS_TC_NOT_SET &&
+	    nla_put_u8(skb, TCA_MPLS_TC, p->tcfm_tc))
+		goto nla_put_failure;
+
+	if (p->tcfm_ttl && nla_put_u8(skb, TCA_MPLS_TTL, p->tcfm_ttl))
+		goto nla_put_failure;
+
+	if (nla_put_be16(skb, TCA_MPLS_PROTO, p->tcfm_proto))
+		goto nla_put_failure;
+
+	tcf_tm_dump(&t, &m->tcf_tm);
+
+	if (nla_put_64bit(skb, TCA_MPLS_TM, sizeof(t), &t, TCA_MPLS_PAD))
+		goto nla_put_failure;
+
+	spin_unlock_bh(&m->tcf_lock);
+
+	return skb->len;
+
+nla_put_failure:
+	spin_unlock_bh(&m->tcf_lock);
+	nlmsg_trim(skb, b);
+	return -EMSGSIZE;
+}
+
+static int tcf_mpls_walker(struct net *net, struct sk_buff *skb,
+			   struct netlink_callback *cb, int type,
+			   const struct tc_action_ops *ops,
+			   struct netlink_ext_ack *extack)
+{
+	struct tc_action_net *tn = net_generic(net, mpls_net_id);
+
+	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
+}
+
+static int tcf_mpls_search(struct net *net, struct tc_action **a, u32 index)
+{
+	struct tc_action_net *tn = net_generic(net, mpls_net_id);
+
+	return tcf_idr_search(tn, a, index);
+}
+
+static struct tc_action_ops act_mpls_ops = {
+	.kind		=	"mpls",
+	.id		=	TCA_ID_MPLS,
+	.owner		=	THIS_MODULE,
+	.act		=	tcf_mpls_act,
+	.dump		=	tcf_mpls_dump,
+	.init		=	tcf_mpls_init,
+	.cleanup	=	tcf_mpls_cleanup,
+	.walk		=	tcf_mpls_walker,
+	.lookup		=	tcf_mpls_search,
+	.size		=	sizeof(struct tcf_mpls),
+};
+
+static __net_init int mpls_init_net(struct net *net)
+{
+	struct tc_action_net *tn = net_generic(net, mpls_net_id);
+
+	return tc_action_net_init(tn, &act_mpls_ops);
+}
+
+static void __net_exit mpls_exit_net(struct list_head *net_list)
+{
+	tc_action_net_exit(net_list, mpls_net_id);
+}
+
+static struct pernet_operations mpls_net_ops = {
+	.init = mpls_init_net,
+	.exit_batch = mpls_exit_net,
+	.id   = &mpls_net_id,
+	.size = sizeof(struct tc_action_net),
+};
+
+static int __init mpls_init_module(void)
+{
+	return tcf_register_action(&act_mpls_ops, &mpls_net_ops);
+}
+
+static void __exit mpls_cleanup_module(void)
+{
+	tcf_unregister_action(&act_mpls_ops, &mpls_net_ops);
+}
+
+module_init(mpls_init_module);
+module_exit(mpls_cleanup_module);
+
+MODULE_AUTHOR("Netronome Systems <oss-drivers@netronome.com>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("MPLS manipulation actions");
-- 
2.7.4


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

* [PATCH net-next v3 2/2] selftests: tc-tests: actions: add MPLS tests
  2019-06-14 14:58 [PATCH net-next v3 0/2] Add MPLS actions to TC John Hurley
  2019-06-14 14:58 ` [PATCH net-next v3 1/2] net: sched: add mpls manipulation " John Hurley
@ 2019-06-14 14:58 ` John Hurley
  1 sibling, 0 replies; 6+ messages in thread
From: John Hurley @ 2019-06-14 14:58 UTC (permalink / raw)
  To: netdev
  Cc: davem, jiri, xiyou.wangcong, dcaratti, simon.horman,
	jakub.kicinski, oss-drivers, John Hurley

Add a new series of selftests to verify the functionality of act_mpls in
TC.

Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 .../tc-testing/tc-tests/actions/mpls.json          | 744 +++++++++++++++++++++
 1 file changed, 744 insertions(+)
 create mode 100644 tools/testing/selftests/tc-testing/tc-tests/actions/mpls.json

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/mpls.json b/tools/testing/selftests/tc-testing/tc-tests/actions/mpls.json
new file mode 100644
index 0000000..e1a14f4e0
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/mpls.json
@@ -0,0 +1,744 @@
+[
+    {
+        "id": "a933",
+        "name": "Add MPLS dec_ttl action with pipe opcode",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls dec_ttl pipe index 8",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*dec_ttl.*pipe.*index 8 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mpls"
+        ]
+    },
+    {
+        "id": "08d1",
+        "name": "Add mpls dec_ttl action with pass opcode",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls dec_ttl pass index 8",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action mpls index 8",
+        "matchPattern": "action order [0-9]+: mpls.*dec_ttl.*pass.*index 8 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mpls"
+        ]
+    },
+    {
+        "id": "d786",
+        "name": "Add mpls dec_ttl action with drop opcode",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls dec_ttl drop index 8",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action mpls index 8",
+        "matchPattern": "action order [0-9]+: mpls.*dec_ttl.*drop.*index 8 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mpls"
+        ]
+    },
+    {
+        "id": "f334",
+        "name": "Add mpls dec_ttl action with reclassify opcode",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls dec_ttl reclassify index 8",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action mpls index 8",
+        "matchPattern": "action order [0-9]+: mpls.*dec_ttl.*reclassify.*index 8 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mpls"
+        ]
+    },
+    {
+        "id": "29bd",
+        "name": "Add mpls dec_ttl action with continue opcode",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls dec_ttl continue index 8",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action mpls index 8",
+        "matchPattern": "action order [0-9]+: mpls.*dec_ttl.*continue.*index 8 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mpls"
+        ]
+    },
+    {
+        "id": "48df",
+        "name": "Add mpls dec_ttl action with jump opcode",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls dec_ttl jump 10 index 8",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*jump 10.*index 8 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mpls"
+        ]
+    },
+    {
+        "id": "62eb",
+        "name": "Add mpls dec_ttl action with trap opcode",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls dec_ttl trap index 8",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*dec_ttl trap.*index 8 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mpls"
+        ]
+    },
+    {
+        "id": "9118",
+        "name": "Add mpls dec_ttl action with invalid opcode",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls dec_ttl foo index 8",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*dec_ttl.*foo.*index 8 ref",
+        "matchCount": "0",
+        "teardown": []
+    },
+    {
+        "id": "6ce1",
+        "name": "Add mpls dec_ttl action with label (invalid)",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls dec_ttl label 20",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*dec_ttl.*label.*20.*pipe",
+        "matchCount": "0",
+        "teardown": []
+    },
+    {
+        "id": "352f",
+        "name": "Add mpls dec_ttl action with tc (invalid)",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls dec_ttl tc 3",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*dec_ttl.*tc.*3.*pipe",
+        "matchCount": "0",
+        "teardown": []
+    },
+    {
+        "id": "fa1c",
+        "name": "Add mpls dec_ttl action with ttl (invalid)",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls dec_ttl ttl 20",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*dec_ttl.*ttl.*20.*pipe",
+        "matchCount": "0",
+        "teardown": []
+    },
+    {
+        "id": "d4c4",
+        "name": "Add mpls pop action with ip proto",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls pop protocol ipv4",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*pop.*protocol.*ip.*pipe",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mpls"
+        ]
+    },
+    {
+        "id": "92fe",
+        "name": "Add mpls pop action with mpls proto",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls pop protocol mpls_mc",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*pop.*protocol.*mpls_mc.*pipe",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mpls"
+        ]
+    },
+    {
+        "id": "7e23",
+        "name": "Add mpls pop action with no protocol (invalid)",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls pop",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*pop.*pipe",
+        "matchCount": "0",
+        "teardown": []
+    },
+    {
+        "id": "6182",
+        "name": "Add mpls pop action with label (invalid)",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls pop protocol ipv4 label 20",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*pop.*label.*20.*pipe",
+        "matchCount": "0",
+        "teardown": []
+    },
+    {
+        "id": "6475",
+        "name": "Add mpls pop action with tc (invalid)",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls pop protocol ipv4 tc 3",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*pop.*tc.*3.*pipe",
+        "matchCount": "0",
+        "teardown": []
+    },
+    {
+        "id": "067b",
+        "name": "Add mpls pop action with ttl (invalid)",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls pop protocol ipv4 ttl 20",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*pop.*ttl.*20.*pipe",
+        "matchCount": "0",
+        "teardown": []
+    },
+    {
+        "id": "38cc",
+        "name": "Add mpls push action with label",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls push label 20",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*push.*protocol.*mpls_uc.*label.*20.*ttl.*[0-9]+.*pipe",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mpls"
+        ]
+    },
+    {
+        "id": "c281",
+        "name": "Add mpls push action with mpls_mc protocol",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls push protocol mpls_mc label 20",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*push.*protocol.*mpls_mc.*label.*20.*ttl.*[0-9]+.*pipe",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mpls"
+        ]
+    },
+    {
+        "id": "5db4",
+        "name": "Add mpls push action with label, tc and ttl",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls push label 20 tc 3 ttl 128",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*push.*protocol.*mpls_uc.*label.*20.*tc.*3.*ttl.*128.*pipe",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mpls"
+        ]
+    },
+    {
+        "id": "d69d",
+        "name": "Add mpls push action with no label (invalid)",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls push",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*push.*protocol.*mpls_uc.*pipe",
+        "matchCount": "0",
+        "teardown": []
+    },
+    {
+        "id": "e8e4",
+        "name": "Add mpls push action with ipv4 protocol (invalid)",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls push protocol ipv4 label 20",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*push.*protocol.*mpls_uc.*label.*20.*ttl.*[0-9]+.*pipe",
+        "matchCount": "0",
+        "teardown": []
+    },
+    {
+        "id": "ecd0",
+        "name": "Add mpls push action with out of range label (invalid)",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls push label 1048576",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*push.*protocol.*mpls_uc.*label.*1048576.*pipe",
+        "matchCount": "0",
+        "teardown": []
+    },
+    {
+        "id": "d303",
+        "name": "Add mpls push action with out of range tc (invalid)",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls push label 20 tc 8",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*push.*protocol.*mpls_uc.*label.*20.*tc.*8.*pipe",
+        "matchCount": "0",
+        "teardown": []
+    },
+    {
+        "id": "fd6e",
+        "name": "Add mpls push action with ttl of 0 (invalid)",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls push label 20 ttl 0",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*push.*protocol.*mpls_uc.*label.*20.*ttl.*0.*pipe",
+        "matchCount": "0",
+        "teardown": []
+    },
+    {
+        "id": "19e9",
+        "name": "Add mpls mod action with mpls label",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls mod label 20",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*modify.*label.*20.*pipe",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mpls"
+        ]
+    },
+    {
+        "id": "04b5",
+        "name": "Add mpls mod action with mpls tc",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls mod tc 3",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*modify.*tc.*3.*pipe",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mpls"
+        ]
+    },
+    {
+        "id": "6ed5",
+        "name": "Add mpls mod action with mpls label",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls mod ttl 128",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*modify.*ttl.*128.*pipe",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mpls"
+        ]
+    },
+    {
+        "id": "db7c",
+        "name": "Add mpls mod action with protocol (invalid)",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "$TC actions add action mpls mod protocol ipv4",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*modify.*protocol.*ip.*pipe",
+        "matchCount": "0",
+        "teardown": []
+    },
+    {
+        "id": "b070",
+        "name": "Replace existing mpls push action with new ID",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ],
+            "$TC actions add action mpls push label 20 pipe index 12"
+        ],
+        "cmdUnderTest": "$TC actions replace action mpls push label 30 pipe index 12",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions get action mpls index 12",
+        "matchPattern": "action order [0-9]+: mpls.*push.*protocol.*mpls_uc.*label.*30.*pipe.*index 12 ref",
+        "matchCount": "1",
+        "teardown": [
+            "$TC actions flush action mpls"
+        ]
+    },
+    {
+        "id": "6cce",
+        "name": "Delete mpls pop action",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ],
+            "$TC actions add action mpls pop protocol ipv4 index 44"
+        ],
+        "cmdUnderTest": "$TC actions del action mpls index 44",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*pop.*index 44 ref",
+        "matchCount": "0",
+        "teardown": []
+    },
+    {
+        "id": "d138",
+        "name": "Flush mpls actions",
+        "category": [
+            "actions",
+            "mpls"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action mpls",
+                0,
+                1,
+                255
+            ],
+            "$TC actions add action mpls push label 10 index 10",
+            "$TC actions add action mpls push label 20 index 20",
+            "$TC actions add action mpls push label 30 index 30",
+            "$TC actions add action mpls push label 40 index 40"
+        ],
+        "cmdUnderTest": "$TC actions flush action mpls",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions list action mpls",
+        "matchPattern": "action order [0-9]+: mpls.*push.*",
+        "matchCount": "0",
+        "teardown": []
+    }
+]
-- 
2.7.4


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

* Re: [PATCH net-next v3 1/2] net: sched: add mpls manipulation actions to TC
  2019-06-14 14:58 ` [PATCH net-next v3 1/2] net: sched: add mpls manipulation " John Hurley
@ 2019-06-14 17:22   ` David Ahern
  2019-06-14 23:22     ` John Hurley
  0 siblings, 1 reply; 6+ messages in thread
From: David Ahern @ 2019-06-14 17:22 UTC (permalink / raw)
  To: John Hurley, netdev
  Cc: davem, jiri, xiyou.wangcong, dcaratti, simon.horman,
	jakub.kicinski, oss-drivers

On 6/14/19 8:58 AM, John Hurley wrote:
> Currently, TC offers the ability to match on the MPLS fields of a packet
> through the use of the flow_dissector_key_mpls struct. However, as yet, TC
> actions do not allow the modification or manipulation of such fields.
> 
> Add a new module that registers TC action ops to allow manipulation of
> MPLS. This includes the ability to push and pop headers as well as modify
> the contents of new or existing headers. A further action to decrement the
> TTL field of an MPLS header is also provided.

you have this limited to a single mpls label. It would be more flexible
to allow push/pop of N-labels (push probably being the most useful).

more below

> diff --git a/include/uapi/linux/tc_act/tc_mpls.h b/include/uapi/linux/tc_act/tc_mpls.h
> new file mode 100644
> index 0000000..6e8907b
> --- /dev/null
> +++ b/include/uapi/linux/tc_act/tc_mpls.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/* Copyright (C) 2019 Netronome Systems, Inc. */
> +
> +#ifndef __LINUX_TC_MPLS_H
> +#define __LINUX_TC_MPLS_H
> +
> +#include <linux/pkt_cls.h>
> +
> +#define TCA_MPLS_ACT_POP	1
> +#define TCA_MPLS_ACT_PUSH	2
> +#define TCA_MPLS_ACT_MODIFY	3
> +#define TCA_MPLS_ACT_DEC_TTL	4
> +
> +struct tc_mpls {
> +	tc_gen;
> +	int m_action;
> +};
> +
> +enum {
> +	TCA_MPLS_UNSPEC,
> +	TCA_MPLS_TM,
> +	TCA_MPLS_PARMS,
> +	TCA_MPLS_PAD,
> +	TCA_MPLS_PROTO,
> +	TCA_MPLS_LABEL,
> +	TCA_MPLS_TC,
> +	TCA_MPLS_TTL,
> +	__TCA_MPLS_MAX,
> +};
> +#define TCA_MPLS_MAX (__TCA_MPLS_MAX - 1)
> +
> +#endif

Adding comments to those attributes for userspace writers would be very
helpful. See what I did for the nexthop API -
include/uapi/linux/nexthop.h. My goal there was to document that API
enough that someone adding support to a routing suite (e.g., FRR) never
had to ask a question about the API.


> diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c
> new file mode 100644
> index 0000000..828a8d9
> --- /dev/null
> +++ b/net/sched/act_mpls.c
...

> +	switch (p->tcfm_action) {
> +	case TCA_MPLS_ACT_POP:
> +		if (unlikely(!eth_p_mpls(skb->protocol)))
> +			goto out;
> +
> +		if (unlikely(skb_ensure_writable(skb, ETH_HLEN + MPLS_HLEN)))
> +			goto drop;
> +
> +		skb_postpull_rcsum(skb, mpls_hdr(skb), MPLS_HLEN);
> +		memmove(skb->data + MPLS_HLEN, skb->data, ETH_HLEN);
> +
> +		__skb_pull(skb, MPLS_HLEN);
> +		skb_reset_mac_header(skb);
> +		skb_set_network_header(skb, ETH_HLEN);
> +
> +		tcf_mpls_set_eth_type(skb, p->tcfm_proto);
> +		skb->protocol = p->tcfm_proto;

This is pop of a single label. It may or may not be the bottom label, so
it seems like this should handle both cases and may depend on the
packet. If it is the bottom label, then letting the user specify next
seems correct but it is not then the protocol needs to stay MPLS.


> +		break;
> +	case TCA_MPLS_ACT_PUSH:
> +		if (unlikely(skb_cow_head(skb, MPLS_HLEN)))
> +			goto drop;
> +
> +		skb_push(skb, MPLS_HLEN);
> +		memmove(skb->data, skb->data + MPLS_HLEN, ETH_HLEN);
> +		skb_reset_mac_header(skb);
> +		skb_set_network_header(skb, ETH_HLEN);
> +
> +		lse = mpls_hdr(skb);
> +		lse->label_stack_entry = 0;
> +		tcf_mpls_mod_lse(lse, p, !eth_p_mpls(skb->protocol));
> +		skb_postpush_rcsum(skb, lse, MPLS_HLEN);
> +
> +		tcf_mpls_set_eth_type(skb, p->tcfm_proto);
> +		skb->protocol = p->tcfm_proto;

similarly here. If you are pushing one or more labels why allow the user
to specify the protocol? It should be set to ETH_P_MPLS_UC.

> +		break;
> +	case TCA_MPLS_ACT_MODIFY:
> +		if (unlikely(!eth_p_mpls(skb->protocol)))
> +			goto out;
> +
> +		if (unlikely(skb_ensure_writable(skb, ETH_HLEN + MPLS_HLEN)))
> +			goto drop;
> +
> +		lse = mpls_hdr(skb);
> +		skb_postpull_rcsum(skb, lse, MPLS_HLEN);
> +		tcf_mpls_mod_lse(lse, p, false);
> +		skb_postpush_rcsum(skb, lse, MPLS_HLEN);
> +		break;
> +	case TCA_MPLS_ACT_DEC_TTL:
> +		if (unlikely(!eth_p_mpls(skb->protocol)))
> +			goto out;
> +
> +		if (unlikely(skb_ensure_writable(skb, ETH_HLEN + MPLS_HLEN)))
> +			goto drop;
> +
> +		lse = mpls_hdr(skb);
> +		temp_lse = be32_to_cpu(lse->label_stack_entry);
> +		ttl = (temp_lse & MPLS_LS_TTL_MASK) >> MPLS_LS_TTL_SHIFT;
> +		if (!--ttl)
> +			goto drop;
> +
> +		temp_lse &= ~MPLS_LS_TTL_MASK;
> +		temp_lse |= ttl << MPLS_LS_TTL_SHIFT;
> +		skb_postpull_rcsum(skb, lse, MPLS_HLEN);
> +		lse->label_stack_entry = cpu_to_be32(temp_lse);
> +		skb_postpush_rcsum(skb, lse, MPLS_HLEN);
> +		break;
> +	default:
> +		WARN_ONCE(1, "Invalid MPLS action\n");
> +	}
> +
> +out:
> +	if (skb_at_tc_ingress(skb))
> +		skb_pull_rcsum(skb, skb->mac_len);
> +
> +	return ret;
> +
> +drop:
> +	qstats_drop_inc(this_cpu_ptr(m->common.cpu_qstats));
> +	return TC_ACT_SHOT;
> +}
> +
> +static int valid_label(const struct nlattr *attr,
> +		       struct netlink_ext_ack *extack)
> +{
> +	const u32 *label = nla_data(attr);
> +
> +	if (!*label || *label & ~MPLS_LABEL_MASK) {
> +		NL_SET_ERR_MSG_MOD(extack, "MPLS label out of range");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct nla_policy mpls_policy[TCA_MPLS_MAX + 1] = {
> +	[TCA_MPLS_PARMS]	= NLA_POLICY_EXACT_LEN(sizeof(struct tc_mpls)),
> +	[TCA_MPLS_PROTO]	= { .type = NLA_U16 },
> +	[TCA_MPLS_LABEL]	= NLA_POLICY_VALIDATE_FN(NLA_U32, valid_label),
> +	[TCA_MPLS_TC]		= NLA_POLICY_RANGE(NLA_U8, 0, 7),
> +	[TCA_MPLS_TTL]		= NLA_POLICY_MIN(NLA_U8, 1),
> +};

Since this is a new policy, set .strict_start_type in the
TCA_MPLS_UNSPEC entry:
   [TCA_MPLS_UNSPEC] = { .strict_start_type = TCA_MPLS_UNSPEC + 1 },


> +
> +static int tcf_mpls_init(struct net *net, struct nlattr *nla,
> +			 struct nlattr *est, struct tc_action **a,
> +			 int ovr, int bind, bool rtnl_held,
> +			 struct tcf_proto *tp, struct netlink_ext_ack *extack)
> +{
> +	struct tc_action_net *tn = net_generic(net, mpls_net_id);
> +	struct nlattr *tb[TCA_MPLS_MAX + 1];
> +	struct tcf_chain *goto_ch = NULL;
> +	struct tcf_mpls_params *p;
> +	struct tc_mpls *parm;
> +	bool exists = false;
> +	struct tcf_mpls *m;
> +	int ret = 0, err;
> +	u8 mpls_ttl = 0;
> +
> +	if (!nla) {
> +		NL_SET_ERR_MSG_MOD(extack, "missing netlink attributes");
> +		return -EINVAL;
> +	}
> +
> +	err = nla_parse_nested(tb, TCA_MPLS_MAX, nla, mpls_policy, extack);
> +	if (err < 0)
> +		return err;
> +
> +	if (!tb[TCA_MPLS_PARMS]) {
> +		NL_SET_ERR_MSG_MOD(extack, "no MPLS params");
> +		return -EINVAL;
> +	}
> +	parm = nla_data(tb[TCA_MPLS_PARMS]);
> +
> +	/* Verify parameters against action type. */
> +	switch (parm->m_action) {
> +	case TCA_MPLS_ACT_POP:
> +		if (!tb[TCA_MPLS_PROTO] ||
> +		    !eth_proto_is_802_3(nla_get_be16(tb[TCA_MPLS_PROTO]))) {
> +			NL_SET_ERR_MSG_MOD(extack, "MPLS POP: invalid proto");

Please spell out the messages:
  Invalid protocol

> +			return -EINVAL;
> +		}
> +		if (tb[TCA_MPLS_LABEL] || tb[TCA_MPLS_TTL] || tb[TCA_MPLS_TC]) {
> +			NL_SET_ERR_MSG_MOD(extack, "MPLS POP: unsupported attrs");

Be more specific with the error message:
    LABEL, TTL and ??? attributes can not be used with pop action.

> +			return -EINVAL;
> +		}
> +		break;
> +	case TCA_MPLS_ACT_DEC_TTL:
> +		if (tb[TCA_MPLS_PROTO] || tb[TCA_MPLS_LABEL] ||
> +		    tb[TCA_MPLS_TTL] || tb[TCA_MPLS_TC]) {
> +			NL_SET_ERR_MSG_MOD(extack, "MPLS DEC TTL: unsupported attrs");

same here and other extack messages here


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

* Re: [PATCH net-next v3 1/2] net: sched: add mpls manipulation actions to TC
  2019-06-14 17:22   ` David Ahern
@ 2019-06-14 23:22     ` John Hurley
  2019-06-14 23:44       ` David Ahern
  0 siblings, 1 reply; 6+ messages in thread
From: John Hurley @ 2019-06-14 23:22 UTC (permalink / raw)
  To: David Ahern
  Cc: Linux Netdev List, David Miller, Jiri Pirko, Cong Wang,
	Davide Caratti, Simon Horman, Jakub Kicinski, oss-drivers

On Fri, Jun 14, 2019 at 6:22 PM David Ahern <dsahern@gmail.com> wrote:
>
> On 6/14/19 8:58 AM, John Hurley wrote:
> > Currently, TC offers the ability to match on the MPLS fields of a packet
> > through the use of the flow_dissector_key_mpls struct. However, as yet, TC
> > actions do not allow the modification or manipulation of such fields.
> >
> > Add a new module that registers TC action ops to allow manipulation of
> > MPLS. This includes the ability to push and pop headers as well as modify
> > the contents of new or existing headers. A further action to decrement the
> > TTL field of an MPLS header is also provided.
>
> you have this limited to a single mpls label. It would be more flexible
> to allow push/pop of N-labels (push probably being the most useful).
>

Hi David.
Multiple push and pop actions can be done by piping them together.
E.g. for a flower filter that pushes 2 labels to an IP packet you can do:

tc filter add dev eth0 protocol ip parent ffff: \
        flower \
        action mpls push protocol mpls_mc label 10 \
        action mpls push protocol mpls_mc label 20 \
        action mirred egress redirect dev eth1


> more below
>
> > diff --git a/include/uapi/linux/tc_act/tc_mpls.h b/include/uapi/linux/tc_act/tc_mpls.h
> > new file mode 100644
> > index 0000000..6e8907b
> > --- /dev/null
> > +++ b/include/uapi/linux/tc_act/tc_mpls.h
> > @@ -0,0 +1,32 @@
> > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > +/* Copyright (C) 2019 Netronome Systems, Inc. */
> > +
> > +#ifndef __LINUX_TC_MPLS_H
> > +#define __LINUX_TC_MPLS_H
> > +
> > +#include <linux/pkt_cls.h>
> > +
> > +#define TCA_MPLS_ACT_POP     1
> > +#define TCA_MPLS_ACT_PUSH    2
> > +#define TCA_MPLS_ACT_MODIFY  3
> > +#define TCA_MPLS_ACT_DEC_TTL 4
> > +
> > +struct tc_mpls {
> > +     tc_gen;
> > +     int m_action;
> > +};
> > +
> > +enum {
> > +     TCA_MPLS_UNSPEC,
> > +     TCA_MPLS_TM,
> > +     TCA_MPLS_PARMS,
> > +     TCA_MPLS_PAD,
> > +     TCA_MPLS_PROTO,
> > +     TCA_MPLS_LABEL,
> > +     TCA_MPLS_TC,
> > +     TCA_MPLS_TTL,
> > +     __TCA_MPLS_MAX,
> > +};
> > +#define TCA_MPLS_MAX (__TCA_MPLS_MAX - 1)
> > +
> > +#endif
>
> Adding comments to those attributes for userspace writers would be very
> helpful. See what I did for the nexthop API -
> include/uapi/linux/nexthop.h. My goal there was to document that API
> enough that someone adding support to a routing suite (e.g., FRR) never
> had to ask a question about the API.
>

nice example.
I can add similar

>
> > diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c
> > new file mode 100644
> > index 0000000..828a8d9
> > --- /dev/null
> > +++ b/net/sched/act_mpls.c
> ...
>
> > +     switch (p->tcfm_action) {
> > +     case TCA_MPLS_ACT_POP:
> > +             if (unlikely(!eth_p_mpls(skb->protocol)))
> > +                     goto out;
> > +
> > +             if (unlikely(skb_ensure_writable(skb, ETH_HLEN + MPLS_HLEN)))
> > +                     goto drop;
> > +
> > +             skb_postpull_rcsum(skb, mpls_hdr(skb), MPLS_HLEN);
> > +             memmove(skb->data + MPLS_HLEN, skb->data, ETH_HLEN);
> > +
> > +             __skb_pull(skb, MPLS_HLEN);
> > +             skb_reset_mac_header(skb);
> > +             skb_set_network_header(skb, ETH_HLEN);
> > +
> > +             tcf_mpls_set_eth_type(skb, p->tcfm_proto);
> > +             skb->protocol = p->tcfm_proto;
>
> This is pop of a single label. It may or may not be the bottom label, so
> it seems like this should handle both cases and may depend on the
> packet. If it is the bottom label, then letting the user specify next
> seems correct but it is not then the protocol needs to stay MPLS.
>

Yes, the user is expected to indicate the next protocol after the pop
even if another mpls label is next.
We're trying to cater for supporting mpls_uc and mpls_mc ethtypes.
So you could in theory pop the top mpls unicast header and set the
next to multicast.
We expect the user to know what the next header is so enforce that
they give that information.
Do you agree with this or should we add more checks around the BoS bit?

>
> > +             break;
> > +     case TCA_MPLS_ACT_PUSH:
> > +             if (unlikely(skb_cow_head(skb, MPLS_HLEN)))
> > +                     goto drop;
> > +
> > +             skb_push(skb, MPLS_HLEN);
> > +             memmove(skb->data, skb->data + MPLS_HLEN, ETH_HLEN);
> > +             skb_reset_mac_header(skb);
> > +             skb_set_network_header(skb, ETH_HLEN);
> > +
> > +             lse = mpls_hdr(skb);
> > +             lse->label_stack_entry = 0;
> > +             tcf_mpls_mod_lse(lse, p, !eth_p_mpls(skb->protocol));
> > +             skb_postpush_rcsum(skb, lse, MPLS_HLEN);
> > +
> > +             tcf_mpls_set_eth_type(skb, p->tcfm_proto);
> > +             skb->protocol = p->tcfm_proto;
>
> similarly here. If you are pushing one or more labels why allow the user
> to specify the protocol? It should be set to ETH_P_MPLS_UC.
>

Again, this caters for setting ETH_P_MPLS_MC.
For push, the setting of an ethtype is not enforced and UC is used as
the default.

> > +             break;
> > +     case TCA_MPLS_ACT_MODIFY:
> > +             if (unlikely(!eth_p_mpls(skb->protocol)))
> > +                     goto out;
> > +
> > +             if (unlikely(skb_ensure_writable(skb, ETH_HLEN + MPLS_HLEN)))
> > +                     goto drop;
> > +
> > +             lse = mpls_hdr(skb);
> > +             skb_postpull_rcsum(skb, lse, MPLS_HLEN);
> > +             tcf_mpls_mod_lse(lse, p, false);
> > +             skb_postpush_rcsum(skb, lse, MPLS_HLEN);
> > +             break;
> > +     case TCA_MPLS_ACT_DEC_TTL:
> > +             if (unlikely(!eth_p_mpls(skb->protocol)))
> > +                     goto out;
> > +
> > +             if (unlikely(skb_ensure_writable(skb, ETH_HLEN + MPLS_HLEN)))
> > +                     goto drop;
> > +
> > +             lse = mpls_hdr(skb);
> > +             temp_lse = be32_to_cpu(lse->label_stack_entry);
> > +             ttl = (temp_lse & MPLS_LS_TTL_MASK) >> MPLS_LS_TTL_SHIFT;
> > +             if (!--ttl)
> > +                     goto drop;
> > +
> > +             temp_lse &= ~MPLS_LS_TTL_MASK;
> > +             temp_lse |= ttl << MPLS_LS_TTL_SHIFT;
> > +             skb_postpull_rcsum(skb, lse, MPLS_HLEN);
> > +             lse->label_stack_entry = cpu_to_be32(temp_lse);
> > +             skb_postpush_rcsum(skb, lse, MPLS_HLEN);
> > +             break;
> > +     default:
> > +             WARN_ONCE(1, "Invalid MPLS action\n");
> > +     }
> > +
> > +out:
> > +     if (skb_at_tc_ingress(skb))
> > +             skb_pull_rcsum(skb, skb->mac_len);
> > +
> > +     return ret;
> > +
> > +drop:
> > +     qstats_drop_inc(this_cpu_ptr(m->common.cpu_qstats));
> > +     return TC_ACT_SHOT;
> > +}
> > +
> > +static int valid_label(const struct nlattr *attr,
> > +                    struct netlink_ext_ack *extack)
> > +{
> > +     const u32 *label = nla_data(attr);
> > +
> > +     if (!*label || *label & ~MPLS_LABEL_MASK) {
> > +             NL_SET_ERR_MSG_MOD(extack, "MPLS label out of range");
> > +             return -EINVAL;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct nla_policy mpls_policy[TCA_MPLS_MAX + 1] = {
> > +     [TCA_MPLS_PARMS]        = NLA_POLICY_EXACT_LEN(sizeof(struct tc_mpls)),
> > +     [TCA_MPLS_PROTO]        = { .type = NLA_U16 },
> > +     [TCA_MPLS_LABEL]        = NLA_POLICY_VALIDATE_FN(NLA_U32, valid_label),
> > +     [TCA_MPLS_TC]           = NLA_POLICY_RANGE(NLA_U8, 0, 7),
> > +     [TCA_MPLS_TTL]          = NLA_POLICY_MIN(NLA_U8, 1),
> > +};
>
> Since this is a new policy, set .strict_start_type in the
> TCA_MPLS_UNSPEC entry:
>    [TCA_MPLS_UNSPEC] = { .strict_start_type = TCA_MPLS_UNSPEC + 1 },
>

ack

>
> > +
> > +static int tcf_mpls_init(struct net *net, struct nlattr *nla,
> > +                      struct nlattr *est, struct tc_action **a,
> > +                      int ovr, int bind, bool rtnl_held,
> > +                      struct tcf_proto *tp, struct netlink_ext_ack *extack)
> > +{
> > +     struct tc_action_net *tn = net_generic(net, mpls_net_id);
> > +     struct nlattr *tb[TCA_MPLS_MAX + 1];
> > +     struct tcf_chain *goto_ch = NULL;
> > +     struct tcf_mpls_params *p;
> > +     struct tc_mpls *parm;
> > +     bool exists = false;
> > +     struct tcf_mpls *m;
> > +     int ret = 0, err;
> > +     u8 mpls_ttl = 0;
> > +
> > +     if (!nla) {
> > +             NL_SET_ERR_MSG_MOD(extack, "missing netlink attributes");
> > +             return -EINVAL;
> > +     }
> > +
> > +     err = nla_parse_nested(tb, TCA_MPLS_MAX, nla, mpls_policy, extack);
> > +     if (err < 0)
> > +             return err;
> > +
> > +     if (!tb[TCA_MPLS_PARMS]) {
> > +             NL_SET_ERR_MSG_MOD(extack, "no MPLS params");
> > +             return -EINVAL;
> > +     }
> > +     parm = nla_data(tb[TCA_MPLS_PARMS]);
> > +
> > +     /* Verify parameters against action type. */
> > +     switch (parm->m_action) {
> > +     case TCA_MPLS_ACT_POP:
> > +             if (!tb[TCA_MPLS_PROTO] ||
> > +                 !eth_proto_is_802_3(nla_get_be16(tb[TCA_MPLS_PROTO]))) {
> > +                     NL_SET_ERR_MSG_MOD(extack, "MPLS POP: invalid proto");
>
> Please spell out the messages:
>   Invalid protocol
>
> > +                     return -EINVAL;
> > +             }
> > +             if (tb[TCA_MPLS_LABEL] || tb[TCA_MPLS_TTL] || tb[TCA_MPLS_TC]) {
> > +                     NL_SET_ERR_MSG_MOD(extack, "MPLS POP: unsupported attrs");
>
> Be more specific with the error message:
>     LABEL, TTL and ??? attributes can not be used with pop action.
>
> > +                     return -EINVAL;
> > +             }
> > +             break;
> > +     case TCA_MPLS_ACT_DEC_TTL:
> > +             if (tb[TCA_MPLS_PROTO] || tb[TCA_MPLS_LABEL] ||
> > +                 tb[TCA_MPLS_TTL] || tb[TCA_MPLS_TC]) {
> > +                     NL_SET_ERR_MSG_MOD(extack, "MPLS DEC TTL: unsupported attrs");
>
> same here and other extack messages here

ack to the extack comments.
Let me sort.
Thanks for input!

>

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

* Re: [PATCH net-next v3 1/2] net: sched: add mpls manipulation actions to TC
  2019-06-14 23:22     ` John Hurley
@ 2019-06-14 23:44       ` David Ahern
  0 siblings, 0 replies; 6+ messages in thread
From: David Ahern @ 2019-06-14 23:44 UTC (permalink / raw)
  To: John Hurley
  Cc: Linux Netdev List, David Miller, Jiri Pirko, Cong Wang,
	Davide Caratti, Simon Horman, Jakub Kicinski, oss-drivers

On 6/14/19 5:22 PM, John Hurley wrote:
> On Fri, Jun 14, 2019 at 6:22 PM David Ahern <dsahern@gmail.com> wrote:
>>
>> On 6/14/19 8:58 AM, John Hurley wrote:
>>> Currently, TC offers the ability to match on the MPLS fields of a packet
>>> through the use of the flow_dissector_key_mpls struct. However, as yet, TC
>>> actions do not allow the modification or manipulation of such fields.
>>>
>>> Add a new module that registers TC action ops to allow manipulation of
>>> MPLS. This includes the ability to push and pop headers as well as modify
>>> the contents of new or existing headers. A further action to decrement the
>>> TTL field of an MPLS header is also provided.
>>
>> you have this limited to a single mpls label. It would be more flexible
>> to allow push/pop of N-labels (push probably being the most useful).
>>
> 
> Hi David.
> Multiple push and pop actions can be done by piping them together.
> E.g. for a flower filter that pushes 2 labels to an IP packet you can do:
> 
> tc filter add dev eth0 protocol ip parent ffff: \
>         flower \
>         action mpls push protocol mpls_mc label 10 \
>         action mpls push protocol mpls_mc label 20 \
>         action mirred egress redirect dev eth1

ok, that seems reasonable.

>>> diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c
>>> new file mode 100644
>>> index 0000000..828a8d9
>>> --- /dev/null
>>> +++ b/net/sched/act_mpls.c
>> ...
>>
>>> +     switch (p->tcfm_action) {
>>> +     case TCA_MPLS_ACT_POP:
>>> +             if (unlikely(!eth_p_mpls(skb->protocol)))
>>> +                     goto out;
>>> +
>>> +             if (unlikely(skb_ensure_writable(skb, ETH_HLEN + MPLS_HLEN)))
>>> +                     goto drop;
>>> +
>>> +             skb_postpull_rcsum(skb, mpls_hdr(skb), MPLS_HLEN);
>>> +             memmove(skb->data + MPLS_HLEN, skb->data, ETH_HLEN);
>>> +
>>> +             __skb_pull(skb, MPLS_HLEN);
>>> +             skb_reset_mac_header(skb);
>>> +             skb_set_network_header(skb, ETH_HLEN);
>>> +
>>> +             tcf_mpls_set_eth_type(skb, p->tcfm_proto);
>>> +             skb->protocol = p->tcfm_proto;
>>
>> This is pop of a single label. It may or may not be the bottom label, so
>> it seems like this should handle both cases and may depend on the
>> packet. If it is the bottom label, then letting the user specify next
>> seems correct but it is not then the protocol needs to stay MPLS.
>>
> 
> Yes, the user is expected to indicate the next protocol after the pop
> even if another mpls label is next.
> We're trying to cater for supporting mpls_uc and mpls_mc ethtypes.
> So you could in theory pop the top mpls unicast header and set the
> next to multicast.
> We expect the user to know what the next header is so enforce that
> they give that information.
> Do you agree with this or should we add more checks around the BoS bit?

that's a tough one. tc filters are very advanced and users should
understand what they are doing. On the other hand, a mistake means
spewing packets that could cause problems elsewhere and best case are
just dropped. If this is inline with other actions (ability to generate
bogus packets on a mistake) then I guess it is fine.



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

end of thread, other threads:[~2019-06-14 23:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-14 14:58 [PATCH net-next v3 0/2] Add MPLS actions to TC John Hurley
2019-06-14 14:58 ` [PATCH net-next v3 1/2] net: sched: add mpls manipulation " John Hurley
2019-06-14 17:22   ` David Ahern
2019-06-14 23:22     ` John Hurley
2019-06-14 23:44       ` David Ahern
2019-06-14 14:58 ` [PATCH net-next v3 2/2] selftests: tc-tests: actions: add MPLS tests John Hurley

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).