Re: [v4,net-next,3/5] octeontx2-af: npc: cn20k: add subbank search order control

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
octeontx2-af: npc: cn20k: add subbank search order control

CN20K NPC MCAM is split into 32 subbanks that are searched in a
predefined order during allocation. Lower-numbered subbanks have
higher priority than higher-numbered ones. Add a runtime devlink
parameter "srch_order" (DEVLINK_PARAM_TYPE_U32_ARRAY) to control the
order in which subbanks are searched during MCAM allocation.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> index e854b85ced9e..348a72c4ee43 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> @@ -3809,6 +3809,93 @@ static void npc_unlock_all_subbank(void)
>  		mutex_unlock(&npc_priv.sb[i].lock);
>  }
>
> +int npc_cn20k_search_order_set(struct rvu *rvu,
> +			       u32 arr[MAX_NUM_SUB_BANKS], int cnt)
> +{

[ ... ]

> +fail_used:
> +	npc_unlock_all_subbank();
> +	mutex_unlock(&mcam->lock);
> +
> +	return rc;
> +}
> +
> +const u32 *npc_cn20k_search_order_get(bool *restricted_order, u32 *sz)
> +{
> +	*restricted_order = restrict_valid;
> +	*sz = npc_priv.num_subbanks;
> +	return subbank_srch_order;
> +}
           ^^^^^^^^^^^^^^^^^^^

Is there a type mismatch here? The function signature declares the return
type as 'const u32 *' but returns 'subbank_srch_order', which appears to be
declared as 'static int *' elsewhere in the code. While the values stored
(0-31 for subbank indices) fit within both int and u32, and sizeof(int) ==
sizeof(u32) on all kernel-supported architectures, this conversion between
signed and unsigned pointer types is technically incompatible.

The callers rvu_af_dl_npc_srch_order_get() and
rvu_af_dl_npc_srch_order_validate() treat the returned pointer as a u32
array:

drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c:rvu_af_dl_npc_srch_order_get()
{
    const u32 *order;
    ...
    order = npc_cn20k_search_order_get(&restricted_order, &sz);
    ...
    for (int i = 0; i < sz; i++)
        ctx->val.u32arr.val[i] = order[i];
}

Should subbank_srch_order be declared as 'static u32 *' instead of 'static
int *', or should the function return type be changed to 'const int *' to
match the actual type?

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.h b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.h
> index 004a556c7b90..b168ecfbc5c8 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.h
> @@ -343,5 +343,7 @@ int npc_cn20k_defrag(struct rvu *rvu);

[ ... ]
-- 
pw-bot: cr