Re: [PATCH net-next v5 3/9] lib: packing: add pack_fields() and unpack_fields()

[Jacob Keller <jacob.e.keller@intel.com>]

On 11/13/2024 12:32 PM, Masahiro Yamada wrote:
> On Mon, Nov 11, 2024 at 5:08 PM Jacob Keller <jacob.e.keller@intel.com> 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 generat
>>   variable length code (loops), we can't easily use macros to implement the
>>   overlap checks at compile time.
>>
>>   Instead, check for field ordering and overlap in modpost.
> 
> This is over-engineering.
> 
> modpost should not be bothered just for a small library like this.
> 
> Please do sanity checks within lib/packing.c
> 

With the goal of maintaining compile time checks, we end up either
needing to use generated macros which are O(N^2) if we allow arbitrary
overlap. If we instead allow only only ascending or descending order,
this would drop to O(N) which would avoid needing to have 20k lines of
generated code for the case with 50. I think we could implement them
without forcing drivers to specifically call the correct macro by using
something like __builtin_choose_expr(), tho implementing that macro to
select could be quite long.

Otherwise we can fall back to either module load time checks, or go all
the way back to only sanity checking at executing of pack_fields or
unpack_fields.