Re: [PATCH v3 net-next 02/13] octeontx2-af: Verify MCAM entry channel and PF_FUNC

Hi Alexander,

-----Original Message-----
From: Alexander Duyck <redacted>
Sent: Friday, November 13, 2020 1:33 AM
To: Naveen Mamindlapalli <redacted>
Cc: Netdev <redacted>; LKML <redacted>;
Jakub Kicinski [off-list ref]; David Miller [off-list ref];
saeed@kernel.org; Sunil Kovvuri Goutham [off-list ref]; Linu
Cherian [off-list ref]; Geethasowjanya Akula
[off-list ref]; Jerin Jacob Kollanukkaran [off-list ref];
Subbaraya Sundeep Bhatta [off-list ref]; Hariprasad Kelam
[off-list ref]
Subject: Re: [PATCH v3 net-next 02/13] octeontx2-af: Verify MCAM entry
channel and PF_FUNC

On Tue, Nov 10, 2020 at 11:18 PM Naveen Mamindlapalli
[off-list ref] wrote:

quoted

From: Subbaraya Sundeep <sbhatta@marvell.com>

This patch adds support to verify the channel number sent by mailbox
requester before writing MCAM entry for Ingress packets.
Similarly for Egress packets, verifying the PF_FUNC sent by the
mailbox user.

Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
Signed-off-by: Kiran Kumar K <redacted>
Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
Signed-off-by: Naveen Mamindlapalli <redacted>

One minor nit below. Otherwise looks good to me.

Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>

quoted

---
 drivers/net/ethernet/marvell/octeontx2/af/rvu.c    |  4 +-
 drivers/net/ethernet/marvell/octeontx2/af/rvu.h    |  2 +
 .../net/ethernet/marvell/octeontx2/af/rvu_npc.c    | 78

++++++++++++++++++++++

quoted

 3 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c

b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
index a28a518c0eae..e8b5aaf73201 100644

--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c

@@ -2642,7 +2642,7 @@ static void rvu_enable_afvf_intr(struct rvu

*rvu)

 #define PCI_DEVID_OCTEONTX2_LBK 0xA061

-static int lbk_get_num_chans(void)
+int rvu_get_num_lbk_chans(void)
 {
        struct pci_dev *pdev;
        void __iomem *base;

@@ -2677,7 +2677,7 @@ static int rvu_enable_sriov(struct rvu *rvu)
                return 0;
        }

-       chans = lbk_get_num_chans();
+       chans = rvu_get_num_lbk_chans();
        if (chans < 0)
                return chans;

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h

b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
index 5ac9bb12415f..1724dbd18847 100644

--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h

@@ -445,6 +445,7 @@ int rvu_get_lf(struct rvu *rvu, struct rvu_block

*block, u16 pcifunc, u16 slot);  int rvu_lf_reset(struct rvu *rvu,
struct rvu_block *block, int lf);  int rvu_get_blkaddr(struct rvu
*rvu, int blktype, u16 pcifunc);  int rvu_poll_reg(struct rvu *rvu,
u64 block, u64 offset, u64 mask, bool zero);
+int rvu_get_num_lbk_chans(void);

 /* RVU HW reg validation */
 enum regmap_block {

@@ -535,6 +536,7 @@ bool is_npc_intf_tx(u8 intf);  bool

is_npc_intf_rx(u8 intf);  bool is_npc_interface_valid(struct rvu *rvu,
u8 intf);  int rvu_npc_get_tx_nibble_cfg(struct rvu *rvu, u64
nibble_ena);
+int npc_mcam_verify_channel(struct rvu *rvu, u16 pcifunc, u8 intf,
+u16 channel);

 #ifdef CONFIG_DEBUG_FS
 void rvu_dbg_init(struct rvu *rvu);

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c

b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
index 989533a3d2ce..3666159bb6b6 100644

--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c

@@ -28,6 +28,8 @@

 #define NPC_PARSE_RESULT_DMAC_OFFSET   8
 #define NPC_HW_TSTAMP_OFFSET           8
+#define NPC_KEX_CHAN_MASK              0xFFFULL
+#define NPC_KEX_PF_FUNC_MASK           0xFFFFULL

 static const char def_pfl_name[] = "default";

@@ -63,6 +65,54 @@ int rvu_npc_get_tx_nibble_cfg(struct rvu *rvu, u64

nibble_ena)

quoted

        return 0;
 }

