On Wed, Apr 3, 2024 at 1:22 AM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Sat, Mar 30, 2024 at 8:44 PM Yafang Shao <laoar.shao@xxxxxxxxx> wrote:
> >
> > Three new kfuncs, namely bpf_iter_bits_{new,next,destroy}, have been
> > added for the new bpf_iter_bits functionality. These kfuncs enable the
> > iteration of the bits from a given address and a given number of bits.
> >
> > - bpf_iter_bits_new
> > Initialize a new bits iterator for a given memory area. Due to the
> > limitation of bpf memalloc, the max number of bits to be iterated
> > over is (4096 * 8).
> > - bpf_iter_bits_next
> > Get the next bit in a bpf_iter_bits
> > - bpf_iter_bits_destroy
> > Destroy a bpf_iter_bits
> >
> > The bits iterator can be used in any context and on any address.
> >
> > Changes:
> > - v4->v5:
> > - Simplify test cases (Andrii)
>
> hm... I think you oversimplified them :) Your negative tests are good,
I misinterpreted your earlier comment :(
> but there is now no "positive" test that shows that iterator functions
> properly. I'd suggest to add few positive tests (still within
> RUN_TESTS framework) using __retval (grep for other use cases using
> it) to check actual values.
>
> I think you need to validate that iterator works for:
>
> a) bit mask smaller than 8 bytes (and also maybe number of bits that
> are not multiple of 8, e.g., have 20 bit bitmask, but set entire
> 32-bit value to 1s, check that you get only 20 iterator next()
> returns)
>
> b) a typical case of having 8-byte bit mask (you can have some
> irregular pattern of bits, like 0b1111000111001101, and calculate a
> sum of bit positions, returned for __retval check. This will catch
> endianness issues, if there are still any.
>
> c) another typical case where bit mask is > 8 bytes (same idea with
> position sum would work, but probably just counting number of bits
> would be enough)
Thank you for providing such a detailed explanation. I will take some
time to carefully consider it.
--
Regards
Yafang
