Re: [PATCH bpf-next v2] bpf: net: emit anonymous enum with BPF_TCP_CLOSE value explicitly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 16, 2021 at 10:58 PM Yonghong Song <yhs@xxxxxx> wrote:
>
>
>
> On 3/16/21 10:44 PM, Alexei Starovoitov wrote:
> > On Tue, Mar 16, 2021 at 9:29 PM Yonghong Song <yhs@xxxxxx> wrote:
> >> +       BTF_TYPE_EMIT_ENUM(BPF_TCP_ESTABLISHED);
> >> +
> >> +       return 0;
> >> +}
> >> +late_initcall(bpf_emit_btf_type);
> >
> > I think if we burn a dummy function on this it would be a wrong
> > pattern to follow.
>
> Maybe we can pick another initcall to piggyback?
>
> > This is just a nop C statement.
> > Typically we add BUILD_BUG_ON near the places that rely on that constraint.
> > There is such a function already. It's tcp_set_state() as you pointed out.
> > It's not using BTF of course, but I would move above BTF_TYPE_EMIT_ENUM there.
> > I'm not sure why you're calling it "pollute net/ipv4/tcp.c".
>
> This is the minor reason. I first coded in that place and feel awkward
> where we have macro referenced above and we still emit a BTF_TYPE_EMIT
> below although with some comments.
>
> The major reason is I think we may have some uapi type/enum's (e.g., in
> uapi/linux/bpf.h) which will be used in bpf program but not in kernel
> itself. So we cannot generate types in vmlinux btf because of this. So I
> used this case to find a place to generate these btf types.
> BPF_TCP_CLOSE is actually such an example, it happens we have a
> BUILD_BUG_ON in kernel to access it.
> Maybe I am too forward looking?

It's great to be forward looking :)
I'm just having a hard time justifying an empty function with single 'ret' insn
that actually will be called at init time and it will stay empty like this for
foreseeable future. Static analysis tools and whatnot will start sending
patches to remove that empty function.



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux