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://sashiko.dev/#/patchset/20260602060359.1894952-1-rkannoth%40marvell.com says

>
>> -				  u16 *x4_slots)
>> +static int otx2_mcam_pfl_info_get(struct otx2_nic *pfvf, u16 *x4_slots, u8 *kw_type)
>>  {
>>  	struct npc_get_pfl_info_rsp *rsp;
>>  	struct msg_req *req;
>>  	static struct {
>>  		bool is_set;
>> -		bool is_x2;
>> +		u8 kw_type;
>>  		u16 x4_slots;
>>  	} pfl_info;
>This is a pre-existing issue, but does using a static structure to cache
>the hardware profile break multi-device support?
These are value of AF device (singleton device per system). AF process mbox messages serially, so if any PF updates this
first, that value will be same and can be used for all other PFs.

>Additionally, the cache is protected by a per-instance lock, which provides
>no mutual exclusion when multiple distinct devices access it concurrently.
>Could this lead to data races?
No races as value is same and is updated by first PF who request it.

>> @@ -289,6 +291,8 @@ int otx2_mcam_entry_init(struct otx2_nic *pfvf)
>>  	struct npc_mcam_alloc_entry_rsp *rsp;
>>  	int vf_vlan_max_flows, count;
>>  	int rc, ref, prio, ent;
>> +	u8 kw_type = 0;
>> +	u16 x4_slots;
>>  	u16 dft_idx;
>>
>>  	ref = 0;
>> @@ -315,6 +319,16 @@ int otx2_mcam_entry_init(struct otx2_nic *pfvf)
>>  	if (!flow_cfg->def_ent)
>>  		return -ENOMEM;
>This is a pre-existing issue, but does this function leak devres-managed
>memory when triggered by devlink parameters?
Preexting issue, so will be addressed to net tree once this series is merged.

>>  	u8 kw_type = 0;
>>  	u8 *src, *dst;
>>  	u16 x4_slots;
>> -	bool is_x2;
>This is a pre-existing issue, but does the VF validation error path in
>otx2_add_flow_msg() leak a mailbox message buffer?
>A mailbox message is dynamically allocated early in the function:
>	req = otx2_mbox_alloc_msg_npc_install_flow(&pfvf->mbox);
>During the flow specification validation, if the VF is invalid, the
>function unlocks the mailbox and returns an error:
>	vf = ethtool_get_flow_spec_ring_vf(ring_cookie);
>	if (vf > pci_num_vf(pfvf->pdev)) {
>		mutex_unlock(&pfvf->mbox.lock);
>		return -EINVAL;
>	}
>However, it fails to call otx2_mbox_reset(), which means the message slot
>remains active or consumed in the mailbox queue.
>Could a user repeatedly submitting invalid flows exhaust the mailbox queue
>and break communications with the Admin Function?
same as above.