[Intel-wired-lan] [PATCH iwl-net] ice: fix ice_parser_rt::bst_key array size
Simon Horman
horms at kernel.org
Thu Dec 12 16:30:47 UTC 2024
On Wed, Dec 11, 2024 at 02:26:36PM +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.
>
> This fix obsoletes Ahmed's attempt at [1].
>
> [1] https://lore.kernel.org/intel-wired-lan/20240823230847.172295-1-ahmed.zaki@intel.com
> [2] https://lore.kernel.org/intel-wired-lan/20230605054641.2865142-13-junfeng.guo@intel.com
> [3] https://lore.kernel.org/intel-wired-lan/20230817093442.2576997-13-junfeng.guo@intel.com
>
> Reported-by: Dan Carpenter <dan.carpenter at linaro.org>
> Closes: https://lore.kernel.org/intel-wired-lan/b1fb6ff9-b69e-4026-9988-3c783d86c2e0@stanley.mountain
> Fixes: 9a4c07aaa0f5 ("ice: add parser execution main loop")
> CC: Ahmed Zaki <ahmed.zaki at intel.com>
> Reviewed-by: Larysa Zaremba <larysa.zaremba at intel.com>
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel at intel.com>
Hi Przemek,
I agree that these changes are good. But I wonder if it would be best to
only treat the update size of bst_key as a fix.
...
> diff --git a/drivers/net/ethernet/intel/ice/ice_parser_rt.c b/drivers/net/ethernet/intel/ice/ice_parser_rt.c
> index dedf5e854e4b..d9c38ce27e4f 100644
> --- a/drivers/net/ethernet/intel/ice/ice_parser_rt.c
> +++ b/drivers/net/ethernet/intel/ice/ice_parser_rt.c
> @@ -125,22 +125,20 @@ static void ice_bst_key_init(struct ice_parser_rt *rt,
> else
> key[idd] = imem->b_kb.prio;
>
> - idd = ICE_BST_KEY_TCAM_SIZE - 1;
> + idd = ICE_BST_TCAM_KEY_SIZE - 2;
> for (i = idd; i >= 0; i--) {
> int j;
>
> j = ho + idd - i;
> if (j < ICE_PARSER_MAX_PKT_LEN)
> - key[i] = rt->pkt_buf[ho + idd - i];
> + key[i] = rt->pkt_buf[j];
> else
> key[i] = 0;
> }
>
> - ice_debug(rt->psr->hw, ICE_DBG_PARSER, "Generated Boost TCAM Key:\n");
> - ice_debug(rt->psr->hw, ICE_DBG_PARSER, "%02X %02X %02X %02X %02X %02X %02X %02X %02X %02X\n",
> - key[0], key[1], key[2], key[3], key[4],
> - key[5], key[6], key[7], key[8], key[9]);
> - ice_debug(rt->psr->hw, ICE_DBG_PARSER, "\n");
> + ice_debug_array_w_prefix(rt->psr->hw, ICE_DBG_PARSER,
> + KBUILD_MODNAME "Generated Boost TCAM Key",
Should there be a delimeter between KBUILD_MODNAME and "Generated ..." ?
e.g.:
KBUILD_MODNAME ": Generated Boost TCAM Key",
> + key, ICE_BST_TCAM_KEY_SIZE);
> }
>
> static u16 ice_bit_rev_u16(u16 v, int len)
>
> base-commit: 51a00be6a0994da2ba6b4ace3b7a0d9373b4b25e
> --
> 2.46.0
>
>
More information about the Intel-wired-lan
mailing list