Skip to content

Commit b0c4946

Browse files
Florian Westphalummakynes
authored andcommitted
netfilter: nf_tables: switch trans_elem to real flex array
When queueing a set element add or removal operation to the transaction log, check if the previous operation already asks for a the identical operation on the same set. If so, store the element reference in the preceding operation. This significantlty reduces memory consumption when many set add/delete operations appear in a single transaction. Example: 10k elements require 937kb of memory (10k allocations from kmalloc-96 slab). Assuming we can compact 4 elements in the same set, 468 kbytes are needed (64 bytes for base struct, nft_trans_elemn, 32 bytes for nft_trans_one_elem structure, so 2500 allocations from kmalloc-192 slab). For large batch updates we can compact up to 62 elements into one single nft_trans_elem structure (~65% mem reduction): (64 bytes for base struct, nft_trans_elem, 32 byte for nft_trans_one_elem struct). We can halve size of nft_trans_one_elem struct by moving timeout/expire/update_flags into a dynamically allocated structure, this allows to store 124 elements in a 2k slab nft_trans_elem struct. This is done in a followup patch. Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
1 parent 466c9b3 commit b0c4946

File tree

1 file changed

+90
-0
lines changed

1 file changed

+90
-0
lines changed

net/netfilter/nf_tables_api.c

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@
2626
#define NFT_MODULE_AUTOLOAD_LIMIT (MODULE_NAME_LEN - sizeof("nft-expr-255-"))
2727
#define NFT_SET_MAX_ANONLEN 16
2828

29+
/* limit compaction to avoid huge kmalloc/krealloc sizes. */
30+
#define NFT_MAX_SET_NELEMS ((2048 - sizeof(struct nft_trans_elem)) / sizeof(struct nft_trans_one_elem))
31+
2932
unsigned int nf_tables_net_id __read_mostly;
3033

3134
static LIST_HEAD(nf_tables_expressions);
@@ -391,6 +394,86 @@ static void nf_tables_unregister_hook(struct net *net,
391394
return __nf_tables_unregister_hook(net, table, chain, false);
392395
}
393396

397+
static bool nft_trans_collapse_set_elem_allowed(const struct nft_trans_elem *a, const struct nft_trans_elem *b)
398+
{
399+
/* NB: the ->bound equality check is defensive, at this time we only merge
400+
* a new nft_trans_elem transaction request with the transaction tail
401+
* element, but a->bound != b->bound would imply a NEWRULE transaction
402+
* is queued in-between.
403+
*
404+
* The set check is mandatory, the NFT_MAX_SET_NELEMS check prevents
405+
* huge krealloc() requests.
406+
*/
407+
return a->set == b->set && a->bound == b->bound && a->nelems < NFT_MAX_SET_NELEMS;
408+
}
409+
410+
static bool nft_trans_collapse_set_elem(struct nftables_pernet *nft_net,
411+
struct nft_trans_elem *tail,
412+
struct nft_trans_elem *trans,
413+
gfp_t gfp)
414+
{
415+
unsigned int nelems, old_nelems = tail->nelems;
416+
struct nft_trans_elem *new_trans;
417+
418+
if (!nft_trans_collapse_set_elem_allowed(tail, trans))
419+
return false;
420+
421+
/* "cannot happen", at this time userspace element add
422+
* requests always allocate a new transaction element.
423+
*
424+
* This serves as a reminder to adjust the list_add_tail
425+
* logic below in case this ever changes.
426+
*/
427+
if (WARN_ON_ONCE(trans->nelems != 1))
428+
return false;
429+
430+
if (check_add_overflow(old_nelems, trans->nelems, &nelems))
431+
return false;
432+
433+
/* krealloc might free tail which invalidates list pointers */
434+
list_del_init(&tail->nft_trans.list);
435+
436+
new_trans = krealloc(tail, struct_size(tail, elems, nelems), gfp);
437+
if (!new_trans) {
438+
list_add_tail(&tail->nft_trans.list, &nft_net->commit_list);
439+
return false;
440+
}
441+
442+
/*
443+
* new_trans->nft_trans.list contains garbage, but
444+
* list_add_tail() doesn't care.
445+
*/
446+
new_trans->nelems = nelems;
447+
new_trans->elems[old_nelems] = trans->elems[0];
448+
list_add_tail(&new_trans->nft_trans.list, &nft_net->commit_list);
449+
450+
return true;
451+
}
452+
453+
static bool nft_trans_try_collapse(struct nftables_pernet *nft_net,
454+
struct nft_trans *trans, gfp_t gfp)
455+
{
456+
struct nft_trans *tail;
457+
458+
if (list_empty(&nft_net->commit_list))
459+
return false;
460+
461+
tail = list_last_entry(&nft_net->commit_list, struct nft_trans, list);
462+
463+
if (tail->msg_type != trans->msg_type)
464+
return false;
465+
466+
switch (trans->msg_type) {
467+
case NFT_MSG_NEWSETELEM:
468+
case NFT_MSG_DELSETELEM:
469+
return nft_trans_collapse_set_elem(nft_net,
470+
nft_trans_container_elem(tail),
471+
nft_trans_container_elem(trans), gfp);
472+
}
473+
474+
return false;
475+
}
476+
394477
static void nft_trans_commit_list_add_tail(struct net *net, struct nft_trans *trans)
395478
{
396479
struct nftables_pernet *nft_net = nft_pernet(net);
@@ -424,11 +507,18 @@ static void nft_trans_commit_list_add_tail(struct net *net, struct nft_trans *tr
424507
static void nft_trans_commit_list_add_elem(struct net *net, struct nft_trans *trans,
425508
gfp_t gfp)
426509
{
510+
struct nftables_pernet *nft_net = nft_pernet(net);
511+
427512
WARN_ON_ONCE(trans->msg_type != NFT_MSG_NEWSETELEM &&
428513
trans->msg_type != NFT_MSG_DELSETELEM);
429514

430515
might_alloc(gfp);
431516

517+
if (nft_trans_try_collapse(nft_net, trans, gfp)) {
518+
kfree(trans);
519+
return;
520+
}
521+
432522
nft_trans_commit_list_add_tail(net, trans);
433523
}
434524

0 commit comments

Comments
 (0)