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

[Ratheesh Kannoth]

On 2026-05-20 at 07:39:38, 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).
>

>This commit updates the default CN20K NPC rule allocation to use X4 with a
>bank-masked reference index when the silicon uses X4 keys. It also queries
>the NPC profile in the AF flow-install path to select the correct key width
>and fails if an unsupported X4 allocation is requested on an X2 profile.
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
>> index 6ae9cdcb608b0..d20eb0e47d7d8 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
>> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
>[ ... ]
>> @@ -1697,8 +1699,16 @@ rvu_npc_alloc_entry_for_flow_install(struct rvu *rvu,
>>  	kw_bits = kws_rsp.kws * 64;
>>
>>  	*kw_type = NPC_MCAM_KEY_X2;
>> -	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;
>> +	}
>>
>>  	memset(&entry_req, 0, sizeof(entry_req));
>>  	memset(&entry_rsp, 0, sizeof(entry_rsp));
>This is a pre-existing issue, but does this dynamic upgrade to an X4
>key type correctly mask the reference index?

There is no dynamic profile change support.
If an application attempts to request a key configuration
(such as X4 width requirements) in
running X2 profile, the driver rejects the request outright with -EOPNOTSUPP.

>Further down in this function, entry_req.ref_entry is assigned directly
>from fl_req->ref_entry. If the reference index came from a previously
>installed X2 flow, it might be in the larger X2 space. When passed to the
>allocator without being masked against the X4 bank depth, could it trigger a
>bounds check failure and permanently fail valid X4 flow installations?

As mentioned above, there is no dynamic profile switch support

>> -static int otx2_mcam_pfl_info_get(struct otx2_nic *pfvf, bool *is_x2,
>> -				  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 could this function-local static cache
>lead to data races and incorrect hardware capability sharing?
>The pfl_info structure is static, but it is protected by
>mutex_lock(&pfvf->mbox.lock), which is an instance-specific lock. If multiple
>NICs probe concurrently, couldn't they enter this critical section
>simultaneously and cause a data race?
>Also, since the first device populates this global cache, will subsequent
>NICs incorrectly use the first NIC's profile data instead of querying their
>own hardware capabilities?

The 'pfl_info' structure caches global
NPC hardware profile parameters queried directly from the Admin Function (AF)
driver. These properties are invariant and bound to the fixed system-wide SoC
hardware profile configured at boot time. There is no dynamic runtime
reconfiguration or profile switching supported.

>> @@ -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;
>This is a pre-existing issue, but is there a potential use-after-free
>if the firmware returns an allocation count that differs from the requested
>count?
>Later in this function, if rsp->count != req->count, the driver frees the
>array via devm_kfree(pfvf->dev, flow_cfg->def_ent) but does not set the
>pointer to NULL, and returns 0 (success):
>if (rsp->count != req->count) {
>        ...
>        devm_kfree(pfvf->dev, flow_cfg->def_ent);
>        return 0;
>}
>Since it returns success, the device probe completes. If an administrator
>subsequently changes the MCAM count via devlink, it invokes
>otx2_alloc_mcam_entries(), which unconditionally reads from the freed array:
>        if (!is_otx2_vf(pfvf->pcifunc)) {
>                req->ref_prio = NPC_MCAM_HIGHER_PRIO;
>                ref = flow_cfg->def_ent[0];
>        }
>Could this trigger a use-after-free from userspace?
ACk,Pre exsting issue. it is a fix and will be sent as a seperate patch to "net" tree once this patch
series is merged.