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

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Wed, Jul 29, 2026 at 11:07:48PM +0530, Avinash Duduskar wrote:
> nft prints non-existent cgroup names as the raw id using PRIu64, but
> cgroupv2_type_parse() only stats /sys/fs/cgroup/<identifier>, so the
> listing does not load back:
> 
>   # nft list set ip t s
>   table ip t {
>           set s {
>                   type cgroupsv2
>                   elements = { 50834 }
>           }
>   }
>   # nft delete element ip t s { 50834 }
>   Error: cgroupv2 path fails: No such file or directory
> 
> The element cannot be deleted by key once its cgroup is gone, only
> flushed with the set, and a dump does not restore. The json dump has
> the same problem, and the stale id has been visible in the wild since
> 2022 (see Link). tests/shell/testcases/packetpath/cgroupv2 already
> works around this in cleanup().
> 
> Fall back to integer_type_parse() when the path does not resolve, as
> boolean_type_parse() already does. The path lookup stays first, so a
> cgroup named as a number still resolves as a path. Improving the
> integer parser is left for a follow-up as discussed in v1.
> 
> Fixes: 38228087252c ("src: add cgroupsv2 support")
> Link: https://lore.kernel.org/netfilter-devel/fabde324-383a-622c-7e69-32c9b2d06191@gmail.com/
> Signed-off-by: Avinash Duduskar <avinash.duduskar@gmail.com>
> ---
> Changes since v1 (https://lore.kernel.org/netfilter-devel/20260727082427.740789-1-avinash.duduskar@gmail.com/):
> - drop the strict decimal parser, delegate to integer_type_parse()
>   (Pablo, Phil)
> - json test arm uses a here-string instead of a tempfile (Phil)
> - shorter commit message and test
> 
>  src/datatype.c                                |  13 +-
>  .../shell/testcases/parsing/cgroupv2_stale_id | 129 ++++++++++++++++++
>  .../parsing/dumps/cgroupv2_stale_id.json-nft  |  11 ++
>  .../parsing/dumps/cgroupv2_stale_id.nft       |   0
>  4 files changed, 151 insertions(+), 2 deletions(-)
>  create mode 100755 tests/shell/testcases/parsing/cgroupv2_stale_id
>  create mode 100644 tests/shell/testcases/parsing/dumps/cgroupv2_stale_id.json-nft
>  create mode 100644 tests/shell/testcases/parsing/dumps/cgroupv2_stale_id.nft
> 
> diff --git a/src/datatype.c b/src/datatype.c
> index 4dbca16e..5b3b350c 100644
> --- a/src/datatype.c
> +++ b/src/datatype.c
> @@ -1665,9 +1665,18 @@ static struct error_record *cgroupv2_type_parse(struct parse_ctx *ctx,
>  		 SYSFS_CGROUPSV2_PATH, sym->identifier);
>  	cgroupv2_path[sizeof(cgroupv2_path) - 1] = '\0';
>  
> -	if (stat(cgroupv2_path, &st) < 0)
> +	if (stat(cgroupv2_path, &st) < 0) {
> +		struct error_record *erec;
> +		int stat_errno = errno;
> +
> +		/* the listing prints a raw id once the path is gone */
> +		erec = integer_type_parse(ctx, sym, res);
> +		if (!erec)
> +			return NULL;
> +		erec_destroy(erec);

Any reason to ignore the error that integer_type_parse() provides?
Instead I would suggest:

                if (erec)
                        return erec;

Thanks.

>  		return error(&sym->location, "cgroupv2 path fails: %s",
> -			     strerror(errno));
> +			     strerror(stat_errno));
> +	}
>  
>  	ino = st.st_ino;
>  	*res = constant_expr_alloc(&sym->location, &cgroupv2_type,