[patch net-next v2 0/4] net: sched: get rid of cls_flower->egress_dev

[patch net-next v2 0/4] net: sched: get rid of cls_flower->egress_dev

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Introduction of cls_flower->egress_dev was a workaround. Turned out
to be a bit ugly hack. So replace it with more generic and reusable
infrastructure.

This is a dependency of shared block introduction that will be send as
a follow-up patchsets group.
---
v1->v2:
- patch2:
  - take rtnl for register/unregister

Jiri Pirko (4):
  net: sched: make tc_action_ops->get_dev return dev and avoid passing
    net
  net: sched: introduce per-egress action device callbacks
  net: sched: convert cls_flower->egress_dev users to tc_setup_cb_egdev
    infra
  net: sched: remove unused tcf_exts_get_dev helper and
    cls_flower->egress_dev

 drivers/net/ethernet/mellanox/mlx5/core/en.h      |   3 +
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c |   4 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_rep.c  |  31 ++-
 include/net/act_api.h                             |  37 +++-
 include/net/pkt_cls.h                             |   6 +-
 include/net/tc_act/tc_mirred.h                    |   1 +
 net/sched/act_api.c                               | 220 ++++++++++++++++++++++
 net/sched/act_mirred.c                            |  13 +-
 net/sched/cls_api.c                               |  35 ++--
 net/sched/cls_flower.c                            |  63 ++++---
 10 files changed, 348 insertions(+), 65 deletions(-)

-- 
2.9.5

[patch net-next v2 1/4] net: sched: make tc_action_ops->get_dev return dev and avoid passing net

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Return dev directly, NULL if not possible. That is enough.

Makes no sense to pass struct net * to get_dev op, as there is only one
net possible, the one the action was created in. So just store it in
mirred priv and use directly.

