On Fri, Jun 3, 2022 at 3:08 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
[...]
> >
> > for (int i = 0; ...)
>
> Sorry, just to clarify, you want me to pull all loop counter
> declarations to the top of the relevant functions, right? (Affecting 4
> functions in this patch).
Right. There are a few more in later patches.
>
> [...]
>
> > > +static int find_insn_subseq(struct bpf_insn *seq, struct bpf_insn *subseq,
> > > + if (check_unexpected &&
> > > + find_all_insn_subseqs(buf, test->unexpected_insns,
> > > + cnt, MAX_UNEXPECTED_INSNS)) {
> >
> > I wonder whether we want different logic for unexpected_insns. With multiple
> > sub sequences, say seq-A and seq-B, it is more natural to reject any results
> > with either seq-A or seq-B. However, current logic will reject seq-A => seq-B,
> > but will accept seq-B => seq-A. Does this make sense?
>
> Have no strong opinion on this topic. In theory different variants
> might be useful in different cases.
>
> In the test cases for bpf_loop inlining I only had to match a single
> unexpected instruction, so I opted to use same match function in both
> expected and unexpected cases to keep things simple.
I think we can wait until we have an actual use case for multiple seq.
For now, let's keep current logic and document this clearly in struct
bpf_test.
Thanks,
Song
[...]
