Re: [PATCH net-next v9 03/10] lib: packing: add pack_fields() and unpack_fields()

[Vladimir Oltean <vladimir.oltean@nxp.com>]

On Wed, Dec 04, 2024 at 05:22:49PM -0800, Jacob Keller wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> This is new API which caters to the following requirements:
> 
> - Pack or unpack a large number of fields to/from a buffer with a small
>   code footprint. The current alternative is to open-code a large number
>   of calls to pack() and unpack(), or to use packing() to reduce that
>   number to half. But packing() is not const-correct.
> 
> - Use unpacked numbers stored in variables smaller than u64. This
>   reduces the rodata footprint of the stored field arrays.
> 
> - Perform error checking at compile time, rather than runtime, and return
>   void from the API functions. Because the C preprocessor can't generate
>   variable length code (loops), this is a bit tricky to do with macros.
> 
>   To handle this, implement macros which sanity check the packed field
>   definitions based on their size. Finally, a single macro with a chain of
>   __builtin_choose_expr() is used to select the appropriate macros. We
>   enforce the use of ascending or descending order to avoid O(N^2) scaling
>   when checking for overlap. Note that the macros are written with care to
>   ensure that the compilers can correctly evaluate the resulting code at
>   compile time. In particular, care was taken with avoiding too many nested
>   statement expressions. Nested statement expressions trip up some
>   compilers, especially when passing down variables created in previous
>   statement expressions.
> 
>   There are two key design choices intended to keep the overall macro code
>   size small. First, the definition of each CHECK_PACKED_FIELDS_N macro is
>   implemented recursively, by calling the N-1 macro. This avoids needing
>   the code to repeat multiple times.
> 
>   Second, the CHECK_PACKED_FIELD macro enforces that the fields in the
>   array are sorted in order. This allows checking for overlap only with
>   neighboring fields, rather than the general overlap case where each field
>   would need to be checked against other fields.
> 
>   The overlap checks use the first two fields to determine the order of the
>   remaining fields, thus allowing either ascending or descending order.
>   This enables drivers the flexibility to keep the fields ordered in which
>   ever order most naturally fits their hardware design and its associated
>   documentation.
> 
>   The CHECK_PACKED_FIELDS macro is directly called from within pack_fields
>   and unpack_fields, ensuring that all drivers using the API receive the
>   benefits of the compile-time checks. Users do not need to directly call
>   any of the macros directly.
> 
>   The CHECK_PACKED_FIELDS and its helper macros CHECK_PACKED_FIELDS_(0..50)
>   are generated using a simple C program in scripts/gen_packed_field_checks.c
>   This program can be compiled on demand and executed to generate the macro
>   code in include/linux/packing.h. This will aid in the event that a driver
>   needs more than 50 fields. The generator can be updated with a new size,
>   and used to update the packing.h header file. In practice, the ice driver
>   will need to support 27 fields, and the sja1105 driver will need to
>   support 40 fields. This on-demand generation avoids the need to modify
>   Kbuild. We do not anticipate the maximum number of fields to grow very
>   often.
> 
> - Reduced rodata footprint for the storage of the packed field arrays.
>   To that end, we have struct packed_field_s (small) and packed_field_m
>   (medium). More can be added as needed (unlikely for now). On these
>   types, the same generic pack_fields() and unpack_fields() API can be
>   used, thanks to the new C11 _Generic() selection feature, which can
>   call pack_fields_s() or pack_fields_m(), depending on the type of the
>   "fields" array - a simplistic form of polymorphism. It is evaluated at
>   compile time which function will actually be called.
> 
> Over time, packing() is expected to be completely replaced either with
> pack() or with pack_fields().
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> Co-developed-by: Jacob Keller <jacob.e.keller@intel.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---

Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com> # KUnit