Re: [PATCH bpf-next 06/13] bpf: Add tracing_multi link fdinfo support for bpf progs

On 10/8/26 00:20, [email protected] wrote:
>> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
>> index a805a5b73303f..80928401830c7 100644
>> --- a/kernel/trace/bpf_trace.c
>> +++ b/kernel/trace/bpf_trace.c
>> @@ -3759,26 +3759,46 @@ static int bpf_tracing_multi_link_fill_link_info(const struct bpf_link *link,
>>  }
>>
>>  #ifdef CONFIG_PROC_FS
>> +static const char *bpf_prog_func_name(struct bpf_prog *prog, u32 btf_id)
>> +{
>> +	const struct btf *btf = prog->aux->btf;
>> +	const struct btf_type *t;
>> +
>> +	t = btf_type_by_id(btf, btf_id);
>> +	if (!t || !btf_type_is_func(t))
>> +		return "";
>> +
>> +	return btf_name_by_offset(btf, t->name_off);
>> +}
>> +
>>  static void bpf_tracing_multi_show_fdinfo(const struct bpf_link *link,
>>  					  struct seq_file *seq)
>>  {
>>  	struct bpf_tracing_multi_link *tr_link =
>>  		container_of(link, struct bpf_tracing_multi_link, link);
>>  	bool has_cookies = !!tr_link->cookies;
>> +	bool has_progs = !!tr_link->progs;
>>
>>  	seq_printf(seq, "attach_type:\t%u\n", tr_link->link.attach_type);
>>  	seq_printf(seq, "cnt:\t%u\n", tr_link->nodes_cnt);
>>
>> -	seq_printf(seq, "%s\t %s\t %s\t %s\n", "obj-id", "btf-id", "cookie", "func");
>> +	seq_printf(seq, "%s\t %s\t %s\t %s\n", has_progs ? "prog-id" : "obj-id", "btf-id", "cookie",
>> +		   "func");
>>  	for (int i = 0; i < tr_link->nodes_cnt; i++) {
>>  		struct bpf_tracing_multi_node *mnode = &tr_link->nodes[i];
>> +		u64 cookie = has_cookies ? tr_link->cookies[i] : 0;
>>  		u32 btf_id, obj_id;
>>
>>  		bpf_trampoline_unpack_key(mnode->trampoline->key, &obj_id, &btf_id);
>> +		btf_id = has_progs ? (u32) mnode->trampoline->key : btf_id;
>                         ^^^^
> 
> This isn't a bug, but since bpf_trampoline_unpack_key() only masks off
> bit 31, which is never set for prog targets, does this re-assignment
> change anything, or could the unpacked value be used directly?
> 
> bpf_trampoline_unpack_key() already sets btf_id = key & 0x7FFFFFFF, and
> bit 31 is only set by the !tgt_prog branch of bpf_trampoline_compute_key()
> (the kernel-BTF case).
> 
> In the has_progs case the key was built as:
> 
>     ((u64)tgt_prog->aux->id << 32) | ids[i]
> 
> and ids[i] must be a valid BTF type id (<= BTF_MAX_TYPE, 0xfffff) or
> bpf_check_attach_target() would have rejected the attach, so (u32)key
> and the unpacked btf_id are always equal here.
> 

Good catch.

Will drop this line.

>> +		if (has_progs)
>> +			seq_printf(seq, "%u\t %u\t %llu\t %s\n",
>> +				   obj_id, btf_id, cookie,
>> +				   bpf_prog_func_name(tr_link->progs[i], btf_id));
>> +		else
>> +			seq_printf(seq, "%u\t %u\t %llu\t %pS\n",
>> +				   obj_id, btf_id, cookie,
>> +				   (void *) mnode->trampoline->ip);
>>
>>  		cond_resched();
>>  	}
> 
> The commit message shows the sample fdinfo output but doesn't say much
> about the implementation or why this change is needed. In particular:
> 
>> Show the target bpf prog info in the fdinfo.
>>
>> $ cat /proc/self/fdinfo/13
>> pos:    0
>> flags:  02000000
>> mnt_id: 18
>> ino:    1045
>> link_type:      tracing_multi
>> link_id:        9
>> prog_tag:       9a7f9e3ab6f138ab
>> prog_id:        25
>> attach_type:    61
>> cnt:    2
>> prog-id  btf-id  cookie  func
>> 21       4       16706819        target_1
>> 22       5       16706819        target_2
> 
> This isn't a bug, but could the changelog say why the trampoline ip is
> not usable as the func column for prog targets (tr->ip is 0 for JIT'd
> prog addresses), rather than mostly showing the resulting output? It
> also doesn't mention that the first column header changes from "obj-id"
> to "prog-id" for prog-target links.
> 

Hmm, will describe the change in the commit msg.

Thanks,
Leon

> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31320427087