On Wed, Nov 15, 2023 at 2:07 PM Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > On Wed, Nov 15, 2023 at 3:25 PM Alexei Starovoitov > <alexei.starovoitov@xxxxxxxxx> wrote: > > > > On Sat, Nov 11, 2023 at 5:06 PM Andrii Nakryiko <andrii@xxxxxxxxxx> wrote: > > > > > > > > > By default, sanity violation will trigger a warning in verifier log and > > > resetting register bounds to "unbounded" ones. But to aid development > > > and debugging, BPF_F_TEST_SANITY_STRICT flag is added, which will > > > trigger hard failure of verification with -EFAULT on register bounds > > > violations. This allows selftests to catch such issues. veristat will > > > also gain a CLI option to enable this behavior. > > ... > > > + bool test_sanity_strict; /* fail verification on sanity violations */ > > ... > > > +/* The verifier internal test flag. Behavior is undefined */ > > > +#define BPF_F_TEST_SANITY_STRICT (1U << 7) > > > > Applied, but please follow up with a rename. > > > > The name of the flag here in uapi and in the "veristat --test-sanity" > > will be a subject of bad jokes. > > The flag is asking the verifier to test its own sanity? > > Can the verifier go insane? > > Let's call it TEST_RANGE_ACCOUNTING or something. > > I'm guessing you didn't qualify it with 'range' to reuse it > > in the future for other 'sanity' checks? > > We can add another flag later. > > Like BPF_F_TEST_STATE_FREQ is pretty specific and it's a good thing. > > I think being specific like BPF_F_TEST_RANGE_TRACKING or > > RANGE_ACCOUNTING is better long term. > > Sure, I like BPF_F_TEST_RANGE_TRACKING_STRICT. Or you want to drop the > _STRICT suffix? We can also do something like > BPF_F_TEST_REG_INVARIANTS_STRICT or something to keep it a bit more > generic? Both names sound fine. I prefer without _strict mainly because I'm confused by its meaning. With _strict it sounds like the verifier already testing range tracking, but not strict enough. Whereas without the flag there is no enforcement at all.
