Re: [PATCH net-next] octeontx2-af: Add mailbox to read default MCAM entry

[Simon Horman <horms@kernel.org>]

On Thu, Aug 06, 2026 at 01:26:31PM +0530, nshettyj@marvell.com wrote:
> From: Satheesh Paul <psatheesh@marvell.com>
> 
> Add an NPC mailbox command so PF/VF clients can read the default
> unicast MCAM rule associated with their NIX LF.
> 
> Signed-off-by: Satheesh Paul <psatheesh@marvell.com>
> Signed-off-by: Nitin Shetty J <nshettyj@marvell.com>
> ---
>  .../net/ethernet/marvell/octeontx2/af/mbox.h  |  2 ++
>  .../ethernet/marvell/octeontx2/af/rvu_npc.c   | 34 +++++++++++++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> index 73f743e4a83d..cece197d1074 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> @@ -309,6 +309,8 @@ M(NPC_MCAM_GET_DFT_RL_IDXS, 0x601e, npc_get_dft_rl_idxs,	\
>  M(NPC_MCAM_GET_NPC_PFL_INFO, 0x601f, npc_get_pfl_info,		\
>  					msg_req,		\
>  					npc_get_pfl_info_rsp)	\
> +M(NPC_MCAM_READ_DEFAULT_RULE, 0x6021, npc_read_default_rule, msg_req,	\
> +					npc_mcam_read_base_rule_rsp)	\
>  /* NIX mbox IDs (range 0x8000 - 0xFFFF) */				\
>  M(NIX_LF_ALLOC,		0x8000, nix_lf_alloc,				\
>  				 nix_lf_alloc_req, nix_lf_alloc_rsp)	\
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> index db27ea622f35..266bb762691c 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> @@ -4303,6 +4303,40 @@ int rvu_mbox_handler_npc_set_pkind(struct rvu *rvu, struct npc_set_pkind *req,
>  				      req->skip_size);
>  }
>  
> +int rvu_mbox_handler_npc_read_default_rule(struct rvu *rvu,
> +					   struct msg_req *req,
> +					   struct npc_mcam_read_base_rule_rsp *rsp)
> +{
> +	struct npc_mcam *mcam = &rvu->hw->mcam;
> +	int index, blkaddr, nixlf, rc;
> +	u16 pcifunc = req->hdr.pcifunc;
> +	u8 intf, enable;
> +
> +	blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPC, 0);
> +	if (blkaddr < 0)
> +		return NPC_MCAM_INVALID_REQ;
> +
> +	rc = nix_get_nixlf(rvu, pcifunc, &nixlf, NULL);
> +	if (rc < 0)
> +		return rc;
> +
> +	/* Read the default ucast entry */
> +	mutex_lock(&mcam->lock);
> +	index = npc_get_nixlf_mcam_index(mcam, pcifunc, nixlf,
> +					 NIXLF_UCAST_ENTRY);
> +	if (index < 0) {
> +		mutex_unlock(&mcam->lock);
> +		return NIX_AF_ERR_AF_LF_INVALID;
> +	}
> +
> +	/* Read the mcam entry */
> +	npc_read_mcam_entry(rvu, mcam, blkaddr, index, &rsp->entry, &intf,
> +			    &enable);
> +	mutex_unlock(&mcam->lock);
> +
> +	return 0;
> +}
> +

Hi,

There is an extensive AI-generated review of this patch available at
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260806075631.2007542-1-nshettyj%40marvell.com

While I'd appreciate it if you could look over the entire review I think
the most relevant point raised is the one I've pasted immediately below.
Could you provide some feedback on it?

  Does this path return the contents of unrelated registers on CN20K?
  The index comes from the cn20k-aware lookup, whose first branch is:

    npc_get_nixlf_mcam_index() {
	if (is_cn20k(rvu->pdev)) {
		rc = npc_cn20k_dft_rules_idx_get(rvu, pcifunc, &bcast, &mcast,
						 &promisc, &ucast);
	...
    }

  but the entry is then decoded unconditionally with the legacy reader, which
  uses the pre-CN20K register macros:

    npc_read_mcam_entry() {
	cam1 = rvu_read64(rvu, blkaddr,
			  NPC_AF_MCAMEX_BANKX_CAMX_W0(src, bank, 1));
	...
	entry->action = rvu_read64(rvu, blkaddr,
				   NPC_AF_MCAMEX_BANKX_ACTION(src, sbank));
    }

  CN20K MCAM registers use different bases and shifts, for example in
  af/cn20k/reg.h:

    #define NPC_AF_CN20K_MCAMEX_BANKX_CAMX_W0_EXT(a, b, c) ...
	offset = (0x9000000ull | (a) << 4 | (b) << 20 | (c) << 3);

  Elsewhere in the driver the helper choice is guarded, e.g. in
  npc_update_dmac_value():

    if (is_cn20k(rvu->pdev)) {
	if (npc_cn20k_read_mcam_entry(rvu, npcblkaddr, rule->entry,
				      cn20k_entry, &intf,
				      &enable, &hw_prio))
		return -EINVAL;
    } else {
	npc_read_mcam_entry(rvu, mcam, npcblkaddr, rule->entry,
			    entry, &intf, &enable);
    }

  and rvu_mbox_handler_npc_cn20k_read_base_steer_rule() uses
  npc_cn20k_read_mcam_entry() for the equivalent CN20K message.  Without a
  similar branch here, does a CN20K PF/VF get garbage key/action data with a
  return code of 0?