On Mon, Aug 18, 2025 at 11:58:22AM +0100, Loftus, Ciara wrote:
> > >
> > > They're not necessarily needed but I think they improve the readability
> > > when
> > they are used in the next three patches when defining the features of each
> > Rx
> > burst function. If you don't agree I can remove them.
> > >
> >
> > I'd tend to prefer assigning bools to just true/false for type-safety and
> > type-clarity. In terms of readability with defines like above: if we do
> > want separate defines for scattered, flex-desc etc. I'd tend toward having
> > them as enums and the variables below typed using those enums.
> > Unfortunately, that's not really feasible with the struct below, because
> > while bools only occupy 1 byte, even a two-element enum would use up 4
> > bytes per value, which is padding we cannot afford. :-(
> >
> >
>
> I went with splitting out the boolean features into their own sub-structure,
> which you suggested in a previous thread.
>
> The definition can look like this now:
>
> [I40E_RX_AVX512_SCATTERED] = { i40e_recv_scattered_pkts_vec_avx512,
> "Vector AVX512 Scattered", {I40E_RX_VECTOR_OFFLOADS, RTE_VECT_SIMD_512,
> {.scattered = true, .bulk_alloc = true}}},
>
> Let me know what you think. It's perhaps a bit unusual with the nested
> structures but I'm ok with that since it's more readable than a series of
> true/false. However, if you would like to just go with the simple true/false
> approach I'm happy to spin up a new patch.
>
> Ciara
>
I'll review in the v3, but that does indeed look quite readable to me.
/Bruce
