Netdev List
 help / color / mirror / Atom feed
* [patch net-next v2 10/20] net: sched: cls_fw: rename fw_change_attrs function
From: Jiri Pirko @ 2017-08-04 12:29 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170804122915.15177-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

Since the function name is misleading since it is not changing
anything, name it similarly to other cls.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_fw.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index a53fa75..358bf34 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -190,10 +190,9 @@ static const struct nla_policy fw_policy[TCA_FW_MAX + 1] = {
 	[TCA_FW_MASK]		= { .type = NLA_U32 },
 };
 
-static int
-fw_change_attrs(struct net *net, struct tcf_proto *tp, struct fw_filter *f,
-		struct nlattr **tb, struct nlattr **tca, unsigned long base,
-		bool ovr)
+static int fw_set_parms(struct net *net, struct tcf_proto *tp,
+			struct fw_filter *f, struct nlattr **tb,
+			struct nlattr **tca, unsigned long base, bool ovr)
 {
 	struct fw_head *head = rtnl_dereference(tp->root);
 	struct tcf_exts e;
@@ -282,7 +281,7 @@ static int fw_change(struct net *net, struct sk_buff *in_skb,
 			return err;
 		}
 
-		err = fw_change_attrs(net, tp, fnew, tb, tca, base, ovr);
+		err = fw_set_parms(net, tp, fnew, tb, tca, base, ovr);
 		if (err < 0) {
 			tcf_exts_destroy(&fnew->exts);
 			kfree(fnew);
@@ -330,7 +329,7 @@ static int fw_change(struct net *net, struct sk_buff *in_skb,
 	f->id = handle;
 	f->tp = tp;
 
-	err = fw_change_attrs(net, tp, f, tb, tca, base, ovr);
+	err = fw_set_parms(net, tp, f, tb, tca, base, ovr);
 	if (err < 0)
 		goto errout;
 
-- 
2.9.3

^ permalink raw reply related

* [patch net-next v2 09/20] net: sched: cls_bpf: rename cls_bpf_modify_existing function
From: Jiri Pirko @ 2017-08-04 12:29 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170804122915.15177-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

The name cls_bpf_modify_existing is highly misleading, as it indeed does
not modify anything existing. It does not modify at all.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 net/sched/cls_bpf.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index f57bd53..e9ab837 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -382,10 +382,9 @@ static int cls_bpf_prog_from_efd(struct nlattr **tb, struct cls_bpf_prog *prog,
 	return 0;
 }
 
-static int cls_bpf_modify_existing(struct net *net, struct tcf_proto *tp,
-				   struct cls_bpf_prog *prog,
-				   unsigned long base, struct nlattr **tb,
-				   struct nlattr *est, bool ovr)
+static int cls_bpf_set_parms(struct net *net, struct tcf_proto *tp,
+			     struct cls_bpf_prog *prog, unsigned long base,
+			     struct nlattr **tb, struct nlattr *est, bool ovr)
 {
 	bool is_bpf, is_ebpf, have_exts = false;
 	struct tcf_exts exts;
@@ -508,8 +507,7 @@ static int cls_bpf_change(struct net *net, struct sk_buff *in_skb,
 		goto errout;
 	}
 
-	ret = cls_bpf_modify_existing(net, tp, prog, base, tb, tca[TCA_RATE],
-				      ovr);
+	ret = cls_bpf_set_parms(net, tp, prog, base, tb, tca[TCA_RATE], ovr);
 	if (ret < 0)
 		goto errout;
 
-- 
2.9.3

^ permalink raw reply related

* [patch net-next v2 08/20] net: sched: use tcf_exts_has_actions instead of exts->nr_actions
From: Jiri Pirko @ 2017-08-04 12:29 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170804122915.15177-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

For check in tcf_exts_dump use tcf_exts_has_actions helper instead
of exts->nr_actions for checking if there are any actions present.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_api.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 287ae6c..735d556 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -915,7 +915,7 @@ int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts *exts)
 #ifdef CONFIG_NET_CLS_ACT
 	struct nlattr *nest;
 
-	if (exts->action && exts->nr_actions) {
+	if (exts->action && tcf_exts_has_actions(exts)) {
 		/*
 		 * again for backward compatible mode - we want
 		 * to work with both old and new modes of entering
-- 
2.9.3

^ permalink raw reply related

* [patch net-next v2 07/20] net: sched: remove check for number of actions in tcf_exts_exec
From: Jiri Pirko @ 2017-08-04 12:29 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170804122915.15177-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

Leave it to tcf_action_exec to return TC_ACT_OK in case there is no
action present.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/pkt_cls.h | 4 +---
 net/sched/act_api.c   | 3 ++-
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 61ce521..b8959c9 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -192,9 +192,7 @@ tcf_exts_exec(struct sk_buff *skb, struct tcf_exts *exts,
 	      struct tcf_result *res)
 {
 #ifdef CONFIG_NET_CLS_ACT
-	if (tcf_exts_has_actions(exts))
-		return tcf_action_exec(skb, exts->actions, exts->nr_actions,
-				       res);
+	return tcf_action_exec(skb, exts->actions, exts->nr_actions, res);
 #endif
 	return TC_ACT_OK;
 }
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index f19b118..a2915d9 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -473,9 +473,10 @@ static struct tc_action_ops *tc_lookup_action(struct nlattr *kind)
 int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
 		    int nr_actions, struct tcf_result *res)
 {
-	int ret = -1, i;
 	u32 jmp_prgcnt = 0;
 	u32 jmp_ttl = TCA_ACT_MAX_PRIO; /*matches actions per filter */
+	int i;
+	int ret = TC_ACT_OK;
 
 	if (skb_skip_tc_classify(skb))
 		return TC_ACT_OK;
-- 
2.9.3

^ permalink raw reply related

* [patch net-next v2 06/20] net: sched: fix return value of tcf_exts_exec
From: Jiri Pirko @ 2017-08-04 12:29 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170804122915.15177-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

Return the defined TC_ACT_OK instead of 0.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/pkt_cls.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 817badf..61ce521 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -182,7 +182,7 @@ static inline bool tcf_exts_has_one_action(struct tcf_exts *exts)
  * @exts: tc filter extensions handle
  * @res: desired result
  *
- * Executes all configured extensions. Returns 0 on a normal execution,
+ * Executes all configured extensions. Returns TC_ACT_OK on a normal execution,
  * a negative number if the filter must be considered unmatched or
  * a positive action code (TC_ACT_*) which must be returned to the
  * underlying layer.
@@ -196,7 +196,7 @@ tcf_exts_exec(struct sk_buff *skb, struct tcf_exts *exts,
 		return tcf_action_exec(skb, exts->actions, exts->nr_actions,
 				       res);
 #endif
-	return 0;
+	return TC_ACT_OK;
 }
 
 int tcf_exts_validate(struct net *net, struct tcf_proto *tp,
-- 
2.9.3

^ permalink raw reply related

* [patch net-next v2 05/20] net: sched: remove redundant helpers tcf_exts_is_predicative and tcf_exts_is_available
From: Jiri Pirko @ 2017-08-04 12:29 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170804122915.15177-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

These two helpers are doing the same as tcf_exts_has_actions, so remove
them and use tcf_exts_has_actions instead.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/pkt_cls.h   | 30 ------------------------------
 net/sched/cls_fw.c      |  2 +-
 net/sched/cls_route.c   |  2 +-
 net/sched/cls_tcindex.c |  2 +-
 4 files changed, 3 insertions(+), 33 deletions(-)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 322a282..817badf 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -113,36 +113,6 @@ static inline int tcf_exts_init(struct tcf_exts *exts, int action, int police)
 	return 0;
 }
 
-/**
- * tcf_exts_is_predicative - check if a predicative extension is present
- * @exts: tc filter extensions handle
- *
- * Returns 1 if a predicative extension is present, i.e. an extension which
- * might cause further actions and thus overrule the regular tcf_result.
- */
-static inline int
-tcf_exts_is_predicative(struct tcf_exts *exts)
-{
-#ifdef CONFIG_NET_CLS_ACT
-	return exts->nr_actions;
-#else
-	return 0;
-#endif
-}
-
-/**
- * tcf_exts_is_available - check if at least one extension is present
- * @exts: tc filter extensions handle
- *
- * Returns 1 if at least one extension is present.
- */
-static inline int
-tcf_exts_is_available(struct tcf_exts *exts)
-{
-	/* All non-predicative extensions must be added here. */
-	return tcf_exts_is_predicative(exts);
-}
-
 static inline void tcf_exts_to_list(const struct tcf_exts *exts,
 				    struct list_head *actions)
 {
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index d388536..a53fa75 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -387,7 +387,7 @@ static int fw_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 
 	t->tcm_handle = f->id;
 
-	if (!f->res.classid && !tcf_exts_is_available(&f->exts))
+	if (!f->res.classid && !tcf_exts_has_actions(&f->exts))
 		return skb->len;
 
 	nest = nla_nest_start(skb, TCA_OPTIONS);
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index d63d550..26f8636 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -113,7 +113,7 @@ static inline int route4_hash_wild(void)
 #define ROUTE4_APPLY_RESULT()					\
 {								\
 	*res = f->res;						\
-	if (tcf_exts_is_available(&f->exts)) {			\
+	if (tcf_exts_has_actions(&f->exts)) {			\
 		int r = tcf_exts_exec(skb, &f->exts, res);	\
 		if (r < 0) {					\
 			dont_cache = 1;				\
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index 8a8a583..66924d1 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -52,7 +52,7 @@ struct tcindex_data {
 
 static inline int tcindex_filter_is_set(struct tcindex_filter_result *r)
 {
-	return tcf_exts_is_predicative(&r->exts) || r->res.classid;
+	return tcf_exts_has_actions(&r->exts) || r->res.classid;
 }
 
 static struct tcindex_filter_result *tcindex_lookup(struct tcindex_data *p,
-- 
2.9.3

^ permalink raw reply related

* [patch net-next v2 04/20] net: sched: use tcf_exts_has_actions in tcf_exts_exec
From: Jiri Pirko @ 2017-08-04 12:28 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170804122915.15177-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

Use the tcf_exts_has_actions helper instead or directly testing
exts->nr_actions in tcf_exts_exec.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/pkt_cls.h | 46 +++++++++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 7f25636..322a282 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -177,29 +177,6 @@ tcf_exts_stats_update(const struct tcf_exts *exts,
 }
 
 /**
- * tcf_exts_exec - execute tc filter extensions
- * @skb: socket buffer
- * @exts: tc filter extensions handle
- * @res: desired result
- *
- * Executes all configured extensions. Returns 0 on a normal execution,
- * a negative number if the filter must be considered unmatched or
- * a positive action code (TC_ACT_*) which must be returned to the
- * underlying layer.
- */
-static inline int
-tcf_exts_exec(struct sk_buff *skb, struct tcf_exts *exts,
-	       struct tcf_result *res)
-{
-#ifdef CONFIG_NET_CLS_ACT
-	if (exts->nr_actions)
-		return tcf_action_exec(skb, exts->actions, exts->nr_actions,
-				       res);
-#endif
-	return 0;
-}
-
-/**
  * tcf_exts_has_actions - check if at least one action is present
  * @exts: tc filter extensions handle
  *
@@ -229,6 +206,29 @@ static inline bool tcf_exts_has_one_action(struct tcf_exts *exts)
 #endif
 }
 
+/**
+ * tcf_exts_exec - execute tc filter extensions
+ * @skb: socket buffer
+ * @exts: tc filter extensions handle
+ * @res: desired result
+ *
+ * Executes all configured extensions. Returns 0 on a normal execution,
+ * a negative number if the filter must be considered unmatched or
+ * a positive action code (TC_ACT_*) which must be returned to the
+ * underlying layer.
+ */
+static inline int
+tcf_exts_exec(struct sk_buff *skb, struct tcf_exts *exts,
+	      struct tcf_result *res)
+{
+#ifdef CONFIG_NET_CLS_ACT
+	if (tcf_exts_has_actions(exts))
+		return tcf_action_exec(skb, exts->actions, exts->nr_actions,
+				       res);
+#endif
+	return 0;
+}
+
 int tcf_exts_validate(struct net *net, struct tcf_proto *tp,
 		      struct nlattr **tb, struct nlattr *rate_tlv,
 		      struct tcf_exts *exts, bool ovr);
-- 
2.9.3

^ permalink raw reply related

* [patch net-next v2 03/20] net: sched: change names of action number helpers to be aligned with the rest
From: Jiri Pirko @ 2017-08-04 12:28 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170804122915.15177-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

The rest of the helpers are named tcf_exts_*, so change the name of
the action number helpers to be aligned. While at it, change to inline
functions.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c  |  2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c      |  2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c    |  4 +--
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     |  2 +-
 .../net/ethernet/mellanox/mlxsw/spectrum_flower.c  |  2 +-
 drivers/net/ethernet/netronome/nfp/bpf/offload.c   |  4 +--
 include/net/pkt_cls.h                              | 36 ++++++++++++++++------
 net/dsa/slave.c                                    |  2 +-
 net/sched/cls_api.c                                |  2 +-
 9 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c
index ef06ce8..6f734c5 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c
@@ -96,7 +96,7 @@ static int fill_action_fields(struct adapter *adap,
 	LIST_HEAD(actions);
 
 	exts = cls->knode.exts;
-	if (tc_no_actions(exts))
+	if (!tcf_exts_has_actions(exts))
 		return -EINVAL;
 
 	tcf_exts_to_list(exts, &actions);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 96606e3..091fcc7 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -8953,7 +8953,7 @@ static int parse_tc_actions(struct ixgbe_adapter *adapter,
 	LIST_HEAD(actions);
 	int err;
 
-	if (tc_no_actions(exts))
+	if (!tcf_exts_has_actions(exts))
 		return -EINVAL;
 
 	tcf_exts_to_list(exts, &actions);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 3c536f5..78f50d9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -1326,7 +1326,7 @@ static int parse_tc_nic_actions(struct mlx5e_priv *priv, struct tcf_exts *exts,
 	LIST_HEAD(actions);
 	int err;
 
-	if (tc_no_actions(exts))
+	if (!tcf_exts_has_actions(exts))
 		return -EINVAL;
 
 	attr->flow_tag = MLX5_FS_DEFAULT_FLOW_TAG;
@@ -1839,7 +1839,7 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv, struct tcf_exts *exts,
 	bool encap = false;
 	int err = 0;
 
-	if (tc_no_actions(exts))
+	if (!tcf_exts_has_actions(exts))
 		return -EINVAL;
 
 	memset(attr, 0, sizeof(*attr));
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 88b668b..66d511d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -1626,7 +1626,7 @@ static int mlxsw_sp_port_add_cls_matchall(struct mlxsw_sp_port *mlxsw_sp_port,
 	LIST_HEAD(actions);
 	int err;
 
-	if (!tc_single_action(cls->exts)) {
+	if (!tcf_exts_has_one_action(cls->exts)) {
 		netdev_err(mlxsw_sp_port->dev, "only singular actions are supported\n");
 		return -EOPNOTSUPP;
 	}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
index 400ad40..9be48d2e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
@@ -53,7 +53,7 @@ static int mlxsw_sp_flower_parse_actions(struct mlxsw_sp *mlxsw_sp,
 	LIST_HEAD(actions);
 	int err;
 
-	if (tc_no_actions(exts))
+	if (!tcf_exts_has_actions(exts))
 		return 0;
 
 	/* Count action is inserted first */
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index 78d80a3..a88bb5b 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -115,14 +115,14 @@ nfp_net_bpf_get_act(struct nfp_net *nn, struct tc_cls_bpf_offload *cls_bpf)
 
 	/* TC direct action */
 	if (cls_bpf->exts_integrated) {
-		if (tc_no_actions(cls_bpf->exts))
+		if (!tcf_exts_has_actions(cls_bpf->exts))
 			return NN_ACT_DIRECT;
 
 		return -EOPNOTSUPP;
 	}
 
 	/* TC legacy mode */
-	if (!tc_single_action(cls_bpf->exts))
+	if (!tcf_exts_has_one_action(cls_bpf->exts))
 		return -EOPNOTSUPP;
 
 	tcf_exts_to_list(cls_bpf->exts, &actions);
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index f4462ec..7f25636 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -199,17 +199,35 @@ tcf_exts_exec(struct sk_buff *skb, struct tcf_exts *exts,
 	return 0;
 }
 
+/**
+ * tcf_exts_has_actions - check if at least one action is present
+ * @exts: tc filter extensions handle
+ *
+ * Returns true if at least one action is present.
+ */
+static inline bool tcf_exts_has_actions(struct tcf_exts *exts)
+{
 #ifdef CONFIG_NET_CLS_ACT
+	return exts->nr_actions;
+#else
+	return false;
+#endif
+}
 
-#define tc_no_actions(_exts)  ((_exts)->nr_actions == 0)
-#define tc_single_action(_exts) ((_exts)->nr_actions == 1)
-
-#else /* CONFIG_NET_CLS_ACT */
-
-#define tc_no_actions(_exts) true
-#define tc_single_action(_exts) false
-
-#endif /* CONFIG_NET_CLS_ACT */
+/**
+ * tcf_exts_has_one_action - check if exactly one action is present
+ * @exts: tc filter extensions handle
+ *
+ * Returns true if exactly one action is present.
+ */
+static inline bool tcf_exts_has_one_action(struct tcf_exts *exts)
+{
+#ifdef CONFIG_NET_CLS_ACT
+	return exts->nr_actions == 1;
+#else
+	return false;
+#endif
+}
 
 int tcf_exts_validate(struct net *net, struct tcf_proto *tp,
 		      struct nlattr **tb, struct nlattr *rate_tlv,
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index e196562..83252e8 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -779,7 +779,7 @@ static int dsa_slave_add_cls_matchall(struct net_device *dev,
 	if (!ds->ops->port_mirror_add)
 		return err;
 
-	if (!tc_single_action(cls->exts))
+	if (!tcf_exts_has_one_action(cls->exts))
 		return err;
 
 	tcf_exts_to_list(cls->exts, &actions);
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 39da0c5..287ae6c 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -972,7 +972,7 @@ int tcf_exts_get_dev(struct net_device *dev, struct tcf_exts *exts,
 	const struct tc_action *a;
 	LIST_HEAD(actions);
 
-	if (tc_no_actions(exts))
+	if (!tcf_exts_has_actions(exts))
 		return -EINVAL;
 
 	tcf_exts_to_list(exts, &actions);
-- 
2.9.3

^ permalink raw reply related

* [patch net-next v2 02/20] net: sched: remove unneeded tcf_em_tree_change
From: Jiri Pirko @ 2017-08-04 12:28 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170804122915.15177-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

Since tcf_em_tree_validate could be always called on a newly created
filter, there is no need for this change function.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/pkt_cls.h  | 21 ---------------------
 net/sched/cls_basic.c  |  4 +---
 net/sched/cls_cgroup.c |  4 +---
 net/sched/cls_flow.c   | 12 +++++-------
 4 files changed, 7 insertions(+), 34 deletions(-)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 537d0a0..f4462ec 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -333,26 +333,6 @@ int __tcf_em_tree_match(struct sk_buff *, struct tcf_ematch_tree *,
 			struct tcf_pkt_info *);
 
 /**
- * tcf_em_tree_change - replace ematch tree of a running classifier
- *
- * @tp: classifier kind handle
- * @dst: destination ematch tree variable
- * @src: source ematch tree (temporary tree from tcf_em_tree_validate)
- *
- * This functions replaces the ematch tree in @dst with the ematch
- * tree in @src. The classifier in charge of the ematch tree may be
- * running.
- */
-static inline void tcf_em_tree_change(struct tcf_proto *tp,
-				      struct tcf_ematch_tree *dst,
-				      struct tcf_ematch_tree *src)
-{
-	tcf_tree_lock(tp);
-	memcpy(dst, src, sizeof(*dst));
-	tcf_tree_unlock(tp);
-}
-
-/**
  * tcf_em_tree_match - evaulate an ematch tree
  *
  * @skb: socket buffer of the packet in question
@@ -386,7 +366,6 @@ struct tcf_ematch_tree {
 #define tcf_em_tree_validate(tp, tb, t) ((void)(t), 0)
 #define tcf_em_tree_destroy(t) do { (void)(t); } while(0)
 #define tcf_em_tree_dump(skb, t, tlv) (0)
-#define tcf_em_tree_change(tp, dst, src) do { } while(0)
 #define tcf_em_tree_match(skb, t, info) ((void)(info), 1)
 
 #endif /* CONFIG_NET_EMATCH */
diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index c4fd63a..979cd26 100644
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -130,7 +130,6 @@ static int basic_set_parms(struct net *net, struct tcf_proto *tp,
 {
 	int err;
 	struct tcf_exts e;
-	struct tcf_ematch_tree t;
 
 	err = tcf_exts_init(&e, TCA_BASIC_ACT, TCA_BASIC_POLICE);
 	if (err < 0)
@@ -139,7 +138,7 @@ static int basic_set_parms(struct net *net, struct tcf_proto *tp,
 	if (err < 0)
 		goto errout;
 
-	err = tcf_em_tree_validate(tp, tb[TCA_BASIC_EMATCHES], &t);
+	err = tcf_em_tree_validate(tp, tb[TCA_BASIC_EMATCHES], &f->ematches);
 	if (err < 0)
 		goto errout;
 
@@ -149,7 +148,6 @@ static int basic_set_parms(struct net *net, struct tcf_proto *tp,
 	}
 
 	tcf_exts_change(tp, &f->exts, &e);
-	tcf_em_tree_change(tp, &f->ematches, &t);
 	f->tp = tp;
 
 	return 0;
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 12ce547..ce7d38b 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -76,7 +76,6 @@ static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb,
 	struct nlattr *tb[TCA_CGROUP_MAX + 1];
 	struct cls_cgroup_head *head = rtnl_dereference(tp->root);
 	struct cls_cgroup_head *new;
-	struct tcf_ematch_tree t;
 	struct tcf_exts e;
 	int err;
 
@@ -112,14 +111,13 @@ static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb,
 		goto errout;
 	}
 
-	err = tcf_em_tree_validate(tp, tb[TCA_CGROUP_EMATCHES], &t);
+	err = tcf_em_tree_validate(tp, tb[TCA_CGROUP_EMATCHES], &new->ematches);
 	if (err < 0) {
 		tcf_exts_destroy(&e);
 		goto errout;
 	}
 
 	tcf_exts_change(tp, &new->exts, &e);
-	tcf_em_tree_change(tp, &new->ematches, &t);
 
 	rcu_assign_pointer(tp->root, new);
 	if (head)
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index 3065752..71fd1af 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -389,7 +389,6 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
 	struct nlattr *opt = tca[TCA_OPTIONS];
 	struct nlattr *tb[TCA_FLOW_MAX + 1];
 	struct tcf_exts e;
-	struct tcf_ematch_tree t;
 	unsigned int nkeys = 0;
 	unsigned int perturb_period = 0;
 	u32 baseclass = 0;
@@ -432,13 +431,13 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
 	if (err < 0)
 		goto err1;
 
-	err = tcf_em_tree_validate(tp, tb[TCA_FLOW_EMATCHES], &t);
-	if (err < 0)
-		goto err1;
-
 	err = -ENOBUFS;
 	fnew = kzalloc(sizeof(*fnew), GFP_KERNEL);
 	if (!fnew)
+		goto err1;
+
+	err = tcf_em_tree_validate(tp, tb[TCA_FLOW_EMATCHES], &fnew->ematches);
+	if (err < 0)
 		goto err2;
 
 	err = tcf_exts_init(&fnew->exts, TCA_FLOW_ACT, TCA_FLOW_POLICE);
@@ -512,7 +511,6 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
 			       (unsigned long)fnew);
 
 	tcf_exts_change(tp, &fnew->exts, &e);
-	tcf_em_tree_change(tp, &fnew->ematches, &t);
 
 	netif_keep_dst(qdisc_dev(tp->q));
 
@@ -554,8 +552,8 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
 
 err3:
 	tcf_exts_destroy(&fnew->exts);
+	tcf_em_tree_destroy(&fnew->ematches);
 err2:
-	tcf_em_tree_destroy(&t);
 	kfree(fnew);
 err1:
 	tcf_exts_destroy(&e);
-- 
2.9.3

^ permalink raw reply related

* [patch net-next v2 01/20] net: sched: sch_atm: use Qdisc_class_common structure
From: Jiri Pirko @ 2017-08-04 12:28 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170804122915.15177-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

Even if it is only for classid now, use this common struct a be aligned
with the rest of the classful qdiscs.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_atm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/sched/sch_atm.c b/net/sched/sch_atm.c
index 572fe25..0af4b1c 100644
--- a/net/sched/sch_atm.c
+++ b/net/sched/sch_atm.c
@@ -41,6 +41,7 @@
 #define VCC2FLOW(vcc) ((struct atm_flow_data *) ((vcc)->user_back))
 
 struct atm_flow_data {
+	struct Qdisc_class_common common;
 	struct Qdisc		*q;	/* FIFO, TBF, etc. */
 	struct tcf_proto __rcu	*filter_list;
 	struct tcf_block	*block;
@@ -49,7 +50,6 @@ struct atm_flow_data {
 					   struct sk_buff *skb); /* chaining */
 	struct atm_qdisc_data	*parent;	/* parent qdisc */
 	struct socket		*sock;		/* for closing */
-	u32			classid;	/* x:y type ID */
 	int			ref;		/* reference count */
 	struct gnet_stats_basic_packed	bstats;
 	struct gnet_stats_queue	qstats;
@@ -75,7 +75,7 @@ static inline struct atm_flow_data *lookup_flow(struct Qdisc *sch, u32 classid)
 	struct atm_flow_data *flow;
 
 	list_for_each_entry(flow, &p->flows, list) {
-		if (flow->classid == classid)
+		if (flow->common.classid == classid)
 			return flow;
 	}
 	return NULL;
@@ -293,7 +293,7 @@ static int atm_tc_change(struct Qdisc *sch, u32 classid, u32 parent,
 	flow->old_pop = flow->vcc->pop;
 	flow->parent = p;
 	flow->vcc->pop = sch_atm_pop;
-	flow->classid = classid;
+	flow->common.classid = classid;
 	flow->ref = 1;
 	flow->excess = excess;
 	list_add(&flow->list, &p->link.list);
@@ -549,7 +549,7 @@ static int atm_tc_init(struct Qdisc *sch, struct nlattr *opt)
 
 	p->link.vcc = NULL;
 	p->link.sock = NULL;
-	p->link.classid = sch->handle;
+	p->link.common.classid = sch->handle;
 	p->link.ref = 1;
 	tasklet_init(&p->task, sch_atm_dequeue, (unsigned long)sch);
 	return 0;
@@ -594,7 +594,7 @@ static int atm_tc_dump_class(struct Qdisc *sch, unsigned long cl,
 		sch, p, flow, skb, tcm);
 	if (list_empty(&flow->list))
 		return -EINVAL;
-	tcm->tcm_handle = flow->classid;
+	tcm->tcm_handle = flow->common.classid;
 	tcm->tcm_info = flow->q->handle;
 
 	nest = nla_nest_start(skb, TCA_OPTIONS);
@@ -619,7 +619,7 @@ static int atm_tc_dump_class(struct Qdisc *sch, unsigned long cl,
 			goto nla_put_failure;
 	}
 	if (flow->excess) {
-		if (nla_put_u32(skb, TCA_ATM_EXCESS, flow->classid))
+		if (nla_put_u32(skb, TCA_ATM_EXCESS, flow->common.classid))
 			goto nla_put_failure;
 	} else {
 		if (nla_put_u32(skb, TCA_ATM_EXCESS, 0))
-- 
2.9.3

^ permalink raw reply related

* [patch net-next v2 00/20] net: sched: summer cleanup part 1, mainly in exts area
From: Jiri Pirko @ 2017-08-04 12:28 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw

From: Jiri Pirko <jiri@mellanox.com>

This patchset is one of the couple cleanup patchsets I have in queue.
The motivation aside the obvious need to "make things nicer" is also
to prepare for shared filter blocks introduction. That requires tp->q
removal, and therefore removal of all tp->q users.

Patch 1 is just some small thing I spotted on the way
Patch 2 removes one user of tp->q, namely tcf_em_tree_change
Patches 3-8 do preparations for exts->nr_actions removal
Patches 9-10 do simple renames of functions in cls*
Patches 11-19 remove unnecessary calls of tcf_exts_change helper
The last patch changes tcf_exts_change to don't take lock

Tested by tools/testing/selftests/tc-testing

v1->v2:
- removed conversion of action array to list as noted by Cong
- added the past patch instead
- small rebases of patches 11-19

Jiri Pirko (20):
  net: sched: sch_atm: use Qdisc_class_common structure
  net: sched: remove unneeded tcf_em_tree_change
  net: sched: change names of action number helpers to be aligned with
    the rest
  net: sched: use tcf_exts_has_actions in tcf_exts_exec
  net: sched: remove redundant helpers tcf_exts_is_predicative and
    tcf_exts_is_available
  net: sched: fix return value of tcf_exts_exec
  net: sched: remove check for number of actions in tcf_exts_exec
  net: sched: use tcf_exts_has_actions instead of exts->nr_actions
  net: sched: cls_bpf: rename cls_bpf_modify_existing function
  net: sched: cls_fw: rename fw_change_attrs function
  net: sched: cls_flower: no need to call tcf_exts_change for newly
    allocated struct
  net: sched: cls_fw: no need to call tcf_exts_change for newly
    allocated struct
  net: sched: cls_matchall: no need to call tcf_exts_change for newly
    allocated struct
  net: sched: cls_basic: no need to call tcf_exts_change for newly
    allocated struct
  net: sched: cls_bpf: no need to call tcf_exts_change for newly
    allocated struct
  net: sched: cls_cgroup: no need to call tcf_exts_change for newly
    allocated struct
  net: sched: cls_flow: no need to call tcf_exts_change for newly
    allocated struct
  net: sched: cls_route: no need to call tcf_exts_change for newly
    allocated struct
  net: sched: cls_u32: no need to call tcf_exts_change for newly
    allocated struct
  net: sched: avoid atomic swap in tcf_exts_change

 drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c  |   2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c      |   2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c    |   4 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     |   2 +-
 .../net/ethernet/mellanox/mlxsw/spectrum_flower.c  |   2 +-
 drivers/net/ethernet/netronome/nfp/bpf/offload.c   |   4 +-
 include/net/pkt_cls.h                              | 106 +++++++--------------
 net/dsa/slave.c                                    |   2 +-
 net/sched/act_api.c                                |   3 +-
 net/sched/cls_api.c                                |  14 +--
 net/sched/cls_basic.c                              |  17 +---
 net/sched/cls_bpf.c                                |  35 ++-----
 net/sched/cls_cgroup.c                             |  18 +---
 net/sched/cls_flow.c                               |  47 ++++-----
 net/sched/cls_flower.c                             |  13 +--
 net/sched/cls_fw.c                                 |  34 +++----
 net/sched/cls_matchall.c                           |  14 +--
 net/sched/cls_route.c                              |  32 ++-----
 net/sched/cls_rsvp.h                               |   4 +-
 net/sched/cls_tcindex.c                            |   8 +-
 net/sched/cls_u32.c                                |  18 +---
 net/sched/sch_atm.c                                |  12 +--
 22 files changed, 125 insertions(+), 268 deletions(-)

-- 
2.9.3

^ permalink raw reply

* [PATCH net 2/2] bpf, s390: fix build for libbpf and selftest suite
From: Daniel Borkmann @ 2017-08-04 12:20 UTC (permalink / raw)
  To: davem; +Cc: holzheu, ast, netdev, Daniel Borkmann
In-Reply-To: <cover.1501848914.git.daniel@iogearbox.net>

The BPF feature test as well as libbpf is missing the __NR_bpf
define for s390 and currently refuses to compile (selftest suite
depends on libbpf as well). Similar issue was fixed some time
ago via b0c47807d31d ("bpf: Add sparc support to tools and
samples."), just do the same and add definitions.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 tools/build/feature/test-bpf.c | 2 ++
 tools/lib/bpf/bpf.c            | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/tools/build/feature/test-bpf.c b/tools/build/feature/test-bpf.c
index 7598361..da2172f 100644
--- a/tools/build/feature/test-bpf.c
+++ b/tools/build/feature/test-bpf.c
@@ -11,6 +11,8 @@
 #  define __NR_bpf 280
 # elif defined(__sparc__)
 #  define __NR_bpf 349
+# elif defined(__s390__)
+#  define __NR_bpf 351
 # else
 #  error __NR_bpf not defined. libbpf does not support your arch.
 # endif
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 256f571..e5bbb09 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -39,6 +39,8 @@
 #  define __NR_bpf 280
 # elif defined(__sparc__)
 #  define __NR_bpf 349
+# elif defined(__s390__)
+#  define __NR_bpf 351
 # else
 #  error __NR_bpf not defined. libbpf does not support your arch.
 # endif
-- 
1.9.3

^ permalink raw reply related

* [PATCH net 0/2] Two BPF fixes for s390
From: Daniel Borkmann @ 2017-08-04 12:20 UTC (permalink / raw)
  To: davem; +Cc: holzheu, ast, netdev, Daniel Borkmann

Found while testing some other work touching JITs.

Thanks!

Daniel Borkmann (2):
  bpf, s390: fix jit branch offset related to ldimm64
  bpf, s390: fix build for libbpf and selftest suite

 arch/s390/net/bpf_jit_comp.c   | 3 ++-
 tools/build/feature/test-bpf.c | 2 ++
 tools/lib/bpf/bpf.c            | 2 ++
 3 files changed, 6 insertions(+), 1 deletion(-)

-- 
1.9.3

^ permalink raw reply

* [PATCH net 1/2] bpf, s390: fix jit branch offset related to ldimm64
From: Daniel Borkmann @ 2017-08-04 12:20 UTC (permalink / raw)
  To: davem; +Cc: holzheu, ast, netdev, Daniel Borkmann
In-Reply-To: <cover.1501848914.git.daniel@iogearbox.net>

While testing some other work that required JIT modifications, I
run into test_bpf causing a hang when JIT enabled on s390. The
problematic test case was the one from ddc665a4bb4b (bpf, arm64:
fix jit branch offset related to ldimm64), and turns out that we
do have a similar issue on s390 as well. In bpf_jit_prog() we
update next instruction address after returning from bpf_jit_insn()
with an insn_count. bpf_jit_insn() returns either -1 in case of
error (e.g. unsupported insn), 1 or 2. The latter is only the
case for ldimm64 due to spanning 2 insns, however, next address
is only set to i + 1 not taking actual insn_count into account,
thus fix is to use insn_count instead of 1. bpf_jit_enable in
mode 2 provides also disasm on s390:

Before fix:

  000003ff800349b6: a7f40003   brc     15,3ff800349bc                 ; target
  000003ff800349ba: 0000               unknown
  000003ff800349bc: e3b0f0700024       stg     %r11,112(%r15)
  000003ff800349c2: e3e0f0880024       stg     %r14,136(%r15)
  000003ff800349c8: 0db0               basr    %r11,%r0
  000003ff800349ca: c0ef00000000       llilf   %r14,0
  000003ff800349d0: e320b0360004       lg      %r2,54(%r11)
  000003ff800349d6: e330b03e0004       lg      %r3,62(%r11)
  000003ff800349dc: ec23ffeda065       clgrj   %r2,%r3,10,3ff800349b6 ; jmp
  000003ff800349e2: e3e0b0460004       lg      %r14,70(%r11)
  000003ff800349e8: e3e0b04e0004       lg      %r14,78(%r11)
  000003ff800349ee: b904002e   lgr     %r2,%r14
  000003ff800349f2: e3b0f0700004       lg      %r11,112(%r15)
  000003ff800349f8: e3e0f0880004       lg      %r14,136(%r15)
  000003ff800349fe: 07fe               bcr     15,%r14

After fix:

  000003ff80ef3db4: a7f40003   brc     15,3ff80ef3dba
  000003ff80ef3db8: 0000               unknown
  000003ff80ef3dba: e3b0f0700024       stg     %r11,112(%r15)
  000003ff80ef3dc0: e3e0f0880024       stg     %r14,136(%r15)
  000003ff80ef3dc6: 0db0               basr    %r11,%r0
  000003ff80ef3dc8: c0ef00000000       llilf   %r14,0
  000003ff80ef3dce: e320b0360004       lg      %r2,54(%r11)
  000003ff80ef3dd4: e330b03e0004       lg      %r3,62(%r11)
  000003ff80ef3dda: ec230006a065       clgrj   %r2,%r3,10,3ff80ef3de6 ; jmp
  000003ff80ef3de0: e3e0b0460004       lg      %r14,70(%r11)
  000003ff80ef3de6: e3e0b04e0004       lg      %r14,78(%r11)          ; target
  000003ff80ef3dec: b904002e   lgr     %r2,%r14
  000003ff80ef3df0: e3b0f0700004       lg      %r11,112(%r15)
  000003ff80ef3df6: e3e0f0880004       lg      %r14,136(%r15)
  000003ff80ef3dfc: 07fe               bcr     15,%r14

test_bpf.ko suite runs fine after the fix.

Fixes: 054623105728 ("s390/bpf: Add s390x eBPF JIT compiler backend")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Tested-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
---
 arch/s390/net/bpf_jit_comp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 01c6fbc..1803797 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -1253,7 +1253,8 @@ static int bpf_jit_prog(struct bpf_jit *jit, struct bpf_prog *fp)
 		insn_count = bpf_jit_insn(jit, fp, i);
 		if (insn_count < 0)
 			return -1;
-		jit->addrs[i + 1] = jit->prg; /* Next instruction address */
+		/* Next instruction address */
+		jit->addrs[i + insn_count] = jit->prg;
 	}
 	bpf_jit_epilogue(jit);
 
-- 
1.9.3

^ permalink raw reply related

* [patch net 2/2] mlxsw: spectrum_switchdev: Release multicast groups during fini
From: Jiri Pirko @ 2017-08-04 12:12 UTC (permalink / raw)
  To: netdev; +Cc: davem, idosch, mlxsw
In-Reply-To: <20170804121230.2144-1-jiri@resnulli.us>

From: Ido Schimmel <idosch@mellanox.com>

Each multicast group (MID) stores a bitmap of ports to which a packet
should be forwarded to in case an MDB entry associated with the MID is
hit.

Since the initial introduction of IGMP snooping in commit 3a49b4fde2a1
("mlxsw: Adding layer 2 multicast support") the driver didn't correctly
free these multicast groups upon ungraceful situations such as the
removal of the upper bridge device or module removal.

The correct way to fix this is to associate each MID with the bridge
ports member in it and then drop the reference in case the bridge port
is destroyed, but this will result in a lot more code and will be fixed
in net-next.

For now, upon module removal, traverse the MID list and release each
one.

Fixes: 3a49b4fde2a1 ("mlxsw: Adding layer 2 multicast support")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index 73d54df..5eb1606 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -1974,6 +1974,17 @@ static void mlxsw_sp_fdb_fini(struct mlxsw_sp *mlxsw_sp)
 
 }
 
+static void mlxsw_sp_mids_fini(struct mlxsw_sp *mlxsw_sp)
+{
+	struct mlxsw_sp_mid *mid, *tmp;
+
+	list_for_each_entry_safe(mid, tmp, &mlxsw_sp->bridge->mids_list, list) {
+		list_del(&mid->list);
+		clear_bit(mid->mid, mlxsw_sp->bridge->mids_bitmap);
+		kfree(mid);
+	}
+}
+
 int mlxsw_sp_switchdev_init(struct mlxsw_sp *mlxsw_sp)
 {
 	struct mlxsw_sp_bridge *bridge;
@@ -1996,7 +2007,7 @@ int mlxsw_sp_switchdev_init(struct mlxsw_sp *mlxsw_sp)
 void mlxsw_sp_switchdev_fini(struct mlxsw_sp *mlxsw_sp)
 {
 	mlxsw_sp_fdb_fini(mlxsw_sp);
-	WARN_ON(!list_empty(&mlxsw_sp->bridge->mids_list));
+	mlxsw_sp_mids_fini(mlxsw_sp);
 	WARN_ON(!list_empty(&mlxsw_sp->bridge->bridges_list));
 	kfree(mlxsw_sp->bridge);
 }
-- 
2.9.3

^ permalink raw reply related

* [patch net 1/2] mlxsw: spectrum_switchdev: Don't warn about valid situations
From: Jiri Pirko @ 2017-08-04 12:12 UTC (permalink / raw)
  To: netdev; +Cc: davem, idosch, mlxsw
In-Reply-To: <20170804121230.2144-1-jiri@resnulli.us>

From: Ido Schimmel <idosch@mellanox.com>

Some operations in the bridge driver such as MDB deletion are preformed
in an atomic context and thus deferred to a process context by the
switchdev infrastructure.

Therefore, by the time the operation is performed by the underlying
device driver it's possible the bridge port context is already gone.
This is especially true for removal flows, but theoretically can also be
invoked during addition.

Remove the warnings in such situations and return normally.

Fixes: c57529e1d5d8 ("mlxsw: spectrum: Replace vPorts with Port-VLAN")
Fixes: 3922285d96e7 ("net: bridge: Add support for offloading port attributes")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../ethernet/mellanox/mlxsw/spectrum_switchdev.c   | 24 +++++++++++-----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index 656b2d3..73d54df 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -626,8 +626,8 @@ static int mlxsw_sp_port_attr_br_flags_set(struct mlxsw_sp_port *mlxsw_sp_port,
 
 	bridge_port = mlxsw_sp_bridge_port_find(mlxsw_sp_port->mlxsw_sp->bridge,
 						orig_dev);
-	if (WARN_ON(!bridge_port))
-		return -EINVAL;
+	if (!bridge_port)
+		return 0;
 
 	err = mlxsw_sp_bridge_port_flood_table_set(mlxsw_sp_port, bridge_port,
 						   MLXSW_SP_FLOOD_TYPE_UC,
@@ -711,8 +711,8 @@ static int mlxsw_sp_port_attr_mc_router_set(struct mlxsw_sp_port *mlxsw_sp_port,
 
 	bridge_port = mlxsw_sp_bridge_port_find(mlxsw_sp_port->mlxsw_sp->bridge,
 						orig_dev);
-	if (WARN_ON(!bridge_port))
-		return -EINVAL;
+	if (!bridge_port)
+		return 0;
 
 	if (!bridge_port->bridge_device->multicast_enabled)
 		return 0;
@@ -1283,15 +1283,15 @@ static int mlxsw_sp_port_mdb_add(struct mlxsw_sp_port *mlxsw_sp_port,
 		return 0;
 
 	bridge_port = mlxsw_sp_bridge_port_find(mlxsw_sp->bridge, orig_dev);
-	if (WARN_ON(!bridge_port))
-		return -EINVAL;
+	if (!bridge_port)
+		return 0;
 
 	bridge_device = bridge_port->bridge_device;
 	mlxsw_sp_port_vlan = mlxsw_sp_port_vlan_find_by_bridge(mlxsw_sp_port,
 							       bridge_device,
 							       mdb->vid);
-	if (WARN_ON(!mlxsw_sp_port_vlan))
-		return -EINVAL;
+	if (!mlxsw_sp_port_vlan)
+		return 0;
 
 	fid_index = mlxsw_sp_fid_index(mlxsw_sp_port_vlan->fid);
 
@@ -1407,15 +1407,15 @@ static int mlxsw_sp_port_mdb_del(struct mlxsw_sp_port *mlxsw_sp_port,
 	int err = 0;
 
 	bridge_port = mlxsw_sp_bridge_port_find(mlxsw_sp->bridge, orig_dev);
-	if (WARN_ON(!bridge_port))
-		return -EINVAL;
+	if (!bridge_port)
+		return 0;
 
 	bridge_device = bridge_port->bridge_device;
 	mlxsw_sp_port_vlan = mlxsw_sp_port_vlan_find_by_bridge(mlxsw_sp_port,
 							       bridge_device,
 							       mdb->vid);
-	if (WARN_ON(!mlxsw_sp_port_vlan))
-		return -EINVAL;
+	if (!mlxsw_sp_port_vlan)
+		return 0;
 
 	fid_index = mlxsw_sp_fid_index(mlxsw_sp_port_vlan->fid);
 
-- 
2.9.3

^ permalink raw reply related

* [patch net 0/2] mlxsw: Couple of fixes
From: Jiri Pirko @ 2017-08-04 12:12 UTC (permalink / raw)
  To: netdev; +Cc: davem, idosch, mlxsw

From: Jiri Pirko <jiri@mellanox.com>

Ido says:

The first patch prevents us from warning about valid situations that can
happen due to the fact that some operations in switchdev are deferred.

Second patch fixes a long standing problem in which we didn't correctly
free resources upon module removal, resulting in a memory leak.

Ido Schimmel (2):
  mlxsw: spectrum_switchdev: Don't warn about valid situations
  mlxsw: spectrum_switchdev: Release multicast groups during fini

 .../ethernet/mellanox/mlxsw/spectrum_switchdev.c   | 37 ++++++++++++++--------
 1 file changed, 24 insertions(+), 13 deletions(-)

-- 
2.9.3

^ permalink raw reply

* Realtek r8169 driver VLAN TAG stripping / offloading
From: Benoit Panizzon @ 2017-08-04 11:55 UTC (permalink / raw)
  To: netdev

Hi

Yes, someone posted the same problem a couple of years ago:
https://www.spinics.net/lists/netdev/msg152505.html

But as far as I have read the thread, nobody figured out the problem.

I have run in the same strange problem, where I suspect either a driver
of firmware bug, after upgrading a system from kernel 3.16 (whatever
subversion debian used) to the actual: 4.9.30-2+deb9u2

I use native and untagged vlan.

The two ports on the Cisco Switch have this config:

 switchport trunk encapsulation dot1q
 switchport trunk native vlan 3
 switchport trunk allowed vlan 2,3
 switchport mode trunk

So for outgoing packets (switch => Linux) with vlan id 3 that id is
stripped and for ingress packets without tag, vlan id 3 is added.

Packets with vlan id 2 are passed unaltered between switch and linux
box.

Now I have two machines with slightly different r8169 NIC:

01:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd.
RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 02)

=> This one works fine

And

03:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd.
RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 09)

=> This one stripps all ingress vlan tags!

Both use the exact same kernel version.

On rev 02 I don't bother to load the firmware, it works fine without.
On the rev 09 one I did try with and without firmware, no difference.

Observation:

Packets generated on linux on vlan id 2 are sent out to the switch
containing the vlan tag.

Packets which leave the switch with vlan id 2 loose this id after being
processed by the r8169 driver and therefore do not make it out of the
vlan interface on linux.

As an example, the same ARP packet capture on the rev 02 and the rev 09
NIC:

13:51:05.955414 4c:5e:0c:41:0c:83 > ff:ff:ff:ff:ff:ff, ethertype 802.1Q
(0x8100), length 64: vlan 2, p 0, ethertype ARP, Request who-has
157.161.6.10 tell 157.161.6.1, length 46

=> This is fine, see vlan 2?

13:51:05.957652 4c:5e:0c:41:0c:83 > ff:ff:ff:ff:ff:ff, ethertype 802.1Q
(0x8100), length 64: vlan 0, p 0, ethertype ARP, Request who-has
157.161.6.10 tell 157.161.6.1, length 46

=> Wrong vlan tag!

# ethtool -k eth0
Features for eth0:
rx-checksumming: on
tx-checksumming: off
        tx-checksum-ipv4: off
        tx-checksum-ip-generic: off [fixed]
        tx-checksum-ipv6: off
        tx-checksum-fcoe-crc: off [fixed]
        tx-checksum-sctp: off [fixed]
scatter-gather: off
        tx-scatter-gather: off
        tx-scatter-gather-fraglist: off [fixed]
tcp-segmentation-offload: off
        tx-tcp-segmentation: off
        tx-tcp-ecn-segmentation: off [fixed]
        tx-tcp-mangleid-segmentation: off
        tx-tcp6-segmentation: off
udp-fragmentation-offload: off [fixed]
generic-segmentation-offload: off [requested on]
generic-receive-offload: on
large-receive-offload: off [fixed]
rx-vlan-offload: on
tx-vlan-offload: on
ntuple-filters: off [fixed]
receive-hashing: off [fixed]
highdma: on [fixed]
rx-vlan-filter: off [fixed]
vlan-challenged: off [fixed]
tx-lockless: off [fixed]
netns-local: off [fixed]
tx-gso-robust: off [fixed]
tx-fcoe-segmentation: off [fixed]
tx-gre-segmentation: off [fixed]
tx-gre-csum-segmentation: off [fixed]
tx-ipxip4-segmentation: off [fixed]
tx-ipxip6-segmentation: off [fixed]
tx-udp_tnl-segmentation: off [fixed]
tx-udp_tnl-csum-segmentation: off [fixed]
tx-gso-partial: off [fixed]
tx-sctp-segmentation: off [fixed]
fcoe-mtu: off [fixed]
tx-nocache-copy: off
loopback: off [fixed]
rx-fcs: off
rx-all: off
tx-vlan-stag-hw-insert: off [fixed]
rx-vlan-stag-hw-parse: off [fixed]
rx-vlan-stag-filter: off [fixed]
l2-fwd-offload: off [fixed]
busy-poll: off [fixed]
hw-tc-offload: off [fixed]

I have been trying to turn 'off' rx-vlan-offload and tx-vlan-offload
with ethtool, but I don't succeed.

As I suppose the problem was introduced somewhen between kernel 3.16
and 4.9 I will be trying to reboot with 3.16 tonight when I have
physical access to the machine.

Any hints on what could cause the problem or how I could further debug
it, is highly welcome.

-Benoît Panizzon-
-- 
I m p r o W a r e   A G    -    Leiter Commerce Kunden
______________________________________________________

Zurlindenstrasse 29             Tel  +41 61 826 93 00
CH-4133 Pratteln                Fax  +41 61 826 93 01
Schweiz                         Web  http://www.imp.ch
______________________________________________________

^ permalink raw reply

* Re: [RFC] iproute: Add support for extended ack to rtnl_talk
From: Simon Horman @ 2017-08-04 11:31 UTC (permalink / raw)
  To: David Ahern
  Cc: Daniel Borkmann, Stephen Hemminger, Phil Sutter, David Miller,
	netdev
In-Reply-To: <0d84bd27-2fd1-68c0-5787-0195e95ab1af@gmail.com>

On Thu, Aug 03, 2017 at 02:26:58PM -0600, David Ahern wrote:
> On 5/18/17 10:24 PM, David Ahern wrote:
> > On 5/18/17 3:02 AM, Daniel Borkmann wrote:
> >> So effectively this means libmnl has to be used for new stuff, noone
> >> has time to do the work to convert the existing tooling over (which
> >> by itself might be a challenge in testing everything to make sure
> >> there are no regressions) given there's not much activity around
> >> lib/libnetlink.c anyway, and existing users not using libmnl today
> >> won't see/notice new improvements on netlink side when they do an
> >> upgrade. So we'll be stuck with that dual library mess pretty much
> >> for a very long time. :(
> > 
> > lib/libnetlink.c with all of its duplicate functions weighs in at just
> > 947 LOC -- a mere 12% of the code in lib/. From a total SLOC of iproute2
> > it is a negligible part of the code base.
> > 
> > Given that, there is very little gain -- but a lot of risk in
> > regressions -- in converting such a small, low level code base to libmnl
> > just for the sake of using a library - something Phil noted in his
> > cursory attempt at converting ip to libmnl. ie., The level effort
> > required vs the benefit is just not worth it.
> > 
> > There are so many other parts of the ip code base that need work with a
> > much higher return on the time investment.
> > 
> 
> Stephen: It has been 3 months since the first extack patches were posted
> and still nothing in iproute2, all of it hung up on your decision to
> require libmnl. Do you plan to finish the libmnl support any time soon
> and send out patches?

FWIIW I would also like to see some way to get this enhancement accepted.

^ permalink raw reply

* Re: WARN_ON in commit 0ffdaf5 "net/sock: add WARN_ON(parent->sk) in sock_graft()"
From: Sowmini Varadhan @ 2017-08-04 11:19 UTC (permalink / raw)
  To: Guoqing Jiang; +Cc: davem, netdev
In-Reply-To: <598456CE.7020702@suse.com>

On (08/04/17 19:13), Guoqing Jiang wrote:
> 
> HI,
> 
> With commit 0ffdaf5b41cf4435ece14d1d3e977ce69012a20d "net/sock: add
> WARN_ON(parent->sk) in sock_graft()",
> I can see the WARN_ON is triggered easily by dlm subsystem.
> 
> tcp_accept_from_sock() in  fs/dlm/lowcomms.c has the following snippet:
> 
> 1.    result = sock_create_kern(&init_net, dlm_local_addr[0]->ss_family,
>                                       SOCK_STREAM, IPPROTO_TCP, &newsock);
>         sk is set by the path "sock_create_kern -> __sock_creat ->
> pf->create => inet_create -> sock_init_data"
>     ...
> 2.    result = con->sock->ops->accept(con->sock, newsock, O_NONBLOCK, true);
>         Then sock_graft is called by "con->sock->ops->accept => inet_accept
> -> sock_graft "
> 
> Does it mean call accept() after the just created socket is not a acceptable
> behavior? Any comment will be appreciate, thanks.
> 

It means that you have a sk leak, just like rds-tcp did. See
	https://patchwork.ozlabs.org/patch/780356/
for a discussion.

You should be calling sock_create_lite, instead of sock_create_kern.

The ->sk that triggers the WARN_ON is the one that is leaking (you will
see that the ->sk befrore the ->accept (i.e., the one set up by  sock_create_kern)
is different than the one after ->accept)

--Sowmini

^ permalink raw reply

* WARN_ON in commit 0ffdaf5 "net/sock: add WARN_ON(parent->sk) in sock_graft()"
From: Guoqing Jiang @ 2017-08-04 11:13 UTC (permalink / raw)
  To: sowmini.varadhan, davem; +Cc: netdev, Guoqing Jiang

HI,

With commit 0ffdaf5b41cf4435ece14d1d3e977ce69012a20d "net/sock: add 
WARN_ON(parent->sk) in sock_graft()",
I can see the WARN_ON is triggered easily by dlm subsystem.

tcp_accept_from_sock() in  fs/dlm/lowcomms.c has the following snippet:

1.    result = sock_create_kern(&init_net, dlm_local_addr[0]->ss_family,
                                       SOCK_STREAM, IPPROTO_TCP, &newsock);
         sk is set by the path "sock_create_kern -> __sock_creat -> 
pf->create => inet_create -> sock_init_data"
     ...
2.    result = con->sock->ops->accept(con->sock, newsock, O_NONBLOCK, true);
         Then sock_graft is called by "con->sock->ops->accept => 
inet_accept -> sock_graft "

Does it mean call accept() after the just created socket is not a 
acceptable behavior? Any comment will be appreciate, thanks.

Regards,
Guoqing

^ permalink raw reply

* Re: [PATCH v3] ARM: dts: tango4: Request RGMII RX and TX clock delays
From: Arnd Bergmann @ 2017-08-04 11:02 UTC (permalink / raw)
  To: Marc Gonzalez
  Cc: arm-soc, Florian Fainelli, Mans Rullgard, Mason, netdev,
	Linux ARM
In-Reply-To: <d669e207-ae47-4c9e-67c5-75424fde501f@sigmadesigns.com>

On Fri, Jul 28, 2017 at 3:27 PM, Marc Gonzalez
<marc_gonzalez@sigmadesigns.com> wrote:
> RX and TX clock delays are required. Request them explicitly.
>
> Fixes: cad008b8a77e6 ("ARM: dts: tango4: Initial device trees")
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---
> Arnd, Kevin, Olof, can one of you take this patch through arm-soc?

Applied to the fixes branch now.

> Greg will apply it to all stable branches, right?

He usually looks at random patches that have a 'Fixes:' line, but if you already
know that you want a bugfix to be backported, it should also come with a
'Cc: stable@vger.kernel.org' line. I've added that manually this time, but
please include it yourself if you have another patch like that.

 Thanks,

      Arnd

^ permalink raw reply

* Re: [PATCH net] xgene: Always get clk source, but ignore if it's missing for SGMII ports
From: Will Deacon @ 2017-08-04 11:00 UTC (permalink / raw)
  To: Thomas Bogendoerfer
  Cc: isubramanian, kchudgar, qnguyen, netdev, linux-kernel,
	Laura Abbott
In-Reply-To: <20170803140311.34C7E50870B@solo.franken.de>

On Thu, Aug 03, 2017 at 03:43:14PM +0200, Thomas Bogendoerfer wrote:
> From: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> 
> Even the driver doesn't do anything with the clk source for SGMII
> ports it needs to be enabled by doing a devm_clk_get(), if there is
> a clk source in DT.
> 
> Fixes: 0db01097cabd ('xgene: Don't fail probe, if there is no clk resource for SGMII interfaces')
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> Tested-by: Laura Abbott <labbott@redhat.com>
> Acked-by: Iyappan Subramanian <isubramanian@apm.com>
> ---
>  drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Thanks, this fixes a boot-time crash (below) when bringing up the interface
on my xgene board with -rc3, so:

Tested-by: Will Deacon <will.deacon@arm.com>

Will

--->8

[    8.076583] synchronous external abort: synchronous external abort (0x96000010) at 0xffff00000aca0010
[    8.085777] internal error: : 96000010 [#1] preempt smp
[    8.090977] modules linked in:
[    8.094020] cpu: 4 pid: 2345 comm: ip not tainted 4.13.0-rc3-00002-g93bd9a45f608 #2
[    8.101638] hardware name: appliedmicro x-gene mustang board/x-gene mustang board, bios 3.06.25 oct 17 2016
[    8.111330] task: ffff8003e7181c00 task.stack: ffff8003e5afc000
[    8.117227] pc is at xgene_enet_rd_mac+0x80/0x108
[    8.121909] lr is at xgene_enet_rd_mac+0x50/0x108
[    8.126589] pc : [<ffff00000865f140>] lr : [<ffff00000865f110>] pstate: a0000145
[    8.133948] sp : ffff8003e5aff570
[    8.137244] x29: ffff8003e5aff570 x28: 0000000000000001 
[    8.142533] x27: ffff8003eb9cc838 x26: ffff000008663970 
[    8.147820] x25: ffff00000aca000c x24: ffff8003eb9cc000 
[    8.153107] x23: 0000000000000000 x22: ffff00000aca0004 
[    8.158394] x21: ffff8003eb9cc9f0 x20: ffff00000aca0010 
[    8.163680] x19: 000000000000000b x18: 0000000000000001 
[    8.168966] x17: 0000000120177170 x16: ffff000008889dc8 
[    8.174253] x15: ffffffffffffffff x14: 00000001e15a50b8 
[    8.179539] x13: 0000000000000000 x12: ffff8003e8cb0a00 
[    8.184825] x11: 0000000000000000 x10: 0000000000000004 
[    8.190111] x9 : 00000000ffffffff x8 : ffff8003fff75468 
[    8.195398] x7 : 00000000000d7c8c x6 : ffff0000089c0000 
[    8.200684] x5 : 0000000000000001 x4 : ffff8003e7181c00 
[    8.205969] x3 : 0000000000000000 x2 : 0000000000000000 
[    8.211255] x1 : 0000000000330033 x0 : 0000000040000000 
[    8.216543] process ip (pid: 2345, stack limit = 0xffff8003e5afc000)
[    8.222865] stack: (0xffff8003e5aff570 to 0xffff8003e5b00000)
[    8.228582] f560:                                   ffff8003e5aff5c0 ffff0000086600f4
[    8.236374] f580: 0000000000000001 ffff8003eb9cc800 0000000000000001 ffff8003eb9cc000
[    8.244164] f5a0: ffff8003eca16a2e ffff8003ec93a410 ffff000008acdf08 ffff00000863b684
[    8.251955] f5c0: ffff8003e5aff5f0 ffff0000086601f4 ffff8003eb9cc800 0000000000000000
[    8.259745] f5e0: 0000000000000000 ffff000008663ce0 ffff8003e5aff600 ffff000008663cf0
[    8.267536] f600: ffff8003e5aff670 ffff0000088aa3b8 ffff8003eb9cc000 0000000000001003
[    8.275326] f620: ffff000008ace018 ffff8003eb9cc048 0000000000001002 ffff8003e5aff8e0
[    8.283116] f640: ffff000008ace018 ffff8003e8dd8010 ffff8003e5affb78 0000000000000000
[    8.290907] f660: ffff000008663000 ffff000008d185f0 ffff8003e5aff6b0 ffff0000088aa6b8
[    8.298697] f680: ffff8003eb9cc000 0000000000001003 0000000000000001 0000000000000000
[    8.306488] f6a0: ffff8003eb9cc000 ffff8003eb9cc000 ffff8003e5aff6f0 ffff0000088aa798
[    8.314278] f6c0: ffff8003eb9cc000 0000000000001002 0000000000000000 0000000000000000
[    8.322069] f6e0: ffff8003e6868000 000000000000002b ffff8003e5aff720 ffff0000088bc168
[    8.329859] f700: ffff8003e8dd8000 ffff8003eb9cc000 0000000000000000 ffff8003e5aff968
[    8.337650] f720: ffff8003e5aff840 ffff0000088be918 ffff000008d5e000 ffff8003eb9cc000
[    8.345441] f740: 0000000000000000 ffff8003e5affb78 0000000000000000 0000000000000000
[    8.353231] f760: ffff8003e5aff840 ffff000008b159c0 ffff8003e8dd8020 ffff0000081b3a90
[    8.361022] f780: ffff8003e7037b80 ffff7e000f9b0e00 00e80043e6c38f53 ffff8003ecf49450
[    8.368812] f7a0: ffff8003e7037b80 0000000000000001 ffff8003e5aff7d0 ffff0000081bf0cc
[    8.376603] f7c0: ffff7e000f9b0e00 ffff0000081b39c0 ffff8003e5aff7e0 ffff00000818ef70
[    8.384393] f7e0: ffff8003e5aff800 ffff0000081b3a9c ffff8003e5aff840 ffff0000088be3e0
[    8.392184] f800: ffff8003e8dd8000 ffff0000081b44ec 0000000000000000 ffff8003e5affb78
[    8.399974] f820: ffff8003e5aff840 ffff0000088be438 ffff8003e8dd8000 ffff00000858169c
[    8.407765] f840: ffff8003e5affad0 ffff0000088bec24 0000000000000000 ffff8003e8dd8000
[    8.415555] f860: ffff8003e5affb78 0000000000000000 ffff8003e6868000 0000000000000000
[    8.423346] f880: 0000000000000000 0000000000000008 ffff0000089b1000 ffff8003e7181c00
[    8.431136] f8a0: ffff8003e5aff8c0 ffff8003e6868000 0000000000000000 ffff000008fc7e80
[    8.438927] f8c0: ffff000008b15a80 ffff8003e8dd8010 ffff8003e71e3800 ffff8003e71e3f6c
[    8.446717] f8e0: 0000000000000000 00000000000001c0 ffff000008ecd000 ffff000008ed9000
[    8.454508] f900: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    8.462297] f920: 0000000000000000 0000000000000000 0000000000000000 ffff8003e71e3800
[    8.470088] f940: 0000000000000000 00000000000001c0 ffff8003fff8f400 ffff000008ed9bc8
[    8.477878] f960: ffff8003e5aff970 0000000000000000 0000000000000000 0000000000000000
[    8.485668] f980: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    8.493458] f9a0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    8.501255] f9c0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    8.501258] f9e0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    8.501260] fa00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    8.501263] fa20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    8.501265] fa40: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    8.501268] fa60: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    8.501270] fa80: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    8.501273] faa0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    8.501276] fac0: 0000000000000000 0000000000000000 ffff8003e5affb40 ffff0000088d41c8
[    8.501278] fae0: ffff8003e8dd8000 ffff8003e6868000 ffff0000088bea98 ffff8003e6868000
[    8.501281] fb00: ffff8003e5affc08 ffff8003fffd2500 0000000000000140 ffff800300000000
[    8.501284] fb20: ffff8003e5affb70 ffff00000889220c ffff8003e5affb50 ffff0000088d1d98
[    8.501287] fb40: ffff8003e5affba0 ffff0000088bb364 ffff8003e6868000 ffff8003ec6e6c00
[    8.501289] fb60: 0000000000000020 0000000000000000 ffff8003e5affb90 0000000000000000
[    8.501292] fb80: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    8.501294] fba0: ffff8003e5affbc0 ffff0000088d3934 ffff8003ec8aa800 ffff8003ec6e6c00
[    8.501297] fbc0: ffff8003e5affc10 ffff0000088d3dc8 ffff8003e5affe58 0000000000000000
[    8.501300] fbe0: ffff8003e6868000 ffff8003ec6e6c00 0000000000000000 0000000000000020
[    8.501302] fc00: ffff8003e5affc10 7fffffffffffffff ffff8003e5affc90 ffff000008888570
[    8.501305] fc20: ffff8003e5affe58 0000000000000000 0000000000000000 ffff8003eebdd900
[    8.501308] fc40: 0000000000000000 0000000000000000 ffff8003e5affcf8 00000000000000d3
[    8.501311] fc60: ffff8003e5affc80 0000ffffea686a70 ffff8003e8c80500 0000000000000000
[    8.501313] fc80: 0000000000000929 0000000000000000 ffff8003e5affca0 ffff000008888d4c
[    8.501316] fca0: ffff8003e5affe20 ffff000008889d6c ffff8003eebdd900 0000ffffea686a90
[    8.501319] fcc0: 0000000000000000 0000ffff8bd5abf4 0000000080000000 0000000000000015
[    8.501321] fce0: 0000000000000124 ffff000008220d04 0000000000000000 ffff000008220f54
[    8.501324] fd00: ffff8003fa9e1000 ffff8003e7037b80 014000c000000055 0000000ffffffffc
[    8.501327] fd20: 0000000000000010 ffff800300000000 ffff8003e8ffeff8 0000000000000000
[    8.501330] fd40: 0000000000000000 0000000000000000 0000000000000000 ffff8003ecf49460
[    8.501332] fd60: ffff7e000fb3d270 0000000000000000 ffff8003e5affda0 ffff000008097fc0
[    8.501335] fd80: ffff8003e7181c00 0000ffffea68c000 ffff8003e5affda0 ffff000008098008
[    8.501338] fda0: 0000ffffea68eb78 0000000000000020 0000000092000047 ffff0000089b7250
[    8.501340] fdc0: 0000ffffea68c000 ffff8003e5affec0 0000000020000000 0000000000000024
[    8.501343] fde0: ffff8003e5affdf0 ffff0000082235d8 ffff8003e5affe00 ffff000008888180
[    8.501346] fe00: ffff8003e5affe20 ffff000008889d48 0000000000000000 0000ffffea686a90
[    8.501349] fe20: ffff8003e5affeb0 ffff000008889dd8 0000000000000000 00008003f70a8000
[    8.501351] fe40: ffffffffffffffff ffff0000081febbc fffffff700000000 ffff8003e5affd20
[    8.501354] fe60: ffff80030000000c ffff000000000001 0000000000000000 0000000000000000
[    8.501356] fe80: ffff8003e5affdb0 0000000000000000 0000000000000000 0000000000000000
[    8.501359] fea0: 0000000000000000 0000000000000000 0000000000000000 ffff000008083730
[    8.501361] fec0: 0000000000000003 0000ffffea686a90 0000000000000000 0000000000000000
[    8.501364] fee0: 0000ffffea68eac8 0000000000000004 0000000031687465 0000000031687465
[    8.501367] ff00: 00000000000000d3 7f7f7f7fffffffff 0101010101010101 7f7f7f7f7f7f7f7f
[    8.501370] ff20: 0101010101010101 0000000000000000 0000000000000000 00000000000005bf
[    8.501373] ff40: 0000ffff8bd5abc8 0000000120177170 0000000000040d00 0000ffffea68eb78
[    8.501376] ff60: 00000000581b7100 0000000120176000 0000ffffea686ac8 0000ffffea686a90
[    8.501378] ff80: 0000000000000010 0000000000000000 00000001201773b8 0000ffffea68eb50
[    8.501381] ffa0: 0000ffffea68f2c8 0000ffffea6869d0 000000012014ee10 0000ffffea6869d0
[    8.501384] ffc0: 0000ffff8bd5abf4 0000000080000000 0000000000000003 00000000000000d3
[    8.501386] ffe0: 0000000000000000 0000000000000000 1c1c1c1c1c1c1c1c 1c1c1c1c1c1c1c1c
[    8.501388] call trace:
[    8.501391] exception stack(0xffff8003e5aff3a0 to 0xffff8003e5aff4d0)
[    8.501394] f3a0: 000000000000000b 0001000000000000 ffff8003e5aff570 ffff00000865f140
[    8.501397] f3c0: ffff00000865f140 00000000a0000145 0000000000000025 ffff00000aca000c
[    8.501400] f3e0: ffff000008663970 ffff8003eb9cc838 ffff8003e7181c00 ffff8003e5aff570
[    8.501402] f400: ffff00000865f110 ffff8003e5aff440 ffff000008082dfc 0000000060000145
[    8.501405] f420: ffff8003fff75400 ffff0000080e7b74 0001000000000000 ffff0000080e7ba0
[    8.501408] f440: 0000000040000000 0000000000330033 0000000000000000 0000000000000000
[    8.501411] f460: ffff8003e7181c00 0000000000000001 ffff0000089c0000 00000000000d7c8c
[    8.501413] f480: ffff8003fff75468 00000000ffffffff 0000000000000004 0000000000000000
[    8.501416] f4a0: ffff8003e8cb0a00 0000000000000000 00000001e15a50b8 ffffffffffffffff
[    8.501418] f4c0: ffff000008889dc8 0000000120177170
[    8.501424] [<ffff00000865f140>] xgene_enet_rd_mac+0x80/0x108
[    8.501429] [<ffff0000086600f4>] xgene_sgmac_rxtx+0x24/0x50
[    8.501431] [<ffff0000086601f4>] xgene_sgmac_tx_enable+0x14/0x20
[    8.501434] [<ffff000008663cf0>] xgene_enet_open+0x338/0x3f0
[    8.501439] [<ffff0000088aa3b8>] __dev_open+0xc8/0x140
[    8.501443] [<ffff0000088aa6b8>] __dev_change_flags+0x98/0x158
[    8.501446] [<ffff0000088aa798>] dev_change_flags+0x20/0x60
[    8.501451] [<ffff0000088bc168>] do_setlink+0x248/0xce8
[    8.501454] [<ffff0000088be918>] rtnl_newlink+0x5b0/0x730
[    8.501457] [<ffff0000088bec24>] rtnetlink_rcv_msg+0x18c/0x1f8
[    8.501460] [<ffff0000088d41c8>] netlink_rcv_skb+0xb0/0x110
[    8.501463] [<ffff0000088bb364>] rtnetlink_rcv+0x2c/0x40
[    8.501467] [<ffff0000088d3934>] netlink_unicast+0x184/0x220
[    8.501469] [<ffff0000088d3dc8>] netlink_sendmsg+0x300/0x378
[    8.501473] [<ffff000008888570>] sock_sendmsg+0x18/0x30
[    8.501475] [<ffff000008888d4c>] ___sys_sendmsg+0x264/0x298
[    8.501479] [<ffff000008889d6c>] __sys_sendmsg+0x44/0xa0
[    8.501481] [<ffff000008889dd8>] sys_sendmsg+0x10/0x20
[    8.501486] [<ffff000008083730>] el0_svc_naked+0x24/0x28
[    8.501490] code: 52800173 14000003 d28218e0 940c82f9 (b9400282) 
[    8.501493] ---[ end trace a3c9ec71d1295493 ]---
[    8.501516] note: ip[2345] exited with preempt_count 1

^ permalink raw reply

* Re: [PATCH v2 05/11] mtd: cfi: reduce stack size with KASAN
From: Boris Brezillon @ 2017-08-04 10:57 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andrew Morton, kasan-dev, Dmitry Vyukov, Alexander Potapenko,
	Andrey Ryabinin, netdev, linux-kernel, Arend van Spriel,
	David Woodhouse, Brian Norris, Marek Vasut, Richard Weinberger,
	Cyrille Pitchen, linux-mtd
In-Reply-To: <20170614211556.2062728-6-arnd@arndb.de>

On Wed, 14 Jun 2017 23:15:40 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> When CONFIG_KASAN is used, we consume a lot of extra stack space:
> 
> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'do_write_buffer':
> drivers/mtd/chips/cfi_cmdset_0020.c:603:1: error: the frame size of 2184 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'cfi_staa_erase_varsize':
> drivers/mtd/chips/cfi_cmdset_0020.c:972:1: error: the frame size of 1936 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> drivers/mtd/chips/cfi_cmdset_0001.c: In function 'do_write_buffer':
> drivers/mtd/chips/cfi_cmdset_0001.c:1841:1: error: the frame size of 1776 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> 
> This marks some functions as noinline_if_stackbloat to keep reduce the
> overall stack size.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>

> ---
>  drivers/mtd/chips/cfi_cmdset_0020.c | 8 ++++----
>  include/linux/mtd/map.h             | 8 ++++----
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0020.c b/drivers/mtd/chips/cfi_cmdset_0020.c
> index 7d342965f392..5eee5e883f55 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0020.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0020.c
> @@ -244,7 +244,7 @@ static struct mtd_info *cfi_staa_setup(struct map_info *map)
>  }
>  
>  
> -static inline int do_read_onechip(struct map_info *map, struct flchip *chip, loff_t adr, size_t len, u_char *buf)
> +static noinline_if_stackbloat int do_read_onechip(struct map_info *map, struct flchip *chip, loff_t adr, size_t len, u_char *buf)
>  {
>  	map_word status, status_OK;
>  	unsigned long timeo;
> @@ -728,7 +728,7 @@ cfi_staa_writev(struct mtd_info *mtd, const struct kvec *vecs,
>  }
>  
>  
> -static inline int do_erase_oneblock(struct map_info *map, struct flchip *chip, unsigned long adr)
> +static noinline_if_stackbloat int do_erase_oneblock(struct map_info *map, struct flchip *chip, unsigned long adr)
>  {
>  	struct cfi_private *cfi = map->fldrv_priv;
>  	map_word status, status_OK;
> @@ -1029,7 +1029,7 @@ static void cfi_staa_sync (struct mtd_info *mtd)
>  	}
>  }
>  
> -static inline int do_lock_oneblock(struct map_info *map, struct flchip *chip, unsigned long adr)
> +static noinline_if_stackbloat int do_lock_oneblock(struct map_info *map, struct flchip *chip, unsigned long adr)
>  {
>  	struct cfi_private *cfi = map->fldrv_priv;
>  	map_word status, status_OK;
> @@ -1175,7 +1175,7 @@ static int cfi_staa_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>  	}
>  	return 0;
>  }
> -static inline int do_unlock_oneblock(struct map_info *map, struct flchip *chip, unsigned long adr)
> +static noinline_if_stackbloat int do_unlock_oneblock(struct map_info *map, struct flchip *chip, unsigned long adr)
>  {
>  	struct cfi_private *cfi = map->fldrv_priv;
>  	map_word status, status_OK;
> diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
> index 3aa56e3104bb..29db74314db8 100644
> --- a/include/linux/mtd/map.h
> +++ b/include/linux/mtd/map.h
> @@ -316,7 +316,7 @@ static inline map_word map_word_or(struct map_info *map, map_word val1, map_word
>  	return r;
>  }
>  
> -static inline int map_word_andequal(struct map_info *map, map_word val1, map_word val2, map_word val3)
> +static noinline_if_stackbloat int map_word_andequal(struct map_info *map, map_word val1, map_word val2, map_word val3)
>  {
>  	int i;
>  
> @@ -328,7 +328,7 @@ static inline int map_word_andequal(struct map_info *map, map_word val1, map_wor
>  	return 1;
>  }
>  
> -static inline int map_word_bitsset(struct map_info *map, map_word val1, map_word val2)
> +static noinline_if_stackbloat int map_word_bitsset(struct map_info *map, map_word val1, map_word val2)
>  {
>  	int i;
>  
> @@ -362,7 +362,7 @@ static inline map_word map_word_load(struct map_info *map, const void *ptr)
>  	return r;
>  }
>  
> -static inline map_word map_word_load_partial(struct map_info *map, map_word orig, const unsigned char *buf, int start, int len)
> +static noinline_if_stackbloat map_word map_word_load_partial(struct map_info *map, map_word orig, const unsigned char *buf, int start, int len)
>  {
>  	int i;
>  
> @@ -392,7 +392,7 @@ static inline map_word map_word_load_partial(struct map_info *map, map_word orig
>  #define MAP_FF_LIMIT 8
>  #endif
>  
> -static inline map_word map_word_ff(struct map_info *map)
> +static noinline_if_stackbloat map_word map_word_ff(struct map_info *map)
>  {
>  	map_word r;
>  	int i;

^ permalink raw reply

* Re: [PATCH v2 05/11] mtd: cfi: reduce stack size with KASAN
From: Boris Brezillon @ 2017-08-04 10:56 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andrew Morton, kasan-dev, Dmitry Vyukov, Alexander Potapenko,
	Andrey Ryabinin, Networking, Linux Kernel Mailing List,
	Arend van Spriel, David Woodhouse, Brian Norris, Marek Vasut,
	Richard Weinberger, Cyrille Pitchen, linux-mtd
In-Reply-To: <CAK8P3a2p6w_yx82SC796n6dqZwE1=s7Mad5TLqcdK=pJ0Uiy=w@mail.gmail.com>

On Fri, 4 Aug 2017 11:09:53 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Fri, Aug 4, 2017 at 9:42 AM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Wed, 14 Jun 2017 23:15:40 +0200
> > Arnd Bergmann <arnd@arndb.de> wrote:
> >  
> >> When CONFIG_KASAN is used, we consume a lot of extra stack space:
> >>
> >> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'do_write_buffer':
> >> drivers/mtd/chips/cfi_cmdset_0020.c:603:1: error: the frame size of 2184 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> >> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'cfi_staa_erase_varsize':
> >> drivers/mtd/chips/cfi_cmdset_0020.c:972:1: error: the frame size of 1936 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> >> drivers/mtd/chips/cfi_cmdset_0001.c: In function 'do_write_buffer':
> >> drivers/mtd/chips/cfi_cmdset_0001.c:1841:1: error: the frame size of 1776 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> >>
> >> This marks some functions as noinline_if_stackbloat to keep reduce the
> >> overall stack size.
> >>
> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >> ---
> >>  drivers/mtd/chips/cfi_cmdset_0020.c | 8 ++++----
> >>  include/linux/mtd/map.h             | 8 ++++----
> >>  2 files changed, 8 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/mtd/chips/cfi_cmdset_0020.c b/drivers/mtd/chips/cfi_cmdset_0020.c
> >> index 7d342965f392..5eee5e883f55 100644
> >> --- a/drivers/mtd/chips/cfi_cmdset_0020.c
> >> +++ b/drivers/mtd/chips/cfi_cmdset_0020.c
> >> @@ -244,7 +244,7 @@ static struct mtd_info *cfi_staa_setup(struct map_info *map)
> >>  }
> >>
> >>
> >> -static inline int do_read_onechip(struct map_info *map, struct flchip *chip, loff_t adr, size_t len, u_char *buf)
> >> +static noinline_if_stackbloat int do_read_onechip(struct map_info *map, struct flchip *chip, loff_t adr, size_t len, u_char *buf)  
> >
> > Why do we even need to mark those functions inline in the first place?
> > Isn't the compiler smart enough to decide when it should inline things?  
> 
> I'm pretty sure it doesn't need the 'inline' keywork. I see that the code was
> addedlike this in linux-2.4.0-test3pre3 along with the rest of the mtd layer,
> so it has always been 'inline' and nobody ever bothered to remove that
> during later cleanups.
> 
> We could probably just mark this function as 'noinline' here and never
> inline it,
> but I would rather not add more than one variant of noinline_if_stackbloat:
> almost all other users of noinline_if_stackbloat are for functions that have
> to be inline in normal builds, so it is defined as being either
> 'inline' or 'noinline'
> depending on whether KASAN is active.

Okay. Let's keep it like that then.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox