Re: [PATCH net-next v7 6/9] ice: use <linux/packing.h> for Tx and Rx queue context data

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

On 12/3/2024 5:43 AM, Vladimir Oltean wrote:
> On Mon, Dec 02, 2024 at 04:26:29PM -0800, Jacob Keller wrote:
>> +/**
>> + * ice_pack_rxq_ctx - Pack Rx queue context into a HW buffer
>> + * @ctx: the Rx queue context to pack
>> + * @buf: the HW buffer to pack into
>> + *
>> + * Pack the Rx queue context from the CPU-friendly unpacked buffer into its
>> + * bit-packed HW layout.
>> + */
>> +static void ice_pack_rxq_ctx(const struct ice_rlan_ctx *ctx,
>> +			     ice_rxq_ctx_buf_t *buf)
>> +{
>> +	pack_fields(buf, sizeof(*buf), ctx, ice_rlan_ctx_fields,
>> +		    QUIRK_LITTLE_ENDIAN | QUIRK_LSW32_IS_FIRST);
> 
> An alternative pack_fields() design would enforce that the pbuf argument
> has a sizeof() which reveals the packed buffer size. Pro: one macro
> argument less. Con: too intrusive in forcing authors to write code in a
> certain way maybe?
> 

Yea, I had it that way in an earlier version, but I ultimately felt like
the resulting macro was less friendly, as existing code may not have
structured types with the appropriate sizes.

We still have to enforce that the size value is a constant (to get the
compile time checks) which I guess has its own downsides over forcing it
to be sizeof().

I think I'd prefer to keep the simplicity of the 5 argument style that
does not require forcing your buffers to be structures.

>> +}