| Message ID | 20241014111420.29127-1-pablo@netfilter.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
On Mon, 14 Oct 2024 13:14:11 +0200 Pablo Neira Ayuso wrote: > Hi, > > The following series contains Netfilter updates for net-next: > > 1) Fix sparse warning in nf_tables related to use of percpu counters, > from Uros Bizjak. > > 2) use strscpy_pad in nft_meta_bridge, from Justin Stitt. > > 3) A series from patch #3 to patch #7 to reduce memory footprint of set > element transactions, Florian Westphal says: > > When doing a flush on a set or mass adding/removing elements from a > set, each element needs to allocate 96 bytes to hold the transactional > state. > > In such cases, virtually all the information in struct nft_trans_elem > is the same. > > Change nft_trans_elem to a flex-array, i.e. a single nft_trans_elem > can hold multiple set element pointers. > > The number of elements that can be stored in one nft_trans_elem is limited > by the slab allocator, this series limits the compaction to at most 62 > elements as it caps the reallocation to 2048 bytes of memory. > > 4) Document legacy toggles for xtables packet classifiers, from > Bruno Leitao. > > 5) Use kfree_rcu() instead of call_rcu() + kmem_cache_free(), from Julia Lawall. Hi! Are you seeing any failures in nft_audit? I haven't looked closely but it seems that this PR causes: <snip> # testing for cmd: nft reset quotas t1 ... OK # testing for cmd: nft reset quotas t2 ... OK # testing for cmd: nft reset quotas ... OK # testing for cmd: nft delete rule t1 c1 handle 4 ... OK # testing for cmd: nft delete rule t1 c1 handle 5; delete rule t1 c1 handle 6 ... OK # testing for cmd: nft flush chain t1 c2 ... OK # testing for cmd: nft flush table t2 ... OK # testing for cmd: nft delete chain t2 c2 ... OK # testing for cmd: nft delete element t1 s { 22 } ... OK # testing for cmd: nft delete element t1 s { 80, 443 } ... FAIL # -table=t1 family=2 entries=2 op=nft_unregister_setelem # +table=t1 family=2 entries=1 op=nft_unregister_setelem # testing for cmd: nft flush set t1 s2 ... FAIL # -table=t1 family=2 entries=3 op=nft_unregister_setelem # +table=t1 family=2 entries=1 op=nft_unregister_setelem # testing for cmd: nft delete set t1 s2 ... OK # testing for cmd: nft delete set t1 s3 ... OK not ok 1 selftests: net/netfilter: nft_audit.sh # exit=251 https://netdev-3.bots.linux.dev/vmksft-nf-dbg/results/815301/10-nft-audit-sh/stdout
Jakub Kicinski <kuba@kernel.org> wrote: > > 5) Use kfree_rcu() instead of call_rcu() + kmem_cache_free(), from Julia Lawall. > > Hi! Are you seeing any failures in nft_audit? I haven't looked closely > but it seems that this PR causes: > > <snip> > # testing for cmd: nft reset quotas t1 ... OK > # testing for cmd: nft reset quotas t2 ... OK > # testing for cmd: nft reset quotas ... OK > # testing for cmd: nft delete rule t1 c1 handle 4 ... OK > # testing for cmd: nft delete rule t1 c1 handle 5; delete rule t1 c1 handle 6 ... OK > # testing for cmd: nft flush chain t1 c2 ... OK > # testing for cmd: nft flush table t2 ... OK > # testing for cmd: nft delete chain t2 c2 ... OK > # testing for cmd: nft delete element t1 s { 22 } ... OK > # testing for cmd: nft delete element t1 s { 80, 443 } ... FAIL > # -table=t1 family=2 entries=2 op=nft_unregister_setelem > # +table=t1 family=2 entries=1 op=nft_unregister_setelem > # testing for cmd: nft flush set t1 s2 ... FAIL My fault, Pablo, please toss all of my patches. I do not know when I will resend, so do not wait.
On Mon, Oct 14, 2024 at 11:09:25PM +0200, Florian Westphal wrote: > Jakub Kicinski <kuba@kernel.org> wrote: > > > 5) Use kfree_rcu() instead of call_rcu() + kmem_cache_free(), from Julia Lawall. > > > > Hi! Are you seeing any failures in nft_audit? I haven't looked closely > > but it seems that this PR causes: > > > > <snip> > > # testing for cmd: nft reset quotas t1 ... OK > > # testing for cmd: nft reset quotas t2 ... OK > > # testing for cmd: nft reset quotas ... OK > > # testing for cmd: nft delete rule t1 c1 handle 4 ... OK > > # testing for cmd: nft delete rule t1 c1 handle 5; delete rule t1 c1 handle 6 ... OK > > # testing for cmd: nft flush chain t1 c2 ... OK > > # testing for cmd: nft flush table t2 ... OK > > # testing for cmd: nft delete chain t2 c2 ... OK > > # testing for cmd: nft delete element t1 s { 22 } ... OK > > # testing for cmd: nft delete element t1 s { 80, 443 } ... FAIL > > # -table=t1 family=2 entries=2 op=nft_unregister_setelem > > # +table=t1 family=2 entries=1 op=nft_unregister_setelem > > # testing for cmd: nft flush set t1 s2 ... FAIL > > My fault, Pablo, please toss all of my patches. > > I do not know when I will resend, so do not wait. At quick glance, I can see the audit logic is based in transaction objects, so now it counts one single entry for the two elements in one single transaction. I can look into this to fix this. Florian, are you seing any other issues apart for this miscount? Thanks.
Pablo Neira Ayuso <pablo@netfilter.org> wrote: > At quick glance, I can see the audit logic is based in transaction > objects, so now it counts one single entry for the two elements in one > single transaction. I can look into this to fix this. > > Florian, are you seing any other issues apart for this miscount? Yes, crash when bisecting (its "fixed" by later patch, hunk must be moved to earlier patch), nft_trans_elems_activate must emit notify also for update case. Maybe more. Just remove these patches.
On Tue, Oct 15, 2024 at 12:20:59AM +0200, Florian Westphal wrote: > Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > At quick glance, I can see the audit logic is based in transaction > > objects, so now it counts one single entry for the two elements in one > > single transaction. I can look into this to fix this. > > > > Florian, are you seing any other issues apart for this miscount? > > Yes, crash when bisecting (its "fixed" by later patch, > hunk must be moved to earlier patch), nft_trans_elems_activate > must emit notify also for update case. Correct, event is missed. > Maybe more. Just remove these patches. OK, this needs more work.
Hi, The following series contains Netfilter updates for net-next: 1) Fix sparse warning in nf_tables related to use of percpu counters, from Uros Bizjak. 2) use strscpy_pad in nft_meta_bridge, from Justin Stitt. 3) A series from patch #3 to patch #7 to reduce memory footprint of set element transactions, Florian Westphal says: When doing a flush on a set or mass adding/removing elements from a set, each element needs to allocate 96 bytes to hold the transactional state. In such cases, virtually all the information in struct nft_trans_elem is the same. Change nft_trans_elem to a flex-array, i.e. a single nft_trans_elem can hold multiple set element pointers. The number of elements that can be stored in one nft_trans_elem is limited by the slab allocator, this series limits the compaction to at most 62 elements as it caps the reallocation to 2048 bytes of memory. 4) Document legacy toggles for xtables packet classifiers, from Bruno Leitao. 5) Use kfree_rcu() instead of call_rcu() + kmem_cache_free(), from Julia Lawall. Please, pull these changes from: git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next.git nf-next-24-10-14 Thanks. ---------------------------------------------------------------- The following changes since commit f66ebf37d69cc700ca884c6a18c2258caf8b151b: Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net (2024-10-03 10:05:55 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next.git tags/nf-next-24-10-14 for you to fetch changes up to 9539446cc659e390942b46df871f8abdd4750999: netfilter: replace call_rcu by kfree_rcu for simple kmem_cache_free callback (2024-10-14 12:30:20 +0200) ---------------------------------------------------------------- netfilter pull request 24-10-14 ---------------------------------------------------------------- Breno Leitao (1): netfilter: Make legacy configs user selectable Florian Westphal (5): netfilter: nf_tables: prefer nft_trans_elem_alloc helper netfilter: nf_tables: add nft_trans_commit_list_add_elem helper netfilter: nf_tables: prepare for multiple elements in nft_trans_elem structure netfilter: nf_tables: switch trans_elem to real flex array netfilter: nf_tables: allocate element update information dynamically Julia Lawall (1): netfilter: replace call_rcu by kfree_rcu for simple kmem_cache_free callback Justin Stitt (1): netfilter: nf_tables: replace deprecated strncpy with strscpy_pad Uros Bizjak (1): netfilter: nf_tables: Fix percpu address space issues in nf_tables_api.c include/net/netfilter/nf_tables.h | 25 +-- net/bridge/netfilter/Kconfig | 8 +- net/bridge/netfilter/nft_meta_bridge.c | 2 +- net/ipv4/netfilter/Kconfig | 16 +- net/ipv6/netfilter/Kconfig | 9 +- net/netfilter/nf_conncount.c | 10 +- net/netfilter/nf_conntrack_expect.c | 10 +- net/netfilter/nf_tables_api.c | 370 +++++++++++++++++++++++++-------- net/netfilter/xt_hashlimit.c | 9 +- 9 files changed, 330 insertions(+), 129 deletions(-)