netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH nf-next 00/11] netfilter: nf_tables: reduce transaction log memory usage
@ 2024-05-13 13:00 Florian Westphal
  2024-05-13 13:00 ` [PATCH nf-next 01/11] netfilter: nf_tables: make struct nft_trans first member of derived subtypes Florian Westphal
                   ` (10 more replies)
  0 siblings, 11 replies; 20+ messages in thread
From: Florian Westphal @ 2024-05-13 13:00 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

The transaction log can grow to huge values.
Insertion of 1.000.000 elements into a set, or flushing a set with
1.000.000 elements will eat 128 byte per element, i.e. 128 MiBi.

This series compacts the structures. After this series, struct
nft_trans_elem can be allocated from kmalloc-96 slab, resulting
in a 25% memory reduction.

To further reduce flush/mass-insert several approaches come
to mind:

1. allow struct nft_trans_elem to hold several elements.
2. add a kernel-internal, dedicated nft_trans_elem_batch that
   is only used for flushing (similar to 1).
3. Remove 'struct net' from nft_trans struct.  This reduces
   size of nft_trans_elem to 64 bytes, which would halve memory
   needs compared to the current state.

I have tried to do 3), its possible but not very elegant.

You can have a look at the general idea at
https://git.kernel.org/pub/scm/linux/kernel/git/fwestphal/nf-next.git/commit/?h=nft_trans_compact_01&id=5269e591563204490b9fad6ae1e33810a9f4c39d

I have started to look at 1) too, but unlike this compaction
series it looks like this will make things even more complex
as we'll need to be careful wrt. appending more set elements to
an already-queued nft_trans_elem (must be same msg_type, same set,
etc).

This series has seen brief testing with kasan+kmemleak and
nftables.git selftests.

Feedback and comments welcome.

Florian Westphal (11):
  netfilter: nf_tables: make struct nft_trans first member of derived subtypes
  netfilter: nf_tables: move bind list_head into relevant subtypes
  netfilter: nf_tables: compact chain+ft transaction objects
  netfilter: nf_tables: reduce trans->ctx.table references
  netfilter: nf_tables: pass nft_chain to destroy function, not nft_ctx
  netfilter: nf_tables: pass more specific nft_trans_chain where possible
  netfilter: nf_tables: avoid usage of embedded nft_ctx
  netfilter: nf_tables: store chain pointer in rule transaction
  netfilter: nf_tables: reduce trans->ctx.chain references
  netfilter: nf_tables: pass nft_table to destroy function
  netfilter: nf_tables: do not store nft_ctx in transaction objects

 include/net/netfilter/nf_tables.h | 152 +++++++----
 net/netfilter/nf_tables_api.c     | 402 +++++++++++++++++-------------
 net/netfilter/nf_tables_offload.c |  40 +--
 net/netfilter/nft_immediate.c     |   2 +-
 4 files changed, 363 insertions(+), 233 deletions(-)

-- 
2.43.2


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

* [PATCH nf-next 01/11] netfilter: nf_tables: make struct nft_trans first member of derived subtypes
  2024-05-13 13:00 [PATCH nf-next 00/11] netfilter: nf_tables: reduce transaction log memory usage Florian Westphal
@ 2024-05-13 13:00 ` Florian Westphal
  2024-06-18  8:28   ` Pablo Neira Ayuso
  2024-05-13 13:00 ` [PATCH nf-next 02/11] netfilter: nf_tables: move bind list_head into relevant subtypes Florian Westphal
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Florian Westphal @ 2024-05-13 13:00 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

There is 'struct nft_trans', the basic structure for all transactional
objects, and the the various different transactional objects, such as
nft_trans_table, chain, set, set_elem and so on.

Right now 'struct nft_trans' uses a flexible member at the tail
(data[]), and casting is needed to access the actual type-specific
members.

Change this to make the hierarchy visible in source code, i.e. make
struct nft_trans the first member of all derived subtypes.

This has several advantages:
1. pahole output reflects the real size needed by the particular subtype
2. allows to use container_of() to convert the base type to the actual
   object type instead of casting ->data to the overlay structure.
3. It makes it easy to add intermediate types.

'struct nft_trans' contains a 'binding_list' that is only needed
by two subtypes, so it should be part of the two subtypes, not in
the base structure.

But that makes it hard to interate over the binding_list, because
there is no common base structure.

A follow patch moves the bind list to a new struct:

 struct nft_trans_binding {
   struct nft_trans nft_trans;
   struct list_head binding_list;
 };

... and makes that structure the new 'first member' for both
nft_trans_chain and nft_trans_set.

No functional change intended in this patch.

Some numbers:
 struct nft_trans { /* size: 88, cachelines: 2, members: 5 */
 struct nft_trans_chain { /* size: 152, cachelines: 3, members: 10 */
 struct nft_trans_elem { /* size: 112, cachelines: 2, members: 4 */
 struct nft_trans_flowtable { /* size: 128, cachelines: 2, members: 5 */
 struct nft_trans_obj { /* size: 112, cachelines: 2, members: 4 */
 struct nft_trans_rule { /* size: 112, cachelines: 2, members: 5 */
 struct nft_trans_set { /* size: 120, cachelines: 2, members: 8 */
 struct nft_trans_table { /* size: 96, cachelines: 2, members: 2 */

Of particular interest is nft_trans_elem, which needs to be allocated
once for each pending (to be added or removed) set element.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/net/netfilter/nf_tables.h | 88 +++++++++++++++++--------------
 net/netfilter/nf_tables_api.c     | 10 ++--
 2 files changed, 54 insertions(+), 44 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 2796153b03da..af0ef21f3780 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1608,14 +1608,16 @@ static inline int nft_set_elem_is_dead(const struct nft_set_ext *ext)
 }
 
 /**
- *	struct nft_trans - nf_tables object update in transaction
+ * struct nft_trans - nf_tables object update in transaction
  *
- *	@list: used internally
- *	@binding_list: list of objects with possible bindings
- *	@msg_type: message type
- *	@put_net: ctx->net needs to be put
- *	@ctx: transaction context
- *	@data: internal information related to the transaction
+ * @list: used internally
+ * @binding_list: list of objects with possible bindings
+ * @msg_type: message type
+ * @put_net: ctx->net needs to be put
+ * @ctx: transaction context
+ *
+ * This is the information common to all objects in the transaction,
+ * this must always be the first member of derived sub-types.
  */
 struct nft_trans {
 	struct list_head		list;
@@ -1623,10 +1625,10 @@ struct nft_trans {
 	int				msg_type;
 	bool				put_net;
 	struct nft_ctx			ctx;
-	char				data[];
 };
 
 struct nft_trans_rule {
+	struct nft_trans		nft_trans;
 	struct nft_rule			*rule;
 	struct nft_flow_rule		*flow;
 	u32				rule_id;
@@ -1634,15 +1636,16 @@ struct nft_trans_rule {
 };
 
 #define nft_trans_rule(trans)	\
-	(((struct nft_trans_rule *)trans->data)->rule)
+	container_of(trans, struct nft_trans_rule, nft_trans)->rule
 #define nft_trans_flow_rule(trans)	\
-	(((struct nft_trans_rule *)trans->data)->flow)
+	container_of(trans, struct nft_trans_rule, nft_trans)->flow
 #define nft_trans_rule_id(trans)	\
-	(((struct nft_trans_rule *)trans->data)->rule_id)
+	container_of(trans, struct nft_trans_rule, nft_trans)->rule_id
 #define nft_trans_rule_bound(trans)	\
-	(((struct nft_trans_rule *)trans->data)->bound)
+	container_of(trans, struct nft_trans_rule, nft_trans)->bound
 
 struct nft_trans_set {
+	struct nft_trans		nft_trans;
 	struct nft_set			*set;
 	u32				set_id;
 	u32				gc_int;
@@ -1653,21 +1656,22 @@ struct nft_trans_set {
 };
 
 #define nft_trans_set(trans)	\
-	(((struct nft_trans_set *)trans->data)->set)
+	container_of(trans, struct nft_trans_set, nft_trans)->set
 #define nft_trans_set_id(trans)	\
-	(((struct nft_trans_set *)trans->data)->set_id)
+	container_of(trans, struct nft_trans_set, nft_trans)->set_id
 #define nft_trans_set_bound(trans)	\
-	(((struct nft_trans_set *)trans->data)->bound)
+	container_of(trans, struct nft_trans_set, nft_trans)->bound
 #define nft_trans_set_update(trans)	\
-	(((struct nft_trans_set *)trans->data)->update)
+	container_of(trans, struct nft_trans_set, nft_trans)->update
 #define nft_trans_set_timeout(trans)	\
-	(((struct nft_trans_set *)trans->data)->timeout)
+	container_of(trans, struct nft_trans_set, nft_trans)->timeout
 #define nft_trans_set_gc_int(trans)	\
-	(((struct nft_trans_set *)trans->data)->gc_int)
+	container_of(trans, struct nft_trans_set, nft_trans)->gc_int
 #define nft_trans_set_size(trans)	\
-	(((struct nft_trans_set *)trans->data)->size)
+	container_of(trans, struct nft_trans_set, nft_trans)->size
 
 struct nft_trans_chain {
+	struct nft_trans		nft_trans;
 	struct nft_chain		*chain;
 	bool				update;
 	char				*name;
@@ -1680,58 +1684,64 @@ struct nft_trans_chain {
 };
 
 #define nft_trans_chain(trans)	\
-	(((struct nft_trans_chain *)trans->data)->chain)
+	container_of(trans, struct nft_trans_chain, nft_trans)->chain
 #define nft_trans_chain_update(trans)	\
-	(((struct nft_trans_chain *)trans->data)->update)
+	container_of(trans, struct nft_trans_chain, nft_trans)->update
 #define nft_trans_chain_name(trans)	\
-	(((struct nft_trans_chain *)trans->data)->name)
+	container_of(trans, struct nft_trans_chain, nft_trans)->name
 #define nft_trans_chain_stats(trans)	\
-	(((struct nft_trans_chain *)trans->data)->stats)
+	container_of(trans, struct nft_trans_chain, nft_trans)->stats
 #define nft_trans_chain_policy(trans)	\
-	(((struct nft_trans_chain *)trans->data)->policy)
+	container_of(trans, struct nft_trans_chain, nft_trans)->policy
 #define nft_trans_chain_bound(trans)	\
-	(((struct nft_trans_chain *)trans->data)->bound)
+	container_of(trans, struct nft_trans_chain, nft_trans)->bound
 #define nft_trans_chain_id(trans)	\
-	(((struct nft_trans_chain *)trans->data)->chain_id)
+	container_of(trans, struct nft_trans_chain, nft_trans)->chain_id
 #define nft_trans_basechain(trans)	\
-	(((struct nft_trans_chain *)trans->data)->basechain)
+	container_of(trans, struct nft_trans_chain, nft_trans)->basechain
 #define nft_trans_chain_hooks(trans)	\
-	(((struct nft_trans_chain *)trans->data)->hook_list)
+	container_of(trans, struct nft_trans_chain, nft_trans)->hook_list
 
 struct nft_trans_table {
+	struct nft_trans		nft_trans;
 	bool				update;
 };
 
 #define nft_trans_table_update(trans)	\
-	(((struct nft_trans_table *)trans->data)->update)
+	container_of(trans, struct nft_trans_table, nft_trans)->update
 
 struct nft_trans_elem {
+	struct nft_trans		nft_trans;
 	struct nft_set			*set;
 	struct nft_elem_priv		*elem_priv;
 	bool				bound;
 };
 
+#define nft_trans_container_elem(t)	\
+	container_of(t, struct nft_trans_elem, nft_trans)
 #define nft_trans_elem_set(trans)	\
-	(((struct nft_trans_elem *)trans->data)->set)
+	container_of(trans, struct nft_trans_elem, nft_trans)->set
 #define nft_trans_elem_priv(trans)	\
-	(((struct nft_trans_elem *)trans->data)->elem_priv)
+	container_of(trans, struct nft_trans_elem, nft_trans)->elem_priv
 #define nft_trans_elem_set_bound(trans)	\
-	(((struct nft_trans_elem *)trans->data)->bound)
+	container_of(trans, struct nft_trans_elem, nft_trans)->bound
 
 struct nft_trans_obj {
+	struct nft_trans		nft_trans;
 	struct nft_object		*obj;
 	struct nft_object		*newobj;
 	bool				update;
 };
 
 #define nft_trans_obj(trans)	\
-	(((struct nft_trans_obj *)trans->data)->obj)
+	container_of(trans, struct nft_trans_obj, nft_trans)->obj
 #define nft_trans_obj_newobj(trans) \
-	(((struct nft_trans_obj *)trans->data)->newobj)
+	container_of(trans, struct nft_trans_obj, nft_trans)->newobj
 #define nft_trans_obj_update(trans)	\
-	(((struct nft_trans_obj *)trans->data)->update)
+	container_of(trans, struct nft_trans_obj, nft_trans)->update
 
 struct nft_trans_flowtable {
+	struct nft_trans		nft_trans;
 	struct nft_flowtable		*flowtable;
 	bool				update;
 	struct list_head		hook_list;
@@ -1739,13 +1749,13 @@ struct nft_trans_flowtable {
 };
 
 #define nft_trans_flowtable(trans)	\
-	(((struct nft_trans_flowtable *)trans->data)->flowtable)
+	container_of(trans, struct nft_trans_flowtable, nft_trans)->flowtable
 #define nft_trans_flowtable_update(trans)	\
-	(((struct nft_trans_flowtable *)trans->data)->update)
+	container_of(trans, struct nft_trans_flowtable, nft_trans)->update
 #define nft_trans_flowtable_hooks(trans)	\
-	(((struct nft_trans_flowtable *)trans->data)->hook_list)
+	container_of(trans, struct nft_trans_flowtable, nft_trans)->hook_list
 #define nft_trans_flowtable_flags(trans)	\
-	(((struct nft_trans_flowtable *)trans->data)->flags)
+	container_of(trans, struct nft_trans_flowtable, nft_trans)->flags
 
 #define NFT_TRANS_GC_BATCHCOUNT	256
 
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index be3b4c90d2ed..cd355f63b1d2 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -153,7 +153,7 @@ static struct nft_trans *nft_trans_alloc_gfp(const struct nft_ctx *ctx,
 {
 	struct nft_trans *trans;
 
-	trans = kzalloc(sizeof(struct nft_trans) + size, gfp);
+	trans = kzalloc(size, gfp);
 	if (trans == NULL)
 		return NULL;
 
@@ -10348,7 +10348,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 					     trans->msg_type, GFP_KERNEL);
 			break;
 		case NFT_MSG_NEWSETELEM:
-			te = (struct nft_trans_elem *)trans->data;
+			te = nft_trans_container_elem(trans);
 
 			nft_setelem_activate(net, te->set, te->elem_priv);
 			nf_tables_setelem_notify(&trans->ctx, te->set,
@@ -10363,7 +10363,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 			break;
 		case NFT_MSG_DELSETELEM:
 		case NFT_MSG_DESTROYSETELEM:
-			te = (struct nft_trans_elem *)trans->data;
+			te = nft_trans_container_elem(trans);
 
 			nf_tables_setelem_notify(&trans->ctx, te->set,
 						 te->elem_priv,
@@ -10643,7 +10643,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 				nft_trans_destroy(trans);
 				break;
 			}
-			te = (struct nft_trans_elem *)trans->data;
+			te = nft_trans_container_elem(trans);
 			nft_setelem_remove(net, te->set, te->elem_priv);
 			if (!nft_setelem_is_catchall(te->set, te->elem_priv))
 				atomic_dec(&te->set->nelems);
@@ -10656,7 +10656,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 			break;
 		case NFT_MSG_DELSETELEM:
 		case NFT_MSG_DESTROYSETELEM:
-			te = (struct nft_trans_elem *)trans->data;
+			te = nft_trans_container_elem(trans);
 
 			if (!nft_setelem_active_next(net, te->set, te->elem_priv)) {
 				nft_setelem_data_activate(net, te->set, te->elem_priv);
-- 
2.43.2


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

* [PATCH nf-next 02/11] netfilter: nf_tables: move bind list_head into relevant subtypes
  2024-05-13 13:00 [PATCH nf-next 00/11] netfilter: nf_tables: reduce transaction log memory usage Florian Westphal
  2024-05-13 13:00 ` [PATCH nf-next 01/11] netfilter: nf_tables: make struct nft_trans first member of derived subtypes Florian Westphal
