| Message ID | 20260831194905.45045-1-kmehltretter@gmail.com |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | [6.1.y,6.6.y] netfilter: nft_set_pipapo: restore cloned mapping table memcg accounting | expand |
On Mon, Aug 31, 2026 at 09:49:05PM +0200, Karl Mehltretter wrote: > The 6.1.y and 6.6.y backports of commit 69e687cea79f > ("netfilter: nf_tables: missing objects with no memcg accounting") > changed the pipapo_clone() mapping-table allocation to > GFP_KERNEL_ACCOUNT. > > Later stable backports of commit 07ace0bbe03b ("netfilter: > nft_set_pipapo: do not rely on ZERO_SIZE_PTR") rewrote this allocation > and changed the flag back to GFP_KERNEL. Upstream applied the > ZERO_SIZE_PTR change before the memcg accounting change and retains > GFP_KERNEL_ACCOUNT after both. > > nft_pipapo_deactivate() clones a populated set before looking up an > element to delete. If the element is absent, priv->dirty remains false > and the clone is retained. Its mapping tables therefore remain allocated > without being charged to the requesting memory cgroup. > > Restore GFP_KERNEL_ACCOUNT for the cloned mapping table. > > For a 32,768-element two-field concatenation set, A/B tests on 6.1.186 > and 6.6.155 measured exactly 524,288 additional bytes charged to the > memory cgroup after restoring the flag. An unmodified 6.12.107 control > already accounted the corresponding allocation and showed the same > accounting behavior. > > Fixes: a4983e89e3b1 ("netfilter: nft_set_pipapo: do not rely on ZERO_SIZE_PTR") > Fixes: 32bad10de347 ("netfilter: nft_set_pipapo: do not rely on ZERO_SIZE_PTR") This two Fixes: tag do not make any sense to me. > Assisted-by: LLM > Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com> > --- > net/netfilter/nft_set_pipapo.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c > index 7c8d28a031ad..4c3bb5d61a3b 100644 > --- a/net/netfilter/nft_set_pipapo.c > +++ b/net/netfilter/nft_set_pipapo.c > @@ -1465,7 +1465,7 @@ static struct nft_pipapo_match *pipapo_clone(struct nft_pipapo_match *old) > goto out_mt; > > dst->mt = kvmalloc_array(src->rules, sizeof(*src->mt), > - GFP_KERNEL); > + GFP_KERNEL_ACCOUNT); > if (!dst->mt) > goto out_mt; > > -- > 2.39.5 (Apple Git-154) >
On Tue, Sep 01, 2026 at 12:15:29AM +0100, Pablo Neira Ayuso wrote: > > > > Fixes: a4983e89e3b1 ("netfilter: nft_set_pipapo: do not rely on ZERO_SIZE_PTR") > > Fixes: 32bad10de347 ("netfilter: nft_set_pipapo: do not rely on ZERO_SIZE_PTR") > > This two Fixes: tag do not make any sense to me. The two Fixes tags are for the two stable branches: - a4983e89e3b1 is the 6.1.y backport - 32bad10de347 is the 6.6.y backport Both backports changed this allocation from GFP_KERNEL_ACCOUNT back to GFP_KERNEL, which introduced the regression. The accounting was correct before these commits. I used one Fixes tag for each target tree. I am not sure what the usual preference is here: one common patch with both Fixes tags, or separate patches for each tree. Which do you prefer? Thanks, Karl
> Fixes: a4983e89e3b1 ("netfilter: nft_set_pipapo: do not rely on ZERO_SIZE_PTR") > Fixes: 32bad10de347 ("netfilter: nft_set_pipapo: do not rely on ZERO_SIZE_PTR") > > This two Fixes: tag do not make any sense to me. > The two Fixes tags are for the two stable branches: > > - a4983e89e3b1 is the 6.1.y backport > - 32bad10de347 is the 6.6.y backport > > I am not sure what the usual preference is here: one common patch with > both Fixes tags, or separate patches for each tree. Which do you prefer? Whatever Pablo prefers :)
On Wed, Sep 02, 2026 at 06:14:13PM -0400, Sasha Levin wrote: > > Fixes: a4983e89e3b1 ("netfilter: nft_set_pipapo: do not rely on ZERO_SIZE_PTR") > > Fixes: 32bad10de347 ("netfilter: nft_set_pipapo: do not rely on ZERO_SIZE_PTR") > > > > This two Fixes: tag do not make any sense to me. > > > The two Fixes tags are for the two stable branches: > > > > - a4983e89e3b1 is the 6.1.y backport > > - 32bad10de347 is the 6.6.y backport > > > > I am not sure what the usual preference is here: one common patch with > > both Fixes tags, or separate patches for each tree. Which do you prefer? > > Whatever Pablo prefers :) I am fine either way. I just got confused with the double Fixes: tag that point to the -stable tree commit hash (if I understood correctly after Karl's explaination).
On Thu, Sep 03, 2026 at 01:14:03AM +0100, Pablo Neira Ayuso wrote: > On Wed, Sep 02, 2026 at 06:14:13PM -0400, Sasha Levin wrote: > > > > Whatever Pablo prefers :) > > I am fine either way. I just got confused with the double Fixes: tag > that point to the -stable tree commit hash (if I understood correctly > after Karl's explaination). Thanks, I'll leave this submission as is. If there's a similar fix in the future, I will probably split it to make it more clear. Karl
diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c index 7c8d28a031ad..4c3bb5d61a3b 100644 --- a/net/netfilter/nft_set_pipapo.c +++ b/net/netfilter/nft_set_pipapo.c @@ -1465,7 +1465,7 @@ static struct nft_pipapo_match *pipapo_clone(struct nft_pipapo_match *old) goto out_mt; dst->mt = kvmalloc_array(src->rules, sizeof(*src->mt), - GFP_KERNEL); + GFP_KERNEL_ACCOUNT); if (!dst->mt) goto out_mt;
The 6.1.y and 6.6.y backports of commit 69e687cea79f ("netfilter: nf_tables: missing objects with no memcg accounting") changed the pipapo_clone() mapping-table allocation to GFP_KERNEL_ACCOUNT. Later stable backports of commit 07ace0bbe03b ("netfilter: nft_set_pipapo: do not rely on ZERO_SIZE_PTR") rewrote this allocation and changed the flag back to GFP_KERNEL. Upstream applied the ZERO_SIZE_PTR change before the memcg accounting change and retains GFP_KERNEL_ACCOUNT after both. nft_pipapo_deactivate() clones a populated set before looking up an element to delete. If the element is absent, priv->dirty remains false and the clone is retained. Its mapping tables therefore remain allocated without being charged to the requesting memory cgroup. Restore GFP_KERNEL_ACCOUNT for the cloned mapping table. For a 32,768-element two-field concatenation set, A/B tests on 6.1.186 and 6.6.155 measured exactly 524,288 additional bytes charged to the memory cgroup after restoring the flag. An unmodified 6.12.107 control already accounted the corresponding allocation and showed the same accounting behavior. Fixes: a4983e89e3b1 ("netfilter: nft_set_pipapo: do not rely on ZERO_SIZE_PTR") Fixes: 32bad10de347 ("netfilter: nft_set_pipapo: do not rely on ZERO_SIZE_PTR") Assisted-by: LLM Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com> --- net/netfilter/nft_set_pipapo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)