Re: [PATCH nft] datatype: accept a numeric cgroupsv2 id on input

On Tue, Jul 28, 2026 at 09:16:54PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Jul 28, 2026 at 06:19:25PM +0200, Phil Sutter wrote:
> > Hi,
> > 
> > On Mon, Jul 27, 2026 at 01:54:27PM +0530, Avinash Duduskar wrote:
> > > Once a cgroup is removed there is no path left to resolve, so the listing
> > > falls back to printing the raw id:
> > > 
> > >   # nft list set ip t s
> > >   table ip t {
> > >           set s {
> > >                   type cgroupsv2
> > >                   elements = { 50834 }
> > >           }
> > >   }
> > > 
> > > That output is not valid input. cgroupv2_type_parse() only stats
> > > /sys/fs/cgroup/<identifier>, so handing the id back fails:
> > > 
> > >   # nft delete element ip t s { 50834 }
> > >   Error: cgroupv2 path fails: No such file or directory
> > > 
> > > The element cannot be addressed by key at all once its cgroup is gone,
> > > which leaves flushing the containing set as the only way to remove it.
> > > The json dump does not reload either, since the id is serialised as a
> > > string and reaches the same parser.
> > > tests/shell/testcases/packetpath/cgroupv2 works around this in its
> > > cleanup(), added by commit 9fbc77c127e7 ("tests: shell: add cgroupv2
> > > socket match test case"), because "nft can't find the human-readable
> > > names anymore".
> > > 
> > > Take the id back when the path does not resolve, accepting a decimal
> > > number and nothing else. integer_type_parse() converts with base 0 and
> > > expr_evaluate_integer() does not check the sign, so it would take "0755"
> > > as 493, "0x1f" as 31 and "-1" as 1; strtoull() alone still accepts a
> > > leading sign or space. Requiring a leading digit closes both. The path
> > > lookup stays first, so a cgroup named as a number still resolves as a
> > > path.
> > 
> > So with nft printing non-existent cgroup names using PRIu64 and parsing
> > code still performs the path lookup first, what is all the fuss about
> > non-decimal number input?
> 
> They claim they cannot delete entries via numeric value, it seems.

Sure, I wondered why parsing must be so strict. If the string we get is
not a known cgroup name, something's odd anyway and I don't see the
problem if the fallback parser also accepts hex and octal or even
negative values and whitespace-prefixed number strings.

Is it true that integer_type_parse respects negative values as such?
Because that void avoid the mentioned issue of '-1' becoming '1'.
Negative values would just not match.

IMHO we should once try to change symbol_parse to try basetype parsers
also if the dtype parser is present but fails. This might simplify some
datatype parsers. Or cause a big mess because "anything" may then be
written as a number. :D

> > > A plain decimal name that does not resolve now becomes an id rather than
> > > an error, the way meta skuid falls back to the numeric uid when there is
> > > no passwd entry. nft accepting what nft printed is worth more than
> > > diagnosing a mistyped path.
> > > 
> > > Fixes: 38228087252c ("src: add cgroupsv2 support")
> > > Signed-off-by: Avinash Duduskar <[email protected]>
> > > ---
> > > cgroupv2_type_print() still ignores -n and prints the resolved path when it
> > > can. This patch only makes the id nft already prints acceptable as input;
> > > making the print side honour -n is a separate change.
> > > 
> > > dumps/cgroupv2_stale_id.nft is empty and the json dump is the metainfo
> > > boilerplate, because the test leaves no ruleset behind and the ids it uses are
> > > not stable across runs. There is no .nodump, so the dump comparison still runs.
> > > 
> > > No NFT_TEST_REQUIRES: the test uses no socket expression, so neither
> > > NFT_TEST_HAVE_cgroupsv2 nor nft_socket is involved. It skips with 77 without a
> > > writable cgroup2 mount, and the json arm skips unless -j actually works.
> > > 
> > > Searched the netfilter-devel archive for cgroupv2: 133 messages, 2021-04-21 to
> > > 2026-05-28, none proposing that a numeric id be accepted on input.
> > > 
> > > Tested on b315dd55 with a libnftnl built from git 363b0e3, since master needs
> > > nftnl_set_elem_set_imm which is not in libnftnl 1.3.1. The suite gains this one
> > > test and no regressions; two pre-existing failures are unchanged with and
> > > without the patch.
> > 
> > Is this AI output?
> 
> Looks like so, yes. It is very cheap to write lengthy reports these days.
> 
> We will soon have to funnel this text to a LLM to "please, summarize
> in a terse description what this user reports", then write a reply and
> say "please expand it to make it as lengthy but formal as you can" :-)

That's an interesting approach, indeed! Reading the lengthy reports
causes extra work, so make it more expensive for them by requesting more
elaborate reports.

Cheers, Phil