@ 2024-05-13 13:00 ` Florian Westphal
  2024-06-18  8:24   ` Pablo Neira Ayuso
  2024-06-24 19:16   ` Pablo Neira Ayuso
  2024-05-13 13:00 ` [PATCH nf-next 03/11] netfilter: nf_tables: compact chain+ft transaction objects Florian Westphal
                   ` (8 subsequent siblings)
  10 siblings, 2 replies; 20+ messages in thread
From: Florian Westphal @ 2024-05-13 13:00 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

Only nft_trans_chain and nft_trans_set subtypes use the
trans->binding_list member.

Add a new common binding subtype and move the member there.

This reduces size of all other subtypes by 16 bytes on 64bit platforms.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/net/netfilter/nf_tables.h | 54 +++++++++++++++----------
 net/netfilter/nf_tables_api.c     | 66 +++++++++++++++++++++++++------
 2 files changed, 88 insertions(+), 32 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index af0ef21f3780..f914cace0241 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1611,7 +1611,6 @@ static inline int nft_set_elem_is_dead(const struct nft_set_ext *ext)
  * struct nft_trans - nf_tables object update in transaction
  *
  * @list: used internally
- * @binding_list: list of objects with possible bindings
  * @msg_type: message type
  * @put_net: ctx->net needs to be put
  * @ctx: transaction context
@@ -1621,12 +1620,23 @@ static inline int nft_set_elem_is_dead(const struct nft_set_ext *ext)
  */
 struct nft_trans {
 	struct list_head		list;
-	struct list_head		binding_list;
 	int				msg_type;
 	bool				put_net;
 	struct nft_ctx			ctx;
 };
 
+/**
+ * struct nft_trans_binding - nf_tables object with binding support in transaction
+ * @nft_trans:    base structure, MUST be first member
+ * @binding_list: list of objects with possible bindings
+ *
+ * This is the base type used by objects that can be bound to a chain.
+ */
+struct nft_trans_binding {
+	struct nft_trans nft_trans;
+	struct list_head binding_list;
+};
+
 struct nft_trans_rule {
 	struct nft_trans		nft_trans;
 	struct nft_rule			*rule;
@@ -1645,7 +1655,7 @@ struct nft_trans_rule {
 	container_of(trans, struct nft_trans_rule, nft_trans)->bound
 
 struct nft_trans_set {
-	struct nft_trans		nft_trans;
+	struct nft_trans_binding	nft_trans_binding;
 	struct nft_set			*set;
 	u32				set_id;
 	u32				gc_int;
@@ -1655,23 +1665,25 @@ struct nft_trans_set {
 	u32				size;
 };
 
+#define nft_trans_container_set(t)	\
+	container_of(t, struct nft_trans_set, nft_trans_binding.nft_trans)
 #define nft_trans_set(trans)	\
-	container_of(trans, struct nft_trans_set, nft_trans)->set
+	nft_trans_container_set(trans)->set
 #define nft_trans_set_id(trans)	\
-	container_of(trans, struct nft_trans_set, nft_trans)->set_id
+	nft_trans_container_set(trans)->set_id
 #define nft_trans_set_bound(trans)	\
-	container_of(trans, struct nft_trans_set, nft_trans)->bound
+	nft_trans_container_set(trans)->bound
 #define nft_trans_set_update(trans)	\
-	container_of(trans, struct nft_trans_set, nft_trans)->update
+	nft_trans_container_set(trans)->update
 #define nft_trans_set_timeout(trans)	\
-	container_of(trans, struct nft_trans_set, nft_trans)->timeout
+	nft_trans_container_set(trans)->timeout
 #define nft_trans_set_gc_int(trans)	\
-	container_of(trans, struct nft_trans_set, nft_trans)->gc_int
+	nft_trans_container_set(trans)->gc_int
 #define nft_trans_set_size(trans)	\
-	container_of(trans, struct nft_trans_set, nft_trans)->size
+	nft_trans_container_set(trans)->size
 
 struct nft_trans_chain {
-	struct nft_trans		nft_trans;
+	struct nft_trans_binding	nft_trans_binding;
 	struct nft_chain		*chain;
 	bool				update;
 	char				*name;
@@ -1683,24 +1695,26 @@ struct nft_trans_chain {
 	struct list_head		hook_list;
 };
 
+#define nft_trans_container_chain(t)	\
+	container_of(t, struct nft_trans_chain, nft_trans_binding.nft_trans)
 #define nft_trans_chain(trans)	\
-	container_of(trans, struct nft_trans_chain, nft_trans)->chain
+	nft_trans_container_chain(trans)->chain
 #define nft_trans_chain_update(trans)	\
-	container_of(trans, struct nft_trans_chain, nft_trans)->update
+	nft_trans_container_chain(trans)->update
 #define nft_trans_chain_name(trans)	\
-	container_of(trans, struct nft_trans_chain, nft_trans)->name
+	nft_trans_container_chain(trans)->name
 #define nft_trans_chain_stats(trans)	\
-	container_of(trans, struct nft_trans_chain, nft_trans)->stats
+	nft_trans_container_chain(trans)->stats
 #define nft_trans_chain_policy(trans)	\
-	container_of(trans, struct nft_trans_chain, nft_trans)->policy
+	nft_trans_container_chain(trans)->policy
 #define nft_trans_chain_bound(trans)	\
-	container_of(trans, struct nft_trans_chain, nft_trans)->bound
+	nft_trans_container_chain(trans)->bound
 #define nft_trans_chain_id(trans)	\
-	container_of(trans, struct nft_trans_chain, nft_trans)->chain_id
+	nft_trans_container_chain(trans)->chain_id
 #define nft_trans_basechain(trans)	\
-	container_of(trans, struct nft_trans_chain, nft_trans)->basechain
+	nft_trans_container_chain(trans)->basechain
 #define nft_trans_chain_hooks(trans)	\
-	container_of(trans, struct nft_trans_chain, nft_trans)->hook_list
+	nft_trans_container_chain(trans)->hook_list
 
 struct nft_trans_table {
 	struct nft_trans		nft_trans;
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index cd355f63b1d2..b82b592059ea 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -158,7 +158,6 @@ static struct nft_trans *nft_trans_alloc_gfp(const struct nft_ctx *ctx,
 		return NULL;
 
 	INIT_LIST_HEAD(&trans->list);
-	INIT_LIST_HEAD(&trans->binding_list);
 	trans->msg_type = msg_type;
 	trans->ctx	= *ctx;
 
@@ -171,10 +170,26 @@ static struct nft_trans *nft_trans_alloc(const struct nft_ctx *ctx,
 	return nft_trans_alloc_gfp(ctx, msg_type, size, GFP_KERNEL);
 }
 
+static struct nft_trans_binding *nft_trans_get_binding(struct nft_trans *trans)
+{
+	switch (trans->msg_type) {
+	case NFT_MSG_NEWCHAIN:
+	case NFT_MSG_NEWSET:
+		return container_of(trans, struct nft_trans_binding, nft_trans);
+	}
+
+	return NULL;
+}
+
 static void nft_trans_list_del(struct nft_trans *trans)
 {
+	struct nft_trans_binding *trans_binding;
+
 	list_del(&trans->list);
-	list_del(&trans->binding_list);
+
+	trans_binding = nft_trans_get_binding(trans);
+	if (trans_binding)
+		list_del(&trans_binding->binding_list);
 }
 
 static void nft_trans_destroy(struct nft_trans *trans)
@@ -372,21 +387,26 @@ static void nf_tables_unregister_hook(struct net *net,
 static void nft_trans_commit_list_add_tail(struct net *net, struct nft_trans *trans)
 {
 	struct nftables_pernet *nft_net = nft_pernet(net);
+	struct nft_trans_binding *binding;
+
+	list_add_tail(&trans->list, &nft_net->commit_list);
+
+	binding = nft_trans_get_binding(trans);
+	if (!binding)
+		return;
 
 	switch (trans->msg_type) {
 	case NFT_MSG_NEWSET:
 		if (!nft_trans_set_update(trans) &&
 		    nft_set_is_anonymous(nft_trans_set(trans)))
-			list_add_tail(&trans->binding_list, &nft_net->binding_list);
+			list_add_tail(&binding->binding_list, &nft_net->binding_list);
 		break;
 	case NFT_MSG_NEWCHAIN:
 		if (!nft_trans_chain_update(trans) &&
 		    nft_chain_binding(nft_trans_chain(trans)))
-			list_add_tail(&trans->binding_list, &nft_net->binding_list);
+			list_add_tail(&binding->binding_list, &nft_net->binding_list);
 		break;
 	}
-
-	list_add_tail(&trans->list, &nft_net->commit_list);
 }
 
 static int nft_trans_table_add(struct nft_ctx *ctx, int msg_type)
@@ -416,11 +436,26 @@ static int nft_deltable(struct nft_ctx *ctx)
 	return err;
 }
 
-static struct nft_trans *nft_trans_chain_add(struct nft_ctx *ctx, int msg_type)
+static struct nft_trans *
+nft_trans_alloc_chain(const struct nft_ctx *ctx, int msg_type)
 {
 	struct nft_trans *trans;
 
 	trans = nft_trans_alloc(ctx, msg_type, sizeof(struct nft_trans_chain));
+	if (trans) {
+		struct nft_trans_chain *chain = nft_trans_container_chain(trans);
+
+		INIT_LIST_HEAD(&chain->nft_trans_binding.binding_list);
+	}
+
+	return trans;
+}
+
+static struct nft_trans *nft_trans_chain_add(struct nft_ctx *ctx, int msg_type)
+{
+	struct nft_trans *trans;
+
+	trans = nft_trans_alloc_chain(ctx, msg_type);
 	if (trans == NULL)
 		return ERR_PTR(-ENOMEM);
 
@@ -560,12 +595,16 @@ static int __nft_trans_set_add(const struct nft_ctx *ctx, int msg_type,
 			       struct nft_set *set,
 			       const struct nft_set_desc *desc)
 {
+	struct nft_trans_set *trans_set;
 	struct nft_trans *trans;
 
 	trans = nft_trans_alloc(ctx, msg_type, sizeof(struct nft_trans_set));
 	if (trans == NULL)
 		return -ENOMEM;
 
+	trans_set = nft_trans_container_set(trans);
+	INIT_LIST_HEAD(&trans_set->nft_trans_binding.binding_list);
+
 	if (msg_type == NFT_MSG_NEWSET && ctx->nla[NFTA_SET_ID] && !desc) {
 		nft_trans_set_id(trans) =
 			ntohl(nla_get_be32(ctx->nla[NFTA_SET_ID]));
@@ -2698,8 +2737,7 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
 	}
 
 	err = -ENOMEM;
-	trans = nft_trans_alloc(ctx, NFT_MSG_NEWCHAIN,
-				sizeof(struct nft_trans_chain));
+	trans = nft_trans_alloc_chain(ctx, NFT_MSG_NEWCHAIN);
 	if (trans == NULL)
 		goto err_trans;
 
@@ -2915,8 +2953,7 @@ static int nft_delchain_hook(struct nft_ctx *ctx,
 		list_move(&hook->list, &chain_del_list);
 	}
 
-	trans = nft_trans_alloc(ctx, NFT_MSG_DELCHAIN,
-				sizeof(struct nft_trans_chain));
+	trans = nft_trans_alloc_chain(ctx, NFT_MSG_DELCHAIN);
 	if (!trans) {
 		err = -ENOMEM;
 		goto err_chain_del_hook;
@@ -10147,6 +10184,7 @@ static void nft_gc_seq_end(struct nftables_pernet *nft_net, unsigned int gc_seq)
 static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 {
 	struct nftables_pernet *nft_net = nft_pernet(net);
+	struct nft_trans_binding *trans_binding;
 	struct nft_trans *trans, *next;
 	unsigned int base_seq, gc_seq;
 	LIST_HEAD(set_update_list);
@@ -10161,7 +10199,8 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 		return 0;
 	}
 
-	list_for_each_entry(trans, &nft_net->binding_list, binding_list) {
+	list_for_each_entry(trans_binding, &nft_net->binding_list, binding_list) {
+		trans = &trans_binding->nft_trans;
 		switch (trans->msg_type) {
 		case NFT_MSG_NEWSET:
 			if (!nft_trans_set_update(trans) &&
@@ -10179,6 +10218,9 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 				return -EINVAL;
 			}
 			break;
+		default:
+			WARN_ONCE(1, "Unhandled bind type %d", trans->msg_type);
+			break;
 		}
 	}
 
-- 
2.43.2


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

* [PATCH nf-next 03/11] netfilter: nf_tables: compact chain+ft transaction objects
  2024-05-13 13:00 [PATCH nf-next 00/11] netfilter: nf_tables: reduce transaction log memory usage Florian Westphal
  2024-05-13 13:00 ` [PATCH nf-next 01/11] netfilter: nf_tables: make struct nft_trans first member of derived subtypes Florian Westphal
  2024-05-13 13:00 ` [PATCH nf-next 02/11] netfilter: nf_tables: move bind list_head into relevant subtypes Florian Westphal
@ 2024-05-13 13:00 ` Florian Westphal
  2024-05-13 13:00 ` [PATCH nf-next 04/11] netfilter: nf_tables: reduce trans->ctx.table references Florian Westphal
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Florian Westphal @ 2024-05-13 13:00 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

Cover holes to reduce both structures by 8 byte.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/net/netfilter/nf_tables.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index f914cace0241..1f82b9ea8d5d 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1685,10 +1685,10 @@ struct nft_trans_set {
 struct nft_trans_chain {
 	struct nft_trans_binding	nft_trans_binding;
 	struct nft_chain		*chain;
-	bool				update;
 	char				*name;
 	struct nft_stats __percpu	*stats;
 	u8				policy;
+	bool				update;
 	bool				bound;
 	u32				chain_id;
 	struct nft_base_chain		*basechain;
@@ -1757,9 +1757,9 @@ struct nft_trans_obj {
 struct nft_trans_flowtable {
 	struct nft_trans		nft_trans;
 	struct nft_flowtable		*flowtable;
-	bool				update;
 	struct list_head		hook_list;
 	u32				flags;
+	bool				update;
 };
 
 #define nft_trans_flowtable(trans)	\
-- 
2.43.2


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

* [PATCH nf-next 04/11] netfilter: nf_tables: reduce trans->ctx.table references
  2024-05-13 13:00 [PATCH nf-next 00/11] netfilter: nf_tables: reduce transaction log memory usage Florian Westphal
                   ` (2 preceding siblings ...)
  2024-05-13 13:00 ` [PATCH nf-next 03/11] netfilter: nf_tables: compact chain+ft transaction objects Florian Westphal
@ 2024-05-13 13:00 ` Florian Westphal
  2024-05-13 13:00 ` [PATCH nf-next 05/11] netfilter: nf_tables: pass nft_chain to destroy function, not nft_ctx Florian Westphal
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Florian Westphal @ 2024-05-13 13:00 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

nft_ctx is huge, it should not be stored in nft_trans at all,
most information is not needed.

Preparation patch to remove trans->ctx, no change in behaviour intended.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_tables_api.c | 79 ++++++++++++++++++-----------------
 1 file changed, 41 insertions(+), 38 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index b82b592059ea..bd6e93d2f590 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -9471,14 +9471,15 @@ static void nft_chain_commit_drop_policy(struct nft_trans *trans)
 
 static void nft_chain_commit_update(struct nft_trans *trans)
 {
+	struct nft_table *table = trans->ctx.table;
 	struct nft_base_chain *basechain;
 
 	if (nft_trans_chain_name(trans)) {
-		rhltable_remove(&trans->ctx.table->chains_ht,
+		rhltable_remove(&table->chains_ht,
 				&trans->ctx.chain->rhlhead,
 				nft_chain_ht_params);
 		swap(trans->ctx.chain->name, nft_trans_chain_name(trans));
-		rhltable_insert_key(&trans->ctx.table->chains_ht,
+		rhltable_insert_key(&table->chains_ht,
 				    trans->ctx.chain->name,
 				    &trans->ctx.chain->rhlhead,
 				    nft_chain_ht_params);
@@ -10236,9 +10237,10 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 
 	/* 1.  Allocate space for next generation rules_gen_X[] */
 	list_for_each_entry_safe(trans, next, &nft_net->commit_list, list) {
+		struct nft_table *table = trans->ctx.table;
 		int ret;
 
-		ret = nf_tables_commit_audit_alloc(&adl, trans->ctx.table);
+		ret = nf_tables_commit_audit_alloc(&adl, table);
 		if (ret) {
 			nf_tables_commit_chain_prepare_cancel(net);
 			nf_tables_commit_audit_free(&adl);
@@ -10279,28 +10281,29 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 	net->nft.gencursor = nft_gencursor_next(net);
 
 	list_for_each_entry_safe(trans, next, &nft_net->commit_list, list) {
-		nf_tables_commit_audit_collect(&adl, trans->ctx.table,
-					       trans->msg_type);
+		struct nft_table *table = trans->ctx.table;
+
+		nf_tables_commit_audit_collect(&adl, table, trans->msg_type);
 		switch (trans->msg_type) {
 		case NFT_MSG_NEWTABLE:
 			if (nft_trans_table_update(trans)) {
-				if (!(trans->ctx.table->flags & __NFT_TABLE_F_UPDATE)) {
+				if (!(table->flags & __NFT_TABLE_F_UPDATE)) {
 					nft_trans_destroy(trans);
 					break;
 				}
-				if (trans->ctx.table->flags & NFT_TABLE_F_DORMANT)
-					nf_tables_table_disable(net, trans->ctx.table);
+				if (table->flags & NFT_TABLE_F_DORMANT)
+					nf_tables_table_disable(net, table);
 
-				trans->ctx.table->flags &= ~__NFT_TABLE_F_UPDATE;
+				table->flags &= ~__NFT_TABLE_F_UPDATE;
 			} else {
-				nft_clear(net, trans->ctx.table);
+				nft_clear(net, table);
 			}
 			nf_tables_table_notify(&trans->ctx, NFT_MSG_NEWTABLE);
 			nft_trans_destroy(trans);
 			break;
 		case NFT_MSG_DELTABLE:
 		case NFT_MSG_DESTROYTABLE:
-			list_del_rcu(&trans->ctx.table->list);
+			list_del_rcu(&table->list);
 			nf_tables_table_notify(&trans->ctx, trans->msg_type);
 			break;
 		case NFT_MSG_NEWCHAIN:
@@ -10323,7 +10326,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 			if (nft_trans_chain_update(trans)) {
 				nf_tables_chain_notify(&trans->ctx, NFT_MSG_DELCHAIN,
 						       &nft_trans_chain_hooks(trans));
-				if (!(trans->ctx.table->flags & NFT_TABLE_F_DORMANT)) {
+				if (!(table->flags & NFT_TABLE_F_DORMANT)) {
 					nft_netdev_unregister_hooks(net,
 								    &nft_trans_chain_hooks(trans),
 								    true);
@@ -10332,8 +10335,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 				nft_chain_del(trans->ctx.chain);
 				nf_tables_chain_notify(&trans->ctx, NFT_MSG_DELCHAIN,
 						       NULL);
-				nf_tables_unregister_hook(trans->ctx.net,
-							  trans->ctx.table,
+				nf_tables_unregister_hook(trans->ctx.net, table,
 							  trans->ctx.chain);
 			}
 			break;
@@ -10376,7 +10378,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 				 */
 				if (nft_set_is_anonymous(nft_trans_set(trans)) &&
 				    !list_empty(&nft_trans_set(trans)->bindings))
-					nft_use_dec(&trans->ctx.table->use);
+					nft_use_dec(&table->use);
 			}
 			nf_tables_set_notify(&trans->ctx, nft_trans_set(trans),
 					     NFT_MSG_NEWSET, GFP_KERNEL);
@@ -10574,37 +10576,39 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 
 	list_for_each_entry_safe_reverse(trans, next, &nft_net->commit_list,
 					 list) {
+		struct nft_table *table = trans->ctx.table;
+
 		switch (trans->msg_type) {
 		case NFT_MSG_NEWTABLE:
 			if (nft_trans_table_update(trans)) {
-				if (!(trans->ctx.table->flags & __NFT_TABLE_F_UPDATE)) {
+				if (!(table->flags & __NFT_TABLE_F_UPDATE)) {
 					nft_trans_destroy(trans);
 					break;
 				}
-				if (trans->ctx.table->flags & __NFT_TABLE_F_WAS_DORMANT) {
-					nf_tables_table_disable(net, trans->ctx.table);
-					trans->ctx.table->flags |= NFT_TABLE_F_DORMANT;
-				} else if (trans->ctx.table->flags & __NFT_TABLE_F_WAS_AWAKEN) {
-					trans->ctx.table->flags &= ~NFT_TABLE_F_DORMANT;
+				if (table->flags & __NFT_TABLE_F_WAS_DORMANT) {
+					nf_tables_table_disable(net, table);
+					table->flags |= NFT_TABLE_F_DORMANT;
+				} else if (table->flags & __NFT_TABLE_F_WAS_AWAKEN) {
+					table->flags &= ~NFT_TABLE_F_DORMANT;
 				}
-				if (trans->ctx.table->flags & __NFT_TABLE_F_WAS_ORPHAN) {
-					trans->ctx.table->flags &= ~NFT_TABLE_F_OWNER;
-					trans->ctx.table->nlpid = 0;
+				if (table->flags & __NFT_TABLE_F_WAS_ORPHAN) {
+					table->flags &= ~NFT_TABLE_F_OWNER;
+					table->nlpid = 0;
 				}
-				trans->ctx.table->flags &= ~__NFT_TABLE_F_UPDATE;
+				table->flags &= ~__NFT_TABLE_F_UPDATE;
 				nft_trans_destroy(trans);
 			} else {
-				list_del_rcu(&trans->ctx.table->list);
+				list_del_rcu(&table->list);
 			}
 			break;
 		case NFT_MSG_DELTABLE:
 		case NFT_MSG_DESTROYTABLE:
-			nft_clear(trans->ctx.net, trans->ctx.table);
+			nft_clear(trans->ctx.net, table);
 			nft_trans_destroy(trans);
 			break;
 		case NFT_MSG_NEWCHAIN:
 			if (nft_trans_chain_update(trans)) {
-				if (!(trans->ctx.table->flags & NFT_TABLE_F_DORMANT)) {
+				if (!(table->flags & NFT_TABLE_F_DORMANT)) {
 					nft_netdev_unregister_hooks(net,
 								    &nft_trans_chain_hooks(trans),
 								    true);
@@ -10617,10 +10621,9 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 					nft_trans_destroy(trans);
 					break;
 				}
-				nft_use_dec_restore(&trans->ctx.table->use);
+				nft_use_dec_restore(&table->use);
 				nft_chain_del(trans->ctx.chain);
-				nf_tables_unregister_hook(trans->ctx.net,
-							  trans->ctx.table,
+				nf_tables_unregister_hook(trans->ctx.net, table,
 							  trans->ctx.chain);
 			}
 			break;
@@ -10630,7 +10633,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 				list_splice(&nft_trans_chain_hooks(trans),
 					    &nft_trans_basechain(trans)->hook_list);
 			} else {
-				nft_use_inc_restore(&trans->ctx.table->use);
+				nft_use_inc_restore(&table->use);
 				nft_clear(trans->ctx.net, trans->ctx.chain);
 			}
 			nft_trans_destroy(trans);
@@ -10663,7 +10666,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 				nft_trans_destroy(trans);
 				break;
 			}
-			nft_use_dec_restore(&trans->ctx.table->use);
+			nft_use_dec_restore(&table->use);
 			if (nft_trans_set_bound(trans)) {
 				nft_trans_destroy(trans);
 				break;
@@ -10673,7 +10676,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 			break;
 		case NFT_MSG_DELSET:
 		case NFT_MSG_DESTROYSET:
-			nft_use_inc_restore(&trans->ctx.table->use);
+			nft_use_inc_restore(&table->use);
 			nft_clear(trans->ctx.net, nft_trans_set(trans));
 			if (nft_trans_set(trans)->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
 				nft_map_activate(&trans->ctx, nft_trans_set(trans));
@@ -10719,13 +10722,13 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 				nft_obj_destroy(&trans->ctx, nft_trans_obj_newobj(trans));
 				nft_trans_destroy(trans);
 			} else {
-				nft_use_dec_restore(&trans->ctx.table->use);
+				nft_use_dec_restore(&table->use);
 				nft_obj_del(nft_trans_obj(trans));
 			}
 			break;
 		case NFT_MSG_DELOBJ:
 		case NFT_MSG_DESTROYOBJ:
-			nft_use_inc_restore(&trans->ctx.table->use);
+			nft_use_inc_restore(&table->use);
 			nft_clear(trans->ctx.net, nft_trans_obj(trans));
 			nft_trans_destroy(trans);
 			break;
@@ -10734,7 +10737,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 				nft_unregister_flowtable_net_hooks(net,
 						&nft_trans_flowtable_hooks(trans));
 			} else {
-				nft_use_dec_restore(&trans->ctx.table->use);
+				nft_use_dec_restore(&table->use);
 				list_del_rcu(&nft_trans_flowtable(trans)->list);
 				nft_unregister_flowtable_net_hooks(net,
 						&nft_trans_flowtable(trans)->hook_list);
@@ -10746,7 +10749,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 				list_splice(&nft_trans_flowtable_hooks(trans),
 					    &nft_trans_flowtable(trans)->hook_list);
 			} else {
-				nft_use_inc_restore(&trans->ctx.table->use);
+				nft_use_inc_restore(&table->use);
 				nft_clear(trans->ctx.net, nft_trans_flowtable(trans));
 			}
 			nft_trans_destroy(trans);
-- 
2.43.2


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

* [PATCH nf-next 05/11] netfilter: nf_tables: pass nft_chain to destroy function, not nft_ctx
  2024-05-13 13:00 [PATCH nf-next 00/11] netfilter: nf_tables: reduce transaction log memory usage Florian Westphal
                   ` (3 preceding siblings ...)
  2024-05-13 13:00 ` [PATCH nf-next 04/11] netfilter: nf_tables: reduce trans->ctx.table references Florian Westphal
@ 2024-05-13 13:00 ` Florian Westphal
  2024-05-13 13:00 ` [PATCH nf-next 06/11] netfilter: nf_tables: pass more specific nft_trans_chain where possible Florian Westphal
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Florian Westphal @ 2024-05-13 13:00 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

It would be better to not store nft_ctx inside nft_trans object,
the netlink ctx strucutre is huge and most of its information is
never needed in places that use trans->ctx.

Avoid/reduce its usage if possible, no runtime behaviour change
intended.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/net/netfilter/nf_tables.h |  2 +-
 net/netfilter/nf_tables_api.c     | 17 ++++++++---------
 net/netfilter/nft_immediate.c     |  2 +-
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 1f82b9ea8d5d..3ae09dca65ad 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1171,7 +1171,7 @@ static inline bool nft_chain_is_bound(struct nft_chain *chain)
 
 int nft_chain_add(struct nft_table *table, struct nft_chain *chain);
 void nft_chain_del(struct nft_chain *chain);
-void nf_tables_chain_destroy(struct nft_ctx *ctx);
+void nf_tables_chain_destroy(struct nft_chain *chain);
 
 struct nft_stats {
 	u64			bytes;
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index bd6e93d2f590..00e5fdf8977b 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2117,9 +2117,9 @@ static void nf_tables_chain_free_chain_rules(struct nft_chain *chain)
 	kvfree(chain->blob_next);
 }
 
-void nf_tables_chain_destroy(struct nft_ctx *ctx)
+void nf_tables_chain_destroy(struct nft_chain *chain)
 {
-	struct nft_chain *chain = ctx->chain;
+	const struct nft_table *table = chain->table;
 	struct nft_hook *hook, *next;
 
 	if (WARN_ON(chain->use > 0))
@@ -2131,7 +2131,7 @@ void nf_tables_chain_destroy(struct nft_ctx *ctx)
 	if (nft_is_base_chain(chain)) {
 		struct nft_base_chain *basechain = nft_base_chain(chain);
 
-		if (nft_base_chain_netdev(ctx->family, basechain->ops.hooknum)) {
+		if (nft_base_chain_netdev(table->family, basechain->ops.hooknum)) {
 			list_for_each_entry_safe(hook, next,
 						 &basechain->hook_list, list) {
 				list_del_rcu(&hook->list);
@@ -2620,7 +2620,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
 err_trans:
 	nft_use_dec_restore(&table->use);
 err_destroy_chain:
-	nf_tables_chain_destroy(ctx);
+	nf_tables_chain_destroy(chain);
 
 	return err;
 }
@@ -9531,7 +9531,7 @@ static void nft_commit_release(struct nft_trans *trans)
 		if (nft_trans_chain_update(trans))
 			nft_hooks_destroy(&nft_trans_chain_hooks(trans));
 		else
-			nf_tables_chain_destroy(&trans->ctx);
+			nf_tables_chain_destroy(nft_trans_chain(trans));
 		break;
 	case NFT_MSG_DELRULE:
 	case NFT_MSG_DESTROYRULE:
@@ -10523,7 +10523,7 @@ static void nf_tables_abort_release(struct nft_trans *trans)
 		if (nft_trans_chain_update(trans))
 			nft_hooks_destroy(&nft_trans_chain_hooks(trans));
 		else
-			nf_tables_chain_destroy(&trans->ctx);
+			nf_tables_chain_destroy(nft_trans_chain(trans));
 		break;
 	case NFT_MSG_NEWRULE:
 		nf_tables_rule_destroy(&trans->ctx, nft_trans_rule(trans));
@@ -11410,7 +11410,7 @@ int __nft_release_basechain(struct nft_ctx *ctx)
 	}
 	nft_chain_del(ctx->chain);
 	nft_use_dec(&ctx->table->use);
-	nf_tables_chain_destroy(ctx);
+	nf_tables_chain_destroy(ctx->chain);
 
 	return 0;
 }
@@ -11485,10 +11485,9 @@ static void __nft_release_table(struct net *net, struct nft_table *table)
 		nft_obj_destroy(&ctx, obj);
 	}
 	list_for_each_entry_safe(chain, nc, &table->chains, list) {
-		ctx.chain = chain;
 		nft_chain_del(chain);
 		nft_use_dec(&table->use);
-		nf_tables_chain_destroy(&ctx);
+		nf_tables_chain_destroy(chain);
 	}
 	nf_tables_table_destroy(&ctx);
 }
diff --git a/net/netfilter/nft_immediate.c b/net/netfilter/nft_immediate.c
index 6475c7abc1fe..ac2422c215e5 100644
--- a/net/netfilter/nft_immediate.c
+++ b/net/netfilter/nft_immediate.c
@@ -221,7 +221,7 @@ static void nft_immediate_destroy(const struct nft_ctx *ctx,
 			list_del(&rule->list);
 			nf_tables_rule_destroy(&chain_ctx, rule);
 		}
-		nf_tables_chain_destroy(&chain_ctx);
+		nf_tables_chain_destroy(chain);
 		break;
 	default:
 		break;
-- 
2.43.2


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

* [PATCH nf-next 06/11] netfilter: nf_tables: pass more specific nft_trans_chain where possible
  2024-05-13 13:00 [PATCH nf-next 00/11] netfilter: nf_tables: reduce transaction log memory usage Florian Westphal
                   ` (4 preceding siblings ...)
  2024-05-13 13:00 ` [PATCH nf-next 05/11] netfilter: nf_tables: pass nft_chain to destroy function, not nft_ctx Florian Westphal
@ 2024-05-13 13:00 ` Florian Westphal
  2024-05-13 13:00 ` [PATCH nf-next 07/11] netfilter: nf_tables: avoid usage of embedded nft_ctx Florian Westphal
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Florian Westphal @ 2024-05-13 13:00 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

These functions pass a pointer to the base object type, use the
more specific one.  No functional change intended.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_tables_api.c | 51 ++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 00e5fdf8977b..5a40a8040539 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -446,6 +446,7 @@ nft_trans_alloc_chain(const struct nft_ctx *ctx, int msg_type)
 		struct nft_trans_chain *chain = nft_trans_container_chain(trans);
 
 		INIT_LIST_HEAD(&chain->nft_trans_binding.binding_list);
+		chain->chain = ctx->chain;
 	}
 
 	return trans;
@@ -467,7 +468,6 @@ static struct nft_trans *nft_trans_chain_add(struct nft_ctx *ctx, int msg_type)
 				ntohl(nla_get_be32(ctx->nla[NFTA_CHAIN_ID]));
 		}
 	}
-	nft_trans_chain(trans) = ctx->chain;
 	nft_trans_commit_list_add_tail(ctx->net, trans);
 
 	return trans;
@@ -2088,18 +2088,19 @@ static struct nft_stats __percpu *nft_stats_alloc(const struct nlattr *attr)
 	return newstats;
 }
 
-static void nft_chain_stats_replace(struct nft_trans *trans)
+static void nft_chain_stats_replace(struct nft_trans_chain *trans)
 {
-	struct nft_base_chain *chain = nft_base_chain(trans->ctx.chain);
+	const struct nft_trans *t = &trans->nft_trans_binding.nft_trans;
+	struct nft_base_chain *chain = nft_base_chain(trans->chain);
 
-	if (!nft_trans_chain_stats(trans))
+	if (!trans->stats)
 		return;
 
-	nft_trans_chain_stats(trans) =
-		rcu_replace_pointer(chain->stats, nft_trans_chain_stats(trans),
-				    lockdep_commit_lock_is_held(trans->ctx.net));
+	trans->stats =
+		rcu_replace_pointer(chain->stats, trans->stats,
+				    lockdep_commit_lock_is_held(t->ctx.net));
 
-	if (!nft_trans_chain_stats(trans))
+	if (!trans->stats)
 		static_branch_inc(&nft_counters_enabled);
 }
 
@@ -9455,47 +9456,47 @@ static int nf_tables_validate(struct net *net)
  *
  * We defer the drop policy until the transaction has been finalized.
  */
-static void nft_chain_commit_drop_policy(struct nft_trans *trans)
+static void nft_chain_commit_drop_policy(struct nft_trans_chain *trans)
 {
 	struct nft_base_chain *basechain;
 
-	if (nft_trans_chain_policy(trans) != NF_DROP)
+	if (trans->policy != NF_DROP)
 		return;
 
-	if (!nft_is_base_chain(trans->ctx.chain))
+	if (!nft_is_base_chain(trans->chain))
 		return;
 
-	basechain = nft_base_chain(trans->ctx.chain);
+	basechain = nft_base_chain(trans->chain);
 	basechain->policy = NF_DROP;
 }
 
-static void nft_chain_commit_update(struct nft_trans *trans)
+static void nft_chain_commit_update(struct nft_trans_chain *trans)
 {
-	struct nft_table *table = trans->ctx.table;
+	struct nft_table *table = trans->nft_trans_binding.nft_trans.ctx.table;
 	struct nft_base_chain *basechain;
 
-	if (nft_trans_chain_name(trans)) {
+	if (trans->name) {
 		rhltable_remove(&table->chains_ht,
-				&trans->ctx.chain->rhlhead,
+				&trans->chain->rhlhead,
 				nft_chain_ht_params);
-		swap(trans->ctx.chain->name, nft_trans_chain_name(trans));
+		swap(trans->chain->name, trans->name);
 		rhltable_insert_key(&table->chains_ht,
-				    trans->ctx.chain->name,
-				    &trans->ctx.chain->rhlhead,
+				    trans->chain->name,
+				    &trans->chain->rhlhead,
 				    nft_chain_ht_params);
 	}
 
-	if (!nft_is_base_chain(trans->ctx.chain))
+	if (!nft_is_base_chain(trans->chain))
 		return;
 
 	nft_chain_stats_replace(trans);
 
-	basechain = nft_base_chain(trans->ctx.chain);
+	basechain = nft_base_chain(trans->chain);
 
-	switch (nft_trans_chain_policy(trans)) {
+	switch (trans->policy) {
 	case NF_DROP:
 	case NF_ACCEPT:
-		basechain->policy = nft_trans_chain_policy(trans);
+		basechain->policy = trans->policy;
 		break;
 	}
 }
@@ -10308,14 +10309,14 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 			break;
 		case NFT_MSG_NEWCHAIN:
 			if (nft_trans_chain_update(trans)) {
-				nft_chain_commit_update(trans);
+				nft_chain_commit_update(nft_trans_container_chain(trans));
 				nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN,
 						       &nft_trans_chain_hooks(trans));
 				list_splice(&nft_trans_chain_hooks(trans),
 					    &nft_trans_basechain(trans)->hook_list);
 				/* trans destroyed after rcu grace period */
 			} else {
-				nft_chain_commit_drop_policy(trans);
+				nft_chain_commit_drop_policy(nft_trans_container_chain(trans));
 				nft_clear(net, trans->ctx.chain);
 				nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN, NULL);
 				nft_trans_destroy(trans);
-- 
2.43.2


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

* [PATCH nf-next 07/11] netfilter: nf_tables: avoid usage of embedded nft_ctx
  2024-05-13 13:00 [PATCH nf-next 00/11] netfilter: nf_tables: reduce transaction log memory usage Florian Westphal
                   ` (5 preceding siblings ...)
  2024-05-13 13:00 ` [PATCH nf-next 06/11] netfilter: nf_tables: pass more specific nft_trans_chain where possible Florian Westphal
@ 2024-05-13 13:00 ` Florian Westphal
  2024-05-13 13:00 ` [PATCH nf-next 08/11] netfilter: nf_tables: store chain pointer in rule transaction Florian Westphal
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Florian Westphal @ 2024-05-13 13:00 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

nft_ctx is stored in nft_trans object, but nft_ctx is large
(48 bytes on 64-bit platforms), it should not be embedded in
the transaction structures.

Reduce its usage so we can remove it eventually.

This replaces trans->ctx.chain with the chain pointer
already available in nft_trans_chain structure.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_tables_api.c     | 10 +++++-----
 net/netfilter/nf_tables_offload.c | 16 ++++++++--------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 5a40a8040539..044007893e79 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -9681,10 +9681,10 @@ static void nf_tables_commit_chain_prepare_cancel(struct net *net)
 	struct nft_trans *trans, *next;
 
 	list_for_each_entry_safe(trans, next, &nft_net->commit_list, list) {
-		struct nft_chain *chain = trans->ctx.chain;
-
 		if (trans->msg_type == NFT_MSG_NEWRULE ||
 		    trans->msg_type == NFT_MSG_DELRULE) {
+			struct nft_chain *chain = trans->ctx.chain;
+
 			kvfree(chain->blob_next);
 			chain->blob_next = NULL;
 		}
@@ -10317,7 +10317,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 				/* trans destroyed after rcu grace period */
 			} else {
 				nft_chain_commit_drop_policy(nft_trans_container_chain(trans));
-				nft_clear(net, trans->ctx.chain);
+				nft_clear(net, nft_trans_chain(trans));
 				nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN, NULL);
 				nft_trans_destroy(trans);
 			}
@@ -10333,11 +10333,11 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 								    true);
 				}
 			} else {
-				nft_chain_del(trans->ctx.chain);
+				nft_chain_del(nft_trans_chain(trans));
 				nf_tables_chain_notify(&trans->ctx, NFT_MSG_DELCHAIN,
 						       NULL);
 				nf_tables_unregister_hook(trans->ctx.net, table,
-							  trans->ctx.chain);
+							  nft_trans_chain(trans));
 			}
 			break;
 		case NFT_MSG_NEWRULE:
diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
index 12ab78fa5d84..8d892a0d2438 100644
--- a/net/netfilter/nf_tables_offload.c
+++ b/net/netfilter/nf_tables_offload.c
@@ -518,18 +518,18 @@ static void nft_flow_rule_offload_abort(struct net *net,
 
 		switch (trans->msg_type) {
 		case NFT_MSG_NEWCHAIN:
-			if (!(trans->ctx.chain->flags & NFT_CHAIN_HW_OFFLOAD) ||
+			if (!(nft_trans_chain(trans)->flags & NFT_CHAIN_HW_OFFLOAD) ||
 			    nft_trans_chain_update(trans))
 				continue;
 
-			err = nft_flow_offload_chain(trans->ctx.chain, NULL,
+			err = nft_flow_offload_chain(nft_trans_chain(trans), NULL,
 						     FLOW_BLOCK_UNBIND);
 			break;
 		case NFT_MSG_DELCHAIN:
-			if (!(trans->ctx.chain->flags & NFT_CHAIN_HW_OFFLOAD))
+			if (!(nft_trans_chain(trans)->flags & NFT_CHAIN_HW_OFFLOAD))
 				continue;
 
-			err = nft_flow_offload_chain(trans->ctx.chain, NULL,
+			err = nft_flow_offload_chain(nft_trans_chain(trans), NULL,
 						     FLOW_BLOCK_BIND);
 			break;
 		case NFT_MSG_NEWRULE:
@@ -569,20 +569,20 @@ int nft_flow_rule_offload_commit(struct net *net)
 
 		switch (trans->msg_type) {
 		case NFT_MSG_NEWCHAIN:
-			if (!(trans->ctx.chain->flags & NFT_CHAIN_HW_OFFLOAD) ||
+			if (!(nft_trans_chain(trans)->flags & NFT_CHAIN_HW_OFFLOAD) ||
 			    nft_trans_chain_update(trans))
 				continue;
 
 			policy = nft_trans_chain_policy(trans);
-			err = nft_flow_offload_chain(trans->ctx.chain, &policy,
+			err = nft_flow_offload_chain(nft_trans_chain(trans), &policy,
 						     FLOW_BLOCK_BIND);
 			break;
 		case NFT_MSG_DELCHAIN:
-			if (!(trans->ctx.chain->flags & NFT_CHAIN_HW_OFFLOAD))
+			if (!(nft_trans_chain(trans)->flags & NFT_CHAIN_HW_OFFLOAD))
 				continue;
 
 			policy = nft_trans_chain_policy(trans);
-			err = nft_flow_offload_chain(trans->ctx.chain, &policy,
+			err = nft_flow_offload_chain(nft_trans_chain(trans), &policy,
 						     FLOW_BLOCK_UNBIND);
 			break;
 		case NFT_MSG_NEWRULE:
-- 
2.43.2


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

* [PATCH nf-next 08/11] netfilter: nf_tables: store chain pointer in rule transaction
  2024-05-13 13:00 [PATCH nf-next 00/11] netfilter: nf_tables: reduce transaction log memory usage Florian Westphal
                   ` (6 preceding siblings ...)
  2024-05-13 13:00 ` [PATCH nf-next 07/11] netfilter: nf_tables: avoid usage of embedded nft_ctx Florian Westphal
@ 2024-05-13 13:00 ` Florian Westphal
  2024-05-13 13:00 ` [PATCH nf-next 09/11] netfilter: nf_tables: reduce trans->ctx.chain references Florian Westphal
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Florian Westphal @ 2024-05-13 13:00 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

Currently the chain can be derived from trans->ctx.chain, but
the ctx will go away soon.

Thus add the chain pointer to nft_trans_rule structure itself.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/net/netfilter/nf_tables.h |  3 +++
 net/netfilter/nf_tables_api.c     | 21 +++++++++++----------
 net/netfilter/nf_tables_offload.c | 16 ++++++++--------
 3 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 3ae09dca65ad..11a38fcf4dec 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1640,6 +1640,7 @@ struct nft_trans_binding {
 struct nft_trans_rule {
 	struct nft_trans		nft_trans;
 	struct nft_rule			*rule;
+	struct nft_chain		*chain;
 	struct nft_flow_rule		*flow;
 	u32				rule_id;
 	bool				bound;
@@ -1653,6 +1654,8 @@ struct nft_trans_rule {
 	container_of(trans, struct nft_trans_rule, nft_trans)->rule_id
 #define nft_trans_rule_bound(trans)	\
 	container_of(trans, struct nft_trans_rule, nft_trans)->bound
+#define nft_trans_rule_chain(trans)	\
+	container_of(trans, struct nft_trans_rule, nft_trans)->chain
 
 struct nft_trans_set {
 	struct nft_trans_binding	nft_trans_binding;
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 044007893e79..9d8fc31bbfc6 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -251,7 +251,7 @@ static void __nft_chain_trans_bind(const struct nft_ctx *ctx,
 				nft_trans_chain_bound(trans) = bind;
 			break;
 		case NFT_MSG_NEWRULE:
-			if (trans->ctx.chain == chain)
+			if (nft_trans_rule_chain(trans) == chain)
 				nft_trans_rule_bound(trans) = bind;
 			break;
 		}
@@ -540,6 +540,7 @@ static struct nft_trans *nft_trans_rule_add(struct nft_ctx *ctx, int msg_type,
 			ntohl(nla_get_be32(ctx->nla[NFTA_RULE_ID]));
 	}
 	nft_trans_rule(trans) = rule;
+	nft_trans_rule_chain(trans) = ctx->chain;
 	nft_trans_commit_list_add_tail(ctx->net, trans);
 
 	return trans;
@@ -4226,7 +4227,7 @@ static struct nft_rule *nft_rule_lookup_byid(const struct net *net,
 
 	list_for_each_entry(trans, &nft_net->commit_list, list) {
 		if (trans->msg_type == NFT_MSG_NEWRULE &&
-		    trans->ctx.chain == chain &&
+		    nft_trans_rule_chain(trans) == chain &&
 		    id == nft_trans_rule_id(trans))
 			return nft_trans_rule(trans);
 	}
@@ -9683,7 +9684,7 @@ static void nf_tables_commit_chain_prepare_cancel(struct net *net)
 	list_for_each_entry_safe(trans, next, &nft_net->commit_list, list) {
 		if (trans->msg_type == NFT_MSG_NEWRULE ||
 		    trans->msg_type == NFT_MSG_DELRULE) {
-			struct nft_chain *chain = trans->ctx.chain;
+			struct nft_chain *chain = nft_trans_rule_chain(trans);
 
 			kvfree(chain->blob_next);
 			chain->blob_next = NULL;
@@ -10249,7 +10250,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 		}
 		if (trans->msg_type == NFT_MSG_NEWRULE ||
 		    trans->msg_type == NFT_MSG_DELRULE) {
-			chain = trans->ctx.chain;
+			chain = nft_trans_rule_chain(trans);
 
 			ret = nf_tables_commit_chain_prepare(net, chain);
 			if (ret < 0) {
@@ -10345,7 +10346,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 			nf_tables_rule_notify(&trans->ctx,
 					      nft_trans_rule(trans),
 					      NFT_MSG_NEWRULE);
-			if (trans->ctx.chain->flags & NFT_CHAIN_HW_OFFLOAD)
+			if (nft_trans_rule_chain(trans)->flags & NFT_CHAIN_HW_OFFLOAD)
 				nft_flow_rule_destroy(nft_trans_flow_rule(trans));
 
 			nft_trans_destroy(trans);
@@ -10360,7 +10361,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 						 nft_trans_rule(trans),
 						 NFT_TRANS_COMMIT);
 
-			if (trans->ctx.chain->flags & NFT_CHAIN_HW_OFFLOAD)
+			if (nft_trans_rule_chain(trans)->flags & NFT_CHAIN_HW_OFFLOAD)
 				nft_flow_rule_destroy(nft_trans_flow_rule(trans));
 			break;
 		case NFT_MSG_NEWSET:
@@ -10644,20 +10645,20 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 				nft_trans_destroy(trans);
 				break;
 			}
-			nft_use_dec_restore(&trans->ctx.chain->use);
+			nft_use_dec_restore(&nft_trans_rule_chain(trans)->use);
 			list_del_rcu(&nft_trans_rule(trans)->list);
 			nft_rule_expr_deactivate(&trans->ctx,
 						 nft_trans_rule(trans),
 						 NFT_TRANS_ABORT);
-			if (trans->ctx.chain->flags & NFT_CHAIN_HW_OFFLOAD)
+			if (nft_trans_rule_chain(trans)->flags & NFT_CHAIN_HW_OFFLOAD)
 				nft_flow_rule_destroy(nft_trans_flow_rule(trans));
 			break;
 		case NFT_MSG_DELRULE:
 		case NFT_MSG_DESTROYRULE:
-			nft_use_inc_restore(&trans->ctx.chain->use);
+			nft_use_inc_restore(&nft_trans_rule_chain(trans)->use);
 			nft_clear(trans->ctx.net, nft_trans_rule(trans));
 			nft_rule_expr_activate(&trans->ctx, nft_trans_rule(trans));
-			if (trans->ctx.chain->flags & NFT_CHAIN_HW_OFFLOAD)
+			if (nft_trans_rule_chain(trans)->flags & NFT_CHAIN_HW_OFFLOAD)
 				nft_flow_rule_destroy(nft_trans_flow_rule(trans));
 
 			nft_trans_destroy(trans);
diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
index 8d892a0d2438..0619feb10abb 100644
--- a/net/netfilter/nf_tables_offload.c
+++ b/net/netfilter/nf_tables_offload.c
@@ -533,18 +533,18 @@ static void nft_flow_rule_offload_abort(struct net *net,
 						     FLOW_BLOCK_BIND);
 			break;
 		case NFT_MSG_NEWRULE:
-			if (!(trans->ctx.chain->flags & NFT_CHAIN_HW_OFFLOAD))
+			if (!(nft_trans_rule_chain(trans)->flags & NFT_CHAIN_HW_OFFLOAD))
 				continue;
 
-			err = nft_flow_offload_rule(trans->ctx.chain,
+			err = nft_flow_offload_rule(nft_trans_rule_chain(trans),
 						    nft_trans_rule(trans),
 						    NULL, FLOW_CLS_DESTROY);
 			break;
 		case NFT_MSG_DELRULE:
-			if (!(trans->ctx.chain->flags & NFT_CHAIN_HW_OFFLOAD))
+			if (!(nft_trans_rule_chain(trans)->flags & NFT_CHAIN_HW_OFFLOAD))
 				continue;
 
-			err = nft_flow_offload_rule(trans->ctx.chain,
+			err = nft_flow_offload_rule(nft_trans_rule_chain(trans),
 						    nft_trans_rule(trans),
 						    nft_trans_flow_rule(trans),
 						    FLOW_CLS_REPLACE);
@@ -586,7 +586,7 @@ int nft_flow_rule_offload_commit(struct net *net)
 						     FLOW_BLOCK_UNBIND);
 			break;
 		case NFT_MSG_NEWRULE:
-			if (!(trans->ctx.chain->flags & NFT_CHAIN_HW_OFFLOAD))
+			if (!(nft_trans_rule_chain(trans)->flags & NFT_CHAIN_HW_OFFLOAD))
 				continue;
 
 			if (trans->ctx.flags & NLM_F_REPLACE ||
@@ -594,16 +594,16 @@ int nft_flow_rule_offload_commit(struct net *net)
 				err = -EOPNOTSUPP;
 				break;
 			}
-			err = nft_flow_offload_rule(trans->ctx.chain,
+			err = nft_flow_offload_rule(nft_trans_rule_chain(trans),
 						    nft_trans_rule(trans),
 						    nft_trans_flow_rule(trans),
 						    FLOW_CLS_REPLACE);
 			break;
 		case NFT_MSG_DELRULE:
-			if (!(trans->ctx.chain->flags & NFT_CHAIN_HW_OFFLOAD))
+			if (!(nft_trans_rule_chain(trans)->flags & NFT_CHAIN_HW_OFFLOAD))
 				continue;
 
-			err = nft_flow_offload_rule(trans->ctx.chain,
+			err = nft_flow_offload_rule(nft_trans_rule_chain(trans),
 						    nft_trans_rule(trans),
 						    NULL, FLOW_CLS_DESTROY);
 			break;
-- 
2.43.2


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

* [PATCH nf-next 09/11] netfilter: nf_tables: reduce trans->ctx.chain references
  2024-05-13 13:00 [PATCH nf-next 00/11] netfilter: nf_tables: reduce transaction log memory usage Florian Westphal
                   ` (7 preceding siblings ...)
  2024-05-13 13:00 ` [PATCH nf-next 08/11] netfilter: nf_tables: store chain pointer in rule transaction Florian Westphal
@ 2024-05-13 13:00 ` Florian Westphal
  2024-05-13 13:00 ` [PATCH nf-next 10/11] netfilter: nf_tables: pass nft_table to destroy function Florian Westphal
  2024-05-13 13:00 ` [PATCH nf-next 11/11] netfilter: nf_tables: do not store nft_ctx in transaction objects Florian Westphal
  10 siblings, 0 replies; 20+ messages in thread
