Re: [PATCH v18 net-next 7/8] octeontx2: cn20k: Respect NPC MCAM X2/X4 profile in flows and DFT alloc

[Ratheesh Kannoth]

On 2026-06-02 at 11:33:58, Ratheesh Kannoth (rkannoth@xxxxxxxxxxx) wrote:
> Default CN20K NPC rule allocation now keys off the active MCAM keyword
> width: use X4 with a bank-masked reference index when the silicon uses
> X4 keys, and X2 with the raw index otherwise (replacing the previous
> always-X2 / eidx + 1 behaviour).
>
> In the AF flow-install path, flows that need more than 256 key bits
> query the NPC profile; if the platform is fixed to X2 entries, fail
> with -EOPNOTSUPP instead of requesting X4. Otherwise select X4 for the
> MCAM alloc.
>
> On the PF, cache and pass the profile kw_type from npc_get_pfl_info
> through otx2_mcam_pfl_info_get(), and use it when allocating MCAM
> entries for RSS/defaults and when installing ethtool flows on CN20K,
> including masking the reference index for X4 slot layout.
>
> Signed-off-by: Ratheesh Kannoth <rkannoth@xxxxxxxxxxx>

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260602060359.1894952-1-rkannoth%40marvell.com says:

>> --- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
>> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
>> @@ -4533,11 +4539,18 @@ int npc_cn20k_dft_rules_alloc(struct rvu *rvu, u16 pcifunc)
>>  	 * as NPC_DFT_RULE_PRIO - 1 (higher hw priority)
>>  	 */
>>  	req.contig = false;
>> -	req.kw_type = NPC_MCAM_KEY_X2;
>>  	req.count = cnt;
>>  	req.hdr.pcifunc = pcifunc;
>>  	req.ref_prio = NPC_MCAM_LOWER_PRIO;
>> -	req.ref_entry = eidx + 1;
>> +
>> +	if (npc_priv.kw == NPC_MCAM_KEY_X4) {
>> +		req.kw_type = NPC_MCAM_KEY_X4;
>> +		req.ref_entry = eidx & (npc_priv.bank_depth - 1);
>> +	} else {
>> +		req.kw_type = NPC_MCAM_KEY_X2;
>> +		req.ref_entry = eidx;
>> +	}
>> +
>The first allocation block in npc_cn20k_dft_rules_alloc() previously used
>req.ref_entry = eidx, so adding only the X4 path to it is purely additive.
>This second (LOWER_PRIO fallback) block is different: it used to use
>req.ref_entry = eidx + 1, and the new X2 branch silently drops the +1 and
>uses eidx instead.
>Was the previous eidx + 1 (that is, 2 * npc_priv.bank_depth) in the
>fallback path intentional?
This is intentional to fix a bug  as eidx + 1 may cross the boundary of the resource.

>> -	if (kw_bits > 256)
>> +	if (kw_bits > 256) {
>> +		rvu_mbox_handler_npc_get_pfl_info(rvu, &req, &rsp);
>> +		if (rsp.kw_type == NPC_MCAM_KEY_X2) {
>> +			dev_err(rvu->dev,
>> +				"Only X2 entries are supported in X2 profile\n");
>> +			return -EOPNOTSUPP;
>> +		}
>> +
>>  		*kw_type = NPC_MCAM_KEY_X4;
>> +	}
>A couple of concerns here.
>The struct msg_req req; is left uninitialized on the stack and then passed
>to rvu_mbox_handler_npc_get_pfl_info(). The handler does not currently
>read req, but it is a registered mbox dispatch entry whose contract has
>its hdr filled in by the mbox layer, so should req be zero-initialized at
>the declaration to match that contract, similar to the on-the-same-line
>struct npc_get_pfl_info_rsp rsp = { 0 } above it?
>The int return of rvu_mbox_handler_npc_get_pfl_info() is also discarded,
>and rsp.kw_type is then tested unconditionally. Since rsp starts at zero
>and NPC_MCAM_KEY_X1 is 0, a hypothetical handler failure would skip the
>X2-profile guard and select NPC_MCAM_KEY_X4. The handler's only failure
>return today is the !is_cn20k() check, which the function has already
>short-circuited above, so there is no live failure path; would it still
>make sense to capture the return and bail out, matching the rc-checking
>pattern used for rvu_mbox_handler_npc_get_num_kws() just a few lines
>earlier?
rvu_mbox_handler_npc_get_pfl_info() behaves purely as a local accessor function in this execution path.
It retrieves a statically cached profile configuration without processing any mailboxes.
However, to maintain strict consistency with the mailbox contract and prevent
any future static analysis tools from flagging uninitialized stack usage, we will add a error handler
as per of hardening series once this patch series is merged.