Re: [PATCH v11 net-next 5/7] octeontx2-af: npc: cn20k: add subbank search order control

[Paolo Abeni <pabeni@redhat.com>]

On 4/9/26 4:50 AM, Ratheesh Kannoth wrote:
> 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.
> 
> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
> ---
>  .../ethernet/marvell/octeontx2/af/cn20k/npc.c | 91 +++++++++++++++++-
>  .../ethernet/marvell/octeontx2/af/cn20k/npc.h |  2 +
>  .../marvell/octeontx2/af/rvu_devlink.c        | 92 +++++++++++++++++--
>  3 files changed, 173 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> index e854b85ced9e..153765b3e504 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> @@ -3317,7 +3317,7 @@ rvu_mbox_handler_npc_cn20k_get_kex_cfg(struct rvu *rvu,
>  	return 0;
>  }
>  
> -static int *subbank_srch_order;
> +static u32 *subbank_srch_order;
>  
>  static void npc_populate_restricted_idxs(int num_subbanks)
>  {
> @@ -3329,7 +3329,7 @@ static int npc_create_srch_order(int cnt)
>  {
>  	int val = 0;
>  
> -	subbank_srch_order = kcalloc(cnt, sizeof(int),
> +	subbank_srch_order = kcalloc(cnt, sizeof(u32),
>  				     GFP_KERNEL);
>  	if (!subbank_srch_order)
>  		return -ENOMEM;
> @@ -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,
> +			       u64 arr[MAX_NUM_SUB_BANKS], int cnt)
> +{
> +	struct npc_mcam *mcam = &rvu->hw->mcam;
> +	u32 fslots[MAX_NUM_SUB_BANKS][2];
> +	u32 uslots[MAX_NUM_SUB_BANKS][2];
> +	int fcnt = 0, ucnt = 0;
> +	struct npc_subbank *sb;
> +	int idx, val, rc = 0;
> +
> +	unsigned long index;
> +	void *v;
> +
> +	if (cnt != npc_priv.num_subbanks) {
> +		dev_err(rvu->dev, "Number of entries(%u) != %u\n",
> +			cnt, npc_priv.num_subbanks);
> +		return -EINVAL;
> +	}
> +
> +	mutex_lock(&mcam->lock);
> +	npc_lock_all_subbank();
> +	restrict_valid = false;
> +
> +	for (int i = 0; i < cnt; i++)
> +		subbank_srch_order[i] = (u32)arr[i];
> +
> +	xa_for_each(&npc_priv.xa_sb_used, index, v) {
> +		val = xa_to_value(v);
> +		uslots[ucnt][0] = index;
> +		uslots[ucnt][1] = val;
> +		xa_erase(&npc_priv.xa_sb_used, index);
> +		ucnt++;
> +	}
> +
> +	xa_for_each(&npc_priv.xa_sb_free, index, v) {
> +		val = xa_to_value(v);
> +		fslots[fcnt][0] = index;
> +		fslots[fcnt][1] = val;
> +		xa_erase(&npc_priv.xa_sb_free, index);
> +		fcnt++;
> +	}
> +
> +	/* xa_store() is done under lock. If xa_store fails
> +	 * ,no rollback is planned as it might also fail.

Why do you need to go throuh erase and add loop? Why can't you directly
xa_store() the new value? Note that xa_store() can fail due to memory
pressure.

Avoiding the previous erase will prevent deallocation and re allocation
and will avoid any reasonable xa_store() failure.

AFAICS there are a few more items reported by sashiko, please have a look:

https://sashiko.dev/#/patchset/20260409025055.1664053-1-rkannoth%40marvell.com

/P