[PATCH net-next 1/3] net: flow_offload: remove netns parameter from flow_block_cb_alloc()

[PATCH net-next 1/3] net: flow_offload: remove netns parameter from flow_block_cb_alloc()

[Pablo Neira Ayuso]

No need to annotate the netns on the flow block callback object,
flow_block_cb_is_busy() already checks for used blocks.

Fixes: d63db30c8537 ("net: flow_offload: add flow_block_cb_alloc() and flow_block_cb_free()")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_rep.c    | 3 +--
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c      | 5 ++---
 drivers/net/ethernet/mscc/ocelot_flower.c           | 3 +--
 drivers/net/ethernet/mscc/ocelot_tc.c               | 2 +-
 drivers/net/ethernet/netronome/nfp/flower/offload.c | 6 ++----
 include/net/flow_offload.h                          | 3 +--
 net/core/flow_offload.c                             | 9 +++------
 net/dsa/slave.c                                     | 2 +-
 8 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index 7245d287633d..2162412073c5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -735,8 +735,7 @@ mlx5e_rep_indr_setup_tc_block(struct net_device *netdev,
 		list_add(&indr_priv->list,
 			 &rpriv->uplink_priv.tc_indr_block_priv_list);
 
-		block_cb = flow_block_cb_alloc(f->net,
-					       mlx5e_rep_indr_setup_block_cb,
+		block_cb = flow_block_cb_alloc(mlx5e_rep_indr_setup_block_cb,
 					       indr_priv, indr_priv,
 					       mlx5e_rep_indr_tc_block_unbind);
 		if (IS_ERR(block_cb)) {
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 4d34d42b3b0e..a469035400cf 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -1610,8 +1610,7 @@ mlxsw_sp_setup_tc_block_flower_bind(struct mlxsw_sp_port *mlxsw_sp_port,
 		acl_block = mlxsw_sp_acl_block_create(mlxsw_sp, f->net);
 		if (!acl_block)
 			return -ENOMEM;
-		block_cb = flow_block_cb_alloc(f->net,
-					       mlxsw_sp_setup_tc_block_cb_flower,
+		block_cb = flow_block_cb_alloc(mlxsw_sp_setup_tc_block_cb_flower,
 					       mlxsw_sp, acl_block,
 					       mlxsw_sp_tc_block_flower_release);
 		if (IS_ERR(block_cb)) {
@@ -1702,7 +1701,7 @@ static int mlxsw_sp_setup_tc_block(struct mlxsw_sp_port *mlxsw_sp_port,
 					  &mlxsw_sp_block_cb_list))
 			return -EBUSY;
 
-		block_cb = flow_block_cb_alloc(f->net, cb, mlxsw_sp_port,
+		block_cb = flow_block_cb_alloc(cb, mlxsw_sp_port,
 					       mlxsw_sp_port, NULL);
 		if (IS_ERR(block_cb))
 			return PTR_ERR(block_cb);
diff --git a/drivers/net/ethernet/mscc/ocelot_flower.c b/drivers/net/ethernet/mscc/ocelot_flower.c
index 7aaddc09c185..6a11aea8b186 100644
--- a/drivers/net/ethernet/mscc/ocelot_flower.c
+++ b/drivers/net/ethernet/mscc/ocelot_flower.c
@@ -323,8 +323,7 @@ int ocelot_setup_tc_block_flower_bind(struct ocelot_port *port,
 		if (!port_block)
 			return -ENOMEM;
 
-		block_cb = flow_block_cb_alloc(f->net,
-					       ocelot_setup_tc_block_cb_flower,
+		block_cb = flow_block_cb_alloc(ocelot_setup_tc_block_cb_flower,
 					       port, port_block,
 					       ocelot_tc_block_unbind);
 		if (IS_ERR(block_cb)) {
diff --git a/drivers/net/ethernet/mscc/ocelot_tc.c b/drivers/net/ethernet/mscc/ocelot_tc.c
index 9e6464ffae5d..abbcb66bf5ac 100644
--- a/drivers/net/ethernet/mscc/ocelot_tc.c
+++ b/drivers/net/ethernet/mscc/ocelot_tc.c
@@ -156,7 +156,7 @@ static int ocelot_setup_tc_block(struct ocelot_port *port,
 		if (flow_block_cb_is_busy(cb, port, &ocelot_block_cb_list))
 			return -EBUSY;
 
-		block_cb = flow_block_cb_alloc(f->net, cb, port, port, NULL);
+		block_cb = flow_block_cb_alloc(cb, port, port, NULL);
 		if (IS_ERR(block_cb))
 			return PTR_ERR(block_cb);
 
diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 7e725fa60347..a0f8892bb4b5 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -1324,8 +1324,7 @@ static int nfp_flower_setup_tc_block(struct net_device *netdev,
 					  &nfp_block_cb_list))
 			return -EBUSY;
 
-		block_cb = flow_block_cb_alloc(f->net,
-					       nfp_flower_setup_tc_block_cb,
+		block_cb = flow_block_cb_alloc(nfp_flower_setup_tc_block_cb,
 					       repr, repr, NULL);
 		if (IS_ERR(block_cb))
 			return PTR_ERR(block_cb);
@@ -1430,8 +1429,7 @@ nfp_flower_setup_indr_tc_block(struct net_device *netdev, struct nfp_app *app,
 		cb_priv->app = app;
 		list_add(&cb_priv->list, &priv->indr_block_cb_priv);
 
-		block_cb = flow_block_cb_alloc(f->net,
-					       nfp_flower_setup_indr_block_cb,
+		block_cb = flow_block_cb_alloc(nfp_flower_setup_indr_block_cb,
 					       cb_priv, cb_priv,
 					       nfp_flower_setup_indr_tc_release);
 		if (IS_ERR(block_cb)) {
diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index db337299e81e..aa9b5287b231 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -264,7 +264,6 @@ struct flow_block_offload {
 struct flow_block_cb {
 	struct list_head	driver_list;
 	struct list_head	list;
-	struct net		*net;
 	tc_setup_cb_t		*cb;
 	void			*cb_ident;
 	void			*cb_priv;
@@ -272,7 +271,7 @@ struct flow_block_cb {
 	unsigned int		refcnt;
 };
 
-struct flow_block_cb *flow_block_cb_alloc(struct net *net, tc_setup_cb_t *cb,
+struct flow_block_cb *flow_block_cb_alloc(tc_setup_cb_t *cb,
 					  void *cb_ident, void *cb_priv,
 					  void (*release)(void *cb_priv));
 void flow_block_cb_free(struct flow_block_cb *block_cb);
diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c
index 76f8db3841d7..507de4b48815 100644
--- a/net/core/flow_offload.c
+++ b/net/core/flow_offload.c
@@ -165,7 +165,7 @@ void flow_rule_match_enc_opts(const struct flow_rule *rule,
 }
 EXPORT_SYMBOL(flow_rule_match_enc_opts);
 
-struct flow_block_cb *flow_block_cb_alloc(struct net *net, tc_setup_cb_t *cb,
+struct flow_block_cb *flow_block_cb_alloc(tc_setup_cb_t *cb,
 					  void *cb_ident, void *cb_priv,
 					  void (*release)(void *cb_priv))
 {
@@ -175,7 +175,6 @@ struct flow_block_cb *flow_block_cb_alloc(struct net *net, tc_setup_cb_t *cb,
 	if (!block_cb)
 		return ERR_PTR(-ENOMEM);
 
-	block_cb->net = net;
 	block_cb->cb = cb;
 	block_cb->cb_ident = cb_ident;
 	block_cb->cb_priv = cb_priv;
@@ -200,8 +199,7 @@ struct flow_block_cb *flow_block_cb_lookup(struct flow_block_offload *f,
 	struct flow_block_cb *block_cb;
 
 	list_for_each_entry(block_cb, f->driver_block_list, driver_list) {
-		if (block_cb->net == f->net &&
-		    block_cb->cb == cb &&
+		if (block_cb->cb == cb &&
 		    block_cb->cb_ident == cb_ident)
 			return block_cb;
 	}
@@ -261,8 +259,7 @@ int flow_block_cb_setup_simple(struct flow_block_offload *f,
 		if (flow_block_cb_is_busy(cb, cb_ident, driver_block_list))
 			return -EBUSY;
 
-		block_cb = flow_block_cb_alloc(f->net, cb, cb_ident,
-					       cb_priv, NULL);
+		block_cb = flow_block_cb_alloc(cb, cb_ident, cb_priv, NULL);
 		if (IS_ERR(block_cb))
 			return PTR_ERR(block_cb);
 
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 614c38ece104..6ca9ec58f881 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -967,7 +967,7 @@ static int dsa_slave_setup_tc_block(struct net_device *dev,
 		if (flow_block_cb_is_busy(cb, dev, &dsa_slave_block_cb_list))
 			return -EBUSY;
 
-		block_cb = flow_block_cb_alloc(f->net, cb, dev, dev, NULL);
+		block_cb = flow_block_cb_alloc(cb, dev, dev, NULL);
 		if (IS_ERR(block_cb))
 			return PTR_ERR(block_cb);
 
-- 
2.11.0

[PATCH net-next 2/3] net: flow_offload: rename tc_setup_cb_t to flow_setup_cb_t

[Pablo Neira Ayuso]

Rename this type definition and adapt users.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
This patch is a dependency for patch 3/3, so include/net/flow_offload.h
does not need to include include/net/sch_cls.h, and hence avoid a
circular inclusion.

 drivers/net/ethernet/mellanox/mlxsw/spectrum.c |  2 +-
 drivers/net/ethernet/mscc/ocelot_tc.c          |  2 +-
 include/net/flow_offload.h                     | 16 ++++++++++------
 include/net/pkt_cls.h                          |  4 ++--
 include/net/sch_generic.h                      |  6 ++----
 net/core/flow_offload.c                        |  9 +++++----
 net/dsa/slave.c                                |  2 +-
 net/sched/cls_api.c                            |  2 +-
 net/sched/cls_bpf.c                            |  2 +-
 net/sched/cls_flower.c                         |  2 +-
 net/sched/cls_matchall.c                       |  2 +-
 net/sched/cls_u32.c                            |  6 +++---
 12 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index a469035400cf..51cd0b6f1f3e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -1679,7 +1679,7 @@ static int mlxsw_sp_setup_tc_block(struct mlxsw_sp_port *mlxsw_sp_port,
 				   struct flow_block_offload *f)
 {
 	struct flow_block_cb *block_cb;
-	tc_setup_cb_t *cb;
+	flow_setup_cb_t *cb;
 	bool ingress;
 	int err;
 
diff --git a/drivers/net/ethernet/mscc/ocelot_tc.c b/drivers/net/ethernet/mscc/ocelot_tc.c
index abbcb66bf5ac..fba9512e9ca6 100644
--- a/drivers/net/ethernet/mscc/ocelot_tc.c
+++ b/drivers/net/ethernet/mscc/ocelot_tc.c
@@ -134,7 +134,7 @@ static int ocelot_setup_tc_block(struct ocelot_port *port,
 				 struct flow_block_offload *f)
 {
 	struct flow_block_cb *block_cb;
-	tc_setup_cb_t *cb;
+	flow_setup_cb_t *cb;
 	int err;
 
 	netdev_dbg(port->dev, "tc_block command %d, binder_type %d\n",
diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index aa9b5287b231..98bf3af5c84d 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -3,7 +3,6 @@
 
 #include <linux/kernel.h>
 #include <net/flow_dissector.h>
-#include <net/sch_generic.h>
 
 struct flow_match {
 	struct flow_dissector	*dissector;
@@ -261,23 +260,27 @@ struct flow_block_offload {
 	struct netlink_ext_ack *extack;
 };
 
+enum tc_setup_type;
+typedef int flow_setup_cb_t(enum tc_setup_type type, void *type_data,
+			    void *cb_priv);
+
 struct flow_block_cb {
 	struct list_head	driver_list;
 	struct list_head	list;
-	tc_setup_cb_t		*cb;
+	flow_setup_cb_t		*cb;
 	void			*cb_ident;
 	void			*cb_priv;
 	void			(*release)(void *cb_priv);
 	unsigned int		refcnt;
 };
 
-struct flow_block_cb *flow_block_cb_alloc(tc_setup_cb_t *cb,
+struct flow_block_cb *flow_block_cb_alloc(flow_setup_cb_t *cb,
 					  void *cb_ident, void *cb_priv,
 					  void (*release)(void *cb_priv));
 void flow_block_cb_free(struct flow_block_cb *block_cb);
 
 struct flow_block_cb *flow_block_cb_lookup(struct flow_block_offload *offload,
-					   tc_setup_cb_t *cb, void *cb_ident);
+					   flow_setup_cb_t *cb, void *cb_ident);
 
 void *flow_block_cb_priv(struct flow_block_cb *block_cb);
 void flow_block_cb_incref(struct flow_block_cb *block_cb);
@@ -295,11 +298,12 @@ static inline void flow_block_cb_remove(struct flow_block_cb *block_cb,
 	list_move(&block_cb->list, &offload->cb_list);
 }
 
-bool flow_block_cb_is_busy(tc_setup_cb_t *cb, void *cb_ident,
+bool flow_block_cb_is_busy(flow_setup_cb_t *cb, void *cb_ident,
 			   struct list_head *driver_block_list);
 
 int flow_block_cb_setup_simple(struct flow_block_offload *f,
-			       struct list_head *driver_list, tc_setup_cb_t *cb,
+			       struct list_head *driver_list,
+			       flow_setup_cb_t *cb,
 			       void *cb_ident, void *cb_priv, bool ingress_only);
 
 enum flow_cls_command {
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 841faadceb6e..cee651b76a1f 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -126,14 +126,14 @@ static inline struct Qdisc *tcf_block_q(struct tcf_block *block)
 }
 
 static inline
-int tc_setup_cb_block_register(struct tcf_block *block, tc_setup_cb_t *cb,
+int tc_setup_cb_block_register(struct tcf_block *block, flow_setup_cb_t *cb,
 			       void *cb_priv)
 {
 	return 0;
 }
 
 static inline
-void tc_setup_cb_block_unregister(struct tcf_block *block, tc_setup_cb_t *cb,
+void tc_setup_cb_block_unregister(struct tcf_block *block, flow_setup_cb_t *cb,
 				  void *cb_priv)
 {
 }
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 855167bbc372..9482e060483b 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -15,6 +15,7 @@
 #include <linux/mutex.h>
 #include <net/gen_stats.h>
 #include <net/rtnetlink.h>
+#include <net/flow_offload.h>
 
 struct Qdisc_ops;
 struct qdisc_walker;
@@ -22,9 +23,6 @@ struct tcf_walker;
 struct module;
 struct bpf_flow_keys;
 
-typedef int tc_setup_cb_t(enum tc_setup_type type,
-			  void *type_data, void *cb_priv);
-
 typedef int tc_indr_block_bind_cb_t(struct net_device *dev, void *cb_priv,
 				    enum tc_setup_type type, void *type_data);
 
@@ -313,7 +311,7 @@ struct tcf_proto_ops {
 	void			(*walk)(struct tcf_proto *tp,
 					struct tcf_walker *arg, bool rtnl_held);
 	int			(*reoffload)(struct tcf_proto *tp, bool add,
-					     tc_setup_cb_t *cb, void *cb_priv,
+					     flow_setup_cb_t *cb, void *cb_priv,
 					     struct netlink_ext_ack *extack);
 	void			(*bind_class)(void *, u32, unsigned long);
 	void *			(*tmplt_create)(struct net *net,
diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c
index 507de4b48815..a800fa78d96c 100644
--- a/net/core/flow_offload.c
+++ b/net/core/flow_offload.c
@@ -165,7 +165,7 @@ void flow_rule_match_enc_opts(const struct flow_rule *rule,
 }
 EXPORT_SYMBOL(flow_rule_match_enc_opts);
 
-struct flow_block_cb *flow_block_cb_alloc(tc_setup_cb_t *cb,
+struct flow_block_cb *flow_block_cb_alloc(flow_setup_cb_t *cb,
 					  void *cb_ident, void *cb_priv,
 					  void (*release)(void *cb_priv))
 {
@@ -194,7 +194,7 @@ void flow_block_cb_free(struct flow_block_cb *block_cb)
 EXPORT_SYMBOL(flow_block_cb_free);
 
 struct flow_block_cb *flow_block_cb_lookup(struct flow_block_offload *f,
-					   tc_setup_cb_t *cb, void *cb_ident)
+					   flow_setup_cb_t *cb, void *cb_ident)
 {
 	struct flow_block_cb *block_cb;
 
@@ -226,7 +226,7 @@ unsigned int flow_block_cb_decref(struct flow_block_cb *block_cb)
 }
 EXPORT_SYMBOL(flow_block_cb_decref);
 
-bool flow_block_cb_is_busy(tc_setup_cb_t *cb, void *cb_ident,
+bool flow_block_cb_is_busy(flow_setup_cb_t *cb, void *cb_ident,
 			   struct list_head *driver_block_list)
 {
 	struct flow_block_cb *block_cb;
@@ -243,7 +243,8 @@ EXPORT_SYMBOL(flow_block_cb_is_busy);
 
 int flow_block_cb_setup_simple(struct flow_block_offload *f,
 			       struct list_head *driver_block_list,
-			       tc_setup_cb_t *cb, void *cb_ident, void *cb_priv,
+			       flow_setup_cb_t *cb,
+			       void *cb_ident, void *cb_priv,
 			       bool ingress_only)
 {
 	struct flow_block_cb *block_cb;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 6ca9ec58f881..d697a64fb564 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -951,7 +951,7 @@ static int dsa_slave_setup_tc_block(struct net_device *dev,
 				    struct flow_block_offload *f)
 {
 	struct flow_block_cb *block_cb;
-	tc_setup_cb_t *cb;
+	flow_setup_cb_t *cb;
 
 	if (f->binder_type == FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS)
 		cb = dsa_slave_setup_tc_block_cb_ig;
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 278014e26aec..51fbe6e95a92 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1514,7 +1514,7 @@ void tcf_block_put(struct tcf_block *block)
 EXPORT_SYMBOL(tcf_block_put);
 
 static int
-tcf_block_playback_offloads(struct tcf_block *block, tc_setup_cb_t *cb,
+tcf_block_playback_offloads(struct tcf_block *block, flow_setup_cb_t *cb,
 			    void *cb_priv, bool add, bool offload_in_use,
 			    struct netlink_ext_ack *extack)
 {
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 691f71830134..3f7a9c02b70c 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -651,7 +651,7 @@ static void cls_bpf_walk(struct tcf_proto *tp, struct tcf_walker *arg,
 	}
 }
 
-static int cls_bpf_reoffload(struct tcf_proto *tp, bool add, tc_setup_cb_t *cb,
+static int cls_bpf_reoffload(struct tcf_proto *tp, bool add, flow_setup_cb_t *cb,
 			     void *cb_priv, struct netlink_ext_ack *extack)
 {
 	struct cls_bpf_head *head = rtnl_dereference(tp->root);
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 38d6e85693fc..054123742e32 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1800,7 +1800,7 @@ fl_get_next_hw_filter(struct tcf_proto *tp, struct cls_fl_filter *f, bool add)
 	return NULL;
 }
 
-static int fl_reoffload(struct tcf_proto *tp, bool add, tc_setup_cb_t *cb,
+static int fl_reoffload(struct tcf_proto *tp, bool add, flow_setup_cb_t *cb,
 			void *cb_priv, struct netlink_ext_ack *extack)
 {
 	struct tcf_block *block = tp->chain->block;
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index a30d2f8feb32..455ea2793f9b 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -282,7 +282,7 @@ static void mall_walk(struct tcf_proto *tp, struct tcf_walker *arg,
 	arg->count++;
 }
 
-static int mall_reoffload(struct tcf_proto *tp, bool add, tc_setup_cb_t *cb,
+static int mall_reoffload(struct tcf_proto *tp, bool add, flow_setup_cb_t *cb,
 			  void *cb_priv, struct netlink_ext_ack *extack)
 {
 	struct cls_mall_head *head = rtnl_dereference(tp->root);
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index be9e46c77e8b..8614088edd1b 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -1152,7 +1152,7 @@ static void u32_walk(struct tcf_proto *tp, struct tcf_walker *arg,
 }
 
 static int u32_reoffload_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht,
-			       bool add, tc_setup_cb_t *cb, void *cb_priv,
+			       bool add, flow_setup_cb_t *cb, void *cb_priv,
 			       struct netlink_ext_ack *extack)
 {
 	struct tc_cls_u32_offload cls_u32 = {};
@@ -1172,7 +1172,7 @@ static int u32_reoffload_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht,
 }
 
 static int u32_reoffload_knode(struct tcf_proto *tp, struct tc_u_knode *n,
-			       bool add, tc_setup_cb_t *cb, void *cb_priv,
+			       bool add, flow_setup_cb_t *cb, void *cb_priv,
 			       struct netlink_ext_ack *extack)
 {
 	struct tc_u_hnode *ht = rtnl_dereference(n->ht_down);
@@ -1213,7 +1213,7 @@ static int u32_reoffload_knode(struct tcf_proto *tp, struct tc_u_knode *n,
 	return 0;
 }
 
-static int u32_reoffload(struct tcf_proto *tp, bool add, tc_setup_cb_t *cb,
+static int u32_reoffload(struct tcf_proto *tp, bool add, flow_setup_cb_t *cb,
 			 void *cb_priv, struct netlink_ext_ack *extack)
 {
 	struct tc_u_common *tp_c = tp->data;
-- 
2.11.0

[PATCH net-next 3/3] net: flow_offload: add flow_block structure and use it

[Pablo Neira Ayuso]

This object stores the flow block callbacks that are attached to this
block. This patch restores block sharing.

Fixes: da3eeb904ff4 ("net: flow_offload: add list handling functions")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/flow_offload.h        |  5 +++++
 include/net/netfilter/nf_tables.h |  5 +++--
 include/net/sch_generic.h         |  2 +-
 net/core/flow_offload.c           |  2 +-
 net/netfilter/nf_tables_api.c     |  2 +-
 net/netfilter/nf_tables_offload.c |  5 +++--
 net/sched/cls_api.c               | 10 +++++++---
 7 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index 98bf3af5c84d..e50d94736829 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -248,6 +248,10 @@ enum flow_block_binder_type {
 	FLOW_BLOCK_BINDER_TYPE_CLSACT_EGRESS,
 };
 
+struct flow_block {
+	struct list_head cb_list;
+};
+
 struct netlink_ext_ack;
 
 struct flow_block_offload {
@@ -255,6 +259,7 @@ struct flow_block_offload {
 	enum flow_block_binder_type binder_type;
 	bool block_shared;
 	struct net *net;
+	struct flow_block *block;
 	struct list_head cb_list;
 	struct list_head *driver_block_list;
 	struct netlink_ext_ack *extack;
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 35dfdd9f69b3..00658462f89b 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -11,6 +11,7 @@
 #include <linux/rhashtable.h>
 #include <net/netfilter/nf_flow_table.h>
 #include <net/netlink.h>
+#include <net/flow_offload.h>
 
 struct module;
 
@@ -951,7 +952,7 @@ struct nft_stats {
  *	@stats: per-cpu chain stats
  *	@chain: the chain
  *	@dev_name: device name that this base chain is attached to (if any)
- *	@cb_list: list of flow block callbacks (for hardware offload)
+ *	@block: flow block (for hardware offload)
  */
 struct nft_base_chain {
 	struct nf_hook_ops		ops;
@@ -961,7 +962,7 @@ struct nft_base_chain {
 	struct nft_stats __percpu	*stats;
 	struct nft_chain		chain;
 	char 				dev_name[IFNAMSIZ];
-	struct list_head		cb_list;
+	struct flow_block		block;
 };
 
 static inline struct nft_base_chain *nft_base_chain(const struct nft_chain *chain)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 9482e060483b..58041cb0ce15 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -399,7 +399,7 @@ struct tcf_block {
 	refcount_t refcnt;
 	struct net *net;
 	struct Qdisc *q;
-	struct list_head cb_list;
+	struct flow_block flow;
 	struct list_head owner_list;
 	bool keep_dst;
 	unsigned int offloadcnt; /* Number of oddloaded filters */
diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c
index a800fa78d96c..935c7f81a9ef 100644
--- a/net/core/flow_offload.c
+++ b/net/core/flow_offload.c
@@ -198,7 +198,7 @@ struct flow_block_cb *flow_block_cb_lookup(struct flow_block_offload *f,
 {
 	struct flow_block_cb *block_cb;
 
-	list_for_each_entry(block_cb, f->driver_block_list, driver_list) {
+	list_for_each_entry(block_cb, &f->block->cb_list, list) {
 		if (block_cb->cb == cb &&
 		    block_cb->cb_ident == cb_ident)
 			return block_cb;
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index ed17a7c29b86..c565f146435b 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1662,7 +1662,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
 
 		chain->flags |= NFT_BASE_CHAIN | flags;
 		basechain->policy = NF_ACCEPT;
-		INIT_LIST_HEAD(&basechain->cb_list);
+		INIT_LIST_HEAD(&basechain->block.cb_list);
 	} else {
 		chain = kzalloc(sizeof(*chain), GFP_KERNEL);
 		if (chain == NULL)
diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
index 2c3302845f67..2a184277ee58 100644
--- a/net/netfilter/nf_tables_offload.c
+++ b/net/netfilter/nf_tables_offload.c
@@ -116,7 +116,7 @@ static int nft_setup_cb_call(struct nft_base_chain *basechain,
 	struct flow_block_cb *block_cb;
 	int err;
 
-	list_for_each_entry(block_cb, &basechain->cb_list, list) {
+	list_for_each_entry(block_cb, &basechain->block.cb_list, list) {
 		err = block_cb->cb(type, type_data, block_cb->cb_priv);
 		if (err < 0)
 			return err;
@@ -154,7 +154,7 @@ static int nft_flow_offload_rule(struct nft_trans *trans,
 static int nft_flow_offload_bind(struct flow_block_offload *bo,
 				 struct nft_base_chain *basechain)
 {
-	list_splice(&bo->cb_list, &basechain->cb_list);
+	list_splice(&bo->cb_list, &basechain->block.cb_list);
 	return 0;
 }
 
@@ -198,6 +198,7 @@ static int nft_flow_offload_chain(struct nft_trans *trans,
 		return -EOPNOTSUPP;
 
 	bo.command = cmd;
+	bo.block = &basechain->block;
 	bo.binder_type = FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS;
 	bo.extack = &extack;
 	INIT_LIST_HEAD(&bo.cb_list);
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 51fbe6e95a92..66181961ad6f 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -691,6 +691,8 @@ static void tc_indr_block_ing_cmd(struct tc_indr_block_dev *indr_dev,
 	if (!indr_dev->block)
 		return;
 
+	bo.block = &indr_dev->block->flow;
+
 	indr_block_cb->cb(indr_dev->dev, indr_block_cb->cb_priv, TC_SETUP_BLOCK,
 			  &bo);
 	tcf_block_setup(indr_dev->block, &bo);
@@ -775,6 +777,7 @@ static void tc_indr_block_call(struct tcf_block *block, struct net_device *dev,
 		.command	= command,
 		.binder_type	= ei->binder_type,
 		.net		= dev_net(dev),
+		.block		= &block->flow,
 		.block_shared	= tcf_block_shared(block),
 		.extack		= extack,
 	};
@@ -810,6 +813,7 @@ static int tcf_block_offload_cmd(struct tcf_block *block,
 	bo.net = dev_net(dev);
 	bo.command = command;
 	bo.binder_type = ei->binder_type;
+	bo.block = &block->flow;
 	bo.block_shared = tcf_block_shared(block);
 	bo.extack = extack;
 	INIT_LIST_HEAD(&bo.cb_list);
@@ -988,7 +992,7 @@ static struct tcf_block *tcf_block_create(struct net *net, struct Qdisc *q,
 	}
 	mutex_init(&block->lock);
 	INIT_LIST_HEAD(&block->chain_list);
-	INIT_LIST_HEAD(&block->cb_list);
+	INIT_LIST_HEAD(&block->flow.cb_list);
 	INIT_LIST_HEAD(&block->owner_list);
 	INIT_LIST_HEAD(&block->chain0.filter_chain_list);
 
@@ -1570,7 +1574,7 @@ static int tcf_block_bind(struct tcf_block *block,
 
 		i++;
 	}
-	list_splice(&bo->cb_list, &block->cb_list);
+	list_splice(&bo->cb_list, &block->flow.cb_list);
 
 	return 0;
 
@@ -3155,7 +3159,7 @@ int tc_setup_cb_call(struct tcf_block *block, enum tc_setup_type type,
 	if (block->nooffloaddevcnt && err_stop)
 		return -EOPNOTSUPP;
 
-	list_for_each_entry(block_cb, &block->cb_list, list) {
+	list_for_each_entry(block_cb, &block->flow.cb_list, list) {
 		err = block_cb->cb(type, type_data, block_cb->cb_priv);
 		if (err) {
 			if (err_stop)
-- 
2.11.0

Re: [PATCH net-next 3/3] net: flow_offload: add flow_block structure and use it

[Jiri Pirko]

Thu, Jul 11, 2019 at 02:12:35AM CEST, pablo@netfilter.org wrote:
>This object stores the flow block callbacks that are attached to this
>block. This patch restores block sharing.
>
>Fixes: da3eeb904ff4 ("net: flow_offload: add list handling functions")
>Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
>---
> include/net/flow_offload.h        |  5 +++++
> include/net/netfilter/nf_tables.h |  5 +++--
> include/net/sch_generic.h         |  2 +-
> net/core/flow_offload.c           |  2 +-
> net/netfilter/nf_tables_api.c     |  2 +-
> net/netfilter/nf_tables_offload.c |  5 +++--
> net/sched/cls_api.c               | 10 +++++++---
> 7 files changed, 21 insertions(+), 10 deletions(-)
>
>diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
>index 98bf3af5c84d..e50d94736829 100644
>--- a/include/net/flow_offload.h
>+++ b/include/net/flow_offload.h
>@@ -248,6 +248,10 @@ enum flow_block_binder_type {
> 	FLOW_BLOCK_BINDER_TYPE_CLSACT_EGRESS,
> };
> 
>+struct flow_block {
>+	struct list_head cb_list;
>+};
>+
> struct netlink_ext_ack;
> 
> struct flow_block_offload {
>@@ -255,6 +259,7 @@ struct flow_block_offload {
> 	enum flow_block_binder_type binder_type;
> 	bool block_shared;
> 	struct net *net;
>+	struct flow_block *block;
> 	struct list_head cb_list;
> 	struct list_head *driver_block_list;
> 	struct netlink_ext_ack *extack;
>diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
>index 35dfdd9f69b3..00658462f89b 100644
>--- a/include/net/netfilter/nf_tables.h
>+++ b/include/net/netfilter/nf_tables.h
>@@ -11,6 +11,7 @@
> #include <linux/rhashtable.h>
> #include <net/netfilter/nf_flow_table.h>
> #include <net/netlink.h>
>+#include <net/flow_offload.h>
> 
> struct module;
> 
>@@ -951,7 +952,7 @@ struct nft_stats {
>  *	@stats: per-cpu chain stats
>  *	@chain: the chain
>  *	@dev_name: device name that this base chain is attached to (if any)
>- *	@cb_list: list of flow block callbacks (for hardware offload)
>+ *	@block: flow block (for hardware offload)
>  */
> struct nft_base_chain {
> 	struct nf_hook_ops		ops;
>@@ -961,7 +962,7 @@ struct nft_base_chain {
> 	struct nft_stats __percpu	*stats;
> 	struct nft_chain		chain;
> 	char 				dev_name[IFNAMSIZ];
>-	struct list_head		cb_list;
>+	struct flow_block		block;
> };
> 
> static inline struct nft_base_chain *nft_base_chain(const struct nft_chain *chain)
>diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
>index 9482e060483b..58041cb0ce15 100644
>--- a/include/net/sch_generic.h
>+++ b/include/net/sch_generic.h
>@@ -399,7 +399,7 @@ struct tcf_block {
> 	refcount_t refcnt;
> 	struct net *net;
> 	struct Qdisc *q;
>-	struct list_head cb_list;
>+	struct flow_block flow;

It is not a "flow", that is confusing. It should be named "flow_block".


> 	struct list_head owner_list;
> 	bool keep_dst;
> 	unsigned int offloadcnt; /* Number of oddloaded filters */
>diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c
>index a800fa78d96c..935c7f81a9ef 100644
>--- a/net/core/flow_offload.c
>+++ b/net/core/flow_offload.c
>@@ -198,7 +198,7 @@ struct flow_block_cb *flow_block_cb_lookup(struct flow_block_offload *f,
> {
> 	struct flow_block_cb *block_cb;
> 
>-	list_for_each_entry(block_cb, f->driver_block_list, driver_list) {
>+	list_for_each_entry(block_cb, &f->block->cb_list, list) {

Please made struct flow_block *block and argument of cb_lookup instead
of struct flow_block_offload *f (as it was previously).


> 		if (block_cb->cb == cb &&
> 		    block_cb->cb_ident == cb_ident)
> 			return block_cb;
>diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
>index ed17a7c29b86..c565f146435b 100644
>--- a/net/netfilter/nf_tables_api.c
>+++ b/net/netfilter/nf_tables_api.c
>@@ -1662,7 +1662,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
> 
> 		chain->flags |= NFT_BASE_CHAIN | flags;
> 		basechain->policy = NF_ACCEPT;
>-		INIT_LIST_HEAD(&basechain->cb_list);
>+		INIT_LIST_HEAD(&basechain->block.cb_list);
> 	} else {
> 		chain = kzalloc(sizeof(*chain), GFP_KERNEL);
> 		if (chain == NULL)
>diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
>index 2c3302845f67..2a184277ee58 100644
>--- a/net/netfilter/nf_tables_offload.c
>+++ b/net/netfilter/nf_tables_offload.c
>@@ -116,7 +116,7 @@ static int nft_setup_cb_call(struct nft_base_chain *basechain,
> 	struct flow_block_cb *block_cb;
> 	int err;
> 
>-	list_for_each_entry(block_cb, &basechain->cb_list, list) {
>+	list_for_each_entry(block_cb, &basechain->block.cb_list, list) {
> 		err = block_cb->cb(type, type_data, block_cb->cb_priv);
> 		if (err < 0)
> 			return err;
>@@ -154,7 +154,7 @@ static int nft_flow_offload_rule(struct nft_trans *trans,
> static int nft_flow_offload_bind(struct flow_block_offload *bo,
> 				 struct nft_base_chain *basechain)
> {
>-	list_splice(&bo->cb_list, &basechain->cb_list);
>+	list_splice(&bo->cb_list, &basechain->block.cb_list);
> 	return 0;
> }
> 
>@@ -198,6 +198,7 @@ static int nft_flow_offload_chain(struct nft_trans *trans,
> 		return -EOPNOTSUPP;
> 
> 	bo.command = cmd;
>+	bo.block = &basechain->block;
> 	bo.binder_type = FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS;
> 	bo.extack = &extack;
> 	INIT_LIST_HEAD(&bo.cb_list);
>diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
>index 51fbe6e95a92..66181961ad6f 100644
>--- a/net/sched/cls_api.c
>+++ b/net/sched/cls_api.c
>@@ -691,6 +691,8 @@ static void tc_indr_block_ing_cmd(struct tc_indr_block_dev *indr_dev,
> 	if (!indr_dev->block)
> 		return;
> 
>+	bo.block = &indr_dev->block->flow;
>+
> 	indr_block_cb->cb(indr_dev->dev, indr_block_cb->cb_priv, TC_SETUP_BLOCK,
> 			  &bo);
> 	tcf_block_setup(indr_dev->block, &bo);
>@@ -775,6 +777,7 @@ static void tc_indr_block_call(struct tcf_block *block, struct net_device *dev,
> 		.command	= command,
> 		.binder_type	= ei->binder_type,
> 		.net		= dev_net(dev),
>+		.block		= &block->flow,
> 		.block_shared	= tcf_block_shared(block),
> 		.extack		= extack,
> 	};
>@@ -810,6 +813,7 @@ static int tcf_block_offload_cmd(struct tcf_block *block,
> 	bo.net = dev_net(dev);
> 	bo.command = command;
> 	bo.binder_type = ei->binder_type;
>+	bo.block = &block->flow;
> 	bo.block_shared = tcf_block_shared(block);
> 	bo.extack = extack;
> 	INIT_LIST_HEAD(&bo.cb_list);
>@@ -988,7 +992,7 @@ static struct tcf_block *tcf_block_create(struct net *net, struct Qdisc *q,
> 	}
> 	mutex_init(&block->lock);
> 	INIT_LIST_HEAD(&block->chain_list);
>-	INIT_LIST_HEAD(&block->cb_list);
>+	INIT_LIST_HEAD(&block->flow.cb_list);

With introduction of struct flow_block, please introduce also a helper
to init this struct. Does not look right to init it from user codes
(tc/nft).


> 	INIT_LIST_HEAD(&block->owner_list);
> 	INIT_LIST_HEAD(&block->chain0.filter_chain_list);
> 
>@@ -1570,7 +1574,7 @@ static int tcf_block_bind(struct tcf_block *block,
> 
> 		i++;
> 	}
>-	list_splice(&bo->cb_list, &block->cb_list);
>+	list_splice(&bo->cb_list, &block->flow.cb_list);
> 
> 	return 0;
> 
>@@ -3155,7 +3159,7 @@ int tc_setup_cb_call(struct tcf_block *block, enum tc_setup_type type,
> 	if (block->nooffloaddevcnt && err_stop)
> 		return -EOPNOTSUPP;
> 
>-	list_for_each_entry(block_cb, &block->cb_list, list) {
>+	list_for_each_entry(block_cb, &block->flow.cb_list, list) {
> 		err = block_cb->cb(type, type_data, block_cb->cb_priv);
> 		if (err) {
> 			if (err_stop)
>-- 
>2.11.0
>
>

Re: [PATCH net-next 2/3] net: flow_offload: rename tc_setup_cb_t to flow_setup_cb_t

[Jiri Pirko]

Thu, Jul 11, 2019 at 02:12:34AM CEST, pablo@netfilter.org wrote:
>Rename this type definition and adapt users.
>
>Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

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

Re: [PATCH net-next 1/3] net: flow_offload: remove netns parameter from flow_block_cb_alloc()

[Jiri Pirko]

Thu, Jul 11, 2019 at 02:12:33AM CEST, pablo@netfilter.org wrote:
>No need to annotate the netns on the flow block callback object,
>flow_block_cb_is_busy() already checks for used blocks.
>
>Fixes: d63db30c8537 ("net: flow_offload: add flow_block_cb_alloc() and flow_block_cb_free()")
>Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

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

Re: [PATCH net-next 3/3] net: flow_offload: add flow_block structure and use it

[Pablo Neira Ayuso]

On Thu, Jul 11, 2019 at 10:06:09AM +0200, Jiri Pirko wrote:
> Thu, Jul 11, 2019 at 02:12:35AM CEST, pablo@netfilter.org wrote:
> >This object stores the flow block callbacks that are attached to this
> >block. This patch restores block sharing.
> >
> >Fixes: da3eeb904ff4 ("net: flow_offload: add list handling functions")
> >Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> >---
> > include/net/flow_offload.h        |  5 +++++
> > include/net/netfilter/nf_tables.h |  5 +++--
> > include/net/sch_generic.h         |  2 +-
> > net/core/flow_offload.c           |  2 +-
> > net/netfilter/nf_tables_api.c     |  2 +-
> > net/netfilter/nf_tables_offload.c |  5 +++--
> > net/sched/cls_api.c               | 10 +++++++---
> > 7 files changed, 21 insertions(+), 10 deletions(-)
> >
> >diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
> >index 98bf3af5c84d..e50d94736829 100644
> >--- a/include/net/flow_offload.h
> >+++ b/include/net/flow_offload.h
> >@@ -248,6 +248,10 @@ enum flow_block_binder_type {
> > 	FLOW_BLOCK_BINDER_TYPE_CLSACT_EGRESS,
> > };
> > 
> >+struct flow_block {
> >+	struct list_head cb_list;
> >+};
> >+
> > struct netlink_ext_ack;
> > 
> > struct flow_block_offload {
> >@@ -255,6 +259,7 @@ struct flow_block_offload {
> > 	enum flow_block_binder_type binder_type;
> > 	bool block_shared;
> > 	struct net *net;
> >+	struct flow_block *block;
> > 	struct list_head cb_list;
> > 	struct list_head *driver_block_list;
> > 	struct netlink_ext_ack *extack;
> >diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
> >index 35dfdd9f69b3..00658462f89b 100644
> >--- a/include/net/netfilter/nf_tables.h
> >+++ b/include/net/netfilter/nf_tables.h
> >@@ -11,6 +11,7 @@
> > #include <linux/rhashtable.h>
> > #include <net/netfilter/nf_flow_table.h>
> > #include <net/netlink.h>
> >+#include <net/flow_offload.h>
> > 
> > struct module;
> > 
> >@@ -951,7 +952,7 @@ struct nft_stats {
> >  *	@stats: per-cpu chain stats
> >  *	@chain: the chain
> >  *	@dev_name: device name that this base chain is attached to (if any)
> >- *	@cb_list: list of flow block callbacks (for hardware offload)
> >+ *	@block: flow block (for hardware offload)
> >  */
> > struct nft_base_chain {
> > 	struct nf_hook_ops		ops;
> >@@ -961,7 +962,7 @@ struct nft_base_chain {
> > 	struct nft_stats __percpu	*stats;
> > 	struct nft_chain		chain;
> > 	char 				dev_name[IFNAMSIZ];
> >-	struct list_head		cb_list;
> >+	struct flow_block		block;
> > };
> > 
> > static inline struct nft_base_chain *nft_base_chain(const struct nft_chain *chain)
> >diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> >index 9482e060483b..58041cb0ce15 100644
> >--- a/include/net/sch_generic.h
> >+++ b/include/net/sch_generic.h
> >@@ -399,7 +399,7 @@ struct tcf_block {
> > 	refcount_t refcnt;
> > 	struct net *net;
> > 	struct Qdisc *q;
> >-	struct list_head cb_list;
> >+	struct flow_block flow;
> 
> It is not a "flow", that is confusing. It should be named "flow_block".

Done.

> > 	struct list_head owner_list;
> > 	bool keep_dst;
> > 	unsigned int offloadcnt; /* Number of oddloaded filters */
> >diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c
> >index a800fa78d96c..935c7f81a9ef 100644
> >--- a/net/core/flow_offload.c
> >+++ b/net/core/flow_offload.c
> >@@ -198,7 +198,7 @@ struct flow_block_cb *flow_block_cb_lookup(struct flow_block_offload *f,
> > {
> > 	struct flow_block_cb *block_cb;
> > 
> >-	list_for_each_entry(block_cb, f->driver_block_list, driver_list) {
> >+	list_for_each_entry(block_cb, &f->block->cb_list, list) {
> 
> Please made struct flow_block *block and argument of cb_lookup instead
> of struct flow_block_offload *f (as it was previously).

I can do so if you insist, this will make this fix larger.

> > 		if (block_cb->cb == cb &&
> > 		    block_cb->cb_ident == cb_ident)
> > 			return block_cb;
> >diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> >index ed17a7c29b86..c565f146435b 100644
> >--- a/net/netfilter/nf_tables_api.c
> >+++ b/net/netfilter/nf_tables_api.c
> >@@ -1662,7 +1662,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
> > 
> > 		chain->flags |= NFT_BASE_CHAIN | flags;
> > 		basechain->policy = NF_ACCEPT;
> >-		INIT_LIST_HEAD(&basechain->cb_list);
> >+		INIT_LIST_HEAD(&basechain->block.cb_list);
> > 	} else {
> > 		chain = kzalloc(sizeof(*chain), GFP_KERNEL);
> > 		if (chain == NULL)
> >diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
> >index 2c3302845f67..2a184277ee58 100644
> >--- a/net/netfilter/nf_tables_offload.c
> >+++ b/net/netfilter/nf_tables_offload.c
> >@@ -116,7 +116,7 @@ static int nft_setup_cb_call(struct nft_base_chain *basechain,
> > 	struct flow_block_cb *block_cb;
> > 	int err;
> > 
> >-	list_for_each_entry(block_cb, &basechain->cb_list, list) {
> >+	list_for_each_entry(block_cb, &basechain->block.cb_list, list) {
> > 		err = block_cb->cb(type, type_data, block_cb->cb_priv);
> > 		if (err < 0)
> > 			return err;
> >@@ -154,7 +154,7 @@ static int nft_flow_offload_rule(struct nft_trans *trans,
> > static int nft_flow_offload_bind(struct flow_block_offload *bo,
> > 				 struct nft_base_chain *basechain)
> > {
> >-	list_splice(&bo->cb_list, &basechain->cb_list);
> >+	list_splice(&bo->cb_list, &basechain->block.cb_list);
> > 	return 0;
> > }
> > 
> >@@ -198,6 +198,7 @@ static int nft_flow_offload_chain(struct nft_trans *trans,
> > 		return -EOPNOTSUPP;
> > 
> > 	bo.command = cmd;
> >+	bo.block = &basechain->block;
> > 	bo.binder_type = FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS;
> > 	bo.extack = &extack;
> > 	INIT_LIST_HEAD(&bo.cb_list);
> >diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> >index 51fbe6e95a92..66181961ad6f 100644
> >--- a/net/sched/cls_api.c
> >+++ b/net/sched/cls_api.c
> >@@ -691,6 +691,8 @@ static void tc_indr_block_ing_cmd(struct tc_indr_block_dev *indr_dev,
> > 	if (!indr_dev->block)
> > 		return;
> > 
> >+	bo.block = &indr_dev->block->flow;
> >+
> > 	indr_block_cb->cb(indr_dev->dev, indr_block_cb->cb_priv, TC_SETUP_BLOCK,
> > 			  &bo);
> > 	tcf_block_setup(indr_dev->block, &bo);
> >@@ -775,6 +777,7 @@ static void tc_indr_block_call(struct tcf_block *block, struct net_device *dev,
> > 		.command	= command,
> > 		.binder_type	= ei->binder_type,
> > 		.net		= dev_net(dev),
> >+		.block		= &block->flow,
> > 		.block_shared	= tcf_block_shared(block),
> > 		.extack		= extack,
> > 	};
> >@@ -810,6 +813,7 @@ static int tcf_block_offload_cmd(struct tcf_block *block,
> > 	bo.net = dev_net(dev);
> > 	bo.command = command;
> > 	bo.binder_type = ei->binder_type;
> >+	bo.block = &block->flow;
> > 	bo.block_shared = tcf_block_shared(block);
> > 	bo.extack = extack;
> > 	INIT_LIST_HEAD(&bo.cb_list);
> >@@ -988,7 +992,7 @@ static struct tcf_block *tcf_block_create(struct net *net, struct Qdisc *q,
> > 	}
> > 	mutex_init(&block->lock);
> > 	INIT_LIST_HEAD(&block->chain_list);
> >-	INIT_LIST_HEAD(&block->cb_list);
> >+	INIT_LIST_HEAD(&block->flow.cb_list);
> 
> With introduction of struct flow_block, please introduce also a helper
> to init this struct. Does not look right to init it from user codes
> (tc/nft).

Done.

Re: [PATCH net-next 3/3] net: flow_offload: add flow_block structure and use it

[Jiri Pirko]

Thu, Jul 11, 2019 at 03:13:00PM CEST, pablo@netfilter.org wrote:
>On Thu, Jul 11, 2019 at 10:06:09AM +0200, Jiri Pirko wrote:
>> Thu, Jul 11, 2019 at 02:12:35AM CEST, pablo@netfilter.org wrote:
>> >This object stores the flow block callbacks that are attached to this
>> >block. This patch restores block sharing.
>> >
>> >Fixes: da3eeb904ff4 ("net: flow_offload: add list handling functions")
>> >Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
>> >---
>> > include/net/flow_offload.h        |  5 +++++
>> > include/net/netfilter/nf_tables.h |  5 +++--
>> > include/net/sch_generic.h         |  2 +-
>> > net/core/flow_offload.c           |  2 +-
>> > net/netfilter/nf_tables_api.c     |  2 +-
>> > net/netfilter/nf_tables_offload.c |  5 +++--
>> > net/sched/cls_api.c               | 10 +++++++---
>> > 7 files changed, 21 insertions(+), 10 deletions(-)
>> >
>> >diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
>> >index 98bf3af5c84d..e50d94736829 100644
>> >--- a/include/net/flow_offload.h
>> >+++ b/include/net/flow_offload.h
>> >@@ -248,6 +248,10 @@ enum flow_block_binder_type {
>> > 	FLOW_BLOCK_BINDER_TYPE_CLSACT_EGRESS,
>> > };
>> > 
>> >+struct flow_block {
>> >+	struct list_head cb_list;
>> >+};
>> >+
>> > struct netlink_ext_ack;
>> > 
>> > struct flow_block_offload {
>> >@@ -255,6 +259,7 @@ struct flow_block_offload {
>> > 	enum flow_block_binder_type binder_type;
>> > 	bool block_shared;
>> > 	struct net *net;
>> >+	struct flow_block *block;
>> > 	struct list_head cb_list;
>> > 	struct list_head *driver_block_list;
>> > 	struct netlink_ext_ack *extack;
>> >diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
>> >index 35dfdd9f69b3..00658462f89b 100644
>> >--- a/include/net/netfilter/nf_tables.h
>> >+++ b/include/net/netfilter/nf_tables.h
>> >@@ -11,6 +11,7 @@
>> > #include <linux/rhashtable.h>
>> > #include <net/netfilter/nf_flow_table.h>
>> > #include <net/netlink.h>
>> >+#include <net/flow_offload.h>
>> > 
>> > struct module;
>> > 
>> >@@ -951,7 +952,7 @@ struct nft_stats {
>> >  *	@stats: per-cpu chain stats
>> >  *	@chain: the chain
>> >  *	@dev_name: device name that this base chain is attached to (if any)
>> >- *	@cb_list: list of flow block callbacks (for hardware offload)
>> >+ *	@block: flow block (for hardware offload)
>> >  */
>> > struct nft_base_chain {
>> > 	struct nf_hook_ops		ops;
>> >@@ -961,7 +962,7 @@ struct nft_base_chain {
>> > 	struct nft_stats __percpu	*stats;
>> > 	struct nft_chain		chain;
>> > 	char 				dev_name[IFNAMSIZ];
>> >-	struct list_head		cb_list;
>> >+	struct flow_block		block;
>> > };
>> > 
>> > static inline struct nft_base_chain *nft_base_chain(const struct nft_chain *chain)
>> >diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
>> >index 9482e060483b..58041cb0ce15 100644
>> >--- a/include/net/sch_generic.h
>> >+++ b/include/net/sch_generic.h
>> >@@ -399,7 +399,7 @@ struct tcf_block {
>> > 	refcount_t refcnt;
>> > 	struct net *net;
>> > 	struct Qdisc *q;
>> >-	struct list_head cb_list;
>> >+	struct flow_block flow;
>> 
>> It is not a "flow", that is confusing. It should be named "flow_block".
>
>Done.
>
>> > 	struct list_head owner_list;
>> > 	bool keep_dst;
>> > 	unsigned int offloadcnt; /* Number of oddloaded filters */
>> >diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c
>> >index a800fa78d96c..935c7f81a9ef 100644
>> >--- a/net/core/flow_offload.c
>> >+++ b/net/core/flow_offload.c
>> >@@ -198,7 +198,7 @@ struct flow_block_cb *flow_block_cb_lookup(struct flow_block_offload *f,
>> > {
>> > 	struct flow_block_cb *block_cb;
>> > 
>> >-	list_for_each_entry(block_cb, f->driver_block_list, driver_list) {
>> >+	list_for_each_entry(block_cb, &f->block->cb_list, list) {
>> 
>> Please made struct flow_block *block and argument of cb_lookup instead
>> of struct flow_block_offload *f (as it was previously).
>
>I can do so if you insist, this will make this fix larger.

Yes please. Thanks!


>
>> > 		if (block_cb->cb == cb &&
>> > 		    block_cb->cb_ident == cb_ident)
>> > 			return block_cb;
>> >diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
>> >index ed17a7c29b86..c565f146435b 100644
>> >--- a/net/netfilter/nf_tables_api.c
>> >+++ b/net/netfilter/nf_tables_api.c
>> >@@ -1662,7 +1662,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
>> > 
>> > 		chain->flags |= NFT_BASE_CHAIN | flags;
>> > 		basechain->policy = NF_ACCEPT;
>> >-		INIT_LIST_HEAD(&basechain->cb_list);
>> >+		INIT_LIST_HEAD(&basechain->block.cb_list);
>> > 	} else {
>> > 		chain = kzalloc(sizeof(*chain), GFP_KERNEL);
>> > 		if (chain == NULL)
>> >diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
>> >index 2c3302845f67..2a184277ee58 100644
>> >--- a/net/netfilter/nf_tables_offload.c
>> >+++ b/net/netfilter/nf_tables_offload.c
>> >@@ -116,7 +116,7 @@ static int nft_setup_cb_call(struct nft_base_chain *basechain,
>> > 	struct flow_block_cb *block_cb;
>> > 	int err;
>> > 
>> >-	list_for_each_entry(block_cb, &basechain->cb_list, list) {
>> >+	list_for_each_entry(block_cb, &basechain->block.cb_list, list) {
>> > 		err = block_cb->cb(type, type_data, block_cb->cb_priv);
>> > 		if (err < 0)
>> > 			return err;
>> >@@ -154,7 +154,7 @@ static int nft_flow_offload_rule(struct nft_trans *trans,
>> > static int nft_flow_offload_bind(struct flow_block_offload *bo,
>> > 				 struct nft_base_chain *basechain)
>> > {
>> >-	list_splice(&bo->cb_list, &basechain->cb_list);
>> >+	list_splice(&bo->cb_list, &basechain->block.cb_list);
>> > 	return 0;
>> > }
>> > 
>> >@@ -198,6 +198,7 @@ static int nft_flow_offload_chain(struct nft_trans *trans,
>> > 		return -EOPNOTSUPP;
>> > 
>> > 	bo.command = cmd;
>> >+	bo.block = &basechain->block;
>> > 	bo.binder_type = FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS;
>> > 	bo.extack = &extack;
>> > 	INIT_LIST_HEAD(&bo.cb_list);
>> >diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
>> >index 51fbe6e95a92..66181961ad6f 100644
>> >--- a/net/sched/cls_api.c
>> >+++ b/net/sched/cls_api.c
>> >@@ -691,6 +691,8 @@ static void tc_indr_block_ing_cmd(struct tc_indr_block_dev *indr_dev,
>> > 	if (!indr_dev->block)
>> > 		return;
>> > 
>> >+	bo.block = &indr_dev->block->flow;
>> >+
>> > 	indr_block_cb->cb(indr_dev->dev, indr_block_cb->cb_priv, TC_SETUP_BLOCK,
>> > 			  &bo);
>> > 	tcf_block_setup(indr_dev->block, &bo);
>> >@@ -775,6 +777,7 @@ static void tc_indr_block_call(struct tcf_block *block, struct net_device *dev,
>> > 		.command	= command,
>> > 		.binder_type	= ei->binder_type,
>> > 		.net		= dev_net(dev),
>> >+		.block		= &block->flow,
>> > 		.block_shared	= tcf_block_shared(block),
>> > 		.extack		= extack,
>> > 	};
>> >@@ -810,6 +813,7 @@ static int tcf_block_offload_cmd(struct tcf_block *block,
>> > 	bo.net = dev_net(dev);
>> > 	bo.command = command;
>> > 	bo.binder_type = ei->binder_type;
>> >+	bo.block = &block->flow;
>> > 	bo.block_shared = tcf_block_shared(block);
>> > 	bo.extack = extack;
>> > 	INIT_LIST_HEAD(&bo.cb_list);
>> >@@ -988,7 +992,7 @@ static struct tcf_block *tcf_block_create(struct net *net, struct Qdisc *q,
>> > 	}
>> > 	mutex_init(&block->lock);
>> > 	INIT_LIST_HEAD(&block->chain_list);
>> >-	INIT_LIST_HEAD(&block->cb_list);
>> >+	INIT_LIST_HEAD(&block->flow.cb_list);
>> 
>> With introduction of struct flow_block, please introduce also a helper
>> to init this struct. Does not look right to init it from user codes
>> (tc/nft).
>
>Done.