Re: [PATCH bpf-next v3 3/3] bpftool: Add support for BPF_F_PREORDER cgroup attach flag

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: bpftool JSON ABI breakage for attach_flags field
--

commit 7ba7f84eaca8177e2045dffce655aae4da559465
Author: Hui Su <sh_def@163.com>

bpftool: Add support for BPF_F_PREORDER cgroup attach flag

This commit adds support for the BPF_F_PREORDER cgroup attach flag to
bpftool. It updates attach operations to accept the new flag and modifies
the show operations to output combinations of attach flags, rather than
just a single flag.

> diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
> index ce69d1e5468e3..fee1a260f6d1a 100644
> --- a/tools/bpf/bpftool/cgroup.c
> +++ b/tools/bpf/bpftool/cgroup.c

[ ... ]

> +static void show_attach_flags_json(__u32 flags)
> +{
> +	char buf[32];
> +	size_t i;
> +
> +	jsonw_name(json_wtr, "attach_flags");
> +	jsonw_start_array(json_wtr);
> +
> +	for (i = 0; i < ARRAY_SIZE(attach_flag_names); i++) {
> +		if (!(flags & attach_flag_names[i].flag))
> +			continue;
> +
> +		jsonw_string(json_wtr, attach_flag_names[i].name);
> +		flags &= ~attach_flag_names[i].flag;
> +	}
> +
> +	if (flags) {
> +		snprintf(buf, sizeof(buf), "unknown(%x)", flags);
> +		jsonw_string(json_wtr, buf);
> +	}
> +
> +	jsonw_end_array(json_wtr);
> +}

[ ... ]

> @@ -182,7 +240,7 @@ static int show_bpf_prog(int id, enum bpf_attach_type attach_type,
>  		else
>  			jsonw_uint_field(json_wtr, "attach_type", attach_type);
>  		if (!(query_flags & BPF_F_QUERY_EFFECTIVE))
> -			jsonw_string_field(json_wtr, "attach_flags", attach_flags_str);
> +			show_attach_flags_json(attach_flags);

[Severity: Medium]
Does this change break bpftool's JSON ABI by changing the attach_flags
field from a string to an array?

Previously, this field was output as a single string. Changing the data type
to an array could cause external scripts or applications using strict JSON
parsers to crash or fail to unmarshal the output when running 
bpftool cgroup show -j.

Would it be safer to format the multiple flags as a single comma-separated
string in the JSON output, similar to what is done for the plain-text output,
to maintain backwards compatibility?

>  		jsonw_string_field(json_wtr, "name", prog_name);
>  		if (attach_btf_name)
>  			jsonw_string_field(json_wtr, "attach_btf_name", attach_btf_name);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922025442.3176057-1-sh_def@163.com?part=3