Re: [PATCH bpf-next 1/2] bpf: Simplify cnum contains() and normalize() implementation

[Shung-Hsi Yu <shung-hsi.yu@suse.com>]

On Tue, Jul 28, 2026 at 01:56:58AM +0000, Vinicius Sampaio wrote:
> This patch introduces two optimizations one for the normalize() function
> and other for the contains() function.

Nit: is there a reason these two optimizations need to land as a single
commit?

> In normalize(), there's no need to compare cnum.base with ST_MAX,
> because any cnum with size == UT_MAX represents the full unsigned domain
> independently of base, so normalize() can canonicalize all such values
> to base 0.
> 
> In contains(), bearing in mind that a non-empty cnum represents a
> inclusive circular range in the corresponding unsigned integer domain,
> membership could be tested by checking whether the distance from the
> range base to the queried value is within the range size.
> 
> This makes the explicit wrapping and non-wrapping cases in contains()
> unnecessary, since:
> 
>         v - cnum.base <= cnum.size
> 
> is equivalent for both ordinary ranges and ranges that cross the
> unsigned wrap boundary.
[...]
> @@ -181,7 +181,7 @@ void FN(intersect_with_srange)(struct cnum_t *dst, st min, st max)
>  
>  static inline struct cnum_t FN(normalize)(struct cnum_t cnum)
>  {
> -	if (cnum.size == UT_MAX && cnum.base != 0 && cnum.base != (ut)ST_MAX)
> +	if (cnum.size == UT_MAX && cnum.base != 0)
>  		cnum.base = 0;

Seems right, or at least I could see what semantic `base == ST_MAX` is
encoding. And normalize does not deal with the empty semantic anyway[1].
Could we go even further and also drop the `cnum.base != 0`, just
relying on the `cnum.size == UT_MAX` guard solely?

>  	return cnum;
>  }
> @@ -210,12 +210,7 @@ bool FN(is_empty)(struct cnum_t cnum)
>  
>  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;
>  }

Make sense, with `v - cnum.base` we are pivoting to a cnum where

  { .base = 0, .size = cnum.size }

If `v - cnum.base` is within the pivoted cnum, then `v` should be in the
original cnum as well.

Acked-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>

1: https://lore.kernel.org/bpf/7a5c9c3e8ef2cb86b7ae38fb4593e7337e9962c0.camel@gmail.com/