[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aW2wVTnV_WNsyX70@rkannoth-OptiPlex-7090>
Date: Mon, 19 Jan 2026 09:47:25 +0530
From: Ratheesh Kannoth <rkannoth@...vell.com>
To: Jakub Kicinski <kuba@...nel.org>, <sumang@...vell.com>
CC: <linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org>
Subject: Re: [net-next,v4,04/13] ocetontx2-af: npc: cn20k: MKEX profile
support
On 2026-01-18 at 06:10:20, Jakub Kicinski (kuba@...nel.org) wrote:
> 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?
Suman, Could you please check this comment ?
>
> > +
> > + for (entry = 0; entry < entries; entry++) {
> > + profile->kpu[kpu].cam[entry] = cam[entry];
> > + profile->kpu[kpu].action[entry] = action[entry];
> > + }
> > + }
> > +
> > + return 0;
> > +}
Powered by blists - more mailing lists