On Tue, Dec 6, 2022 at 1:42 PM John Fastabend <john.fastabend@xxxxxxxxx> wrote:
>
> Andrii Nakryiko wrote:
> > BPF verifier marks some instructions as prune points. Currently these
> > prune points serve two purposes.
> >
> > It's a point where verifier tries to find previously verified state and
> > check current state's equivalence to short circuit verification for
> > current code path.
> >
> > But also currently it's a point where jump history, used for precision
> > backtracking, is updated. This is done so that non-linear flow of
> > execution could be properly backtracked.
> >
> > Such coupling is coincidental and unnecessary. Some prune points are not
> > part of some non-linear jump path, so don't need update of jump history.
> > On the other hand, not all instructions which have to be recorded in
> > jump history necessarily are good prune points.
> >
> > This patch splits prune and jump points into independent flags.
> > Currently all prune points are marked as jump points to minimize amount
> > of changes in this patch, but next patch will perform some optimization
> > of prune vs jmp point placement.
> >
> > No functional changes are intended.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
>
> Acked-by: John Fastabend <john.fastabend@xxxxxxxxx>
>
> > ---
> > include/linux/bpf_verifier.h | 1 +
> > kernel/bpf/verifier.c | 57 +++++++++++++++++++++++++++---------
> > 2 files changed, 44 insertions(+), 14 deletions(-)
> >
> > diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> > index b5090e89cb3f..9870d1d0df01 100644
> > --- a/include/linux/bpf_verifier.h
> > +++ b/include/linux/bpf_verifier.h
> > @@ -452,6 +452,7 @@ struct bpf_insn_aux_data {
> > /* below fields are initialized once */
> > unsigned int orig_idx; /* original instruction index */
> > bool prune_point;
> > + bool jmp_point;
>
> Random thought we might want to make these flags in the future so you
> can have,
>
> type = BPF_PRUNE_POINT | BPF_JMP_POINT | BPF_ITERATOR_POINT
>
> and so on without a bunch of bools.
Bunch of bools are nice because they are explicit :) As long as we
have up to 8 of them, we should be fine, I think. I don't envision
adding BPF_ITERATOR_POINT, I just need prune_point on a few special
helpers.
>
> > };
> >