+static int npc_mcam_verify_pf_func(struct rvu *rvu,
+                                  struct mcam_entry *entry_data, u8 intf,
+                                  u16 pcifunc) {
+       u16 pf_func, pf_func_mask;
+
+       if (is_npc_intf_rx(intf))
+               return 0;
+
+       pf_func_mask = (entry_data->kw_mask[0] >> 32) &
+               NPC_KEX_PF_FUNC_MASK;
+       pf_func = (entry_data->kw[0] >> 32) & NPC_KEX_PF_FUNC_MASK;
+
+       pf_func = be16_to_cpu((__force __be16)pf_func);
+       if (pf_func_mask != NPC_KEX_PF_FUNC_MASK ||
+           ((pf_func & ~RVU_PFVF_FUNC_MASK) !=
+            (pcifunc & ~RVU_PFVF_FUNC_MASK)))
+               return -EINVAL;
+
+       return 0;
+}
+
+int npc_mcam_verify_channel(struct rvu *rvu, u16 pcifunc, u8 intf,
+u16 channel) {
+       int pf = rvu_get_pf(pcifunc);
+       u8 cgx_id, lmac_id;
+       int base = 0, end;
+
+       if (is_npc_intf_tx(intf))
+               return 0;
+
+       if (is_afvf(pcifunc)) {
+               end = rvu_get_num_lbk_chans();
+               if (end < 0)
+                       return -EINVAL;
+       } else {
+               rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], &cgx_id,

&lmac_id);

quoted

+               base = NIX_CHAN_CGX_LMAC_CHX(cgx_id, lmac_id, 0x0);
+               /* CGX mapped functions has maximum of 16 channels */
+               end = NIX_CHAN_CGX_LMAC_CHX(cgx_id, lmac_id, 0xF);
+       }
+
+       if (channel < base || channel > end)
+               return -EINVAL;
+
+       return 0;
+}
+
 void rvu_npc_set_pkind(struct rvu *rvu, int pkind, struct rvu_pfvf
*pfvf)  {
        int blkaddr;

@@ -1935,6 +1985,7 @@ int

rvu_mbox_handler_npc_mcam_write_entry(struct rvu *rvu,

quoted

        struct rvu_pfvf *pfvf = rvu_get_pfvf(rvu, req->hdr.pcifunc);
        struct npc_mcam *mcam = &rvu->hw->mcam;
        u16 pcifunc = req->hdr.pcifunc;
+       u16 channel, chan_mask;
        int blkaddr, rc;
        u8 nix_intf;

@@ -1942,6 +1993,10 @@ int

rvu_mbox_handler_npc_mcam_write_entry(struct rvu *rvu,

quoted

        if (blkaddr < 0)
                return NPC_MCAM_INVALID_REQ;

+       chan_mask = req->entry_data.kw_mask[0] & NPC_KEX_CHAN_MASK;
+       channel = req->entry_data.kw[0] & NPC_KEX_CHAN_MASK;
+       channel &= chan_mask;
+
        mutex_lock(&mcam->lock);
        rc = npc_mcam_verify_entry(mcam, pcifunc, req->entry);
        if (rc)

@@ -1963,6 +2018,17 @@ int

rvu_mbox_handler_npc_mcam_write_entry(struct rvu *rvu,

quoted

        else
                nix_intf = pfvf->nix_rx_intf;

+       if (npc_mcam_verify_channel(rvu, pcifunc, req->intf, channel)) {
+               rc = NPC_MCAM_INVALID_REQ;
+               goto exit;
+       }
+
+       if (npc_mcam_verify_pf_func(rvu, &req->entry_data, req->intf,
+                                   pcifunc)) {
+               rc = NPC_MCAM_INVALID_REQ;
+               goto exit;
+       }
+
        npc_config_mcam_entry(rvu, mcam, blkaddr, req->entry, nix_intf,
                              &req->entry_data, req->enable_entry);

@@ -2299,6 +2365,7 @@ int

rvu_mbox_handler_npc_mcam_alloc_and_write_entry(struct rvu *rvu,

quoted

        struct npc_mcam *mcam = &rvu->hw->mcam;
        u16 entry = NPC_MCAM_ENTRY_INVALID;
        u16 cntr = NPC_MCAM_ENTRY_INVALID;
+       u16 channel, chan_mask;
        int blkaddr, rc;
        u8 nix_intf;

@@ -2309,6 +2376,17 @@ int

rvu_mbox_handler_npc_mcam_alloc_and_write_entry(struct rvu *rvu,

quoted

        if (!is_npc_interface_valid(rvu, req->intf))
                return NPC_MCAM_INVALID_REQ;

+       chan_mask = req->entry_data.kw_mask[0] & NPC_KEX_CHAN_MASK;
+       channel = req->entry_data.kw[0] & NPC_KEX_CHAN_MASK;
+       channel &= chan_mask;
+
+       if (npc_mcam_verify_channel(rvu, req->hdr.pcifunc, req->intf, channel))
+               return NPC_MCAM_INVALID_REQ;
+

Why not just move the code for pulling the channel into the
npc_mcam_verify_channel function like you did with the pf_func? Then you can
avoid declaring variables here that won't be used anywhere else in the function.

The npc_mcam_verify_channel is called by other functions that can't pass the raw MCAM entry data. See patch 04.

quoted

+       if (npc_mcam_verify_pf_func(rvu, &req->entry_data, req->intf,
+                                   req->hdr.pcifunc))
+               return NPC_MCAM_INVALID_REQ;
+
        /* Try to allocate a MCAM entry */
        entry_req.hdr.pcifunc = req->hdr.pcifunc;
        entry_req.contig = true;
--
2.16.5