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

[Ratheesh Kannoth]

On 2026-06-09 at 09:34:49, Ratheesh Kannoth (rkannoth@xxxxxxxxxxx) 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@xxxxxxxxxxx>

https://netdev-ai.bots.linux.dev/sashiko/#/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++) {
>> +		sb_idx = narr[prio];
>> +		sb = &npc_priv.sb[sb_idx];
>[ ... ]
>> +		sb->arr_idx = prio;
>> +	}
>[ ... ]
>> +	for (int i = 0; i < cnt; i++)
>> +		subbank_srch_order[i] = (u32)narr[i];
>Do the get and set paths agree with init on what subbank_srch_order[]
>means?  The init path appears to treat the array as indexed by sb_idx
>with priority as the value:
>    /* npc_create_srch_order() */
>    for (int i = 0; i < cnt; i += 2) {
>        subbank_srch_order[i]     = cnt / 2 - val - 1;
>        subbank_srch_order[i + 1] = cnt / 2 + 1 + val;
>        val++;
>    }
>    subbank_srch_order[cnt - 1] = cnt / 2;
>    /* npc_subbank_init() */
>    sb->idx = idx;
>    sb->arr_idx = subbank_srch_order[idx];
>So initially subbank_srch_order[sb_idx] == priority (arr_idx).
>The new set helper, however, treats narr as indexed by priority with
>sb_idx as the value:
>    for (prio = 0; prio < cnt; prio++) {
>            sb_idx = narr[prio];
>            sb = &npc_priv.sb[sb_idx];
>            ...
>            sb->arr_idx = prio;
>    }
>    ...
>    for (int i = 0; i < cnt; i++)
>            subbank_srch_order[i] = (u32)narr[i];
>and the get helper returns subbank_srch_order verbatim.
>With cnt = 4 the round-trip then looks like:
>  after init:        subbank_srch_order = [1, 3, 0, 2]
>                     arr_idx[sb0..sb3]  = [1, 3, 0, 2]
>                     priority -> sb     = [2, 0, 3, 1]
>  user issues GET  -> [1, 3, 0, 2]  (validate accepts: distinct, < cnt)
>  user issues SET with the same array:
>                     arr_idx[sb0..sb3]  = [2, 0, 3, 1]
>                     priority -> sb     = [1, 3, 0, 2]
>That is, an unmodified GET -> SET echo silently inverts the runtime
>priority order of the MCAM subbanks.
>Should init be reworked so subbank_srch_order is indexed by priority
>(with npc_subbank_init() doing something like
>sb[subbank_srch_order[p]].arr_idx = p), or should set/get use the same
>indexing convention as init so a no-op round-trip is actually a no-op?
>> +	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;

There is due to bug in npc_subbank_init() (not introduced by this patch).
Will post a patch to "net" tree.