Re: [PATCH v4 net 10/10] octeontx2-af: npc: cn20k: Reject missing default-rule MCAM indices

[Ratheesh Kannoth]

On 2026-04-27 at 12:02:13, Ratheesh Kannoth (rkannoth@xxxxxxxxxxx) wrote:
> When cn20k default L2 rules are not installed,
> npc_cn20k_dft_rules_idx_get() leaves broadcast, multicast,
> promiscuous, and unicast slots at USHRT_MAX.
> npc_get_nixlf_mcam_index() previously returned that sentinel as a

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> index 5d349d131fdb..9e6c79d6fadf 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
[ ... ]
>> @@ -3897,6 +4006,14 @@ int rvu_mbox_handler_npc_read_base_steer_rule(struct rvu *rvu,
>>  	/* Read the default ucast entry if there is no pkt steering rule */
>>  	index = npc_get_nixlf_mcam_index(mcam, pcifunc, nixlf,
>>  					 NIXLF_UCAST_ENTRY);
>> +	if (index < 0) {
>> +		mutex_unlock(&mcam->lock);
>> +		dev_err(rvu->dev,
>> +			"%s: Error to get ucast entry for pcifunc=%#x\n",
>> +			__func__, pcifunc);
>> +		goto out;
>> +	}
>> +
>Should this path set rc before jumping to out?
>Looking at the surrounding function, rc is initialized to 0 and the out
>label simply returns rc:
>    int index, blkaddr, nixlf, rc = 0;
>    ...
>    out:
>        return rc;
>The adjacent nix_get_nixlf() failure path assigns rc before the goto,
>and other error paths in this file use return NPC_MCAM_INVALID_REQ.
>On this new error path rc stays 0, npc_read_mcam_entry() is skipped, and
>the handler returns success with rsp->entry left zero-filled. Would the
>requester be unable to distinguish this failure from a legitimate reply?
>Would assigning rc = NPC_MCAM_INVALID_REQ (or another negative errno)
>before goto out be more appropriate here?
>> read_entry:
>>  	/* Read the mcam entry */
>>  	npc_read_mcam_entry(rvu, mcam, blkaddr, index, &rsp->entry, &intf,

pw-bot: changes-requested

ACK. will set rc for this error path.