Re: [PATCH v2 bpf-next 0/4] Add internal-only BPF per-CPU instruction

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

 



On Wed, Apr 3, 2024 at 11:04 AM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Wed, Apr 3, 2024 at 10:41 AM Alexei Starovoitov
> <alexei.starovoitov@xxxxxxxxx> wrote:
> >
> > On Mon, Apr 1, 2024 at 7:13 PM Andrii Nakryiko <andrii@xxxxxxxxxx> wrote:
> > >
> > > Add a new BPF instruction for resolving per-CPU memory addresses.
> > >
> > > New instruction is a special form of BPF_ALU64 | BPF_MOV | BPF_DW, with
> > > insns->off set to BPF_ADDR_PERCPU (== -1). It resolves provided per-CPU offset
> > > to an absolute address where per-CPU data resides for "this" CPU.
> >
> > Fixed the typo s/BPF_DW/BPF_X/ while applying.
>
> Oops, thanks!
>
> >
> > I'm still a bit worried that there is no BUILD_BUG_ON
>
> BUILD_BUG_ON is problematic, because those gen_lookup callbacks are
> compiled on 32-bit arches just fine (just not used). I'm totally fine
> adding extra checks as a follow up. Let's just agree on some
> consistent approach her?
>
> See [0] for the verifier check I mentioned. I can
>
> a) add #ifdef guards around all current gen_lookup callbacks to only
> build them on 64-bit arches
>
> or
>
> b) add runtime checks inside get_lookup to return -EOPNOTSUPP if
> architecture isn't 64-bit.
>
> I don't particularly care which one, I care that it's done for all
> callbacks if we are doing this. Please let me know your preference.
>
>   [0] https://github.com/torvalds/linux/blob/master/kernel/bpf/verifier.c#L19942-L19946

Ahh. BITS_PER_LONG == 64 check is for a different reason,
but that works. It won't be lifted any time soon,
so not worried any more about that assumption inside gen_lookup.
Let's keep things as-is.





[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