Re: [PATCH bpf-next v4 1/3] bpf: Simplify cnum contains() by removing redundant branches
[email protected] Fri, 31 Jul 2026 21:38:19 +0000 (UTC)
| Newsgroups | org.kernel.vger.bpf |
|---|---|
| Message-ID | <0f2e2ab963398c775f5eed7438c6c4054d61e3da6e0a7ae731b56813ad016ab1@mail.kernel.org> |
> diff --git a/kernel/bpf/cnum_defs.h b/kernel/bpf/cnum_defs.h
> index a90e317e3578..ac1f62a0195d 100644
> --- a/kernel/bpf/cnum_defs.h
> +++ b/kernel/bpf/cnum_defs.h
[ ... ]
> bool FN(contains)(struct cnum_t cnum, ut v)
> {
> - if (FN(is_empty)(cnum))
> - return false;
> - if (FN(urange_overflow)(cnum))
> - return v >= cnum.base || v <= (ut)cnum.base + cnum.size;
> - else
> - return v >= cnum.base && v <= (ut)cnum.base + cnum.size;
> + return !FN(is_empty)(cnum) && v - cnum.base <= cnum.size;
> }
[ ... ]
> We've also run some benchmarks to compare the performance of the new
> implementation, for example, for empty ranges, overflowing ranges
> non-overflowing ranges. We've run the tests with O2 optimization.
>
> Results:
>
> | Test | contains | contains_new |
> | -------------------- | ----------- | ------------ |
> | empty | 1.511 ns/op | 1.479 ns/op |
> | non-overflow inside | 1.156 ns/op | 1.270 ns/op |
> | non-overflow outside | 1.053 ns/op | 1.258 ns/op |
> | overflow high side | 1.469 ns/op | 1.257 ns/op |
> | overflow low side | 1.469 ns/op | 1.270 ns/op |
> | overflow outside | 1.478 ns/op | 1.256 ns/op |
> | singleton inside | 1.049 ns/op | 1.256 ns/op |
> | singleton outside | 1.051 ns/op | 1.254 ns/op |
> | full range | 1.053 ns/op | 1.256 ns/op |
This isn't a bug, but would the changelog be tighter without the
benchmark table? The deltas look like measurement noise and several
rows show the new version as slightly slower. The substantive part is
the equivalence argument showing why 'v - cnum.base <= cnum.size' works
for both wrapping and non-wrapping cases. If the numbers are meant to
demonstrate no performance regression, would a one-line summary be
clearer?
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/30534991827