Re: [net-next,v6,08/13] octeontx2-af: npc: cn20k: Add new mailboxes for CN20K silicon

[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
---
octeontx2-af: npc: cn20k: Add new mailboxes for CN20K silicon

This patch adds new mailboxes for CN20K silicon due to changes in
struct mcam_entry where the keyword arrays were increased from size 7
to 8, breaking backward compatibility.

> 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 d73e447bedca..000280a4181b 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c

[ ... ]

> @@ -1782,23 +1850,25 @@ static int npc_update_dmac_value(struct rvu *rvu, int npcblkaddr,
>  				 struct rvu_npc_mcam_rule *rule,
>  				 struct rvu_pfvf *pfvf)
>  {
> +	struct npc_cn20k_mcam_write_entry_req cn20k_wreq = { 0 };
>  	struct npc_mcam_write_entry_req write_req = { 0 };
> -	struct npc_mcam *mcam = &rvu->hw->mcam;
>  	struct mcam_entry_mdata mdata = { };
> +	struct npc_mcam *mcam = &rvu->hw->mcam;
> +	struct cn20k_mcam_entry *cn20k_entry;
>  	struct mcam_entry *entry;
>  	u8 intf, enable, hw_prio;
>  	struct msg_rsp rsp;
>  	int err;
>
> +	cn20k_entry = &cn20k_wreq.entry_data;
>  	entry = &write_req.entry_data;
> -
> -	npc_populate_mcam_mdata(rvu, &mdata, entry);
> +	npc_populate_mcam_mdata(rvu, &mdata, cn20k_entry, entry);
>
>  	ether_addr_copy(rule->packet.dmac, pfvf->mac_addr);
>
>  	if (is_cn20k(rvu->pdev))
>  		npc_cn20k_read_mcam_entry(rvu, npcblkaddr, rule->entry,
> -					  entry, &intf,
> +					  cn20k_entry, &intf,
>  					  &enable, &hw_prio);
>  	else
>  		npc_read_mcam_entry(rvu, mcam, npcblkaddr, rule->entry,
> @@ -1813,7 +1883,13 @@ static int npc_update_dmac_value(struct rvu *rvu, int npcblkaddr,
>  	write_req.intf = pfvf->nix_rx_intf;
>
>  	mutex_unlock(&mcam->lock);
> -	err = rvu_mbox_handler_npc_mcam_write_entry(rvu, &write_req, &rsp);
> +	if (is_cn20k(rvu->pdev))
> +		err = rvu_mbox_handler_npc_cn20k_mcam_write_entry(rvu,
> +								  &cn20k_wreq,
> +								  &rsp);

In the CN20K path, cn20k_wreq is passed to the handler but only
write_req fields were initialized above:

    write_req.hdr.pcifunc = rule->owner;
    write_req.entry = rule->entry;
    write_req.intf = pfvf->nix_rx_intf;

The cn20k_wreq.entry, cn20k_wreq.hdr.pcifunc, and cn20k_wreq.intf fields
remain zero-initialized. This causes the handler to write to MCAM entry 0
instead of rule->entry, potentially corrupting an unrelated MCAM entry
and leaving the intended rule unchanged.

Should cn20k_wreq.entry, cn20k_wreq.hdr.pcifunc, and cn20k_wreq.intf be
initialized similar to how write_req is set up in the non-CN20K path?

> +	else
> +		err = rvu_mbox_handler_npc_mcam_write_entry(rvu, &write_req,
> +							    &rsp);
>  	mutex_lock(&mcam->lock);
>
>  	return err;