Move the timeout/expire/flag members from nft_trans_one_elem struct into a dybamically allocated structure, only needed when timeout update was requested. This halves size of nft_trans_one_elem struct and allows to compact up to 124 elements in one transaction container rather than 62. This halves memory requirements for a large flush or insert transaction, where ->update remains NULL. Care has to be taken to release the extra data in all spots, including abort path. Signed-off-by: Florian Westphal <fw@xxxxxxxxx> --- v2: nft_trans_set_elem_destroy must skip entries with NULL priv field, those were update requests that have been handled already during error unwinding. include/net/netfilter/nf_tables.h | 10 +++-- net/netfilter/nf_tables_api.c | 61 ++++++++++++++++++++----------- 2 files changed, 46 insertions(+), 25 deletions(-) diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h index 2a2631edab2b..1caf142900fe 100644 --- a/include/net/netfilter/nf_tables.h +++ b/include/net/netfilter/nf_tables.h @@ -1754,11 +1754,15 @@ enum nft_trans_elem_flags { NFT_TRANS_UPD_EXPIRATION = (1 << 1), }; -struct nft_trans_one_elem { - struct nft_elem_priv *priv; +struct nft_elem_update { u64 timeout; u64 expiration; - u8 update_flags; + u8 flags; +}; + +struct nft_trans_one_elem { + struct nft_elem_priv *priv; + struct nft_elem_update *update; }; struct nft_trans_elem { diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index acd574986647..2c98005daf84 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -6665,7 +6665,8 @@ static void nft_trans_set_elem_destroy(const struct nft_ctx *ctx, struct nft_tra int i; for (i = 0; i < te->nelems; i++) { - if (te->elems[i].update_flags) + /* skip update request, see nft_trans_elems_new_abort() */ + if (!te->elems[i].priv) continue; __nft_set_elem_destroy(ctx, te->set, te->elems[i].priv, true); @@ -6856,24 +6857,28 @@ static void nft_trans_elem_update(const struct nft_set *set, const struct nft_trans_one_elem *elem) { const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv); + const struct nft_elem_update *update = elem->update; - if (elem->update_flags & NFT_TRANS_UPD_TIMEOUT) - WRITE_ONCE(nft_set_ext_timeout(ext)->timeout, elem->timeout); + if (update->flags & NFT_TRANS_UPD_TIMEOUT) + WRITE_ONCE(nft_set_ext_timeout(ext)->timeout, update->timeout); - if (elem->update_flags & NFT_TRANS_UPD_EXPIRATION) - WRITE_ONCE(nft_set_ext_timeout(ext)->expiration, get_jiffies_64() + elem->expiration); + if (update->flags & NFT_TRANS_UPD_EXPIRATION) + WRITE_ONCE(nft_set_ext_timeout(ext)->expiration, get_jiffies_64() + update->expiration); } static void nft_trans_elems_activate(const struct nft_ctx *ctx, - const struct nft_trans_elem *te) + struct nft_trans_elem *te) { int i; for (i = 0; i < te->nelems; i++) { - const struct nft_trans_one_elem *elem = &te->elems[i]; + struct nft_trans_one_elem *elem = &te->elems[i]; - if (elem->update_flags) { + if (elem->update) { nft_trans_elem_update(te->set, elem); + kfree(elem->update); + elem->update = NULL; + elem->priv = NULL; continue; } @@ -6971,6 +6976,8 @@ static void nft_trans_elems_remove(const struct nft_ctx *ctx, int i; for (i = 0; i < te->nelems; i++) { + WARN_ON_ONCE(te->elems[i].update); + nf_tables_setelem_notify(ctx, te->set, te->elems[i].priv, te->nft_trans.msg_type); @@ -7019,7 +7026,6 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set, struct nft_data_desc desc; enum nft_registers dreg; struct nft_trans *trans; - u8 update_flags; u64 expiration; u64 timeout; int err, i; @@ -7334,26 +7340,32 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set, else if (!(nlmsg_flags & NLM_F_EXCL)) { err = 0; if (nft_set_ext_exists(ext2, NFT_SET_EXT_TIMEOUT)) { - struct nft_trans_one_elem *update; - - update = &nft_trans_container_elem(trans)->elems[0]; + struct nft_elem_update update = { }; - update_flags = 0; if (timeout != nft_set_ext_timeout(ext2)->timeout) { - update->timeout = timeout; + update.timeout = timeout; if (expiration == 0) expiration = timeout; - update_flags |= NFT_TRANS_UPD_TIMEOUT; + update.flags |= NFT_TRANS_UPD_TIMEOUT; } if (expiration) { - update->expiration = expiration; - update_flags |= NFT_TRANS_UPD_EXPIRATION; + update.expiration = expiration; + update.flags |= NFT_TRANS_UPD_EXPIRATION; } - if (update_flags) { - update->priv = elem_priv; - update->update_flags = update_flags; + if (update.flags) { + struct nft_trans_one_elem *ue; + + ue = &nft_trans_container_elem(trans)->elems[0]; + + ue->update = kmemdup(&update, sizeof(update), GFP_KERNEL); + if (!ue->update) { + err = -ENOMEM; + goto err_element_clash; + } + + ue->priv = elem_priv; nft_trans_commit_list_add_elem(ctx->net, trans, GFP_KERNEL); goto err_elem_free; } @@ -7521,14 +7533,19 @@ void nft_setelem_data_deactivate(const struct net *net, * Returns true if set had been added to (i.e., elements need to be removed again). */ static bool nft_trans_elems_new_abort(const struct nft_ctx *ctx, - const struct nft_trans_elem *te) + struct nft_trans_elem *te) { bool removed = false; int i; for (i = 0; i < te->nelems; i++) { - if (te->elems[i].update_flags) + if (te->elems[i].update) { + kfree(te->elems[i].update); + te->elems[i].update = NULL; + /* Update request, so do not release this element */ + te->elems[i].priv = NULL; continue; + } if (!te->set->ops->abort || nft_setelem_is_catchall(te->set, te->elems[i].priv)) nft_setelem_remove(ctx->net, te->set, te->elems[i].priv); -- 2.45.2