Re: [PATCH v19 net-next 6/9] octeontx2: cn20k: Coordinate default rules with NIX LF lifecycle

[Ratheesh Kannoth]

On 2026-06-05 at 12:02:42, Ratheesh Kannoth (rkannoth@xxxxxxxxxxx) wrote:
> Add NIX_LF_DONT_FREE_DFT_IDXS so the PF can send NIX LF free during hw
> reinit or teardown without the AF freeing CN20K default NPC rule indexes
> while the driver still owns that state (otx2_init_hw_resources and
> otx2_free_hw_resources).
>
> On CN20K, allocate default NPC rules from NIX LF alloc before
> nix_interface_init, roll back with npc_cn20k_dft_rules_free on failure,
> and free from NIX LF free when the new flag is not set. Tighten
> rvu_mbox_handler_nix_lf_alloc error handling: use a single rc, propagate
> qmem_alloc and other errors, and set -ENOMEM only when kcalloc fails
> (remove the blanket -ENOMEM at the free_mem path).
>
> Signed-off-by: Ratheesh Kannoth <rkannoth@xxxxxxxxxxx>

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

>>  		intf = NIX_INTF_TYPE_SDP;
>>
>> -	err = nix_interface_init(rvu, pcifunc, intf, nixlf, rsp,
>> -				 !!(req->flags & NIX_LF_LBK_BLK_SEL));
>> -	if (err)
>> -		goto free_mem;
>> +	if (is_cn20k(rvu->pdev)) {
>> +		rc = npc_cn20k_dft_rules_idx_get(rvu, pcifunc, &bcast, &mcast,
>> +						 &promisc, &ucast);
>> +		if (rc) {
>> +			rc = npc_cn20k_dft_rules_alloc(rvu, pcifunc);
>> +			if (rc)
>> +				goto free_mem;
>> +
>> +			rules_created = true;
>> +		}
>> +	}
>Does the rules_created flag faithfully reflect whether allocation
>actually happened?
>npc_cn20k_dft_rules_alloc() returns 0 without doing any work when the
>pcifunc is not CGX or LBK mapped (for example SDP):
>	if (!npc_is_cgx_or_lbk(rvu, pcifunc)) {
>		dev_dbg(rvu->dev,
>			"%s: dft rule allocation is only for cgx mapped device, pcifunc=%#x\n",
>			__func__, pcifunc);
>		return 0;
>	}
>For such a pcifunc, npc_cn20k_dft_rules_idx_get() returns -ESRCH, the
>if branch is taken, dft_rules_alloc() is a no-op returning 0, and
>rules_created becomes true even though no rule was created.
>The free_dft path then calls npc_cn20k_dft_rules_free(), which itself
>early-returns on the same !npc_is_cgx_or_lbk predicate, so the safety
>relies on that hidden coupling rather than an explicit signal.
>Would it be clearer to have npc_cn20k_dft_rules_alloc() return a
>distinct created/not-created indication, or to gate rules_created on
>npc_is_cgx_or_lbk(rvu, pcifunc) at the call site?
>> +
dft rules are only for cgx/lbk interfaces. So for sdp, eventhough npc_cn20k_dft_rules_alloc() returns 0
and rules_created is set to true, failure case, it will call dft_rules_free(), but there is nothing
to free. So this will work.

>> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
>> @@ -4085,12 +4095,10 @@ void rvu_npc_clear_ucast_entry(struct rvu *rvu, int pcifunc, int nixlf)
>>
>>  	ucast_idx = npc_get_nixlf_mcam_index(mcam, pcifunc,
>>  					     nixlf, NIXLF_UCAST_ENTRY);
>> -	if (ucast_idx < 0) {
>> -		dev_err(rvu->dev,
>> -			"%s: Error to get ucast entry for pcifunc=%#x\n",
>> -			__func__, pcifunc);
>> +
>> +	/* In cn20k, default rules are freed before detach rsrc */
>> +	if (ucast_idx < 0)
>>  		return;
>> -	}
>The comment says this silent return is for the CN20K case where default
>rules are freed before detach rsrc, but the change drops the dev_err
>unconditionally for every silicon that reaches rvu_npc_clear_ucast_entry()
>via the generic rvu_detach_block() path, including CN10K and OcteonTX2.
>On legacy silicon a negative ucast_idx was previously treated as
>exceptional and worth logging; should this silent return be gated on
>is_cn20k(rvu->pdev) so the dev_err is preserved for the other variants?
No. In cn10k, ucast entry is not dynamic, so it is treated as error. This is
a teardown path, so want to keep same code for cn10k and cn20k.