From: Florian Westphal @ 2024-05-13 13:00 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

These objects are the trans_chain subtype, so use the helper instead
of referencing trans->ctx, which will be removed soon.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_tables_api.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 9d8fc31bbfc6..70d65a09787b 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1261,7 +1261,7 @@ static bool nft_table_pending_update(const struct nft_ctx *ctx)
 		    ((trans->msg_type == NFT_MSG_NEWCHAIN &&
 		      nft_trans_chain_update(trans)) ||
 		     (trans->msg_type == NFT_MSG_DELCHAIN &&
-		      nft_is_base_chain(trans->ctx.chain))))
+		      nft_is_base_chain(nft_trans_chain(trans)))))
 			return true;
 	}
 
@@ -2814,13 +2814,11 @@ static struct nft_chain *nft_chain_lookup_byid(const struct net *net,
 	struct nft_trans *trans;
 
 	list_for_each_entry(trans, &nft_net->commit_list, list) {
-		struct nft_chain *chain = trans->ctx.chain;
-
 		if (trans->msg_type == NFT_MSG_NEWCHAIN &&
-		    chain->table == table &&
+		    nft_trans_chain(trans)->table == table &&
 		    id == nft_trans_chain_id(trans) &&
-		    nft_active_genmask(chain, genmask))
-			return chain;
+		    nft_active_genmask(nft_trans_chain(trans), genmask))
+			return nft_trans_chain(trans);
 	}
 	return ERR_PTR(-ENOENT);
 }
