Subject: Re: [PATCH iwl-net v3] ice: fix ice_parser_rt::bst_key array size

From: Simon Horman <[email-masked]>
Date: Mon, 6 Jan 2025 10:23:34 +0000

On Thu, Dec 19, 2024 at 12:55:16PM +0100, Przemek Kitszel wrote:
> Fix &ice_parser_rt::bst_key size. It was wrongly set to 10 instead of 20
> in the initial impl commit (see Fixes tag). All usage code assumed it was
> of size 20. That was also the initial size present up to v2 of the intro
> series [2], but halved by v3 [3] refactor described as "Replace magic
> hardcoded values with macros." The introducing series was so big that
> some ugliness was unnoticed, same for bugs :/
> 
> ICE_BST_KEY_TCAM_SIZE and ICE_BST_TCAM_KEY_SIZE were differing by one.
> There was tmp variable @j in the scope of edited function, but was not
> used in all places. This ugliness is now gone.
> I'm moving ice_parser_rt::pg_prio a few positions up, to fill up one of
> the holes in order to compensate for the added 10 bytes to the ::bst_key,
> resulting in the same size of the whole as prior to the fix, and miminal
> changes in the offsets of the fields.
> 
> Extend also the debug dump print of the key to cover all bytes. To not
> have string with 20 "%02x" and 20 params, switch to
> ice_debug_array_w_prefix().
> 
> This fix obsoletes Ahmed's attempt at [1].
> 
> [1] https://lore.kernel.org/intel-wired-lan/[email-masked]
> [2] https://lore.kernel.org/intel-wired-lan/[email-masked]
> [3] https://lore.kernel.org/intel-wired-lan/[email-masked]
> 
> Reported-by: Dan Carpenter <[email-masked]>
> Closes: https://lore.kernel.org/intel-wired-lan/[email-masked]
> Fixes: 9a4c07aaa0f5 ("ice: add parser execution main loop")
> CC: Ahmed Zaki <[email-masked]>
> Reviewed-by: Larysa Zaremba <[email-masked]>
> Signed-off-by: Przemek Kitszel <[email-masked]>
> ---
> v3: mention printing change in commit msg, separate prefix from the debug log (Simon)
> 
> v2: same as v3, but lacks code change :(
> 
> v1: https://lore.kernel.org/intel-wired-lan/[email-masked]/T/#mbf984a0faa12a5bdb53460b150201fdd7cc1826a

Thanks for the updates, much appreciated.

Reviewed-by: Simon Horman <[email-masked]>