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

[Ratheesh Kannoth <rkannoth@marvell.com>]

On 2026-06-09 at 09:34:49, Ratheesh Kannoth (rkannoth@marvell.com) 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 "srch_order" to control the order in which
> subbanks are searched during MCAM allocation.
>
> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
> ---

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

>> @@ -3906,6 +3906,122 @@ static void npc_unlock_all_subbank(void)
>>  		mutex_unlock(&npc_priv.sb[i].lock);
>>  }
>>
>> +int npc_cn20k_search_order_set(struct rvu *rvu,
>> +			       u64 narr[MAX_NUM_SUB_BANKS], int cnt)
>> +{
>[ ... ]
>> +	for (prio = 0; prio < cnt; prio++) {
>> +		if (rsrc[FREE][prio] == -1)
>> +			xa_erase(&npc_priv.xa_sb_free, prio);
>> +
>> +		if (rsrc[USED][prio] == -1)
>> +			xa_erase(&npc_priv.xa_sb_used, prio);
>> +	}
>> +
>> +	for (int i = 0; i < cnt; i++)
>> +		subbank_srch_order[i] = (u32)narr[i];
>Does this change the semantic meaning of the subbank_srch_order array?
>During initialization in npc_create_srch_order(), subbank_srch_order is
>mapped as subbank_idx -> priority. The devlink parameter npc_srch_order
>expects an array ordered by priority (narr[priority] = subbank_idx),
>which is handled correctly by the earlier loops in this function.
>However, assigning subbank_srch_order[i] = narr[i] here silently changes
>the array in-place from mapping (idx -> priority) to mapping (priority -> idx).
Thank you. npc_create_srch_order() is creating it properly but npc_subbank_init() is
not settig this order properly. This is a bug. This patch did not introduce this bug.
Will post a patch to "net" tree.

>> +
>> +	restrict_valid = false;
>> +
>> +	npc_unlock_all_subbank();
>> +	mutex_unlock(&mcam->lock);
>> +
>> +	return 0;
>[ ... ]
>> +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;
>Could this result in scrambled subbank search orders if a user space tool
>reads the default order and then writes it back?
>Since this directly returns subbank_srch_order, if it is called before any
>set operation occurs, it returns the original idx -> priority mapping.
>If called after a set operation, it returns a priority -> subbank_idx
>mapping.
>If the original default order is read by devlink and then written back
>unmodified, the set path will misinterpret the original idx -> priority
>array as a priority -> subbank_idx array, scrambling the search order.

Same comment as above.