Re: [net-next,v4,04/13] ocetontx2-af: npc: cn20k: MKEX profile support

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
ocetontx2-af: npc: cn20k: MKEX profile support

This patch adds MKEX (MicroCode Key EXtraction) profile support for the
CN20K silicon variant, introducing a new parser profile with 24 extractors
per packet parsing profile instead of the previous two layer-data
information per key field type.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> index 0717fd66e8a1..c3f41f4ea99c 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/npc.c

[ ... ]

> +static u8 npc_map2cn20k_flag(u8 flag)
> +{
> +	switch (flag) {
> +	case NPC_F_LC_U_IP_FRAG:
> +		return NPC_CN20K_F_LC_L_IP_FRAG;
> +
> +	case NPC_F_LC_U_IP6_FRAG:
> +		return NPC_CN20K_F_LC_L_IP6_FRAG;
> +
> +	case NPC_F_LC_L_6TO4:
> +		return NPC_CN20K_F_LC_L_6TO4;
> +
> +	case NPC_F_LC_L_MPLS_IN_IP:
> +		return NPC_CN20K_F_LC_U_MPLS_IN_IP;
> +
> +	case NPC_F_LC_L_IP6_TUN_IP6:
> +		return NPC_CN20K_F_LC_U_IP6_TUN_IP6;
> +
> +	case NPC_F_LC_L_IP6_MPLS_IN_IP:
> +		return NPC_CN20K_F_LC_U_IP6_MPLS_IN_IP;
> +
> +	default:
> +		break;
> +	}
> +
> +	return -1;
> +}

[ ... ]

> +int npc_cn20k_apply_custom_kpu(struct rvu *rvu,
> +			       struct npc_kpu_profile_adapter *profile)
> +{

[ ... ]

> +	for (kpu = 0; kpu < fw->kpus; kpu++) {
> +		fw_kpu = (struct npc_kpu_fwdata *)(fw->data + offset);

[ ... ]

> +		npc_cn20k_update_action_entries_n_flags(rvu, profile);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Is it intentional to call npc_cn20k_update_action_entries_n_flags() inside
the kpu loop? This function iterates through all kpus in the profile and
remaps flag values from the old enum to the CN20K enum. When called
multiple times, the mapping appears to be bidirectional:

  First call:  NPC_F_LC_L_MPLS_IN_IP (2) -> NPC_CN20K_F_LC_U_MPLS_IN_IP (0x20)
  Second call: 0x20 matches NPC_F_LC_U_IP_FRAG -> NPC_CN20K_F_LC_L_IP_FRAG (2)

The old and new enum values overlap (2,3,4 map to 0x20,0x40,0x80 and
vice-versa), so calling this function fw->kpus times will flip-flop the
flag values. If fw->kpus is even, the flags will have incorrect values
after the loop completes.

Should the call be moved outside the loop, or should there be a guard to
ensure it only runs once?

> +
> +		for (entry = 0; entry < entries; entry++) {
> +			profile->kpu[kpu].cam[entry] = cam[entry];
> +			profile->kpu[kpu].action[entry] = action[entry];
> +		}
> +	}
> +
> +	return 0;
> +}