Rename the mirred op callback function.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/act_api.h          |  3 +--
 include/net/tc_act/tc_mirred.h |  1 +
 net/sched/act_mirred.c         | 13 +++++--------
 net/sched/cls_api.c            |  6 ++----
 4 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index b944e0e..900168a 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -93,8 +93,7 @@ struct tc_action_ops {
 	int     (*walk)(struct net *, struct sk_buff *,
 			struct netlink_callback *, int, const struct tc_action_ops *);
 	void	(*stats_update)(struct tc_action *, u64, u32, u64);
-	int	(*get_dev)(const struct tc_action *a, struct net *net,
-			   struct net_device **mirred_dev);
+	struct net_device *(*get_dev)(const struct tc_action *a);
 };
 
 struct tc_action_net {
diff --git a/include/net/tc_act/tc_mirred.h b/include/net/tc_act/tc_mirred.h
index 604bc31..21a6565 100644
--- a/include/net/tc_act/tc_mirred.h
+++ b/include/net/tc_act/tc_mirred.h
@@ -10,6 +10,7 @@ struct tcf_mirred {
 	int			tcfm_ifindex;
 	bool			tcfm_mac_header_xmit;
 	struct net_device __rcu	*tcfm_dev;
+	struct net		*net;
 	struct list_head	tcfm_list;
 };
 #define to_mirred(a) ((struct tcf_mirred *)a)
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 416627c..8b3e5938 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -140,6 +140,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
 	m->tcfm_eaction = parm->eaction;
 	if (dev != NULL) {
 		m->tcfm_ifindex = parm->ifindex;
+		m->net = net;
 		if (ret != ACT_P_CREATED)
 			dev_put(rcu_dereference_protected(m->tcfm_dev, 1));
 		dev_hold(dev);
@@ -313,15 +314,11 @@ static struct notifier_block mirred_device_notifier = {
 	.notifier_call = mirred_device_event,
 };
 
-static int tcf_mirred_device(const struct tc_action *a, struct net *net,
-			     struct net_device **mirred_dev)
+static struct net_device *tcf_mirred_get_dev(const struct tc_action *a)
 {
-	int ifindex = tcf_mirred_ifindex(a);
+	struct tcf_mirred *m = to_mirred(a);
 
-	*mirred_dev = __dev_get_by_index(net, ifindex);
-	if (!*mirred_dev)
-		return -EINVAL;
-	return 0;
+	return __dev_get_by_index(m->net, m->tcfm_ifindex);
 }
 
 static struct tc_action_ops act_mirred_ops = {
@@ -336,7 +333,7 @@ static struct tc_action_ops act_mirred_ops = {
 	.walk		=	tcf_mirred_walker,
 	.lookup		=	tcf_mirred_search,
 	.size		=	sizeof(struct tcf_mirred),
-	.get_dev	=	tcf_mirred_device,
+	.get_dev	=	tcf_mirred_get_dev,
 };
 
 static __net_init int mirred_init_net(struct net *net)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 0b2219a..450873b 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1016,10 +1016,8 @@ int tcf_exts_get_dev(struct net_device *dev, struct tcf_exts *exts,
 
 	tcf_exts_to_list(exts, &actions);
 	list_for_each_entry(a, &actions, list) {
-		if (a->ops->get_dev) {
-			a->ops->get_dev(a, dev_net(dev), hw_dev);
-			break;
-		}
+		if (a->ops->get_dev)
+			*hw_dev = a->ops->get_dev(a);
 	}
 	if (*hw_dev)
 		return 0;
-- 
2.9.5

[patch net-next v2 2/4] net: sched: introduce per-egress action device callbacks

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Introduce infrastructure that allows drivers to register callbacks that
are called whenever tc would offload inserted rule and specified device
acts as tc action egress device.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v1->v2:
- take rtnl for register/unregister
---
 include/net/act_api.h |  34 ++++++++
 include/net/pkt_cls.h |   2 +
 net/sched/act_api.c   | 220 ++++++++++++++++++++++++++++++++++++++++++++++++++
 net/sched/cls_api.c   |  30 +++++++
 4 files changed, 286 insertions(+)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index 900168a..f5e8c90 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -174,4 +174,38 @@ static inline void tcf_action_stats_update(struct tc_action *a, u64 bytes,
 #endif
 }
 
+typedef int tc_setup_cb_t(enum tc_setup_type type,
+			  void *type_data, void *cb_priv);
+
+#ifdef CONFIG_NET_CLS_ACT
+int tc_setup_cb_egdev_register(const struct net_device *dev,
+			       tc_setup_cb_t *cb, void *cb_priv);
+void tc_setup_cb_egdev_unregister(const struct net_device *dev,
+				  tc_setup_cb_t *cb, void *cb_priv);
+int tc_setup_cb_egdev_call(const struct net_device *dev,
+			   enum tc_setup_type type, void *type_data,
+			   bool err_stop);
+#else
+static inline
+int tc_setup_cb_egdev_register(const struct net_device *dev,
+			       tc_setup_cb_t *cb, void *cb_priv)
+{
+	return 0;
+}
+
+static inline
+void tc_setup_cb_egdev_unregister(const struct net_device *dev,
+				  tc_setup_cb_t *cb, void *cb_priv)
+{
+}
+
+static inline
+int tc_setup_cb_egdev_call(const struct net_device *dev,
+			   enum tc_setup_type type, void *type_data,
+			   bool err_stop)
+{
+	return 0;
+}
+#endif
+
 #endif
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index e80edd8..6f8149c 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -206,6 +206,8 @@ int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts *exts);
 int tcf_exts_dump_stats(struct sk_buff *skb, struct tcf_exts *exts);
 int tcf_exts_get_dev(struct net_device *dev, struct tcf_exts *exts,
 		     struct net_device **hw_dev);
+int tcf_exts_egdev_cb_call(struct tcf_exts *exts, enum tc_setup_type type,
+			   void *type_data, bool err_stop);
 
 /**
  * struct tcf_pkt_info - packet information
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index da6fa82..ac97db9 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -21,6 +21,8 @@
 #include <linux/kmod.h>
 #include <linux/err.h>
 #include <linux/module.h>
+#include <linux/rhashtable.h>
+#include <linux/list.h>
 #include <net/net_namespace.h>
 #include <net/sock.h>
 #include <net/sch_generic.h>
@@ -1249,8 +1251,226 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
 	return skb->len;
 }
 
+struct tcf_action_net {
+	struct rhashtable egdev_ht;
+};
+
+static unsigned int tcf_action_net_id;
+
+struct tcf_action_egdev_cb {
+	struct list_head list;
+	tc_setup_cb_t *cb;
+	void *cb_priv;
+};
+
+struct tcf_action_egdev {
+	struct rhash_head ht_node;
+	const struct net_device *dev;
+	unsigned int refcnt;
+	struct list_head cb_list;
+};
+
+static const struct rhashtable_params tcf_action_egdev_ht_params = {
+	.key_offset = offsetof(struct tcf_action_egdev, dev),
+	.head_offset = offsetof(struct tcf_action_egdev, ht_node),
+	.key_len = sizeof(const struct net_device *),
+};
+
+static struct tcf_action_egdev *
+tcf_action_egdev_lookup(const struct net_device *dev)
+{
+	struct net *net = dev_net(dev);
+	struct tcf_action_net *tan = net_generic(net, tcf_action_net_id);
+
+	return rhashtable_lookup_fast(&tan->egdev_ht, &dev,
+				      tcf_action_egdev_ht_params);
+}
+
+static struct tcf_action_egdev *
+tcf_action_egdev_get(const struct net_device *dev)
+{
+	struct tcf_action_egdev *egdev;
+	struct tcf_action_net *tan;
+
+	egdev = tcf_action_egdev_lookup(dev);
+	if (egdev)
+		goto inc_ref;
+
+	egdev = kzalloc(sizeof(*egdev), GFP_KERNEL);
+	if (!egdev)
+		return NULL;
+	INIT_LIST_HEAD(&egdev->cb_list);
+	tan = net_generic(dev_net(dev), tcf_action_net_id);
+	rhashtable_insert_fast(&tan->egdev_ht, &egdev->ht_node,
+			       tcf_action_egdev_ht_params);
+
+inc_ref:
+	egdev->refcnt++;
+	return egdev;
+}
+
+static void tcf_action_egdev_put(struct tcf_action_egdev *egdev)
+{
+	struct tcf_action_net *tan;
+
+	if (--egdev->refcnt)
+		return;
+	tan = net_generic(dev_net(egdev->dev), tcf_action_net_id);
+	rhashtable_remove_fast(&tan->egdev_ht, &egdev->ht_node,
+			       tcf_action_egdev_ht_params);
+	kfree(egdev);
+}
+
+static struct tcf_action_egdev_cb *
+tcf_action_egdev_cb_lookup(struct tcf_action_egdev *egdev,
+			   tc_setup_cb_t *cb, void *cb_priv)
+{
+	struct tcf_action_egdev_cb *egdev_cb;
+
+	list_for_each_entry(egdev_cb, &egdev->cb_list, list)
+		if (egdev_cb->cb == cb && egdev_cb->cb_priv == cb_priv)
+			return egdev_cb;
+	return NULL;
+}
+
+static int tcf_action_egdev_cb_call(struct tcf_action_egdev *egdev,
+				    enum tc_setup_type type,
+				    void *type_data, bool err_stop)
+{
+	struct tcf_action_egdev_cb *egdev_cb;
+	int ok_count = 0;
+	int err;
+
+	list_for_each_entry(egdev_cb, &egdev->cb_list, list) {
+		err = egdev_cb->cb(type, type_data, egdev_cb->cb_priv);
+		if (err) {
+			if (err_stop)
+				return err;
+		} else {
+			ok_count++;
+		}
+	}
+	return ok_count;
+}
+
+static int tcf_action_egdev_cb_add(struct tcf_action_egdev *egdev,
+				   tc_setup_cb_t *cb, void *cb_priv)
+{
+	struct tcf_action_egdev_cb *egdev_cb;
+
+	egdev_cb = tcf_action_egdev_cb_lookup(egdev, cb, cb_priv);
+	if (WARN_ON(egdev_cb))
+		return -EEXIST;
+	egdev_cb = kzalloc(sizeof(*egdev_cb), GFP_KERNEL);
+	if (!egdev_cb)
+		return -ENOMEM;
+	egdev_cb->cb = cb;
+	egdev_cb->cb_priv = cb_priv;
+	list_add(&egdev_cb->list, &egdev->cb_list);
+	return 0;
+}
+
+static void tcf_action_egdev_cb_del(struct tcf_action_egdev *egdev,
+				    tc_setup_cb_t *cb, void *cb_priv)
+{
+	struct tcf_action_egdev_cb *egdev_cb;
+
+	egdev_cb = tcf_action_egdev_cb_lookup(egdev, cb, cb_priv);
+	if (WARN_ON(!egdev_cb))
+		return;
+	list_del(&egdev_cb->list);
+	kfree(egdev_cb);
+}
+
+static int __tc_setup_cb_egdev_register(const struct net_device *dev,
+					tc_setup_cb_t *cb, void *cb_priv)
+{
+	struct tcf_action_egdev *egdev = tcf_action_egdev_get(dev);
+	int err;
+
+	if (!egdev)
+		return -ENOMEM;
+	err = tcf_action_egdev_cb_add(egdev, cb, cb_priv);
+	if (err)
+		goto err_cb_add;
+	return 0;
+
+err_cb_add:
+	tcf_action_egdev_put(egdev);
+	return err;
+}
+int tc_setup_cb_egdev_register(const struct net_device *dev,
+			       tc_setup_cb_t *cb, void *cb_priv)
+{
+	int err;
+
+	rtnl_lock();
+	err = __tc_setup_cb_egdev_register(dev, cb, cb_priv);
+	rtnl_unlock();
+	return err;
+}
+EXPORT_SYMBOL_GPL(tc_setup_cb_egdev_register);
+
+static void __tc_setup_cb_egdev_unregister(const struct net_device *dev,
+					   tc_setup_cb_t *cb, void *cb_priv)
+{
+	struct tcf_action_egdev *egdev = tcf_action_egdev_lookup(dev);
+
+	if (WARN_ON(!egdev))
+		return;
+	tcf_action_egdev_cb_del(egdev, cb, cb_priv);
+	tcf_action_egdev_put(egdev);
+}
+void tc_setup_cb_egdev_unregister(const struct net_device *dev,
+				  tc_setup_cb_t *cb, void *cb_priv)
+{
+	rtnl_lock();
+	__tc_setup_cb_egdev_unregister(dev, cb, cb_priv);
+	rtnl_unlock();
+}
+EXPORT_SYMBOL_GPL(tc_setup_cb_egdev_unregister);
+
+int tc_setup_cb_egdev_call(const struct net_device *dev,
+			   enum tc_setup_type type, void *type_data,
+			   bool err_stop)
+{
+	struct tcf_action_egdev *egdev = tcf_action_egdev_lookup(dev);
+
+	if (!egdev)
+		return 0;
+	return tcf_action_egdev_cb_call(egdev, type, type_data, err_stop);
+}
+EXPORT_SYMBOL_GPL(tc_setup_cb_egdev_call);
+
+static __net_init int tcf_action_net_init(struct net *net)
+{
+	struct tcf_action_net *tan = net_generic(net, tcf_action_net_id);
+
+	return rhashtable_init(&tan->egdev_ht, &tcf_action_egdev_ht_params);
+}
+
+static void __net_exit tcf_action_net_exit(struct net *net)
+{
+	struct tcf_action_net *tan = net_generic(net, tcf_action_net_id);
+
+	rhashtable_destroy(&tan->egdev_ht);
+}
+
+static struct pernet_operations tcf_action_net_ops = {
+	.init = tcf_action_net_init,
+	.exit = tcf_action_net_exit,
+	.id = &tcf_action_net_id,
+	.size = sizeof(struct tcf_action_net),
+};
+
 static int __init tc_action_init(void)
 {
+	int err;
+
+	err = register_pernet_subsys(&tcf_action_net_ops);
+	if (err)
+		return err;
+
 	rtnl_register(PF_UNSPEC, RTM_NEWACTION, tc_ctl_action, NULL, 0);
 	rtnl_register(PF_UNSPEC, RTM_DELACTION, tc_ctl_action, NULL, 0);
 	rtnl_register(PF_UNSPEC, RTM_GETACTION, tc_ctl_action, tc_dump_action,
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 450873b..99f9432 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1026,6 +1026,36 @@ int tcf_exts_get_dev(struct net_device *dev, struct tcf_exts *exts,
 }
 EXPORT_SYMBOL(tcf_exts_get_dev);
 
+int tcf_exts_egdev_cb_call(struct tcf_exts *exts, enum tc_setup_type type,
+			   void *type_data, bool err_stop)
+{
+	int ok_count = 0;
+#ifdef CONFIG_NET_CLS_ACT
+	const struct tc_action *a;
+	struct net_device *dev;
+	LIST_HEAD(actions);
+	int ret;
+
+	if (!tcf_exts_has_actions(exts))
+		return 0;
+
+	tcf_exts_to_list(exts, &actions);
+	list_for_each_entry(a, &actions, list) {
+		if (!a->ops->get_dev)
+			continue;
+		dev = a->ops->get_dev(a);
+		if (!dev || !tc_can_offload(dev))
+			continue;
+		ret = tc_setup_cb_egdev_call(dev, type, type_data, err_stop);
+		if (ret < 0)
+			return ret;
+		ok_count += ret;
+	}
+#endif
+	return ok_count;
+}
+EXPORT_SYMBOL(tcf_exts_egdev_cb_call);
+
 static int __init tc_filter_init(void)
 {
 	rtnl_register(PF_UNSPEC, RTM_NEWTFILTER, tc_ctl_tfilter, NULL, 0);
-- 
2.9.5

RE: [patch net-next v2 2/4] net: sched: introduce per-egress action device callbacks

[Chopra, Manish]

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Jiri Pirko
> Sent: Wednesday, October 11, 2017 1:11 PM
> To: netdev@vger.kernel.org
> Cc: davem@davemloft.net; jhs@mojatatu.com; xiyou.wangcong@gmail.com;
> saeedm@mellanox.com; matanb@mellanox.com; leonro@mellanox.com;
> mlxsw@mellanox.com; David.Laight@ACULAB.COM; gerlitz.or@gmail.com
> Subject: [patch net-next v2 2/4] net: sched: introduce per-egress action device
> callbacks
> 
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Introduce infrastructure that allows drivers to register callbacks that are called
> whenever tc would offload inserted rule and specified device acts as tc action
> egress device.
> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> v1->v2:
> - take rtnl for register/unregister
> ---
>  include/net/act_api.h |  34 ++++++++
>  include/net/pkt_cls.h |   2 +
>  net/sched/act_api.c   | 220
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  net/sched/cls_api.c   |  30 +++++++
>  4 files changed, 286 insertions(+)
> 
> diff --git a/include/net/act_api.h b/include/net/act_api.h index
> 900168a..f5e8c90 100644
> --- a/include/net/act_api.h
> +++ b/include/net/act_api.h
> @@ -174,4 +174,38 @@ static inline void tcf_action_stats_update(struct
> tc_action *a, u64 bytes,  #endif  }
> 
> +typedef int tc_setup_cb_t(enum tc_setup_type type,
> +			  void *type_data, void *cb_priv);
> +
> +#ifdef CONFIG_NET_CLS_ACT
> +int tc_setup_cb_egdev_register(const struct net_device *dev,
> +			       tc_setup_cb_t *cb, void *cb_priv); void
> +tc_setup_cb_egdev_unregister(const struct net_device *dev,
> +				  tc_setup_cb_t *cb, void *cb_priv); int
> +tc_setup_cb_egdev_call(const struct net_device *dev,
> +			   enum tc_setup_type type, void *type_data,
> +			   bool err_stop);
> +#else
> +static inline
> +int tc_setup_cb_egdev_register(const struct net_device *dev,
> +			       tc_setup_cb_t *cb, void *cb_priv) {
> +	return 0;
> +}
> +
> +static inline
> +void tc_setup_cb_egdev_unregister(const struct net_device *dev,
> +				  tc_setup_cb_t *cb, void *cb_priv) { }
> +
> +static inline
> +int tc_setup_cb_egdev_call(const struct net_device *dev,
> +			   enum tc_setup_type type, void *type_data,
> +			   bool err_stop)
> +{
> +	return 0;
> +}
> +#endif
> +
>  #endif
> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index
> e80edd8..6f8149c 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -206,6 +206,8 @@ int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts
> *exts);  int tcf_exts_dump_stats(struct sk_buff *skb, struct tcf_exts *exts);  int
> tcf_exts_get_dev(struct net_device *dev, struct tcf_exts *exts,
>  		     struct net_device **hw_dev);
> +int tcf_exts_egdev_cb_call(struct tcf_exts *exts, enum tc_setup_type type,
> +			   void *type_data, bool err_stop);
> 
>  /**
>   * struct tcf_pkt_info - packet information diff --git a/net/sched/act_api.c
> b/net/sched/act_api.c index da6fa82..ac97db9 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -21,6 +21,8 @@
>  #include <linux/kmod.h>
>  #include <linux/err.h>
>  #include <linux/module.h>
> +#include <linux/rhashtable.h>
> +#include <linux/list.h>
>  #include <net/net_namespace.h>
>  #include <net/sock.h>
>  #include <net/sch_generic.h>
> @@ -1249,8 +1251,226 @@ static int tc_dump_action(struct sk_buff *skb,
> struct netlink_callback *cb)
>  	return skb->len;
>  }
> 
> +struct tcf_action_net {
> +	struct rhashtable egdev_ht;
> +};
> +
> +static unsigned int tcf_action_net_id;
> +
> +struct tcf_action_egdev_cb {
> +	struct list_head list;
> +	tc_setup_cb_t *cb;
> +	void *cb_priv;
> +};
> +
> +struct tcf_action_egdev {
> +	struct rhash_head ht_node;
> +	const struct net_device *dev;
> +	unsigned int refcnt;
> +	struct list_head cb_list;
> +};
> +
> +static const struct rhashtable_params tcf_action_egdev_ht_params = {
> +	.key_offset = offsetof(struct tcf_action_egdev, dev),
> +	.head_offset = offsetof(struct tcf_action_egdev, ht_node),
> +	.key_len = sizeof(const struct net_device *), };
> +
> +static struct tcf_action_egdev *
> +tcf_action_egdev_lookup(const struct net_device *dev) {
> +	struct net *net = dev_net(dev);
> +	struct tcf_action_net *tan = net_generic(net, tcf_action_net_id);
> +
> +	return rhashtable_lookup_fast(&tan->egdev_ht, &dev,
> +				      tcf_action_egdev_ht_params);
> +}
> +
> +static struct tcf_action_egdev *
> +tcf_action_egdev_get(const struct net_device *dev) {
> +	struct tcf_action_egdev *egdev;
> +	struct tcf_action_net *tan;
> +
> +	egdev = tcf_action_egdev_lookup(dev);
> +	if (egdev)
> +		goto inc_ref;
> +
> +	egdev = kzalloc(sizeof(*egdev), GFP_KERNEL);
> +	if (!egdev)
> +		return NULL;
> +	INIT_LIST_HEAD(&egdev->cb_list);
> +	tan = net_generic(dev_net(dev), tcf_action_net_id);
> +	rhashtable_insert_fast(&tan->egdev_ht, &egdev->ht_node,
> +			       tcf_action_egdev_ht_params);
> +
> +inc_ref:
> +	egdev->refcnt++;
> +	return egdev;
> +}
> +
> +static void tcf_action_egdev_put(struct tcf_action_egdev *egdev) {
> +	struct tcf_action_net *tan;
> +
> +	if (--egdev->refcnt)
> +		return;
> +	tan = net_generic(dev_net(egdev->dev), tcf_action_net_id);
> +	rhashtable_remove_fast(&tan->egdev_ht, &egdev->ht_node,
> +			       tcf_action_egdev_ht_params);
> +	kfree(egdev);
> +}
> +
> +static struct tcf_action_egdev_cb *
> +tcf_action_egdev_cb_lookup(struct tcf_action_egdev *egdev,
> +			   tc_setup_cb_t *cb, void *cb_priv) {
> +	struct tcf_action_egdev_cb *egdev_cb;
> +
> +	list_for_each_entry(egdev_cb, &egdev->cb_list, list)
> +		if (egdev_cb->cb == cb && egdev_cb->cb_priv == cb_priv)
> +			return egdev_cb;
> +	return NULL;
> +}
> +
> +static int tcf_action_egdev_cb_call(struct tcf_action_egdev *egdev,
> +				    enum tc_setup_type type,
> +				    void *type_data, bool err_stop) {
> +	struct tcf_action_egdev_cb *egdev_cb;
> +	int ok_count = 0;
> +	int err;
> +
> +	list_for_each_entry(egdev_cb, &egdev->cb_list, list) {
> +		err = egdev_cb->cb(type, type_data, egdev_cb->cb_priv);
> +		if (err) {
> +			if (err_stop)
> +				return err;
> +		} else {
> +			ok_count++;
> +		}
> +	}
> +	return ok_count;
> +}
> +
> +static int tcf_action_egdev_cb_add(struct tcf_action_egdev *egdev,
> +				   tc_setup_cb_t *cb, void *cb_priv) {
> +	struct tcf_action_egdev_cb *egdev_cb;
> +
> +	egdev_cb = tcf_action_egdev_cb_lookup(egdev, cb, cb_priv);
> +	if (WARN_ON(egdev_cb))
> +		return -EEXIST;
> +	egdev_cb = kzalloc(sizeof(*egdev_cb), GFP_KERNEL);
> +	if (!egdev_cb)
> +		return -ENOMEM;
> +	egdev_cb->cb = cb;
> +	egdev_cb->cb_priv = cb_priv;
> +	list_add(&egdev_cb->list, &egdev->cb_list);
> +	return 0;
> +}
> +
> +static void tcf_action_egdev_cb_del(struct tcf_action_egdev *egdev,
> +				    tc_setup_cb_t *cb, void *cb_priv) {
> +	struct tcf_action_egdev_cb *egdev_cb;
> +
> +	egdev_cb = tcf_action_egdev_cb_lookup(egdev, cb, cb_priv);
> +	if (WARN_ON(!egdev_cb))
> +		return;
> +	list_del(&egdev_cb->list);
> +	kfree(egdev_cb);
> +}
> +
> +static int __tc_setup_cb_egdev_register(const struct net_device *dev,
> +					tc_setup_cb_t *cb, void *cb_priv)
> +{
> +	struct tcf_action_egdev *egdev = tcf_action_egdev_get(dev);
> +	int err;
> +
> +	if (!egdev)
> +		return -ENOMEM;
> +	err = tcf_action_egdev_cb_add(egdev, cb, cb_priv);
> +	if (err)
> +		goto err_cb_add;
> +	return 0;
> +
> +err_cb_add:
> +	tcf_action_egdev_put(egdev);
> +	return err;
> +}
> +int tc_setup_cb_egdev_register(const struct net_device *dev,
> +			       tc_setup_cb_t *cb, void *cb_priv) {
> +	int err;
> +
> +	rtnl_lock();
> +	err = __tc_setup_cb_egdev_register(dev, cb, cb_priv);
> +	rtnl_unlock();
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(tc_setup_cb_egdev_register);
> +
> +static void __tc_setup_cb_egdev_unregister(const struct net_device *dev,
> +					   tc_setup_cb_t *cb, void *cb_priv) {
> +	struct tcf_action_egdev *egdev = tcf_action_egdev_lookup(dev);
> +
> +	if (WARN_ON(!egdev))
> +		return;
> +	tcf_action_egdev_cb_del(egdev, cb, cb_priv);
> +	tcf_action_egdev_put(egdev);
> +}
> +void tc_setup_cb_egdev_unregister(const struct net_device *dev,
> +				  tc_setup_cb_t *cb, void *cb_priv) {
> +	rtnl_lock();
> +	__tc_setup_cb_egdev_unregister(dev, cb, cb_priv);
> +	rtnl_unlock();
> +}
> +EXPORT_SYMBOL_GPL(tc_setup_cb_egdev_unregister);
> +
> +int tc_setup_cb_egdev_call(const struct net_device *dev,
> +			   enum tc_setup_type type, void *type_data,
> +			   bool err_stop)
> +{
> +	struct tcf_action_egdev *egdev = tcf_action_egdev_lookup(dev);
> +
> +	if (!egdev)
> +		return 0;
> +	return tcf_action_egdev_cb_call(egdev, type, type_data, err_stop); }
> +EXPORT_SYMBOL_GPL(tc_setup_cb_egdev_call);
> +
> +static __net_init int tcf_action_net_init(struct net *net) {
> +	struct tcf_action_net *tan = net_generic(net, tcf_action_net_id);
> +
> +	return rhashtable_init(&tan->egdev_ht, &tcf_action_egdev_ht_params);
> }
> +
> +static void __net_exit tcf_action_net_exit(struct net *net) {
> +	struct tcf_action_net *tan = net_generic(net, tcf_action_net_id);
> +
> +	rhashtable_destroy(&tan->egdev_ht);
> +}
> +
> +static struct pernet_operations tcf_action_net_ops = {
> +	.init = tcf_action_net_init,
> +	.exit = tcf_action_net_exit,
> +	.id = &tcf_action_net_id,
> +	.size = sizeof(struct tcf_action_net), };
> +
>  static int __init tc_action_init(void)
>  {
> +	int err;
> +
> +	err = register_pernet_subsys(&tcf_action_net_ops);
> +	if (err)
> +		return err;
> +
>  	rtnl_register(PF_UNSPEC, RTM_NEWACTION, tc_ctl_action, NULL, 0);
>  	rtnl_register(PF_UNSPEC, RTM_DELACTION, tc_ctl_action, NULL, 0);
>  	rtnl_register(PF_UNSPEC, RTM_GETACTION, tc_ctl_action,
> tc_dump_action, diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index
> 450873b..99f9432 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -1026,6 +1026,36 @@ int tcf_exts_get_dev(struct net_device *dev, struct
> tcf_exts *exts,  }  EXPORT_SYMBOL(tcf_exts_get_dev);
> 
> +int tcf_exts_egdev_cb_call(struct tcf_exts *exts, enum tc_setup_type type,
> +			   void *type_data, bool err_stop)
> +{
> +	int ok_count = 0;
> +#ifdef CONFIG_NET_CLS_ACT
> +	const struct tc_action *a;
> +	struct net_device *dev;
> +	LIST_HEAD(actions);
> +	int ret;
> +
> +	if (!tcf_exts_has_actions(exts))
> +		return 0;
> +
> +	tcf_exts_to_list(exts, &actions);
> +	list_for_each_entry(a, &actions, list) {
> +		if (!a->ops->get_dev)
> +			continue;
> +		dev = a->ops->get_dev(a);
> +		if (!dev || !tc_can_offload(dev))
> +			continue;
> +		ret = tc_setup_cb_egdev_call(dev, type, type_data, err_stop);
> +		if (ret < 0)
> +			return ret;
> +		ok_count += ret;
> +	}
> +#endif
> +	return ok_count;
> +}
> +EXPORT_SYMBOL(tcf_exts_egdev_cb_call);
> +
>  static int __init tc_filter_init(void)
>  {
>  	rtnl_register(PF_UNSPEC, RTM_NEWTFILTER, tc_ctl_tfilter, NULL, 0);
> --
> 2.9.5

Hi Jiri,  I am just trying to understand this a bit specially in context of offloaded macvlan devices.
I am assuming that any action device [fox example - offloaded macvlan device] can register for tc callback function using " tc_setup_cb_egdev_register()"
It was hard for me to call this above API from offloaded macvlan flow [.ndo_dfwd_add_station] in PF driver,
since it already grabs rtnl_lock(), so I tried to register it from different context in PF driver somehow for the offloaded macvlan device.

Now, I didn't understand actually the usage of egdev callback registration here -
Consider this example -

tc qdisc add dev <pf_device> ingress
tc filter add dev <pf_device> prio 1 protocol ip parent ffff: flower skip_sw dst_ip 192.168.55.55/24 action mirred egress redirect dev mvlan_0

The above filter add command is received by the PF registered callback [which was supplied by tcf_block_cb_register()], the same command is also
received by the callback function which was supplied in tc_setup_cb_egdev_register().

What is the significance of getting two callbacks here ? Since, I can add the above said filter in first callback function even.
What am I supposed to do in second callback ? is there any special motivation for egdev callback registration ?

Thanks,
Manish

Re: [patch net-next v2 2/4] net: sched: introduce per-egress action device callbacks

[Jiri Pirko]

Sun, Nov 19, 2017 at 08:37:49PM CET, Manish.Chopra@cavium.com wrote:
>> -----Original Message-----
>> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
>> On Behalf Of Jiri Pirko
>> Sent: Wednesday, October 11, 2017 1:11 PM
>> To: netdev@vger.kernel.org
>> Cc: davem@davemloft.net; jhs@mojatatu.com; xiyou.wangcong@gmail.com;
>> saeedm@mellanox.com; matanb@mellanox.com; leonro@mellanox.com;
>> mlxsw@mellanox.com; David.Laight@ACULAB.COM; gerlitz.or@gmail.com
>> Subject: [patch net-next v2 2/4] net: sched: introduce per-egress action device
>> callbacks
>> 
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Introduce infrastructure that allows drivers to register callbacks that are called
>> whenever tc would offload inserted rule and specified device acts as tc action
>> egress device.
>> 
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>> v1->v2:
>> - take rtnl for register/unregister
>> ---
>>  include/net/act_api.h |  34 ++++++++
>>  include/net/pkt_cls.h |   2 +
>>  net/sched/act_api.c   | 220
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  net/sched/cls_api.c   |  30 +++++++
>>  4 files changed, 286 insertions(+)
>> 
>> diff --git a/include/net/act_api.h b/include/net/act_api.h index
>> 900168a..f5e8c90 100644
>> --- a/include/net/act_api.h
>> +++ b/include/net/act_api.h
>> @@ -174,4 +174,38 @@ static inline void tcf_action_stats_update(struct
>> tc_action *a, u64 bytes,  #endif  }
>> 
>> +typedef int tc_setup_cb_t(enum tc_setup_type type,
>> +			  void *type_data, void *cb_priv);
>> +
>> +#ifdef CONFIG_NET_CLS_ACT
>> +int tc_setup_cb_egdev_register(const struct net_device *dev,
>> +			       tc_setup_cb_t *cb, void *cb_priv); void
>> +tc_setup_cb_egdev_unregister(const struct net_device *dev,
>> +				  tc_setup_cb_t *cb, void *cb_priv); int
>> +tc_setup_cb_egdev_call(const struct net_device *dev,
>> +			   enum tc_setup_type type, void *type_data,
>> +			   bool err_stop);
>> +#else
>> +static inline
>> +int tc_setup_cb_egdev_register(const struct net_device *dev,
>> +			       tc_setup_cb_t *cb, void *cb_priv) {
>> +	return 0;
>> +}
>> +
>> +static inline
>> +void tc_setup_cb_egdev_unregister(const struct net_device *dev,
>> +				  tc_setup_cb_t *cb, void *cb_priv) { }
>> +
>> +static inline
>> +int tc_setup_cb_egdev_call(const struct net_device *dev,
>> +			   enum tc_setup_type type, void *type_data,
>> +			   bool err_stop)
>> +{
>> +	return 0;
>> +}
>> +#endif
>> +
>>  #endif
>> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index
>> e80edd8..6f8149c 100644
>> --- a/include/net/pkt_cls.h
>> +++ b/include/net/pkt_cls.h
>> @@ -206,6 +206,8 @@ int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts
>> *exts);  int tcf_exts_dump_stats(struct sk_buff *skb, struct tcf_exts *exts);  int
>> tcf_exts_get_dev(struct net_device *dev, struct tcf_exts *exts,
>>  		     struct net_device **hw_dev);
>> +int tcf_exts_egdev_cb_call(struct tcf_exts *exts, enum tc_setup_type type,
>> +			   void *type_data, bool err_stop);
>> 
>>  /**
>>   * struct tcf_pkt_info - packet information diff --git a/net/sched/act_api.c
>> b/net/sched/act_api.c index da6fa82..ac97db9 100644
>> --- a/net/sched/act_api.c
>> +++ b/net/sched/act_api.c
>> @@ -21,6 +21,8 @@
>>  #include <linux/kmod.h>
>>  #include <linux/err.h>
>>  #include <linux/module.h>
>> +#include <linux/rhashtable.h>
>> +#include <linux/list.h>
>>  #include <net/net_namespace.h>
>>  #include <net/sock.h>
>>  #include <net/sch_generic.h>
>> @@ -1249,8 +1251,226 @@ static int tc_dump_action(struct sk_buff *skb,
>> struct netlink_callback *cb)
>>  	return skb->len;
>>  }
>> 
>> +struct tcf_action_net {
>> +	struct rhashtable egdev_ht;
>> +};
>> +
>> +static unsigned int tcf_action_net_id;
>> +
>> +struct tcf_action_egdev_cb {
>> +	struct list_head list;
>> +	tc_setup_cb_t *cb;
>> +	void *cb_priv;
>> +};
>> +
>> +struct tcf_action_egdev {
>> +	struct rhash_head ht_node;
>> +	const struct net_device *dev;
>> +	unsigned int refcnt;
>> +	struct list_head cb_list;
>> +};
>> +
>> +static const struct rhashtable_params tcf_action_egdev_ht_params = {
>> +	.key_offset = offsetof(struct tcf_action_egdev, dev),
>> +	.head_offset = offsetof(struct tcf_action_egdev, ht_node),
>> +	.key_len = sizeof(const struct net_device *), };
>> +
>> +static struct tcf_action_egdev *
>> +tcf_action_egdev_lookup(const struct net_device *dev) {
>> +	struct net *net = dev_net(dev);
>> +	struct tcf_action_net *tan = net_generic(net, tcf_action_net_id);
>> +
>> +	return rhashtable_lookup_fast(&tan->egdev_ht, &dev,
>> +				      tcf_action_egdev_ht_params);
>> +}
>> +
>> +static struct tcf_action_egdev *
>> +tcf_action_egdev_get(const struct net_device *dev) {
>> +	struct tcf_action_egdev *egdev;
>> +	struct tcf_action_net *tan;
>> +
>> +	egdev = tcf_action_egdev_lookup(dev);
>> +	if (egdev)
>> +		goto inc_ref;
>> +
>> +	egdev = kzalloc(sizeof(*egdev), GFP_KERNEL);
>> +	if (!egdev)
>> +		return NULL;
>> +	INIT_LIST_HEAD(&egdev->cb_list);
>> +	tan = net_generic(dev_net(dev), tcf_action_net_id);
>> +	rhashtable_insert_fast(&tan->egdev_ht, &egdev->ht_node,
>> +			       tcf_action_egdev_ht_params);
>> +
>> +inc_ref:
>> +	egdev->refcnt++;
>> +	return egdev;
>> +}
>> +
>> +static void tcf_action_egdev_put(struct tcf_action_egdev *egdev) {
>> +	struct tcf_action_net *tan;
>> +
>> +	if (--egdev->refcnt)
>> +		return;
>> +	tan = net_generic(dev_net(egdev->dev), tcf_action_net_id);
>> +	rhashtable_remove_fast(&tan->egdev_ht, &egdev->ht_node,
>> +			       tcf_action_egdev_ht_params);
>> +	kfree(egdev);
>> +}
>> +
>> +static struct tcf_action_egdev_cb *
>> +tcf_action_egdev_cb_lookup(struct tcf_action_egdev *egdev,
>> +			   tc_setup_cb_t *cb, void *cb_priv) {
>> +	struct tcf_action_egdev_cb *egdev_cb;
>> +
>> +	list_for_each_entry(egdev_cb, &egdev->cb_list, list)
>> +		if (egdev_cb->cb == cb && egdev_cb->cb_priv == cb_priv)
>> +			return egdev_cb;
>> +	return NULL;
>> +}
>> +
>> +static int tcf_action_egdev_cb_call(struct tcf_action_egdev *egdev,
>> +				    enum tc_setup_type type,
>> +				    void *type_data, bool err_stop) {
>> +	struct tcf_action_egdev_cb *egdev_cb;
>> +	int ok_count = 0;
>> +	int err;
>> +
>> +	list_for_each_entry(egdev_cb, &egdev->cb_list, list) {
>> +		err = egdev_cb->cb(type, type_data, egdev_cb->cb_priv);
>> +		if (err) {
>> +			if (err_stop)
>> +				return err;
>> +		} else {
>> +			ok_count++;
>> +		}
>> +	}
>> +	return ok_count;
>> +}
>> +
>> +static int tcf_action_egdev_cb_add(struct tcf_action_egdev *egdev,
>> +				   tc_setup_cb_t *cb, void *cb_priv) {
>> +	struct tcf_action_egdev_cb *egdev_cb;
>> +
>> +	egdev_cb = tcf_action_egdev_cb_lookup(egdev, cb, cb_priv);
>> +	if (WARN_ON(egdev_cb))
>> +		return -EEXIST;
>> +	egdev_cb = kzalloc(sizeof(*egdev_cb), GFP_KERNEL);
>> +	if (!egdev_cb)
>> +		return -ENOMEM;
>> +	egdev_cb->cb = cb;
>> +	egdev_cb->cb_priv = cb_priv;
>> +	list_add(&egdev_cb->list, &egdev->cb_list);
>> +	return 0;
>> +}
>> +
>> +static void tcf_action_egdev_cb_del(struct tcf_action_egdev *egdev,
>> +				    tc_setup_cb_t *cb, void *cb_priv) {
>> +	struct tcf_action_egdev_cb *egdev_cb;
>> +
>> +	egdev_cb = tcf_action_egdev_cb_lookup(egdev, cb, cb_priv);
>> +	if (WARN_ON(!egdev_cb))
>> +		return;
>> +	list_del(&egdev_cb->list);
>> +	kfree(egdev_cb);
>> +}
>> +
>> +static int __tc_setup_cb_egdev_register(const struct net_device *dev,
>> +					tc_setup_cb_t *cb, void *cb_priv)
>> +{
>> +	struct tcf_action_egdev *egdev = tcf_action_egdev_get(dev);
>> +	int err;
>> +
>> +	if (!egdev)
>> +		return -ENOMEM;
>> +	err = tcf_action_egdev_cb_add(egdev, cb, cb_priv);
>> +	if (err)
>> +		goto err_cb_add;
>> +	return 0;
>> +
>> +err_cb_add:
>> +	tcf_action_egdev_put(egdev);
>> +	return err;
>> +}
>> +int tc_setup_cb_egdev_register(const struct net_device *dev,
>> +			       tc_setup_cb_t *cb, void *cb_priv) {
>> +	int err;
>> +
>> +	rtnl_lock();
>> +	err = __tc_setup_cb_egdev_register(dev, cb, cb_priv);
>> +	rtnl_unlock();
>> +	return err;
>> +}
>> +EXPORT_SYMBOL_GPL(tc_setup_cb_egdev_register);
>> +
>> +static void __tc_setup_cb_egdev_unregister(const struct net_device *dev,
>> +					   tc_setup_cb_t *cb, void *cb_priv) {
>> +	struct tcf_action_egdev *egdev = tcf_action_egdev_lookup(dev);
>> +
>> +	if (WARN_ON(!egdev))
>> +		return;
>> +	tcf_action_egdev_cb_del(egdev, cb, cb_priv);
>> +	tcf_action_egdev_put(egdev);
>> +}
>> +void tc_setup_cb_egdev_unregister(const struct net_device *dev,
>> +				  tc_setup_cb_t *cb, void *cb_priv) {
>> +	rtnl_lock();
>> +	__tc_setup_cb_egdev_unregister(dev, cb, cb_priv);
>> +	rtnl_unlock();
>> +}
>> +EXPORT_SYMBOL_GPL(tc_setup_cb_egdev_unregister);
>> +
>> +int tc_setup_cb_egdev_call(const struct net_device *dev,
>> +			   enum tc_setup_type type, void *type_data,
>> +			   bool err_stop)
>> +{
>> +	struct tcf_action_egdev *egdev = tcf_action_egdev_lookup(dev);
>> +
>> +	if (!egdev)
>> +		return 0;
>> +	return tcf_action_egdev_cb_call(egdev, type, type_data, err_stop); }
>> +EXPORT_SYMBOL_GPL(tc_setup_cb_egdev_call);
>> +
>> +static __net_init int tcf_action_net_init(struct net *net) {
>> +	struct tcf_action_net *tan = net_generic(net, tcf_action_net_id);
>> +
>> +	return rhashtable_init(&tan->egdev_ht, &tcf_action_egdev_ht_params);
>> }
>> +
>> +static void __net_exit tcf_action_net_exit(struct net *net) {
>> +	struct tcf_action_net *tan = net_generic(net, tcf_action_net_id);
>> +
>> +	rhashtable_destroy(&tan->egdev_ht);
>> +}
>> +
>> +static struct pernet_operations tcf_action_net_ops = {
>> +	.init = tcf_action_net_init,
>> +	.exit = tcf_action_net_exit,
>> +	.id = &tcf_action_net_id,
>> +	.size = sizeof(struct tcf_action_net), };
>> +
>>  static int __init tc_action_init(void)
>>  {
>> +	int err;
>> +
>> +	err = register_pernet_subsys(&tcf_action_net_ops);
>> +	if (err)
>> +		return err;
>> +
>>  	rtnl_register(PF_UNSPEC, RTM_NEWACTION, tc_ctl_action, NULL, 0);
>>  	rtnl_register(PF_UNSPEC, RTM_DELACTION, tc_ctl_action, NULL, 0);
>>  	rtnl_register(PF_UNSPEC, RTM_GETACTION, tc_ctl_action,
>> tc_dump_action, diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index
>> 450873b..99f9432 100644
>> --- a/net/sched/cls_api.c
>> +++ b/net/sched/cls_api.c
>> @@ -1026,6 +1026,36 @@ int tcf_exts_get_dev(struct net_device *dev, struct
>> tcf_exts *exts,  }  EXPORT_SYMBOL(tcf_exts_get_dev);
>> 
>> +int tcf_exts_egdev_cb_call(struct tcf_exts *exts, enum tc_setup_type type,
>> +			   void *type_data, bool err_stop)
>> +{
>> +	int ok_count = 0;
>> +#ifdef CONFIG_NET_CLS_ACT
>> +	const struct tc_action *a;
>> +	struct net_device *dev;
>> +	LIST_HEAD(actions);
>> +	int ret;
>> +
>> +	if (!tcf_exts_has_actions(exts))
>> +		return 0;
>> +
>> +	tcf_exts_to_list(exts, &actions);
>> +	list_for_each_entry(a, &actions, list) {
>> +		if (!a->ops->get_dev)
>> +			continue;
>> +		dev = a->ops->get_dev(a);
>> +		if (!dev || !tc_can_offload(dev))
>> +			continue;
>> +		ret = tc_setup_cb_egdev_call(dev, type, type_data, err_stop);
>> +		if (ret < 0)
>> +			return ret;
>> +		ok_count += ret;
>> +	}
>> +#endif
>> +	return ok_count;
>> +}
>> +EXPORT_SYMBOL(tcf_exts_egdev_cb_call);
>> +
>>  static int __init tc_filter_init(void)
>>  {
>>  	rtnl_register(PF_UNSPEC, RTM_NEWTFILTER, tc_ctl_tfilter, NULL, 0);
>> --
>> 2.9.5
>
>Hi Jiri,  I am just trying to understand this a bit specially in context of offloaded macvlan devices.
>I am assuming that any action device [fox example - offloaded macvlan device] can register for tc callback function using " tc_setup_cb_egdev_register()"
>It was hard for me to call this above API from offloaded macvlan flow [.ndo_dfwd_add_station] in PF driver,
>since it already grabs rtnl_lock(), so I tried to register it from different context in PF driver somehow for the offloaded macvlan device.
>
>Now, I didn't understand actually the usage of egdev callback registration here -
>Consider this example -
>
>tc qdisc add dev <pf_device> ingress
>tc filter add dev <pf_device> prio 1 protocol ip parent ffff: flower skip_sw dst_ip 192.168.55.55/24 action mirred egress redirect dev mvlan_0
>
>The above filter add command is received by the PF registered callback [which was supplied by tcf_block_cb_register()], the same command is also
>received by the callback function which was supplied in tc_setup_cb_egdev_register().
>
>What is the significance of getting two callbacks here ? Since, I can add the above said filter in first callback function even.
>What am I supposed to do in second callback ? is there any special motivation for egdev callback registration ?

The motivation is to be able to handle calls for netdevs that are not
owned by driver, for example tunnels. So in your case, you don't need to
register egdev cb.
>
>
>
>
>
>
>
>
>
>
>
>
>

[patch net-next v2 3/4] net: sched: convert cls_flower->egress_dev users to tc_setup_cb_egdev infra

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

The only user of cls_flower->egress_dev is mlx5. So do the conversion
there alongside with the code originating the call in cls_flower
function fl_hw_replace_filter to the newly introduced egress device
callback infrastucture.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h      |  3 ++
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c |  4 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_rep.c  | 31 +++++++----
 include/net/pkt_cls.h                             |  5 +-
 net/sched/cls_api.c                               | 13 +++--
 net/sched/cls_flower.c                            | 63 ++++++++++++-----------
 6 files changed, 73 insertions(+), 46 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index cc13d3d..5ec6d3e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -1081,6 +1081,9 @@ int mlx5e_ethtool_get_ts_info(struct mlx5e_priv *priv,
 int mlx5e_ethtool_flash_device(struct mlx5e_priv *priv,
 			       struct ethtool_flash *flash);
 
+int mlx5e_setup_tc(struct net_device *dev, enum tc_setup_type type,
+		   void *type_data);
+
 /* mlx5e generic netdev management API */
 struct net_device*
 mlx5e_create_netdev(struct mlx5_core_dev *mdev, const struct mlx5e_profile *profile,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index cc11bbb..2a32102 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -3108,8 +3108,8 @@ static int mlx5e_setup_tc_cls_flower(struct net_device *dev,
 }
 #endif
 
-static int mlx5e_setup_tc(struct net_device *dev, enum tc_setup_type type,
-			  void *type_data)
+int mlx5e_setup_tc(struct net_device *dev, enum tc_setup_type type,
+		   void *type_data)
 {
 	switch (type) {
 #ifdef CONFIG_MLX5_ESWITCH
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index 45e03c4..765fc74 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -34,6 +34,7 @@
 #include <linux/mlx5/fs.h>
 #include <net/switchdev.h>
 #include <net/pkt_cls.h>
+#include <net/act_api.h>
 #include <net/netevent.h>
 #include <net/arp.h>
 
@@ -667,14 +668,6 @@ mlx5e_rep_setup_tc_cls_flower(struct net_device *dev,
 	    cls_flower->common.chain_index)
 		return -EOPNOTSUPP;
 
-	if (cls_flower->egress_dev) {
-		struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
-
-		dev = mlx5_eswitch_get_uplink_netdev(esw);
-		return dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_CLSFLOWER,
-						     cls_flower);
-	}
-
 	switch (cls_flower->command) {
 	case TC_CLSFLOWER_REPLACE:
 		return mlx5e_configure_flower(priv, cls_flower);
@@ -698,6 +691,14 @@ static int mlx5e_rep_setup_tc(struct net_device *dev, enum tc_setup_type type,
 	}
 }
 
+static int mlx5e_rep_setup_tc_cb(enum tc_setup_type type, void *type_data,
+				 void *cb_priv)
+{
+	struct net_device *dev = cb_priv;
+
+	return mlx5e_setup_tc(dev, type, type_data);
+}
+
 bool mlx5e_is_uplink_rep(struct mlx5e_priv *priv)
 {
 	struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
@@ -1017,15 +1018,24 @@ mlx5e_vport_rep_load(struct mlx5_eswitch *esw, struct mlx5_eswitch_rep *rep)
 		goto err_detach_netdev;
 	}
 
+	err = tc_setup_cb_egdev_register(netdev, mlx5e_rep_setup_tc_cb,
+					 mlx5_eswitch_get_uplink_netdev(esw));
+	if (err)
+		goto err_neigh_cleanup;
+
 	err = register_netdev(netdev);
 	if (err) {
 		pr_warn("Failed to register representor netdev for vport %d\n",
 			rep->vport);
-		goto err_neigh_cleanup;
+		goto err_egdev_cleanup;
 	}
 
 	return 0;
 
+err_egdev_cleanup:
+	tc_setup_cb_egdev_unregister(netdev, mlx5e_rep_setup_tc_cb,
+				     mlx5_eswitch_get_uplink_netdev(esw));
+
 err_neigh_cleanup:
 	mlx5e_rep_neigh_cleanup(rpriv);
 
@@ -1047,7 +1057,8 @@ mlx5e_vport_rep_unload(struct mlx5_eswitch *esw, struct mlx5_eswitch_rep *rep)
 	void *ppriv = priv->ppriv;
 
 	unregister_netdev(rep->netdev);
-
+	tc_setup_cb_egdev_unregister(netdev, mlx5e_rep_setup_tc_cb,
+				     mlx5_eswitch_get_uplink_netdev(esw));
 	mlx5e_rep_neigh_cleanup(rpriv);
 	mlx5e_detach_netdev(priv);
 	mlx5e_destroy_netdev(priv);
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 6f8149c..c0bdf5c 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -206,8 +206,6 @@ int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts *exts);
 int tcf_exts_dump_stats(struct sk_buff *skb, struct tcf_exts *exts);
 int tcf_exts_get_dev(struct net_device *dev, struct tcf_exts *exts,
 		     struct net_device **hw_dev);
-int tcf_exts_egdev_cb_call(struct tcf_exts *exts, enum tc_setup_type type,
-			   void *type_data, bool err_stop);
 
 /**
  * struct tcf_pkt_info - packet information
@@ -407,6 +405,9 @@ tcf_match_indev(struct sk_buff *skb, int ifindex)
 }
 #endif /* CONFIG_NET_CLS_IND */
 
+int tc_setup_cb_call(struct tcf_exts *exts, enum tc_setup_type type,
+		     void *type_data, bool err_stop);
+
 struct tc_cls_common_offload {
 	u32 chain_index;
 	__be16 protocol;
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 99f9432..51994a2 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1026,8 +1026,9 @@ int tcf_exts_get_dev(struct net_device *dev, struct tcf_exts *exts,
 }
 EXPORT_SYMBOL(tcf_exts_get_dev);
 
-int tcf_exts_egdev_cb_call(struct tcf_exts *exts, enum tc_setup_type type,
-			   void *type_data, bool err_stop)
+static int tc_exts_setup_cb_egdev_call(struct tcf_exts *exts,
+				       enum tc_setup_type type,
+				       void *type_data, bool err_stop)
 {
 	int ok_count = 0;
 #ifdef CONFIG_NET_CLS_ACT
@@ -1054,7 +1055,13 @@ int tcf_exts_egdev_cb_call(struct tcf_exts *exts, enum tc_setup_type type,
 #endif
 	return ok_count;
 }
-EXPORT_SYMBOL(tcf_exts_egdev_cb_call);
+
+int tc_setup_cb_call(struct tcf_exts *exts, enum tc_setup_type type,
+		     void *type_data, bool err_stop)
+{
+	return tc_exts_setup_cb_egdev_call(exts, type, type_data, err_stop);
+}
+EXPORT_SYMBOL(tc_setup_cb_call);
 
 static int __init tc_filter_init(void)
 {
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index db831ac..5b7bb96 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -88,7 +88,6 @@ struct cls_fl_filter {
 	u32 handle;
 	u32 flags;
 	struct rcu_head	rcu;
-	struct net_device *hw_dev;
 };
 
 static unsigned short int fl_mask_range(const struct fl_flow_mask *mask)
@@ -201,16 +200,17 @@ static void fl_destroy_filter(struct rcu_head *head)
 static void fl_hw_destroy_filter(struct tcf_proto *tp, struct cls_fl_filter *f)
 {
 	struct tc_cls_flower_offload cls_flower = {};
-	struct net_device *dev = f->hw_dev;
-
-	if (!tc_can_offload(dev))
-		return;
+	struct net_device *dev = tp->q->dev_queue->dev;
 
 	tc_cls_common_offload_init(&cls_flower.common, tp);
 	cls_flower.command = TC_CLSFLOWER_DESTROY;
 	cls_flower.cookie = (unsigned long) f;
 
-	dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_CLSFLOWER, &cls_flower);
+	if (tc_can_offload(dev))
+		dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_CLSFLOWER,
+					      &cls_flower);
+	tc_setup_cb_call(&f->exts, TC_SETUP_CLSFLOWER,
+			 &cls_flower, false);
 }
 
 static int fl_hw_replace_filter(struct tcf_proto *tp,
@@ -220,20 +220,9 @@ static int fl_hw_replace_filter(struct tcf_proto *tp,
 {
 	struct net_device *dev = tp->q->dev_queue->dev;
 	struct tc_cls_flower_offload cls_flower = {};
+	bool skip_sw = tc_skip_sw(f->flags);
 	int err;
 
-	if (!tc_can_offload(dev)) {
-		if (tcf_exts_get_dev(dev, &f->exts, &f->hw_dev) ||
-		    (f->hw_dev && !tc_can_offload(f->hw_dev))) {
-			f->hw_dev = dev;
-			return tc_skip_sw(f->flags) ? -EINVAL : 0;
-		}
-		dev = f->hw_dev;
-		cls_flower.egress_dev = true;
-	} else {
-		f->hw_dev = dev;
-	}
-
 	tc_cls_common_offload_init(&cls_flower.common, tp);
 	cls_flower.command = TC_CLSFLOWER_REPLACE;
 	cls_flower.cookie = (unsigned long) f;
@@ -242,31 +231,47 @@ static int fl_hw_replace_filter(struct tcf_proto *tp,
 	cls_flower.key = &f->mkey;
 	cls_flower.exts = &f->exts;
 
-	err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_CLSFLOWER,
-					    &cls_flower);
-	if (!err)
-		f->flags |= TCA_CLS_FLAGS_IN_HW;
+	if (tc_can_offload(dev)) {
+		err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_CLSFLOWER,
+						    &cls_flower);
+		if (err) {
+			if (skip_sw)
+				return err;
+		} else {
+			f->flags |= TCA_CLS_FLAGS_IN_HW;
+		}
+	}
 
-	if (tc_skip_sw(f->flags))
+	err = tc_setup_cb_call(&f->exts, TC_SETUP_CLSFLOWER,
+			       &cls_flower, skip_sw);
+	if (err < 0) {
+		fl_hw_destroy_filter(tp, f);
 		return err;
+	} else if (err > 0) {
+		f->flags |= TCA_CLS_FLAGS_IN_HW;
+	}
+
+	if (skip_sw && !(f->flags & TCA_CLS_FLAGS_IN_HW))
+		return -EINVAL;
+
 	return 0;
 }
 
 static void fl_hw_update_stats(struct tcf_proto *tp, struct cls_fl_filter *f)
 {
 	struct tc_cls_flower_offload cls_flower = {};
-	struct net_device *dev = f->hw_dev;
-
-	if (!tc_can_offload(dev))
-		return;
+	struct net_device *dev = tp->q->dev_queue->dev;
 
 	tc_cls_common_offload_init(&cls_flower.common, tp);
 	cls_flower.command = TC_CLSFLOWER_STATS;
 	cls_flower.cookie = (unsigned long) f;
 	cls_flower.exts = &f->exts;
 
-	dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_CLSFLOWER,
-				      &cls_flower);
+	if (tc_can_offload(dev))
+		dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_CLSFLOWER,
+					      &cls_flower);
+	tc_setup_cb_call(&f->exts, TC_SETUP_CLSFLOWER,
+			 &cls_flower, false);
 }
 
 static void __fl_delete(struct tcf_proto *tp, struct cls_fl_filter *f)
-- 
2.9.5

[patch net-next v2 4/4] net: sched: remove unused tcf_exts_get_dev helper and cls_flower->egress_dev

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

The helper and the struct field ares no longer used by any code,
so remove them.

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

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index c0bdf5c..f526374 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -204,8 +204,6 @@ void tcf_exts_destroy(struct tcf_exts *exts);
 void tcf_exts_change(struct tcf_exts *dst, struct tcf_exts *src);
 int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts *exts);
 int tcf_exts_dump_stats(struct sk_buff *skb, struct tcf_exts *exts);
-int tcf_exts_get_dev(struct net_device *dev, struct tcf_exts *exts,
-		     struct net_device **hw_dev);
 
 /**
  * struct tcf_pkt_info - packet information
@@ -517,7 +515,6 @@ struct tc_cls_flower_offload {
 	struct fl_flow_key *mask;
 	struct fl_flow_key *key;
 	struct tcf_exts *exts;
-	bool egress_dev;
 };
 
 enum tc_matchall_command {
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 51994a2..2977b8a 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1004,28 +1004,6 @@ int tcf_exts_dump_stats(struct sk_buff *skb, struct tcf_exts *exts)
 }
 EXPORT_SYMBOL(tcf_exts_dump_stats);
 
-int tcf_exts_get_dev(struct net_device *dev, struct tcf_exts *exts,
-		     struct net_device **hw_dev)
-{
-#ifdef CONFIG_NET_CLS_ACT
-	const struct tc_action *a;
-	LIST_HEAD(actions);
-
-	if (!tcf_exts_has_actions(exts))
-		return -EINVAL;
-
-	tcf_exts_to_list(exts, &actions);
-	list_for_each_entry(a, &actions, list) {
-		if (a->ops->get_dev)
-			*hw_dev = a->ops->get_dev(a);
-	}
-	if (*hw_dev)
-		return 0;
-#endif
-	return -EOPNOTSUPP;
-}
-EXPORT_SYMBOL(tcf_exts_get_dev);
-
 static int tc_exts_setup_cb_egdev_call(struct tcf_exts *exts,
 				       enum tc_setup_type type,
 				       void *type_data, bool err_stop)
-- 
2.9.5

Re: [patch net-next v2 0/4] net: sched: get rid of cls_flower->egress_dev

[David Miller]

From: Jiri Pirko <jiri@resnulli.us>
Date: Wed, 11 Oct 2017 09:41:06 +0200

> From: Jiri Pirko <jiri@mellanox.com>
> 
> Introduction of cls_flower->egress_dev was a workaround. Turned out
> to be a bit ugly hack. So replace it with more generic and reusable
> infrastructure.
> 
> This is a dependency of shared block introduction that will be send as
> a follow-up patchsets group.

Series applied, thanks.