@@ -10624,9 +10622,9 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 					break;
 				}
 				nft_use_dec_restore(&table->use);
-				nft_chain_del(trans->ctx.chain);
+				nft_chain_del(nft_trans_chain(trans));
 				nf_tables_unregister_hook(trans->ctx.net, table,
-							  trans->ctx.chain);
+							  nft_trans_chain(trans));
 			}
 			break;
 		case NFT_MSG_DELCHAIN:
@@ -10636,7 +10634,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 					    &nft_trans_basechain(trans)->hook_list);
 			} else {
 				nft_use_inc_restore(&table->use);
-				nft_clear(trans->ctx.net, trans->ctx.chain);
+				nft_clear(trans->ctx.net, nft_trans_chain(trans));
 			}
 			nft_trans_destroy(trans);
 			break;
-- 
2.43.2


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

* [PATCH nf-next 10/11] netfilter: nf_tables: pass nft_table to destroy function
  2024-05-13 13:00 [PATCH nf-next 00/11] netfilter: nf_tables: reduce transaction log memory usage Florian Westphal
                   ` (8 preceding siblings ...)
  2024-05-13 13:00 ` [PATCH nf-next 09/11] netfilter: nf_tables: reduce trans->ctx.chain references Florian Westphal
@ 2024-05-13 13:00 ` Florian Westphal
  2024-05-13 13:00 ` [PATCH nf-next 11/11] netfilter: nf_tables: do not store nft_ctx in transaction objects Florian Westphal
  10 siblings, 0 replies; 20+ messages in thread
