On Tue, Feb 25, 2025 at 5:59 AM Leon Hwang <leon.hwang@xxxxxxxxx> wrote:
>
>
>
> On 2025/2/25 06:08, Andrii Nakryiko wrote:
> > On Mon, Feb 24, 2025 at 11:41 AM Alexei Starovoitov
> > <alexei.starovoitov@xxxxxxxxx> wrote:
> >>
> >> On Mon, Feb 24, 2025 at 7:34 AM Leon Hwang <leon.hwang@xxxxxxxxx> wrote:
> >>>
> >>> @@ -3539,7 +3540,7 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
> >>> */
> >>> struct bpf_attach_target_info tgt_info = {};
> >>>
> >>> - err = bpf_check_attach_target(NULL, prog, tgt_prog, btf_id,
> >>> + err = bpf_check_attach_target(log, prog, tgt_prog, btf_id,
> >>> &tgt_info);
> >>
> >> I still don't like this uapi addition.
> >>
> >> It only helps a rare corner case of freplace usage:
> >> /* If there is no saved target, or the specified target is
> >> * different from the destination specified at load time, we
> >> * need a new trampoline and a check for compatibility
> >> */
> >>
> >> If it was useful in more than one case we could consider it,
> >> but uapi addition for a single rare use, is imo wrong trade off.
> >
> > Agreed. I think the idea of verbose log is useful for bpf() syscall,
> > given how complicated some of its conditions are. But it should be
> > done more generically, ideally at syscall (or at least the entire BPF
> > command) level, not for one particular kind of link.
> >
>
> Cool!
>
> But, how can we achieve it?
There is no *elegant* way to do this, but I think we could retrofit
this as extra common bpf_attrs into existing bpf() syscall. Something
along the lines of:
struct bpf_common_attr {
__u64 log_buf;
__u32 log_size;
}
#define BPF_COMMON_ATTRS 0x80000000
static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int
size, bpfptr_t uattr_common, unsigned int size_common)
{
if (cmd & BPF_COMMON_ATTRS) {
cmd &= ~BPF_COMMON_ATTRS;
/* read out bpf_common_attr from uattr_common */
}
}
i.e., we add two extra arguments to bpf() syscall, but only look at
them if we have an extra flag set in cmd.
>
> Thanks,
> Leon
>