From: Florian Westphal @ 2024-05-13 13:00 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

No functional change intended.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_tables_api.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 70d65a09787b..769c5ced7f2e 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1655,15 +1655,15 @@ static int nf_tables_deltable(struct sk_buff *skb, const struct nfnl_info *info,
 	return nft_flush_table(&ctx);
 }
 
-static void nf_tables_table_destroy(struct nft_ctx *ctx)
+static void nf_tables_table_destroy(struct nft_table *table)
 {
-	if (WARN_ON(ctx->table->use > 0))
+	if (WARN_ON(table->use > 0))
 		return;
 
-	rhltable_destroy(&ctx->table->chains_ht);
-	kfree(ctx->table->name);
-	kfree(ctx->table->udata);
-	kfree(ctx->table);
+	rhltable_destroy(&table->chains_ht);
+	kfree(table->name);
+	kfree(table->udata);
+	kfree(table);
 }
 
 void nft_register_chain_type(const struct nft_chain_type *ctype)
@@ -9520,7 +9520,7 @@ static void nft_commit_release(struct nft_trans *trans)
 	switch (trans->msg_type) {
 	case NFT_MSG_DELTABLE:
 	case NFT_MSG_DESTROYTABLE:
-		nf_tables_table_destroy(&trans->ctx);
+		nf_tables_table_destroy(trans->ctx.table);
 		break;
 	case NFT_MSG_NEWCHAIN:
 		free_percpu(nft_trans_chain_stats(trans));
@@ -10517,7 +10517,7 @@ static void nf_tables_abort_release(struct nft_trans *trans)
 {
 	switch (trans->msg_type) {
 	case NFT_MSG_NEWTABLE:
-		nf_tables_table_destroy(&trans->ctx);
+		nf_tables_table_destroy(trans->ctx.table);
 		break;
 	case NFT_MSG_NEWCHAIN:
 		if (nft_trans_chain_update(trans))
@@ -11489,7 +11489,7 @@ static void __nft_release_table(struct net *net, struct nft_table *table)
 		nft_use_dec(&table->use);
 		nf_tables_chain_destroy(chain);
 	}
-	nf_tables_table_destroy(&ctx);
+	nf_tables_table_destroy(table);
 }
 
 static void __nft_release_tables(struct net *net)
-- 
2.43.2


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

* [PATCH nf-next 11/11] netfilter: nf_tables: do not store nft_ctx in transaction objects
  2024-05-13 13:00 [PATCH nf-next 00/11] netfilter: nf_tables: reduce transaction log memory usage Florian Westphal
                   ` (9 preceding siblings ...)
  2024-05-13 13:00 ` [PATCH nf-next 10/11] netfilter: nf_tables: pass nft_table to destroy function Florian Westphal
@ 2024-05-13 13:00 ` Florian Westphal
  10 siblings, 0 replies; 20+ messages in thread
From: Florian Westphal @ 2024-05-13 13:00 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

nft_ctx is huge and most of the information stored within isn't used
at all.

Remove nft_ctx member from the base transaction structure and store
only what is needed.

After this change, relevant struct sizes are:

struct nft_trans_chain { /* size: 120 (-32), cachelines: 2, members: 10 */
struct nft_trans_elem { /* size: 72 (-40), cachelines: 2, members: 4 */
struct nft_trans_flowtable { /* size: 80 (-48), cachelines: 2, members: 5 */
struct nft_trans_obj { /* size: 72 (-40), cachelines: 2, members: 4 */
struct nft_trans_rule { /* size: 80 (-32), cachelines: 2, members: 6 */
struct nft_trans_set { /* size: 96 (-24), cachelines: 2, members: 8 */
struct nft_trans_table { /* size: 56 (-40), cachelines: 1, members: 2 */

struct nft_trans_elem can now be allocated from kmalloc-96 instead of
kmalloc-128 slab.
A further reduction by 8 bytes would even allow for kmalloc-64.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/net/netfilter/nf_tables.h |  43 ++++++++-
 net/netfilter/nf_tables_api.c     | 140 +++++++++++++++++-------------
 net/netfilter/nf_tables_offload.c |   8 +-
 3 files changed, 125 insertions(+), 66 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 11a38fcf4dec..8ca4a027f0e5 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1611,18 +1611,26 @@ static inline int nft_set_elem_is_dead(const struct nft_set_ext *ext)
  * struct nft_trans - nf_tables object update in transaction
  *
  * @list: used internally
+ * @net: struct net
+ * @table: struct nft_table the object resides in
  * @msg_type: message type
- * @put_net: ctx->net needs to be put
- * @ctx: transaction context
+ * @seq: netlink sequence number
+ * @flags: modifiers to new request
+ * @report: notify via unicast netlink message
+ * @put_net: net needs to be put
  *
  * This is the information common to all objects in the transaction,
  * this must always be the first member of derived sub-types.
  */
 struct nft_trans {
 	struct list_head		list;
+	struct net			*net;
+	struct nft_table		*table;
 	int				msg_type;
-	bool				put_net;
-	struct nft_ctx			ctx;
+	u32				seq;
+	u16				flags;
+	u8				report:1;
+	u8				put_net:1;
 };
 
 /**
@@ -1786,6 +1794,33 @@ struct nft_trans_gc {
 	struct rcu_head		rcu;
 };
 
+static inline void nft_ctx_update(struct nft_ctx *ctx,
+				  const struct nft_trans *trans)
+{
+	switch (trans->msg_type) {
+	case NFT_MSG_NEWRULE:
+	case NFT_MSG_DELRULE:
+	case NFT_MSG_DESTROYRULE:
+		ctx->chain = nft_trans_rule_chain(trans);
+		break;
+	case NFT_MSG_NEWCHAIN:
+	case NFT_MSG_DELCHAIN:
+	case NFT_MSG_DESTROYCHAIN:
+		ctx->chain = nft_trans_chain(trans);
+		break;
+	default:
+		ctx->chain = NULL;
+		break;
+	}
+
+	ctx->net = trans->net;
+	ctx->table = trans->table;
+	ctx->family = trans->table->family;
+	ctx->report = trans->report;
+	ctx->flags = trans->flags;
+	ctx->seq = trans->seq;
+}
+
 struct nft_trans_gc *nft_trans_gc_alloc(struct nft_set *set,
 					unsigned int gc_seq, gfp_t gfp);
 void nft_trans_gc_destroy(struct nft_trans_gc *trans);
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 769c5ced7f2e..6e9cbaca6009 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -159,7 +159,12 @@ static struct nft_trans *nft_trans_alloc_gfp(const struct nft_ctx *ctx,
 
 	INIT_LIST_HEAD(&trans->list);
 	trans->msg_type = msg_type;
-	trans->ctx	= *ctx;
+
+	trans->net = ctx->net;
+	trans->table = ctx->table;
+	trans->seq = ctx->seq;
+	trans->flags = ctx->flags;
+	trans->report = ctx->report;
 
 	return trans;
 }
@@ -1257,7 +1262,7 @@ static bool nft_table_pending_update(const struct nft_ctx *ctx)
 		return true;
 
 	list_for_each_entry(trans, &nft_net->commit_list, list) {
-		if (trans->ctx.table == ctx->table &&
+		if (trans->table == ctx->table &&
 		    ((trans->msg_type == NFT_MSG_NEWCHAIN &&
 		      nft_trans_chain_update(trans)) ||
 		     (trans->msg_type == NFT_MSG_DELCHAIN &&
@@ -2099,7 +2104,7 @@ static void nft_chain_stats_replace(struct nft_trans_chain *trans)
 
 	trans->stats =
 		rcu_replace_pointer(chain->stats, trans->stats,
-				    lockdep_commit_lock_is_held(t->ctx.net));
+				    lockdep_commit_lock_is_held(t->net));
 
 	if (!trans->stats)
 		static_branch_inc(&nft_counters_enabled);
@@ -2765,7 +2770,7 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
 		err = -EEXIST;
 		list_for_each_entry(tmp, &nft_net->commit_list, list) {
 			if (tmp->msg_type == NFT_MSG_NEWCHAIN &&
-			    tmp->ctx.table == table &&
+			    tmp->table == table &&
 			    nft_trans_chain_update(tmp) &&
 			    nft_trans_chain_name(tmp) &&
 			    strcmp(name, nft_trans_chain_name(tmp)) == 0) {
@@ -9471,7 +9476,7 @@ static void nft_chain_commit_drop_policy(struct nft_trans_chain *trans)
 
 static void nft_chain_commit_update(struct nft_trans_chain *trans)
 {
-	struct nft_table *table = trans->nft_trans_binding.nft_trans.ctx.table;
+	struct nft_table *table = trans->nft_trans_binding.nft_trans.table;
 	struct nft_base_chain *basechain;
 
 	if (trans->name) {
@@ -9500,7 +9505,8 @@ static void nft_chain_commit_update(struct nft_trans_chain *trans)
 	}
 }
 
-static void nft_obj_commit_update(struct nft_trans *trans)
+static void nft_obj_commit_update(const struct nft_ctx *ctx,
+				  struct nft_trans *trans)
 {
 	struct nft_object *newobj;
 	struct nft_object *obj;
@@ -9512,15 +9518,21 @@ static void nft_obj_commit_update(struct nft_trans *trans)
 		return;
 
 	obj->ops->update(obj, newobj);
-	nft_obj_destroy(&trans->ctx, newobj);
+	nft_obj_destroy(ctx, newobj);
 }
 
 static void nft_commit_release(struct nft_trans *trans)
 {
+	struct nft_ctx ctx = {
+		.net = trans->net,
+	};
+
+	nft_ctx_update(&ctx, trans);
+
 	switch (trans->msg_type) {
 	case NFT_MSG_DELTABLE:
 	case NFT_MSG_DESTROYTABLE:
-		nf_tables_table_destroy(trans->ctx.table);
+		nf_tables_table_destroy(trans->table);
 		break;
 	case NFT_MSG_NEWCHAIN:
 		free_percpu(nft_trans_chain_stats(trans));
@@ -9535,21 +9547,21 @@ static void nft_commit_release(struct nft_trans *trans)
 		break;
 	case NFT_MSG_DELRULE:
 	case NFT_MSG_DESTROYRULE:
-		nf_tables_rule_destroy(&trans->ctx, nft_trans_rule(trans));
+		nf_tables_rule_destroy(&ctx, nft_trans_rule(trans));
 		break;
 	case NFT_MSG_DELSET:
 	case NFT_MSG_DESTROYSET:
-		nft_set_destroy(&trans->ctx, nft_trans_set(trans));
+		nft_set_destroy(&ctx, nft_trans_set(trans));
 		break;
 	case NFT_MSG_DELSETELEM:
 	case NFT_MSG_DESTROYSETELEM:
-		nf_tables_set_elem_destroy(&trans->ctx,
+		nf_tables_set_elem_destroy(&ctx,
 					   nft_trans_elem_set(trans),
 					   nft_trans_elem_priv(trans));
 		break;
 	case NFT_MSG_DELOBJ:
 	case NFT_MSG_DESTROYOBJ:
-		nft_obj_destroy(&trans->ctx, nft_trans_obj(trans));
+		nft_obj_destroy(&ctx, nft_trans_obj(trans));
 		break;
 	case NFT_MSG_DELFLOWTABLE:
 	case NFT_MSG_DESTROYFLOWTABLE:
@@ -9561,7 +9573,7 @@ static void nft_commit_release(struct nft_trans *trans)
 	}
 
 	if (trans->put_net)
-		put_net(trans->ctx.net);
+		put_net(trans->net);
 
 	kfree(trans);
 }
@@ -10041,7 +10053,7 @@ static void nf_tables_commit_release(struct net *net)
 
 	trans = list_last_entry(&nft_net->commit_list,
 				struct nft_trans, list);
-	get_net(trans->ctx.net);
+	get_net(trans->net);
 	WARN_ON_ONCE(trans->put_net);
 
 	trans->put_net = true;
@@ -10185,6 +10197,7 @@ static void nft_gc_seq_end(struct nftables_pernet *nft_net, unsigned int gc_seq)
 static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 {
 	struct nftables_pernet *nft_net = nft_pernet(net);
+	const struct nlmsghdr *nlh = nlmsg_hdr(skb);
 	struct nft_trans_binding *trans_binding;
 	struct nft_trans *trans, *next;
 	unsigned int base_seq, gc_seq;
@@ -10192,6 +10205,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 	struct nft_trans_elem *te;
 	struct nft_chain *chain;
 	struct nft_table *table;
+	struct nft_ctx ctx;
 	LIST_HEAD(adl);
 	int err;
 
@@ -10200,6 +10214,8 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 		return 0;
 	}
 
+	nft_ctx_init(&ctx, net, skb, nlh, NFPROTO_UNSPEC, NULL, NULL, NULL);
+
 	list_for_each_entry(trans_binding, &nft_net->binding_list, binding_list) {
 		trans = &trans_binding->nft_trans;
 		switch (trans->msg_type) {
@@ -10237,7 +10253,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 
 	/* 1.  Allocate space for next generation rules_gen_X[] */
 	list_for_each_entry_safe(trans, next, &nft_net->commit_list, list) {
-		struct nft_table *table = trans->ctx.table;
+		struct nft_table *table = trans->table;
 		int ret;
 
 		ret = nf_tables_commit_audit_alloc(&adl, table);
@@ -10281,7 +10297,9 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 	net->nft.gencursor = nft_gencursor_next(net);
 
 	list_for_each_entry_safe(trans, next, &nft_net->commit_list, list) {
-		struct nft_table *table = trans->ctx.table;
+		struct nft_table *table = trans->table;
+
+		nft_ctx_update(&ctx, trans);
 
 		nf_tables_commit_audit_collect(&adl, table, trans->msg_type);
 		switch (trans->msg_type) {
@@ -10298,18 +10316,18 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 			} else {
 				nft_clear(net, table);
 			}
-			nf_tables_table_notify(&trans->ctx, NFT_MSG_NEWTABLE);
+			nf_tables_table_notify(&ctx, NFT_MSG_NEWTABLE);
 			nft_trans_destroy(trans);
 			break;
 		case NFT_MSG_DELTABLE:
 		case NFT_MSG_DESTROYTABLE:
 			list_del_rcu(&table->list);
-			nf_tables_table_notify(&trans->ctx, trans->msg_type);
+			nf_tables_table_notify(&ctx, trans->msg_type);
 			break;
 		case NFT_MSG_NEWCHAIN:
 			if (nft_trans_chain_update(trans)) {
 				nft_chain_commit_update(nft_trans_container_chain(trans));
-				nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN,
+				nf_tables_chain_notify(&ctx, NFT_MSG_NEWCHAIN,
 						       &nft_trans_chain_hooks(trans));
 				list_splice(&nft_trans_chain_hooks(trans),
 					    &nft_trans_basechain(trans)->hook_list);
@@ -10317,14 +10335,14 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 			} else {
 				nft_chain_commit_drop_policy(nft_trans_container_chain(trans));
 				nft_clear(net, nft_trans_chain(trans));
-				nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN, NULL);
+				nf_tables_chain_notify(&ctx, NFT_MSG_NEWCHAIN, NULL);
 				nft_trans_destroy(trans);
 			}
 			break;
 		case NFT_MSG_DELCHAIN:
 		case NFT_MSG_DESTROYCHAIN:
 			if (nft_trans_chain_update(trans)) {
-				nf_tables_chain_notify(&trans->ctx, NFT_MSG_DELCHAIN,
+				nf_tables_chain_notify(&ctx, NFT_MSG_DELCHAIN,
 						       &nft_trans_chain_hooks(trans));
 				if (!(table->flags & NFT_TABLE_F_DORMANT)) {
 					nft_netdev_unregister_hooks(net,
@@ -10333,16 +10351,15 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 				}
 			} else {
 				nft_chain_del(nft_trans_chain(trans));
-				nf_tables_chain_notify(&trans->ctx, NFT_MSG_DELCHAIN,
+				nf_tables_chain_notify(&ctx, NFT_MSG_DELCHAIN,
 						       NULL);
-				nf_tables_unregister_hook(trans->ctx.net, table,
+				nf_tables_unregister_hook(ctx.net, ctx.table,
 							  nft_trans_chain(trans));
 			}
 			break;
 		case NFT_MSG_NEWRULE:
-			nft_clear(trans->ctx.net, nft_trans_rule(trans));
-			nf_tables_rule_notify(&trans->ctx,
-					      nft_trans_rule(trans),
+			nft_clear(net, nft_trans_rule(trans));
+			nf_tables_rule_notify(&ctx, nft_trans_rule(trans),
 					      NFT_MSG_NEWRULE);
 			if (nft_trans_rule_chain(trans)->flags & NFT_CHAIN_HW_OFFLOAD)
 				nft_flow_rule_destroy(nft_trans_flow_rule(trans));
@@ -10352,11 +10369,9 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 		case NFT_MSG_DELRULE:
 		case NFT_MSG_DESTROYRULE:
 			list_del_rcu(&nft_trans_rule(trans)->list);
-			nf_tables_rule_notify(&trans->ctx,
-					      nft_trans_rule(trans),
+			nf_tables_rule_notify(&ctx, nft_trans_rule(trans),
 					      trans->msg_type);
-			nft_rule_expr_deactivate(&trans->ctx,
-						 nft_trans_rule(trans),
+			nft_rule_expr_deactivate(&ctx, nft_trans_rule(trans),
 						 NFT_TRANS_COMMIT);
 
 			if (nft_trans_rule_chain(trans)->flags & NFT_CHAIN_HW_OFFLOAD)
@@ -10380,7 +10395,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 				    !list_empty(&nft_trans_set(trans)->bindings))
 					nft_use_dec(&table->use);
 			}
-			nf_tables_set_notify(&trans->ctx, nft_trans_set(trans),
+			nf_tables_set_notify(&ctx, nft_trans_set(trans),
 					     NFT_MSG_NEWSET, GFP_KERNEL);
 			nft_trans_destroy(trans);
 			break;
@@ -10388,14 +10403,14 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 		case NFT_MSG_DESTROYSET:
 			nft_trans_set(trans)->dead = 1;
 			list_del_rcu(&nft_trans_set(trans)->list);
-			nf_tables_set_notify(&trans->ctx, nft_trans_set(trans),
+			nf_tables_set_notify(&ctx, nft_trans_set(trans),
 					     trans->msg_type, GFP_KERNEL);
 			break;
 		case NFT_MSG_NEWSETELEM:
 			te = nft_trans_container_elem(trans);
 
 			nft_setelem_activate(net, te->set, te->elem_priv);
-			nf_tables_setelem_notify(&trans->ctx, te->set,
+			nf_tables_setelem_notify(&ctx, te->set,
 						 te->elem_priv,
 						 NFT_MSG_NEWSETELEM);
 			if (te->set->ops->commit &&
@@ -10409,7 +10424,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 		case NFT_MSG_DESTROYSETELEM:
 			te = nft_trans_container_elem(trans);
 
-			nf_tables_setelem_notify(&trans->ctx, te->set,
+			nf_tables_setelem_notify(&ctx, te->set,
 						 te->elem_priv,
 						 trans->msg_type);
 			nft_setelem_remove(net, te->set, te->elem_priv);
@@ -10425,13 +10440,13 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 			break;
 		case NFT_MSG_NEWOBJ:
 			if (nft_trans_obj_update(trans)) {
-				nft_obj_commit_update(trans);
-				nf_tables_obj_notify(&trans->ctx,
+				nft_obj_commit_update(&ctx, trans);
+				nf_tables_obj_notify(&ctx,
 						     nft_trans_obj(trans),
 						     NFT_MSG_NEWOBJ);
 			} else {
 				nft_clear(net, nft_trans_obj(trans));
-				nf_tables_obj_notify(&trans->ctx,
+				nf_tables_obj_notify(&ctx,
 						     nft_trans_obj(trans),
 						     NFT_MSG_NEWOBJ);
 				nft_trans_destroy(trans);
@@ -10440,14 +10455,14 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 		case NFT_MSG_DELOBJ:
 		case NFT_MSG_DESTROYOBJ:
 			nft_obj_del(nft_trans_obj(trans));
-			nf_tables_obj_notify(&trans->ctx, nft_trans_obj(trans),
+			nf_tables_obj_notify(&ctx, nft_trans_obj(trans),
 					     trans->msg_type);
 			break;
 		case NFT_MSG_NEWFLOWTABLE:
 			if (nft_trans_flowtable_update(trans)) {
 				nft_trans_flowtable(trans)->data.flags =
 					nft_trans_flowtable_flags(trans);
-				nf_tables_flowtable_notify(&trans->ctx,
+				nf_tables_flowtable_notify(&ctx,
 							   nft_trans_flowtable(trans),
 							   &nft_trans_flowtable_hooks(trans),
 							   NFT_MSG_NEWFLOWTABLE);
@@ -10455,7 +10470,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 					    &nft_trans_flowtable(trans)->hook_list);
 			} else {
 				nft_clear(net, nft_trans_flowtable(trans));
-				nf_tables_flowtable_notify(&trans->ctx,
+				nf_tables_flowtable_notify(&ctx,
 							   nft_trans_flowtable(trans),
 							   NULL,
 							   NFT_MSG_NEWFLOWTABLE);
@@ -10465,7 +10480,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 		case NFT_MSG_DELFLOWTABLE:
 		case NFT_MSG_DESTROYFLOWTABLE:
 			if (nft_trans_flowtable_update(trans)) {
-				nf_tables_flowtable_notify(&trans->ctx,
+				nf_tables_flowtable_notify(&ctx,
 							   nft_trans_flowtable(trans),
 							   &nft_trans_flowtable_hooks(trans),
 							   trans->msg_type);
@@ -10473,7 +10488,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 								   &nft_trans_flowtable_hooks(trans));
 			} else {
 				list_del_rcu(&nft_trans_flowtable(trans)->list);
-				nf_tables_flowtable_notify(&trans->ctx,
+				nf_tables_flowtable_notify(&ctx,
 							   nft_trans_flowtable(trans),
 							   NULL,
 							   trans->msg_type);
@@ -10515,9 +10530,13 @@ static void nf_tables_module_autoload(struct net *net)
 
 static void nf_tables_abort_release(struct nft_trans *trans)
 {
+	struct nft_ctx ctx = { };
+
+	nft_ctx_update(&ctx, trans);
+
 	switch (trans->msg_type) {
 	case NFT_MSG_NEWTABLE:
-		nf_tables_table_destroy(trans->ctx.table);
+		nf_tables_table_destroy(trans->table);
 		break;
 	case NFT_MSG_NEWCHAIN:
 		if (nft_trans_chain_update(trans))
@@ -10526,17 +10545,17 @@ static void nf_tables_abort_release(struct nft_trans *trans)
 			nf_tables_chain_destroy(nft_trans_chain(trans));
 		break;
 	case NFT_MSG_NEWRULE:
-		nf_tables_rule_destroy(&trans->ctx, nft_trans_rule(trans));
+		nf_tables_rule_destroy(&ctx, nft_trans_rule(trans));
 		break;
 	case NFT_MSG_NEWSET:
-		nft_set_destroy(&trans->ctx, nft_trans_set(trans));
+		nft_set_destroy(&ctx, nft_trans_set(trans));
 		break;
 	case NFT_MSG_NEWSETELEM:
 		nft_set_elem_destroy(nft_trans_elem_set(trans),
 				     nft_trans_elem_priv(trans), true);
 		break;
 	case NFT_MSG_NEWOBJ:
-		nft_obj_destroy(&trans->ctx, nft_trans_obj(trans));
+		nft_obj_destroy(&ctx, nft_trans_obj(trans));
 		break;
 	case NFT_MSG_NEWFLOWTABLE:
 		if (nft_trans_flowtable_update(trans))
@@ -10569,6 +10588,9 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 	LIST_HEAD(set_update_list);
 	struct nft_trans_elem *te;
 	int err = 0;
+	struct nft_ctx ctx = {
+		.net = net,
+	};
 
 	if (action == NFNL_ABORT_VALIDATE &&
 	    nf_tables_validate(net) < 0)
@@ -10576,7 +10598,9 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 
 	list_for_each_entry_safe_reverse(trans, next, &nft_net->commit_list,
 					 list) {
-		struct nft_table *table = trans->ctx.table;
+		struct nft_table *table = trans->table;
+
+		nft_ctx_update(&ctx, trans);
 
 		switch (trans->msg_type) {
 		case NFT_MSG_NEWTABLE:
@@ -10603,7 +10627,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 			break;
 		case NFT_MSG_DELTABLE:
 		case NFT_MSG_DESTROYTABLE:
-			nft_clear(trans->ctx.net, table);
+			nft_clear(trans->net, table);
 			nft_trans_destroy(trans);
 			break;
 		case NFT_MSG_NEWCHAIN:
@@ -10623,7 +10647,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 				}
 				nft_use_dec_restore(&table->use);
 				nft_chain_del(nft_trans_chain(trans));
-				nf_tables_unregister_hook(trans->ctx.net, table,
+				nf_tables_unregister_hook(trans->net, table,
 							  nft_trans_chain(trans));
 			}
 			break;
@@ -10634,7 +10658,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 					    &nft_trans_basechain(trans)->hook_list);
 			} else {
 				nft_use_inc_restore(&table->use);
-				nft_clear(trans->ctx.net, nft_trans_chain(trans));
+				nft_clear(trans->net, nft_trans_chain(trans));
 			}
 			nft_trans_destroy(trans);
 			break;
@@ -10645,7 +10669,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 			}
 			nft_use_dec_restore(&nft_trans_rule_chain(trans)->use);
 			list_del_rcu(&nft_trans_rule(trans)->list);
-			nft_rule_expr_deactivate(&trans->ctx,
+			nft_rule_expr_deactivate(&ctx,
 						 nft_trans_rule(trans),
 						 NFT_TRANS_ABORT);
 			if (nft_trans_rule_chain(trans)->flags & NFT_CHAIN_HW_OFFLOAD)
@@ -10654,8 +10678,8 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 		case NFT_MSG_DELRULE:
 		case NFT_MSG_DESTROYRULE:
 			nft_use_inc_restore(&nft_trans_rule_chain(trans)->use);
-			nft_clear(trans->ctx.net, nft_trans_rule(trans));
-			nft_rule_expr_activate(&trans->ctx, nft_trans_rule(trans));
+			nft_clear(trans->net, nft_trans_rule(trans));
+			nft_rule_expr_activate(&ctx, nft_trans_rule(trans));
 			if (nft_trans_rule_chain(trans)->flags & NFT_CHAIN_HW_OFFLOAD)
 				nft_flow_rule_destroy(nft_trans_flow_rule(trans));
 
@@ -10677,9 +10701,9 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 		case NFT_MSG_DELSET:
 		case NFT_MSG_DESTROYSET:
 			nft_use_inc_restore(&table->use);
-			nft_clear(trans->ctx.net, nft_trans_set(trans));
+			nft_clear(trans->net, nft_trans_set(trans));
 			if (nft_trans_set(trans)->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
-				nft_map_activate(&trans->ctx, nft_trans_set(trans));
+				nft_map_activate(&ctx, nft_trans_set(trans));
 
 			nft_trans_destroy(trans);
 			break;
@@ -10719,7 +10743,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 			break;
 		case NFT_MSG_NEWOBJ:
 			if (nft_trans_obj_update(trans)) {
-				nft_obj_destroy(&trans->ctx, nft_trans_obj_newobj(trans));
+				nft_obj_destroy(&ctx, nft_trans_obj_newobj(trans));
 				nft_trans_destroy(trans);
 			} else {
 				nft_use_dec_restore(&table->use);
@@ -10729,7 +10753,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 		case NFT_MSG_DELOBJ:
 		case NFT_MSG_DESTROYOBJ:
 			nft_use_inc_restore(&table->use);
-			nft_clear(trans->ctx.net, nft_trans_obj(trans));
+			nft_clear(trans->net, nft_trans_obj(trans));
 			nft_trans_destroy(trans);
 			break;
 		case NFT_MSG_NEWFLOWTABLE:
@@ -10750,7 +10774,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 					    &nft_trans_flowtable(trans)->hook_list);
 			} else {
 				nft_use_inc_restore(&table->use);
-				nft_clear(trans->ctx.net, nft_trans_flowtable(trans));
+				nft_clear(trans->net, nft_trans_flowtable(trans));
 			}
 			nft_trans_destroy(trans);
 			break;
diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
index 0619feb10abb..64675f1c7f29 100644
--- a/net/netfilter/nf_tables_offload.c
+++ b/net/netfilter/nf_tables_offload.c
@@ -513,7 +513,7 @@ static void nft_flow_rule_offload_abort(struct net *net,
 	int err = 0;
 
 	list_for_each_entry_continue_reverse(trans, &nft_net->commit_list, list) {
-		if (trans->ctx.family != NFPROTO_NETDEV)
+		if (trans->table->family != NFPROTO_NETDEV)
 			continue;
 
 		switch (trans->msg_type) {
@@ -564,7 +564,7 @@ int nft_flow_rule_offload_commit(struct net *net)
 	u8 policy;
 
 	list_for_each_entry(trans, &nft_net->commit_list, list) {
-		if (trans->ctx.family != NFPROTO_NETDEV)
+		if (trans->table->family != NFPROTO_NETDEV)
 			continue;
 
 		switch (trans->msg_type) {
@@ -589,8 +589,8 @@ int nft_flow_rule_offload_commit(struct net *net)
 			if (!(nft_trans_rule_chain(trans)->flags & NFT_CHAIN_HW_OFFLOAD))
 				continue;
 
-			if (trans->ctx.flags & NLM_F_REPLACE ||
-			    !(trans->ctx.flags & NLM_F_APPEND)) {
+			if (trans->flags & NLM_F_REPLACE ||
+			    !(trans->flags & NLM_F_APPEND)) {
 				err = -EOPNOTSUPP;
 				break;
 			}
-- 
2.43.2


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

* Re: [PATCH nf-next 02/11] netfilter: nf_tables: move bind list_head into relevant subtypes
  2024-05-13 13:00 ` [PATCH nf-next 02/11] netfilter: nf_tables: move bind list_head into relevant subtypes Florian Westphal
@ 2024-06-18  8:24   ` Pablo Neira Ayuso
  2024-06-18  9:21     ` Florian Westphal
  2024-06-24 19:16   ` Pablo Neira Ayuso
  1 sibling, 1 reply; 20+ messages in thread
From: Pablo Neira Ayuso @ 2024-06-18  8:24 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

Hi Florian,

I like this series, just a comestic glitch.

On Mon, May 13, 2024 at 03:00:42PM +0200, Florian Westphal wrote:
> @@ -416,11 +436,26 @@ static int nft_deltable(struct nft_ctx *ctx)
>  	return err;
>  }
>  
> -static struct nft_trans *nft_trans_chain_add(struct nft_ctx *ctx, int msg_type)
> +static struct nft_trans *
> +nft_trans_alloc_chain(const struct nft_ctx *ctx, int msg_type)
>  {
>  	struct nft_trans *trans;
>  
>  	trans = nft_trans_alloc(ctx, msg_type, sizeof(struct nft_trans_chain));
> +	if (trans) {

May I mangle this to do here:

        if (!trans)
                return NULL;

...

> +		struct nft_trans_chain *chain = nft_trans_container_chain(trans);
> +
> +		INIT_LIST_HEAD(&chain->nft_trans_binding.binding_list);
> +	}
> +
> +	return trans;
> +}
> +
> +static struct nft_trans *nft_trans_chain_add(struct nft_ctx *ctx, int msg_type)
> +{
> +	struct nft_trans *trans;
> +
> +	trans = nft_trans_alloc_chain(ctx, msg_type);
>  	if (trans == NULL)
>  		return ERR_PTR(-ENOMEM);
>  
> @@ -560,12 +595,16 @@ static int __nft_trans_set_add(const struct nft_ctx *ctx, int msg_type,
>  			       struct nft_set *set,
>  			       const struct nft_set_desc *desc)
>  {
> +	struct nft_trans_set *trans_set;
>  	struct nft_trans *trans;
>  
>  	trans = nft_trans_alloc(ctx, msg_type, sizeof(struct nft_trans_set));
>  	if (trans == NULL)
>  		return -ENOMEM;

As it is done here?

>  
> +	trans_set = nft_trans_container_set(trans);
> +	INIT_LIST_HEAD(&trans_set->nft_trans_binding.binding_list);
> +
>  	if (msg_type == NFT_MSG_NEWSET && ctx->nla[NFTA_SET_ID] && !desc) {
>  		nft_trans_set_id(trans) =
>  			ntohl(nla_get_be32(ctx->nla[NFTA_SET_ID]));

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

* Re: [PATCH nf-next 01/11] netfilter: nf_tables: make struct nft_trans first member of derived subtypes
  2024-05-13 13:00 ` [PATCH nf-next 01/11] netfilter: nf_tables: make struct nft_trans first member of derived subtypes Florian Westphal
@ 2024-06-18  8:28   ` Pablo Neira Ayuso
  2024-06-18  9:20     ` Florian Westphal
  0 siblings, 1 reply; 20+ messages in thread
From: Pablo Neira Ayuso @ 2024-06-18  8:28 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

On Mon, May 13, 2024 at 03:00:41PM +0200, Florian Westphal wrote:
> There is 'struct nft_trans', the basic structure for all transactional
> objects, and the the various different transactional objects, such as
> nft_trans_table, chain, set, set_elem and so on.
> 
> Right now 'struct nft_trans' uses a flexible member at the tail
> (data[]), and casting is needed to access the actual type-specific
> members.
> 
> Change this to make the hierarchy visible in source code, i.e. make
> struct nft_trans the first member of all derived subtypes.
> 
> This has several advantages:
> 1. pahole output reflects the real size needed by the particular subtype
> 2. allows to use container_of() to convert the base type to the actual
>    object type instead of casting ->data to the overlay structure.
> 3. It makes it easy to add intermediate types.
> 
> 'struct nft_trans' contains a 'binding_list' that is only needed
> by two subtypes, so it should be part of the two subtypes, not in
> the base structure.
> 
> But that makes it hard to interate over the binding_list, because
> there is no common base structure.
> 
> A follow patch moves the bind list to a new struct:
> 
>  struct nft_trans_binding {
>    struct nft_trans nft_trans;
>    struct list_head binding_list;
>  };
> 
> ... and makes that structure the new 'first member' for both
> nft_trans_chain and nft_trans_set.
> 
> No functional change intended in this patch.
> 
> Some numbers:
>  struct nft_trans { /* size: 88, cachelines: 2, members: 5 */
>  struct nft_trans_chain { /* size: 152, cachelines: 3, members: 10 */
>  struct nft_trans_elem { /* size: 112, cachelines: 2, members: 4 */
>  struct nft_trans_flowtable { /* size: 128, cachelines: 2, members: 5 */
>  struct nft_trans_obj { /* size: 112, cachelines: 2, members: 4 */
>  struct nft_trans_rule { /* size: 112, cachelines: 2, members: 5 */
>  struct nft_trans_set { /* size: 120, cachelines: 2, members: 8 */
>  struct nft_trans_table { /* size: 96, cachelines: 2, members: 2 */
> 
> Of particular interest is nft_trans_elem, which needs to be allocated
> once for each pending (to be added or removed) set element.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  include/net/netfilter/nf_tables.h | 88 +++++++++++++++++--------------
>  net/netfilter/nf_tables_api.c     | 10 ++--
>  2 files changed, 54 insertions(+), 44 deletions(-)
> 
> diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
> index 2796153b03da..af0ef21f3780 100644
> --- a/include/net/netfilter/nf_tables.h
> +++ b/include/net/netfilter/nf_tables.h
> @@ -1608,14 +1608,16 @@ static inline int nft_set_elem_is_dead(const struct nft_set_ext *ext)
>  }
>  
>  /**
> - *	struct nft_trans - nf_tables object update in transaction
> + * struct nft_trans - nf_tables object update in transaction
>   *
> - *	@list: used internally
> - *	@binding_list: list of objects with possible bindings
> - *	@msg_type: message type
> - *	@put_net: ctx->net needs to be put
> - *	@ctx: transaction context
> - *	@data: internal information related to the transaction
> + * @list: used internally
> + * @binding_list: list of objects with possible bindings
> + * @msg_type: message type
> + * @put_net: ctx->net needs to be put
> + * @ctx: transaction context
> + *
> + * This is the information common to all objects in the transaction,
> + * this must always be the first member of derived sub-types.
>   */
>  struct nft_trans {
>  	struct list_head		list;
> @@ -1623,10 +1625,10 @@ struct nft_trans {
>  	int				msg_type;
>  	bool				put_net;
>  	struct nft_ctx			ctx;
> -	char				data[];
>  };
>  
>  struct nft_trans_rule {
> +	struct nft_trans		nft_trans;
>  	struct nft_rule			*rule;
>  	struct nft_flow_rule		*flow;
>  	u32				rule_id;
> @@ -1634,15 +1636,16 @@ struct nft_trans_rule {
>  };
>  
>  #define nft_trans_rule(trans)	\
> -	(((struct nft_trans_rule *)trans->data)->rule)
> +	container_of(trans, struct nft_trans_rule, nft_trans)->rule

Another nitpicking comestic issue:

Maybe I can get this series slightly smaller if

        nft_trans_rule_container

is added here and use it, instead of the opencoded container_of.

For consistency with:

#define nft_trans_container_set(t)

which is used everywhere in this header in a follow up patch.

I mangle this at the expense of adding my own bugs :)

[...]
>  #define nft_trans_set(trans)	\
> -	(((struct nft_trans_set *)trans->data)->set)
> +	container_of(trans, struct nft_trans_set, nft_trans)->set
>  #define nft_trans_set_id(trans)	\
> -	(((struct nft_trans_set *)trans->data)->set_id)
> +	container_of(trans, struct nft_trans_set, nft_trans)->set_id
>  #define nft_trans_set_bound(trans)	\
> -	(((struct nft_trans_set *)trans->data)->bound)
> +	container_of(trans, struct nft_trans_set, nft_trans)->bound
>  #define nft_trans_set_update(trans)	\
> -	(((struct nft_trans_set *)trans->data)->update)
> +	container_of(trans, struct nft_trans_set, nft_trans)->update
>  #define nft_trans_set_timeout(trans)	\
> -	(((struct nft_trans_set *)trans->data)->timeout)
> +	container_of(trans, struct nft_trans_set, nft_trans)->timeout
>  #define nft_trans_set_gc_int(trans)	\
> -	(((struct nft_trans_set *)trans->data)->gc_int)
> +	container_of(trans, struct nft_trans_set, nft_trans)->gc_int
>  #define nft_trans_set_size(trans)	\
> -	(((struct nft_trans_set *)trans->data)->size)
> +	container_of(trans, struct nft_trans_set, nft_trans)->size

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

* Re: [PATCH nf-next 01/11] netfilter: nf_tables: make struct nft_trans first member of derived subtypes
  2024-06-18  8:28   ` Pablo Neira Ayuso
@ 2024-06-18  9:20     ` Florian Westphal
  0 siblings, 0 replies; 20+ messages in thread
From: Florian Westphal @ 2024-06-18  9:20 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Maybe I can get this series slightly smaller if
> 
>         nft_trans_rule_container
> 
> is added here and use it, instead of the opencoded container_of.

Sure, no objections.

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

* Re: [PATCH nf-next 02/11] netfilter: nf_tables: move bind list_head into relevant subtypes
  2024-06-18  8:24   ` Pablo Neira Ayuso
@ 2024-06-18  9:21     ` Florian Westphal
  0 siblings, 0 replies; 20+ messages in thread
From: Florian Westphal @ 2024-06-18  9:21 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> May I mangle this to do here:
> 
>         if (!trans)
>                 return NULL;

Sure.

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

* Re: [PATCH nf-next 02/11] netfilter: nf_tables: move bind list_head into relevant subtypes
  2024-05-13 13:00 ` [PATCH nf-next 02/11] netfilter: nf_tables: move bind list_head into relevant subtypes Florian Westphal
  2024-06-18  8:24   ` Pablo Neira Ayuso
@ 2024-06-24 19:16   ` Pablo Neira Ayuso
  2024-06-24 21:18     ` Florian Westphal
  1 sibling, 1 reply; 20+ messages in thread
From: Pablo Neira Ayuso @ 2024-06-24 19:16 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

Hi Florian,

One more question:

On Mon, May 13, 2024 at 03:00:42PM +0200, Florian Westphal wrote:
[...]
> @@ -1621,12 +1620,23 @@ static inline int nft_set_elem_is_dead(const struct nft_set_ext *ext)
>   */
>  struct nft_trans {
>  	struct list_head		list;
> -	struct list_head		binding_list;
>  	int				msg_type;
>  	bool				put_net;
>  	struct nft_ctx			ctx;
>  };
>  
> +/**
> + * struct nft_trans_binding - nf_tables object with binding support in transaction
> + * @nft_trans:    base structure, MUST be first member

This comment says nft_trans MUST be first.

I can add BUILD_BUG_ON for all nft_trans_* objects to check that
nft_trans always comes first (ie. this comes at offset 0 in this
structure).

Thanks.

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

* Re: [PATCH nf-next 02/11] netfilter: nf_tables: move bind list_head into relevant subtypes
  2024-06-24 19:16   ` Pablo Neira Ayuso
@ 2024-06-24 21:18     ` Florian Westphal
  2024-06-25 18:49       ` Pablo Neira Ayuso
  0 siblings, 1 reply; 20+ messages in thread
From: Florian Westphal @ 2024-06-24 21:18 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Mon, May 13, 2024 at 03:00:42PM +0200, Florian Westphal wrote:
> [...]
> > @@ -1621,12 +1620,23 @@ static inline int nft_set_elem_is_dead(const struct nft_set_ext *ext)
> >   */
> >  struct nft_trans {
> >  	struct list_head		list;
> > -	struct list_head		binding_list;
> >  	int				msg_type;
> >  	bool				put_net;
> >  	struct nft_ctx			ctx;
> >  };
> >  
> > +/**
> > + * struct nft_trans_binding - nf_tables object with binding support in transaction
> > + * @nft_trans:    base structure, MUST be first member
> 
> This comment says nft_trans MUST be first.

Yes, thats because current code assumes that it can
cast any subtype to nft_trans.

Once everything is converted to container_of that would
not be necessary but I think it would still be better to do it
this way.

> I can add BUILD_BUG_ON for all nft_trans_* objects to check that
> nft_trans always comes first (ie. this comes at offset 0 in this
> structure).

Sounds good, thanks!

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

* Re: [PATCH nf-next 02/11] netfilter: nf_tables: move bind list_head into relevant subtypes
  2024-06-24 21:18     ` Florian Westphal
@ 2024-06-25 18:49       ` Pablo Neira Ayuso
  2024-06-26 11:28         ` Pablo Neira Ayuso
  0 siblings, 1 reply; 20+ messages in thread
From: Pablo Neira Ayuso @ 2024-06-25 18:49 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

On Mon, Jun 24, 2024 at 11:18:52PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
[...]
> > I can add BUILD_BUG_ON for all nft_trans_* objects to check that
> > nft_trans always comes first (ie. this comes at offset 0 in this
> > structure).
> 
> Sounds good, thanks!

OK, I have pushed out the patches with the manglings I reported here:

https://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next.git/log/?h=under-review

I am going to collect remaining pending patches for nf-next and
prepare for PR.

Thanks.

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

* Re: [PATCH nf-next 02/11] netfilter: nf_tables: move bind list_head into relevant subtypes
  2024-06-25 18:49       ` Pablo Neira Ayuso
@ 2024-06-26 11:28         ` Pablo Neira Ayuso
  0 siblings, 0 replies; 20+ messages in thread
From: Pablo Neira Ayuso @ 2024-06-26 11:28 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

On Tue, Jun 25, 2024 at 08:49:45PM +0200, Pablo Neira Ayuso wrote:
> On Mon, Jun 24, 2024 at 11:18:52PM +0200, Florian Westphal wrote:
> > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> [...]
> > > I can add BUILD_BUG_ON for all nft_trans_* objects to check that
> > > nft_trans always comes first (ie. this comes at offset 0 in this
> > > structure).
> > 
> > Sounds good, thanks!
> 
> OK, I have pushed out the patches with the manglings I reported here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next.git/log/?h=under-review
> 
> I am going to collect remaining pending patches for nf-next and
> prepare for PR.

For the record, this is now in the netfilter/nf-next tree. Thanks.

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

end of thread, other threads:[~2024-06-26 11:28 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-13 13:00 [PATCH nf-next 00/11] netfilter: nf_tables: reduce transaction log memory usage Florian Westphal
2024-05-13 13:00 ` [PATCH nf-next 01/11] netfilter: nf_tables: make struct nft_trans first member of derived subtypes Florian Westphal
2024-06-18  8:28   ` Pablo Neira Ayuso
2024-06-18  9:20     ` Florian Westphal
2024-05-13 13:00 ` [PATCH nf-next 02/11] netfilter: nf_tables: move bind list_head into relevant subtypes Florian Westphal
2024-06-18  8:24   ` Pablo Neira Ayuso
2024-06-18  9:21     ` Florian Westphal
2024-06-24 19:16   ` Pablo Neira Ayuso
2024-06-24 21:18     ` Florian Westphal
2024-06-25 18:49       ` Pablo Neira Ayuso
2024-06-26 11:28         ` Pablo Neira Ayuso
2024-05-13 13:00 ` [PATCH nf-next 03/11] netfilter: nf_tables: compact chain+ft transaction objects Florian Westphal
2024-05-13 13:00 ` [PATCH nf-next 04/11] netfilter: nf_tables: reduce trans->ctx.table references Florian Westphal
2024-05-13 13:00 ` [PATCH nf-next 05/11] netfilter: nf_tables: pass nft_chain to destroy function, not nft_ctx Florian Westphal
2024-05-13 13:00 ` [PATCH nf-next 06/11] netfilter: nf_tables: pass more specific nft_trans_chain where possible Florian Westphal
2024-05-13 13:00 ` [PATCH nf-next 07/11] netfilter: nf_tables: avoid usage of embedded nft_ctx Florian Westphal
2024-05-13 13:00 ` [PATCH nf-next 08/11] netfilter: nf_tables: store chain pointer in rule transaction Florian Westphal
2024-05-13 13:00 ` [PATCH nf-next 09/11] netfilter: nf_tables: reduce trans->ctx.chain references Florian Westphal
2024-05-13 13:00 ` [PATCH nf-next 10/11] netfilter: nf_tables: pass nft_table to destroy function Florian Westphal
2024-05-13 13:00 ` [PATCH nf-next 11/11] netfilter: nf_tables: do not store nft_ctx in transaction objects Florian Westphal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).