2023-03-29 17:09:51

by Sai Krishna Gajula

[permalink] [raw]
Subject: [net PATCH 0/7] octeontx2: Miscellaneous fixes

This patchset includes following fixes.

Patch #1 Fix for the race condition while updating APR table

Patch #2 Fix for bit positions in NPC, MCAM table entries

Patch #3 Fix driver crash resulting from invalid interface type
information retrieved from firmware

Patch #4 Fix incorrect mask used while installing filters inlovling
fragmented packets

Patch #5 Fixes for NPC field hash extract w.r.t IPV6 hash reduction,
IPV6 filed hash configuration, parser confiuration destination
address hash.

Patch #6 Fix for skipping mbox initialization for PFs disabled by firmware.

Patch #7 Fix disabling packet I/O in case of mailbox timeout.

Geetha sowjanya (1):
octeontx2-af: Secure APR table update with the lock

Hariprasad Kelam (1):
octeontx2-af: Add validation for lmac type

Ratheesh Kannoth (3):
octeontx2-af: Fix start and end bit for scan config
octeontx2-af: Fix issues with NPC field hash extract
octeontx2-af: Skip PFs if not enabled

Subbaraya Sundeep (1):
octeontx2-pf: Disable packet I/O for graceful exit

Suman Ghosh (1):
octeontx2-af: Update correct mask to filter IPv4 fragments

.../net/ethernet/marvell/octeontx2/af/cgx.c | 7 +
.../net/ethernet/marvell/octeontx2/af/mbox.c | 5 +-
.../net/ethernet/marvell/octeontx2/af/mbox.h | 19 ++-
.../net/ethernet/marvell/octeontx2/af/rvu.c | 38 +++++-
.../ethernet/marvell/octeontx2/af/rvu_cn10k.c | 8 +-
.../marvell/octeontx2/af/rvu_npc_fs.c | 28 ++--
.../marvell/octeontx2/af/rvu_npc_fs.h | 4 +
.../marvell/octeontx2/af/rvu_npc_hash.c | 125 ++++++++++--------
.../marvell/octeontx2/af/rvu_npc_hash.h | 10 +-
.../marvell/octeontx2/nic/otx2_common.h | 4 +-
.../ethernet/marvell/octeontx2/nic/otx2_pf.c | 11 +-
.../ethernet/marvell/octeontx2/nic/otx2_tc.c | 2 +-
.../ethernet/marvell/octeontx2/nic/otx2_vf.c | 8 +-
13 files changed, 181 insertions(+), 88 deletions(-)

--
2.25.1


2023-03-29 17:10:41

by Sai Krishna Gajula

[permalink] [raw]
Subject: [net PATCH 3/7] octeontx2-af: Add validation for lmac type

From: Hariprasad Kelam <[email protected]>

Upon physical link change, firmware reports to the kernel about the
change along with the details like speed, lmac_type_id, etc.
Kernel derives lmac_type based on lmac_type_id received from firmware.

In a few scenarios, firmware returns an invalid lmac_type_id, which
is resulting in below kernel panic. This patch adds the missing
validation of the lmac_type_id field.

Internal error: Oops: 96000005 [#1] PREEMPT SMP
[ 35.321595] Modules linked in:
[ 35.328982] CPU: 0 PID: 31 Comm: kworker/0:1 Not tainted
5.4.210-g2e3169d8e1bc-dirty #17
[ 35.337014] Hardware name: Marvell CN103XX board (DT)
[ 35.344297] Workqueue: events work_for_cpu_fn
[ 35.352730] pstate: 40400089 (nZcv daIf +PAN -UAO)
[ 35.360267] pc : strncpy+0x10/0x30
[ 35.366595] lr : cgx_link_change_handler+0x90/0x180

Fixes: 61071a871ea6 ("octeontx2-af: Forward CGX link notifications to PFs")
Signed-off-by: Hariprasad Kelam <[email protected]>
Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
Signed-off-by: Sai Krishna <[email protected]>
---
drivers/net/ethernet/marvell/octeontx2/af/cgx.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
index 724df6398bbe..180aa84cf1c3 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
@@ -1231,6 +1231,13 @@ static inline void link_status_user_format(u64 lstat,
linfo->an = FIELD_GET(RESP_LINKSTAT_AN, lstat);
linfo->fec = FIELD_GET(RESP_LINKSTAT_FEC, lstat);
linfo->lmac_type_id = FIELD_GET(RESP_LINKSTAT_LMAC_TYPE, lstat);
+
+ if (linfo->lmac_type_id >= LMAC_MODE_MAX) {
+ dev_err(&cgx->pdev->dev, "Unknown lmac_type_id %d reported by firmware on cgx port%d:%d",
+ linfo->lmac_type_id, cgx->cgx_id, lmac_id);
+ return;
+ }
+
lmac_string = cgx_lmactype_string[linfo->lmac_type_id];
strncpy(linfo->lmac_type, lmac_string, LMACTYPE_STR_LEN - 1);
}
--
2.25.1

2023-03-29 17:11:13

by Sai Krishna Gajula

[permalink] [raw]
Subject: [net PATCH 2/7] octeontx2-af: Fix start and end bit for scan config

From: Ratheesh Kannoth <[email protected]>

Fix the NPC nibble start and end positions in the bit
map. Fix the depth of cam and mem table configuration.
Increased the field size of dmac filter flows as cn10kb
support large in number.

Fixes: b747923afff8 ("octeontx2-af: Exact match support")
Signed-off-by: Ratheesh Kannoth <[email protected]>
Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
Signed-off-by: Sai Krishna <[email protected]>
---
drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c | 5 ++---
drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c | 4 ++--
drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h | 4 ++--
3 files changed, 6 insertions(+), 7 deletions(-)

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 006beb5cf98d..27603078689a 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
@@ -593,9 +593,8 @@ static int npc_scan_kex(struct rvu *rvu, int blkaddr, u8 intf)
* exact match code.
*/
masked_cfg = cfg & NPC_EXACT_NIBBLE;
- bitnr = NPC_EXACT_NIBBLE_START;
- for_each_set_bit_from(bitnr, (unsigned long *)&masked_cfg,
- NPC_EXACT_NIBBLE_START) {
+ bitnr = NPC_EXACT_NIBBLE_START - 1;
+ for_each_set_bit_from(bitnr, (unsigned long *)&masked_cfg, NPC_EXACT_NIBBLE_END + 1) {
npc_scan_exact_result(mcam, bitnr, key_nibble, intf);
key_nibble++;
}
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
index 20ebb9c95c73..6597af84aa36 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
@@ -1868,9 +1868,9 @@ int rvu_npc_exact_init(struct rvu *rvu)
rvu->hw->table = table;

/* Read table size, ways and depth */
- table->mem_table.depth = FIELD_GET(GENMASK_ULL(31, 24), npc_const3);
table->mem_table.ways = FIELD_GET(GENMASK_ULL(19, 16), npc_const3);
- table->cam_table.depth = FIELD_GET(GENMASK_ULL(15, 0), npc_const3);
+ table->mem_table.depth = FIELD_GET(GENMASK_ULL(15, 0), npc_const3);
+ table->cam_table.depth = FIELD_GET(GENMASK_ULL(31, 24), npc_const3);

dev_dbg(rvu->dev, "%s: NPC exact match 4way_2k table(ways=%d, depth=%d)\n",
__func__, table->mem_table.ways, table->cam_table.depth);
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
index 3d22cc6a2804..99cdc871b59c 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
@@ -335,11 +335,11 @@ struct otx2_flow_config {
#define OTX2_PER_VF_VLAN_FLOWS 2 /* Rx + Tx per VF */
#define OTX2_VF_VLAN_RX_INDEX 0
#define OTX2_VF_VLAN_TX_INDEX 1
- u16 max_flows;
- u8 dmacflt_max_flows;
u32 *bmap_to_dmacindex;
unsigned long *dmacflt_bmap;
struct list_head flow_list;
+ u32 dmacflt_max_flows;
+ u16 max_flows;
};

struct otx2_tc_info {
--
2.25.1

2023-03-29 17:11:36

by Sai Krishna Gajula

[permalink] [raw]
Subject: [net PATCH 7/7] octeontx2-pf: Disable packet I/O for graceful exit

From: Subbaraya Sundeep <[email protected]>

At the stage of enabling packet I/O in otx2_open, If mailbox
timeout occurs then interface ends up in down state where as
hardware packet I/O is enabled. Hence disable packet I/O also
before bailing out. This patch also free the LMTST per cpu structure
on teardown, if the lmt_info pointer is not NULL.

Fixes: 1ea0166da050 ("octeontx2-pf: Fix the device state on error")
Signed-off-by: Subbaraya Sundeep <[email protected]>
Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
Signed-off-by: Sai Krishna <[email protected]>
---
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 11 ++++++++++-
drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c | 8 +++++---
2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
index 179433d0a54a..52a57d2493dc 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
@@ -1835,13 +1835,22 @@ int otx2_open(struct net_device *netdev)
otx2_dmacflt_reinstall_flows(pf);

err = otx2_rxtx_enable(pf, true);
- if (err)
+ /* If a mbox communication error happens at this point then interface
+ * will end up in a state such that it is in down state but hardware
+ * mcam entries are enabled to receive the packets. Hence disable the
+ * packet I/O.
+ */
+ if (err == EIO)
+ goto err_disable_rxtx;
+ else if (err)
goto err_tx_stop_queues;

otx2_do_set_rx_mode(pf);

return 0;

+err_disable_rxtx:
+ otx2_rxtx_enable(pf, false);
err_tx_stop_queues:
netif_tx_stop_all_queues(netdev);
netif_carrier_off(netdev);
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
index ab126f8706c7..6ab4780f12fd 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
@@ -621,7 +621,7 @@ static int otx2vf_probe(struct pci_dev *pdev, const struct pci_device_id *id)

err = otx2vf_realloc_msix_vectors(vf);
if (err)
- goto err_mbox_destroy;
+ goto err_detach_rsrc;

err = otx2_set_real_num_queues(netdev, qcount, qcount);
if (err)
@@ -709,7 +709,8 @@ static int otx2vf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
err_ptp_destroy:
otx2_ptp_destroy(vf);
err_detach_rsrc:
- free_percpu(vf->hw.lmt_info);
+ if (vf->hw.lmt_info)
+ free_percpu(vf->hw.lmt_info);
if (test_bit(CN10K_LMTST, &vf->hw.cap_flag))
qmem_free(vf->dev, vf->dync_lmt);
otx2_detach_resources(&vf->mbox);
@@ -763,7 +764,8 @@ static void otx2vf_remove(struct pci_dev *pdev)
otx2_shutdown_tc(vf);
otx2vf_disable_mbox_intr(vf);
otx2_detach_resources(&vf->mbox);
- free_percpu(vf->hw.lmt_info);
+ if (vf->hw.lmt_info)
+ free_percpu(vf->hw.lmt_info);
if (test_bit(CN10K_LMTST, &vf->hw.cap_flag))
qmem_free(vf->dev, vf->dync_lmt);
otx2vf_vfaf_mbox_destroy(vf);
--
2.25.1

2023-03-29 17:11:49

by Sai Krishna Gajula

[permalink] [raw]
Subject: [net PATCH 6/7] octeontx2-af: Skip PFs if not enabled

From: Ratheesh Kannoth <[email protected]>

Skip mbox initialization of disabled PFs. Firmware configures PFs
and allocate mbox resources etc. Linux should configure particular
PFs, which ever are enabled by firmware.

Fixes: 9bdc47a6e328 ("octeontx2-af: Mbox communication support btw AF and it's VFs")
Signed-off-by: Ratheesh Kannoth <[email protected]>
Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
Signed-off-by: Sai Krishna <[email protected]>
---
.../net/ethernet/marvell/octeontx2/af/mbox.c | 5 ++-
.../net/ethernet/marvell/octeontx2/af/mbox.h | 3 +-
.../net/ethernet/marvell/octeontx2/af/rvu.c | 38 ++++++++++++++++---
3 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.c b/drivers/net/ethernet/marvell/octeontx2/af/mbox.c
index 2898931d5260..9690ac01f02c 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.c
@@ -157,7 +157,7 @@ EXPORT_SYMBOL(otx2_mbox_init);
*/
int otx2_mbox_regions_init(struct otx2_mbox *mbox, void **hwbase,
struct pci_dev *pdev, void *reg_base,
- int direction, int ndevs)
+ int direction, int ndevs, unsigned long *pf_bmap)
{
struct otx2_mbox_dev *mdev;
int devid, err;
@@ -169,6 +169,9 @@ int otx2_mbox_regions_init(struct otx2_mbox *mbox, void **hwbase,
mbox->hwbase = hwbase[0];

for (devid = 0; devid < ndevs; devid++) {
+ if (!test_bit(devid, pf_bmap))
+ continue;
+
mdev = &mbox->dev[devid];
mdev->mbase = hwbase[devid];
mdev->hwbase = hwbase[devid];
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
index 0ce533848536..26636a4d7dcc 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
@@ -96,9 +96,10 @@ void otx2_mbox_destroy(struct otx2_mbox *mbox);
int otx2_mbox_init(struct otx2_mbox *mbox, void __force *hwbase,
struct pci_dev *pdev, void __force *reg_base,
int direction, int ndevs);
+
int otx2_mbox_regions_init(struct otx2_mbox *mbox, void __force **hwbase,
struct pci_dev *pdev, void __force *reg_base,
- int direction, int ndevs);
+ int direction, int ndevs, unsigned long *bmap);
void otx2_mbox_msg_send(struct otx2_mbox *mbox, int devid);
int otx2_mbox_wait_for_rsp(struct otx2_mbox *mbox, int devid);
int otx2_mbox_busy_poll_for_rsp(struct otx2_mbox *mbox, int devid);
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
index 8683ce57ed3f..61c658fa3f28 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
@@ -2282,7 +2282,7 @@ static inline void rvu_afvf_mbox_up_handler(struct work_struct *work)
}

static int rvu_get_mbox_regions(struct rvu *rvu, void **mbox_addr,
- int num, int type)
+ int num, int type, unsigned long *pf_bmap)
{
struct rvu_hwinfo *hw = rvu->hw;
int region;
@@ -2294,6 +2294,9 @@ static int rvu_get_mbox_regions(struct rvu *rvu, void **mbox_addr,
*/
if (type == TYPE_AFVF) {
for (region = 0; region < num; region++) {
+ if (!test_bit(region, pf_bmap))
+ continue;
+
if (hw->cap.per_pf_mbox_regs) {
bar4 = rvu_read64(rvu, BLKADDR_RVUM,
RVU_AF_PFX_BAR4_ADDR(0)) +
@@ -2315,6 +2318,9 @@ static int rvu_get_mbox_regions(struct rvu *rvu, void **mbox_addr,
* RVU_AF_PF_BAR4_ADDR register.
*/
for (region = 0; region < num; region++) {
+ if (!test_bit(region, pf_bmap))
+ continue;
+
if (hw->cap.per_pf_mbox_regs) {
bar4 = rvu_read64(rvu, BLKADDR_RVUM,
RVU_AF_PFX_BAR4_ADDR(region));
@@ -2343,8 +2349,27 @@ static int rvu_mbox_init(struct rvu *rvu, struct mbox_wq_info *mw,
int err = -EINVAL, i, dir, dir_up;
void __iomem *reg_base;
struct rvu_work *mwork;
+ unsigned long *pf_bmap;
void **mbox_regions;
const char *name;
+ u64 cfg;
+
+ pf_bmap = devm_kcalloc(rvu->dev, BITS_TO_LONGS(num), sizeof(long), GFP_KERNEL);
+ if (!pf_bmap)
+ return -ENOMEM;
+
+ /* RVU VFs */
+ if (type == TYPE_AFVF)
+ bitmap_set(pf_bmap, 0, num);
+
+ if (type == TYPE_AFPF) {
+ /* Mark enabled PFs in bitmap */
+ for (i = 0; i < num; i++) {
+ cfg = rvu_read64(rvu, BLKADDR_RVUM, RVU_PRIV_PFX_CFG(i));
+ if (cfg & BIT_ULL(20))
+ set_bit(i, pf_bmap);
+ }
+ }

mbox_regions = kcalloc(num, sizeof(void *), GFP_KERNEL);
if (!mbox_regions)
@@ -2356,7 +2381,7 @@ static int rvu_mbox_init(struct rvu *rvu, struct mbox_wq_info *mw,
dir = MBOX_DIR_AFPF;
dir_up = MBOX_DIR_AFPF_UP;
reg_base = rvu->afreg_base;
- err = rvu_get_mbox_regions(rvu, mbox_regions, num, TYPE_AFPF);
+ err = rvu_get_mbox_regions(rvu, mbox_regions, num, TYPE_AFPF, pf_bmap);
if (err)
goto free_regions;
break;
@@ -2365,7 +2390,7 @@ static int rvu_mbox_init(struct rvu *rvu, struct mbox_wq_info *mw,
dir = MBOX_DIR_PFVF;
dir_up = MBOX_DIR_PFVF_UP;
reg_base = rvu->pfreg_base;
- err = rvu_get_mbox_regions(rvu, mbox_regions, num, TYPE_AFVF);
+ err = rvu_get_mbox_regions(rvu, mbox_regions, num, TYPE_AFVF, pf_bmap);
if (err)
goto free_regions;
break;
@@ -2396,16 +2421,19 @@ static int rvu_mbox_init(struct rvu *rvu, struct mbox_wq_info *mw,
}

err = otx2_mbox_regions_init(&mw->mbox, mbox_regions, rvu->pdev,
- reg_base, dir, num);
+ reg_base, dir, num, pf_bmap);
if (err)
goto exit;

err = otx2_mbox_regions_init(&mw->mbox_up, mbox_regions, rvu->pdev,
- reg_base, dir_up, num);
+ reg_base, dir_up, num, pf_bmap);
if (err)
goto exit;

for (i = 0; i < num; i++) {
+ if (!test_bit(i, pf_bmap))
+ continue;
+
mwork = &mw->mbox_wrk[i];
mwork->rvu = rvu;
INIT_WORK(&mwork->work, mbox_handler);
--
2.25.1

2023-03-29 17:12:47

by Sai Krishna Gajula

[permalink] [raw]
Subject: [net PATCH 1/7] octeontx2-af: Secure APR table update with the lock

From: Geetha sowjanya <[email protected]>

APR table contains the lmtst base address of PF/VFs.
These entries are updated by the PF/VF during the
device probe. Due to race condition while updating the
entries are getting corrupted. Hence secure the APR
table update with the lock.

Fixes: 893ae97214c3 ("octeontx2-af: cn10k: Support configurable LMTST regions")
Signed-off-by: Geetha sowjanya <[email protected]>
Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
Signed-off-by: Sai Krishna <[email protected]>
---
drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
index 4ad9ff025c96..8530250f6fba 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
@@ -142,16 +142,17 @@ int rvu_mbox_handler_lmtst_tbl_setup(struct rvu *rvu,
* region, if so, convert that IOVA to physical address and
* populate LMT table with that address
*/
+ mutex_lock(&rvu->rsrc_lock);
if (req->use_local_lmt_region) {
err = rvu_get_lmtaddr(rvu, req->hdr.pcifunc,
req->lmt_iova, &lmt_addr);
if (err < 0)
- return err;
+ goto error;

/* Update the lmt addr for this PFFUNC in the LMT table */
err = rvu_update_lmtaddr(rvu, req->hdr.pcifunc, lmt_addr);
if (err)
- return err;
+ goto error;
}

/* Reconfiguring lmtst map table in lmt region shared mode i.e. make
@@ -181,7 +182,7 @@ int rvu_mbox_handler_lmtst_tbl_setup(struct rvu *rvu,
*/
err = rvu_update_lmtaddr(rvu, req->hdr.pcifunc, val);
if (err)
- return err;
+ goto error;
}

/* This mailbox can also be used to update word1 of APR_LMT_MAP_ENTRY_S
@@ -230,6 +231,7 @@ int rvu_mbox_handler_lmtst_tbl_setup(struct rvu *rvu,
}

error:
+ mutex_unlock(&rvu->rsrc_lock);
return err;
}

--
2.25.1

2023-03-29 17:12:53

by Sai Krishna Gajula

[permalink] [raw]
Subject: [net PATCH 5/7] octeontx2-af: Fix issues with NPC field hash extract

From: Ratheesh Kannoth <[email protected]>

1. Update secret key mbox to provide hash mask and hash control as well.
2. Allow field hash configuration for both source and destination IPv6
3. Fix internal logic for IPv6 source/destination address hash
reduction via ntuple rule
4. Configure hardware parser based on hash extract feature enable flag
for IPv6.

Fixes: 56d9f5fd2246 ("octeontx2-af: Use hashed field in MCAM key")
Signed-off-by: Ratheesh Kannoth <[email protected]>
Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
Signed-off-by: Sai Krishna <[email protected]>
---
.../net/ethernet/marvell/octeontx2/af/mbox.h | 16 ++-
.../marvell/octeontx2/af/rvu_npc_fs.c | 23 +++-
.../marvell/octeontx2/af/rvu_npc_fs.h | 4 +
.../marvell/octeontx2/af/rvu_npc_hash.c | 121 ++++++++++--------
.../marvell/octeontx2/af/rvu_npc_hash.h | 10 +-
5 files changed, 108 insertions(+), 66 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
index 5727d67e0259..0ce533848536 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
@@ -245,9 +245,9 @@ M(NPC_MCAM_READ_BASE_RULE, 0x6011, npc_read_base_steer_rule, \
M(NPC_MCAM_GET_STATS, 0x6012, npc_mcam_entry_stats, \
npc_mcam_get_stats_req, \
npc_mcam_get_stats_rsp) \
-M(NPC_GET_SECRET_KEY, 0x6013, npc_get_secret_key, \
- npc_get_secret_key_req, \
- npc_get_secret_key_rsp) \
+M(NPC_GET_FIELD_HASH_INFO, 0x6013, npc_get_field_hash_info, \
+ npc_get_field_hash_info_req, \
+ npc_get_field_hash_info_rsp) \
M(NPC_GET_FIELD_STATUS, 0x6014, npc_get_field_status, \
npc_get_field_status_req, \
npc_get_field_status_rsp) \
@@ -1524,14 +1524,20 @@ struct npc_mcam_get_stats_rsp {
u8 stat_ena; /* enabled */
};

-struct npc_get_secret_key_req {
+struct npc_get_field_hash_info_req {
struct mbox_msghdr hdr;
u8 intf;
};

-struct npc_get_secret_key_rsp {
+struct npc_get_field_hash_info_rsp {
struct mbox_msghdr hdr;
u64 secret_key[3];
+#define NPC_MAX_HASH 2
+#define NPC_MAX_HASH_MASK 2
+ /* NPC_AF_INTF(0..1)_HASH(0..1)_MASK(0..1) */
+ u64 hash_mask[NPC_MAX_INTF][NPC_MAX_HASH][NPC_MAX_HASH_MASK];
+ /* NPC_AF_INTF(0..1)_HASH(0..1)_RESULT_CTRL */
+ u64 hash_ctrl[NPC_MAX_INTF][NPC_MAX_HASH];
};

enum ptp_op {
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 27603078689a..6d63a0ef6d9c 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
@@ -13,11 +13,6 @@
#include "rvu_npc_fs.h"
#include "rvu_npc_hash.h"

-#define NPC_BYTESM GENMASK_ULL(19, 16)
-#define NPC_HDR_OFFSET GENMASK_ULL(15, 8)
-#define NPC_KEY_OFFSET GENMASK_ULL(5, 0)
-#define NPC_LDATA_EN BIT_ULL(7)
-
static const char * const npc_flow_names[] = {
[NPC_DMAC] = "dmac",
[NPC_SMAC] = "smac",
@@ -442,6 +437,7 @@ static void npc_handle_multi_layer_fields(struct rvu *rvu, int blkaddr, u8 intf)
static void npc_scan_ldata(struct rvu *rvu, int blkaddr, u8 lid,
u8 lt, u64 cfg, u8 intf)
{
+ struct npc_mcam_kex_hash *mkex_hash = rvu->kpu.mkex_hash;
struct npc_mcam *mcam = &rvu->hw->mcam;
u8 hdr, key, nr_bytes, bit_offset;
u8 la_ltype, la_start;
@@ -490,8 +486,21 @@ do { \
NPC_SCAN_HDR(NPC_SIP_IPV4, NPC_LID_LC, NPC_LT_LC_IP, 12, 4);
NPC_SCAN_HDR(NPC_DIP_IPV4, NPC_LID_LC, NPC_LT_LC_IP, 16, 4);
NPC_SCAN_HDR(NPC_IPFRAG_IPV6, NPC_LID_LC, NPC_LT_LC_IP6_EXT, 6, 1);
- NPC_SCAN_HDR(NPC_SIP_IPV6, NPC_LID_LC, NPC_LT_LC_IP6, 8, 16);
- NPC_SCAN_HDR(NPC_DIP_IPV6, NPC_LID_LC, NPC_LT_LC_IP6, 24, 16);
+ if (rvu->hw->cap.npc_hash_extract) {
+ if (mkex_hash->lid_lt_ld_hash_en[intf][lid][lt][0])
+ NPC_SCAN_HDR(NPC_SIP_IPV6, NPC_LID_LC, NPC_LT_LC_IP6, 8, 4);
+ else
+ NPC_SCAN_HDR(NPC_SIP_IPV6, NPC_LID_LC, NPC_LT_LC_IP6, 8, 16);
+
+ if (mkex_hash->lid_lt_ld_hash_en[intf][lid][lt][1])
+ NPC_SCAN_HDR(NPC_DIP_IPV6, NPC_LID_LC, NPC_LT_LC_IP6, 24, 4);
+ else
+ NPC_SCAN_HDR(NPC_DIP_IPV6, NPC_LID_LC, NPC_LT_LC_IP6, 24, 16);
+ } else {
+ NPC_SCAN_HDR(NPC_SIP_IPV6, NPC_LID_LC, NPC_LT_LC_IP6, 8, 16);
+ NPC_SCAN_HDR(NPC_DIP_IPV6, NPC_LID_LC, NPC_LT_LC_IP6, 24, 16);
+ }
+
NPC_SCAN_HDR(NPC_SPORT_UDP, NPC_LID_LD, NPC_LT_LD_UDP, 0, 2);
NPC_SCAN_HDR(NPC_DPORT_UDP, NPC_LID_LD, NPC_LT_LD_UDP, 2, 2);
NPC_SCAN_HDR(NPC_SPORT_TCP, NPC_LID_LD, NPC_LT_LD_TCP, 0, 2);
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.h
index bdd65ce56a32..3f5c9042d10e 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.h
@@ -9,6 +9,10 @@
#define __RVU_NPC_FS_H

#define IPV6_WORDS 4
+#define NPC_BYTESM GENMASK_ULL(19, 16)
+#define NPC_HDR_OFFSET GENMASK_ULL(15, 8)
+#define NPC_KEY_OFFSET GENMASK_ULL(5, 0)
+#define NPC_LDATA_EN BIT_ULL(7)

void npc_update_entry(struct rvu *rvu, enum key_fields type,
struct mcam_entry *entry, u64 val_lo,
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
index 6597af84aa36..51209119f0f2 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
@@ -78,42 +78,43 @@ static u32 rvu_npc_toeplitz_hash(const u64 *data, u64 *key, size_t data_bit_len,
return hash_out;
}

-u32 npc_field_hash_calc(u64 *ldata, struct npc_mcam_kex_hash *mkex_hash,
- u64 *secret_key, u8 intf, u8 hash_idx)
+u32 npc_field_hash_calc(u64 *ldata, struct npc_get_field_hash_info_rsp rsp,
+ u8 intf, u8 hash_idx)
{
u64 hash_key[3];
u64 data_padded[2];
u32 field_hash;

- hash_key[0] = secret_key[1] << 31;
- hash_key[0] |= secret_key[2];
- hash_key[1] = secret_key[1] >> 33;
- hash_key[1] |= secret_key[0] << 31;
- hash_key[2] = secret_key[0] >> 33;
+ hash_key[0] = rsp.secret_key[1] << 31;
+ hash_key[0] |= rsp.secret_key[2];
+ hash_key[1] = rsp.secret_key[1] >> 33;
+ hash_key[1] |= rsp.secret_key[0] << 31;
+ hash_key[2] = rsp.secret_key[0] >> 33;

- data_padded[0] = mkex_hash->hash_mask[intf][hash_idx][0] & ldata[0];
- data_padded[1] = mkex_hash->hash_mask[intf][hash_idx][1] & ldata[1];
+ data_padded[0] = rsp.hash_mask[intf][hash_idx][0] & ldata[0];
+ data_padded[1] = rsp.hash_mask[intf][hash_idx][1] & ldata[1];
field_hash = rvu_npc_toeplitz_hash(data_padded, hash_key, 128, 159);

- field_hash &= mkex_hash->hash_ctrl[intf][hash_idx] >> 32;
- field_hash |= mkex_hash->hash_ctrl[intf][hash_idx];
+ field_hash &= FIELD_GET(GENMASK(63, 32), rsp.hash_ctrl[intf][hash_idx]);
+ field_hash += FIELD_GET(GENMASK(31, 0), rsp.hash_ctrl[intf][hash_idx]);
return field_hash;
}

-static u64 npc_update_use_hash(int lt, int ld)
+static u64 npc_update_use_hash(struct rvu *rvu, int blkaddr,
+ u8 intf, int lid, int lt, int ld)
{
- u64 cfg = 0;
-
- switch (lt) {
- case NPC_LT_LC_IP6:
- /* Update use_hash(bit-20) and bytesm1 (bit-16:19)
- * in KEX_LD_CFG
- */
- cfg = KEX_LD_CFG_USE_HASH(0x1, 0x03,
- ld ? 0x8 : 0x18,
- 0x1, 0x0, 0x10);
- break;
- }
+ u8 hdr, key;
+ u64 cfg;
+
+ cfg = rvu_read64(rvu, blkaddr, NPC_AF_INTFX_LIDX_LTX_LDX_CFG(intf, lid, lt, ld));
+ hdr = FIELD_GET(NPC_HDR_OFFSET, cfg);
+ key = FIELD_GET(NPC_KEY_OFFSET, cfg);
+
+ /* Update use_hash(bit-20) to 'true' and
+ * bytesm1(bit-16:19) to '0x3' in KEX_LD_CFG
+ */
+ cfg = KEX_LD_CFG_USE_HASH(0x1, 0x03,
+ hdr, 0x1, 0x0, key);

return cfg;
}
@@ -132,12 +133,13 @@ static void npc_program_mkex_hash_rx(struct rvu *rvu, int blkaddr,
for (lt = 0; lt < NPC_MAX_LT; lt++) {
for (ld = 0; ld < NPC_MAX_LD; ld++) {
if (mkex_hash->lid_lt_ld_hash_en[intf][lid][lt][ld]) {
- u64 cfg = npc_update_use_hash(lt, ld);
+ u64 cfg;

- hash_cnt++;
if (hash_cnt == NPC_MAX_HASH)
return;

+ cfg = npc_update_use_hash(rvu, blkaddr,
+ intf, lid, lt, ld);
/* Set updated KEX configuration */
SET_KEX_LD(intf, lid, lt, ld, cfg);
/* Set HASH configuration */
@@ -149,6 +151,8 @@ static void npc_program_mkex_hash_rx(struct rvu *rvu, int blkaddr,
mkex_hash->hash_mask[intf][ld][1]);
SET_KEX_LD_HASH_CTRL(intf, ld,
mkex_hash->hash_ctrl[intf][ld]);
+
+ hash_cnt++;
}
}
}
@@ -169,12 +173,13 @@ static void npc_program_mkex_hash_tx(struct rvu *rvu, int blkaddr,
for (lt = 0; lt < NPC_MAX_LT; lt++) {
for (ld = 0; ld < NPC_MAX_LD; ld++)
if (mkex_hash->lid_lt_ld_hash_en[intf][lid][lt][ld]) {
- u64 cfg = npc_update_use_hash(lt, ld);
+ u64 cfg;

- hash_cnt++;
if (hash_cnt == NPC_MAX_HASH)
return;

+ cfg = npc_update_use_hash(rvu, blkaddr,
+ intf, lid, lt, ld);
/* Set updated KEX configuration */
SET_KEX_LD(intf, lid, lt, ld, cfg);
/* Set HASH configuration */
@@ -187,8 +192,6 @@ static void npc_program_mkex_hash_tx(struct rvu *rvu, int blkaddr,
SET_KEX_LD_HASH_CTRL(intf, ld,
mkex_hash->hash_ctrl[intf][ld]);
hash_cnt++;
- if (hash_cnt == NPC_MAX_HASH)
- return;
}
}
}
@@ -238,8 +241,8 @@ void npc_update_field_hash(struct rvu *rvu, u8 intf,
struct flow_msg *omask)
{
struct npc_mcam_kex_hash *mkex_hash = rvu->kpu.mkex_hash;
- struct npc_get_secret_key_req req;
- struct npc_get_secret_key_rsp rsp;
+ struct npc_get_field_hash_info_req req;
+ struct npc_get_field_hash_info_rsp rsp;
u64 ldata[2], cfg;
u32 field_hash;
u8 hash_idx;
@@ -250,7 +253,7 @@ void npc_update_field_hash(struct rvu *rvu, u8 intf,
}

req.intf = intf;
- rvu_mbox_handler_npc_get_secret_key(rvu, &req, &rsp);
+ rvu_mbox_handler_npc_get_field_hash_info(rvu, &req, &rsp);

for (hash_idx = 0; hash_idx < NPC_MAX_HASH; hash_idx++) {
cfg = rvu_read64(rvu, blkaddr, NPC_AF_INTFX_HASHX_CFG(intf, hash_idx));
@@ -266,44 +269,45 @@ void npc_update_field_hash(struct rvu *rvu, u8 intf,
* is hashed to 32 bit value.
*/
case NPC_LT_LC_IP6:
- if (features & BIT_ULL(NPC_SIP_IPV6)) {
+ /* ld[0] == hash_idx[0] == Source IPv6
+ * ld[1] == hash_idx[1] == Destination IPv6
+ */
+ if ((features & BIT_ULL(NPC_SIP_IPV6)) && !hash_idx) {
u32 src_ip[IPV6_WORDS];

be32_to_cpu_array(src_ip, pkt->ip6src, IPV6_WORDS);
- ldata[0] = (u64)src_ip[0] << 32 | src_ip[1];
- ldata[1] = (u64)src_ip[2] << 32 | src_ip[3];
+ ldata[1] = (u64)src_ip[0] << 32 | src_ip[1];
+ ldata[0] = (u64)src_ip[2] << 32 | src_ip[3];
field_hash = npc_field_hash_calc(ldata,
- mkex_hash,
- rsp.secret_key,
+ rsp,
intf,
hash_idx);
npc_update_entry(rvu, NPC_SIP_IPV6, entry,
- field_hash, 0, 32, 0, intf);
+ field_hash, 0,
+ GENMASK(31, 0), 0, intf);
memcpy(&opkt->ip6src, &pkt->ip6src,
sizeof(pkt->ip6src));
memcpy(&omask->ip6src, &mask->ip6src,
sizeof(mask->ip6src));
- break;
- }
-
- if (features & BIT_ULL(NPC_DIP_IPV6)) {
+ } else if ((features & BIT_ULL(NPC_DIP_IPV6)) && hash_idx) {
u32 dst_ip[IPV6_WORDS];

be32_to_cpu_array(dst_ip, pkt->ip6dst, IPV6_WORDS);
- ldata[0] = (u64)dst_ip[0] << 32 | dst_ip[1];
- ldata[1] = (u64)dst_ip[2] << 32 | dst_ip[3];
+ ldata[1] = (u64)dst_ip[0] << 32 | dst_ip[1];
+ ldata[0] = (u64)dst_ip[2] << 32 | dst_ip[3];
field_hash = npc_field_hash_calc(ldata,
- mkex_hash,
- rsp.secret_key,
+ rsp,
intf,
hash_idx);
npc_update_entry(rvu, NPC_DIP_IPV6, entry,
- field_hash, 0, 32, 0, intf);
+ field_hash, 0,
+ GENMASK(31, 0), 0, intf);
memcpy(&opkt->ip6dst, &pkt->ip6dst,
sizeof(pkt->ip6dst));
memcpy(&omask->ip6dst, &mask->ip6dst,
sizeof(mask->ip6dst));
}
+
break;
}
}
@@ -311,13 +315,13 @@ void npc_update_field_hash(struct rvu *rvu, u8 intf,
}
}

-int rvu_mbox_handler_npc_get_secret_key(struct rvu *rvu,
- struct npc_get_secret_key_req *req,
- struct npc_get_secret_key_rsp *rsp)
+int rvu_mbox_handler_npc_get_field_hash_info(struct rvu *rvu,
+ struct npc_get_field_hash_info_req *req,
+ struct npc_get_field_hash_info_rsp *rsp)
{
u64 *secret_key = rsp->secret_key;
u8 intf = req->intf;
- int blkaddr;
+ int i, j, blkaddr;

blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPC, 0);
if (blkaddr < 0) {
@@ -329,6 +333,19 @@ int rvu_mbox_handler_npc_get_secret_key(struct rvu *rvu,
secret_key[1] = rvu_read64(rvu, blkaddr, NPC_AF_INTFX_SECRET_KEY1(intf));
secret_key[2] = rvu_read64(rvu, blkaddr, NPC_AF_INTFX_SECRET_KEY2(intf));

+ for (i = 0; i < NPC_MAX_HASH; i++) {
+ for (j = 0; j < NPC_MAX_HASH_MASK; j++) {
+ rsp->hash_mask[NIX_INTF_RX][i][j] =
+ GET_KEX_LD_HASH_MASK(NIX_INTF_RX, i, j);
+ rsp->hash_mask[NIX_INTF_TX][i][j] =
+ GET_KEX_LD_HASH_MASK(NIX_INTF_TX, i, j);
+ }
+ }
+
+ for (i = 0; i < NPC_MAX_INTF; i++)
+ for (j = 0; j < NPC_MAX_HASH; j++)
+ rsp->hash_ctrl[i][j] = GET_KEX_LD_HASH_CTRL(i, j);
+
return 0;
}

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
index 3efeb09c58de..a1c3d987b804 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
@@ -31,6 +31,12 @@
rvu_write64(rvu, blkaddr, \
NPC_AF_INTFX_HASHX_MASKX(intf, ld, mask_idx), cfg)

+#define GET_KEX_LD_HASH_CTRL(intf, ld) \
+ rvu_read64(rvu, blkaddr, NPC_AF_INTFX_HASHX_RESULT_CTRL(intf, ld))
+
+#define GET_KEX_LD_HASH_MASK(intf, ld, mask_idx) \
+ rvu_read64(rvu, blkaddr, NPC_AF_INTFX_HASHX_MASKX(intf, ld, mask_idx))
+
#define SET_KEX_LD_HASH_CTRL(intf, ld, cfg) \
rvu_write64(rvu, blkaddr, \
NPC_AF_INTFX_HASHX_RESULT_CTRL(intf, ld), cfg)
@@ -56,8 +62,8 @@ void npc_update_field_hash(struct rvu *rvu, u8 intf,
struct flow_msg *omask);
void npc_config_secret_key(struct rvu *rvu, int blkaddr);
void npc_program_mkex_hash(struct rvu *rvu, int blkaddr);
-u32 npc_field_hash_calc(u64 *ldata, struct npc_mcam_kex_hash *mkex_hash,
- u64 *secret_key, u8 intf, u8 hash_idx);
+u32 npc_field_hash_calc(u64 *ldata, struct npc_get_field_hash_info_rsp rsp,
+ u8 intf, u8 hash_idx);

static struct npc_mcam_kex_hash npc_mkex_hash_default __maybe_unused = {
.lid_lt_ld_hash_en = {
--
2.25.1

2023-03-29 17:13:26

by Sai Krishna Gajula

[permalink] [raw]
Subject: [net PATCH 4/7] octeontx2-af: Update correct mask to filter IPv4 fragments

From: Suman Ghosh <[email protected]>

During the initial design, the IPv4 ip_flag mask was set to 0xff.
Which results to filter only fragmets with (fragment_offset == 0).
As part of the fix, updated the mask to 0x20 to filter all the
fragmented packets irrespective of the fragment_offset value.

Fixes: c672e3727989 ("octeontx2-pf: Add support to filter packet based on IP fragment")
Signed-off-by: Suman Ghosh <[email protected]>
Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
Signed-off-by: Sai Krishna <[email protected]>
---
drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
index 044cc211424e..8392f63e433f 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
@@ -544,7 +544,7 @@ static int otx2_tc_prepare_flow(struct otx2_nic *nic, struct otx2_tc_flow *node,
if (match.mask->flags & FLOW_DIS_IS_FRAGMENT) {
if (ntohs(flow_spec->etype) == ETH_P_IP) {
flow_spec->ip_flag = IPV4_FLAG_MORE;
- flow_mask->ip_flag = 0xff;
+ flow_mask->ip_flag = IPV4_FLAG_MORE;
req->features |= BIT_ULL(NPC_IPFRAG_IPV4);
} else if (ntohs(flow_spec->etype) == ETH_P_IPV6) {
flow_spec->next_header = IPPROTO_FRAGMENT;
--
2.25.1

2023-03-30 06:02:35

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [net PATCH 1/7] octeontx2-af: Secure APR table update with the lock

On Wed, Mar 29, 2023 at 10:36:13PM +0530, Sai Krishna wrote:
> From: Geetha sowjanya <[email protected]>
>
> APR table contains the lmtst base address of PF/VFs.
> These entries are updated by the PF/VF during the
> device probe. Due to race condition while updating the
> entries are getting corrupted. Hence secure the APR
> table update with the lock.

However, I don't see rsrc_lock in probe path.
otx2_probe()
-> cn10k_lmtst_init()
-> lmt_base/lmstst is updated with and without mbox.lock.

Where did you take rsrc_lock in probe flow?

Thanks

>
> Fixes: 893ae97214c3 ("octeontx2-af: cn10k: Support configurable LMTST regions")
> Signed-off-by: Geetha sowjanya <[email protected]>
> Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
> Signed-off-by: Sai Krishna <[email protected]>
> ---
> drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
> index 4ad9ff025c96..8530250f6fba 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
> @@ -142,16 +142,17 @@ int rvu_mbox_handler_lmtst_tbl_setup(struct rvu *rvu,
> * region, if so, convert that IOVA to physical address and
> * populate LMT table with that address
> */
> + mutex_lock(&rvu->rsrc_lock);
> if (req->use_local_lmt_region) {
> err = rvu_get_lmtaddr(rvu, req->hdr.pcifunc,
> req->lmt_iova, &lmt_addr);
> if (err < 0)
> - return err;
> + goto error;
>
> /* Update the lmt addr for this PFFUNC in the LMT table */
> err = rvu_update_lmtaddr(rvu, req->hdr.pcifunc, lmt_addr);
> if (err)
> - return err;
> + goto error;
> }
>
> /* Reconfiguring lmtst map table in lmt region shared mode i.e. make
> @@ -181,7 +182,7 @@ int rvu_mbox_handler_lmtst_tbl_setup(struct rvu *rvu,
> */
> err = rvu_update_lmtaddr(rvu, req->hdr.pcifunc, val);
> if (err)
> - return err;
> + goto error;
> }
>
> /* This mailbox can also be used to update word1 of APR_LMT_MAP_ENTRY_S
> @@ -230,6 +231,7 @@ int rvu_mbox_handler_lmtst_tbl_setup(struct rvu *rvu,
> }
>
> error:
> + mutex_unlock(&rvu->rsrc_lock);
> return err;
> }
>
> --
> 2.25.1
>

2023-03-30 06:21:15

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [net PATCH 2/7] octeontx2-af: Fix start and end bit for scan config

On Wed, Mar 29, 2023 at 10:36:14PM +0530, Sai Krishna wrote:
> From: Ratheesh Kannoth <[email protected]>
>
> Fix the NPC nibble start and end positions in the bit
> map. Fix the depth of cam and mem table configuration.
> Increased the field size of dmac filter flows as cn10kb
> support large in number.
>
> Fixes: b747923afff8 ("octeontx2-af: Exact match support")
> Signed-off-by: Ratheesh Kannoth <[email protected]>
> Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
> Signed-off-by: Sai Krishna <[email protected]>
> ---
> drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c | 5 ++---
> drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c | 4 ++--
> drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h | 4 ++--
> 3 files changed, 6 insertions(+), 7 deletions(-)
>

Thanks,
Reviewed-by: Leon Romanovsky <[email protected]>

2023-03-30 06:31:44

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [net PATCH 5/7] octeontx2-af: Fix issues with NPC field hash extract

On Wed, Mar 29, 2023 at 10:36:17PM +0530, Sai Krishna wrote:
> From: Ratheesh Kannoth <[email protected]>
>
> 1. Update secret key mbox to provide hash mask and hash control as well.
> 2. Allow field hash configuration for both source and destination IPv6
> 3. Fix internal logic for IPv6 source/destination address hash
> reduction via ntuple rule
> 4. Configure hardware parser based on hash extract feature enable flag
> for IPv6.

This commit message explained what you did, but not what was the
problem.

Thanks

>
> Fixes: 56d9f5fd2246 ("octeontx2-af: Use hashed field in MCAM key")
> Signed-off-by: Ratheesh Kannoth <[email protected]>
> Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
> Signed-off-by: Sai Krishna <[email protected]>
> ---
> .../net/ethernet/marvell/octeontx2/af/mbox.h | 16 ++-
> .../marvell/octeontx2/af/rvu_npc_fs.c | 23 +++-
> .../marvell/octeontx2/af/rvu_npc_fs.h | 4 +
> .../marvell/octeontx2/af/rvu_npc_hash.c | 121 ++++++++++--------
> .../marvell/octeontx2/af/rvu_npc_hash.h | 10 +-
> 5 files changed, 108 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> index 5727d67e0259..0ce533848536 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> @@ -245,9 +245,9 @@ M(NPC_MCAM_READ_BASE_RULE, 0x6011, npc_read_base_steer_rule, \
> M(NPC_MCAM_GET_STATS, 0x6012, npc_mcam_entry_stats, \
> npc_mcam_get_stats_req, \
> npc_mcam_get_stats_rsp) \
> -M(NPC_GET_SECRET_KEY, 0x6013, npc_get_secret_key, \
> - npc_get_secret_key_req, \
> - npc_get_secret_key_rsp) \
> +M(NPC_GET_FIELD_HASH_INFO, 0x6013, npc_get_field_hash_info, \
> + npc_get_field_hash_info_req, \
> + npc_get_field_hash_info_rsp) \
> M(NPC_GET_FIELD_STATUS, 0x6014, npc_get_field_status, \
> npc_get_field_status_req, \
> npc_get_field_status_rsp) \
> @@ -1524,14 +1524,20 @@ struct npc_mcam_get_stats_rsp {
> u8 stat_ena; /* enabled */
> };
>
> -struct npc_get_secret_key_req {
> +struct npc_get_field_hash_info_req {
> struct mbox_msghdr hdr;
> u8 intf;
> };
>
> -struct npc_get_secret_key_rsp {
> +struct npc_get_field_hash_info_rsp {
> struct mbox_msghdr hdr;
> u64 secret_key[3];
> +#define NPC_MAX_HASH 2
> +#define NPC_MAX_HASH_MASK 2
> + /* NPC_AF_INTF(0..1)_HASH(0..1)_MASK(0..1) */
> + u64 hash_mask[NPC_MAX_INTF][NPC_MAX_HASH][NPC_MAX_HASH_MASK];
> + /* NPC_AF_INTF(0..1)_HASH(0..1)_RESULT_CTRL */
> + u64 hash_ctrl[NPC_MAX_INTF][NPC_MAX_HASH];
> };
>
> enum ptp_op {
> 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 27603078689a..6d63a0ef6d9c 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
> @@ -13,11 +13,6 @@
> #include "rvu_npc_fs.h"
> #include "rvu_npc_hash.h"
>
> -#define NPC_BYTESM GENMASK_ULL(19, 16)
> -#define NPC_HDR_OFFSET GENMASK_ULL(15, 8)
> -#define NPC_KEY_OFFSET GENMASK_ULL(5, 0)
> -#define NPC_LDATA_EN BIT_ULL(7)
> -
> static const char * const npc_flow_names[] = {
> [NPC_DMAC] = "dmac",
> [NPC_SMAC] = "smac",
> @@ -442,6 +437,7 @@ static void npc_handle_multi_layer_fields(struct rvu *rvu, int blkaddr, u8 intf)
> static void npc_scan_ldata(struct rvu *rvu, int blkaddr, u8 lid,
> u8 lt, u64 cfg, u8 intf)
> {
> + struct npc_mcam_kex_hash *mkex_hash = rvu->kpu.mkex_hash;
> struct npc_mcam *mcam = &rvu->hw->mcam;
> u8 hdr, key, nr_bytes, bit_offset;
> u8 la_ltype, la_start;
> @@ -490,8 +486,21 @@ do { \
> NPC_SCAN_HDR(NPC_SIP_IPV4, NPC_LID_LC, NPC_LT_LC_IP, 12, 4);
> NPC_SCAN_HDR(NPC_DIP_IPV4, NPC_LID_LC, NPC_LT_LC_IP, 16, 4);
> NPC_SCAN_HDR(NPC_IPFRAG_IPV6, NPC_LID_LC, NPC_LT_LC_IP6_EXT, 6, 1);
> - NPC_SCAN_HDR(NPC_SIP_IPV6, NPC_LID_LC, NPC_LT_LC_IP6, 8, 16);
> - NPC_SCAN_HDR(NPC_DIP_IPV6, NPC_LID_LC, NPC_LT_LC_IP6, 24, 16);
> + if (rvu->hw->cap.npc_hash_extract) {
> + if (mkex_hash->lid_lt_ld_hash_en[intf][lid][lt][0])
> + NPC_SCAN_HDR(NPC_SIP_IPV6, NPC_LID_LC, NPC_LT_LC_IP6, 8, 4);
> + else
> + NPC_SCAN_HDR(NPC_SIP_IPV6, NPC_LID_LC, NPC_LT_LC_IP6, 8, 16);
> +
> + if (mkex_hash->lid_lt_ld_hash_en[intf][lid][lt][1])
> + NPC_SCAN_HDR(NPC_DIP_IPV6, NPC_LID_LC, NPC_LT_LC_IP6, 24, 4);
> + else
> + NPC_SCAN_HDR(NPC_DIP_IPV6, NPC_LID_LC, NPC_LT_LC_IP6, 24, 16);
> + } else {
> + NPC_SCAN_HDR(NPC_SIP_IPV6, NPC_LID_LC, NPC_LT_LC_IP6, 8, 16);
> + NPC_SCAN_HDR(NPC_DIP_IPV6, NPC_LID_LC, NPC_LT_LC_IP6, 24, 16);
> + }
> +
> NPC_SCAN_HDR(NPC_SPORT_UDP, NPC_LID_LD, NPC_LT_LD_UDP, 0, 2);
> NPC_SCAN_HDR(NPC_DPORT_UDP, NPC_LID_LD, NPC_LT_LD_UDP, 2, 2);
> NPC_SCAN_HDR(NPC_SPORT_TCP, NPC_LID_LD, NPC_LT_LD_TCP, 0, 2);
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.h
> index bdd65ce56a32..3f5c9042d10e 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.h
> @@ -9,6 +9,10 @@
> #define __RVU_NPC_FS_H
>
> #define IPV6_WORDS 4
> +#define NPC_BYTESM GENMASK_ULL(19, 16)
> +#define NPC_HDR_OFFSET GENMASK_ULL(15, 8)
> +#define NPC_KEY_OFFSET GENMASK_ULL(5, 0)
> +#define NPC_LDATA_EN BIT_ULL(7)
>
> void npc_update_entry(struct rvu *rvu, enum key_fields type,
> struct mcam_entry *entry, u64 val_lo,
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
> index 6597af84aa36..51209119f0f2 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
> @@ -78,42 +78,43 @@ static u32 rvu_npc_toeplitz_hash(const u64 *data, u64 *key, size_t data_bit_len,
> return hash_out;
> }
>
> -u32 npc_field_hash_calc(u64 *ldata, struct npc_mcam_kex_hash *mkex_hash,
> - u64 *secret_key, u8 intf, u8 hash_idx)
> +u32 npc_field_hash_calc(u64 *ldata, struct npc_get_field_hash_info_rsp rsp,
> + u8 intf, u8 hash_idx)
> {
> u64 hash_key[3];
> u64 data_padded[2];
> u32 field_hash;
>
> - hash_key[0] = secret_key[1] << 31;
> - hash_key[0] |= secret_key[2];
> - hash_key[1] = secret_key[1] >> 33;
> - hash_key[1] |= secret_key[0] << 31;
> - hash_key[2] = secret_key[0] >> 33;
> + hash_key[0] = rsp.secret_key[1] << 31;
> + hash_key[0] |= rsp.secret_key[2];
> + hash_key[1] = rsp.secret_key[1] >> 33;
> + hash_key[1] |= rsp.secret_key[0] << 31;
> + hash_key[2] = rsp.secret_key[0] >> 33;
>
> - data_padded[0] = mkex_hash->hash_mask[intf][hash_idx][0] & ldata[0];
> - data_padded[1] = mkex_hash->hash_mask[intf][hash_idx][1] & ldata[1];
> + data_padded[0] = rsp.hash_mask[intf][hash_idx][0] & ldata[0];
> + data_padded[1] = rsp.hash_mask[intf][hash_idx][1] & ldata[1];
> field_hash = rvu_npc_toeplitz_hash(data_padded, hash_key, 128, 159);
>
> - field_hash &= mkex_hash->hash_ctrl[intf][hash_idx] >> 32;
> - field_hash |= mkex_hash->hash_ctrl[intf][hash_idx];
> + field_hash &= FIELD_GET(GENMASK(63, 32), rsp.hash_ctrl[intf][hash_idx]);
> + field_hash += FIELD_GET(GENMASK(31, 0), rsp.hash_ctrl[intf][hash_idx]);
> return field_hash;
> }
>
> -static u64 npc_update_use_hash(int lt, int ld)
> +static u64 npc_update_use_hash(struct rvu *rvu, int blkaddr,
> + u8 intf, int lid, int lt, int ld)
> {
> - u64 cfg = 0;
> -
> - switch (lt) {
> - case NPC_LT_LC_IP6:
> - /* Update use_hash(bit-20) and bytesm1 (bit-16:19)
> - * in KEX_LD_CFG
> - */
> - cfg = KEX_LD_CFG_USE_HASH(0x1, 0x03,
> - ld ? 0x8 : 0x18,
> - 0x1, 0x0, 0x10);
> - break;
> - }
> + u8 hdr, key;
> + u64 cfg;
> +
> + cfg = rvu_read64(rvu, blkaddr, NPC_AF_INTFX_LIDX_LTX_LDX_CFG(intf, lid, lt, ld));
> + hdr = FIELD_GET(NPC_HDR_OFFSET, cfg);
> + key = FIELD_GET(NPC_KEY_OFFSET, cfg);
> +
> + /* Update use_hash(bit-20) to 'true' and
> + * bytesm1(bit-16:19) to '0x3' in KEX_LD_CFG
> + */
> + cfg = KEX_LD_CFG_USE_HASH(0x1, 0x03,
> + hdr, 0x1, 0x0, key);
>
> return cfg;
> }
> @@ -132,12 +133,13 @@ static void npc_program_mkex_hash_rx(struct rvu *rvu, int blkaddr,
> for (lt = 0; lt < NPC_MAX_LT; lt++) {
> for (ld = 0; ld < NPC_MAX_LD; ld++) {
> if (mkex_hash->lid_lt_ld_hash_en[intf][lid][lt][ld]) {
> - u64 cfg = npc_update_use_hash(lt, ld);
> + u64 cfg;
>
> - hash_cnt++;
> if (hash_cnt == NPC_MAX_HASH)
> return;
>
> + cfg = npc_update_use_hash(rvu, blkaddr,
> + intf, lid, lt, ld);
> /* Set updated KEX configuration */
> SET_KEX_LD(intf, lid, lt, ld, cfg);
> /* Set HASH configuration */
> @@ -149,6 +151,8 @@ static void npc_program_mkex_hash_rx(struct rvu *rvu, int blkaddr,
> mkex_hash->hash_mask[intf][ld][1]);
> SET_KEX_LD_HASH_CTRL(intf, ld,
> mkex_hash->hash_ctrl[intf][ld]);
> +
> + hash_cnt++;
> }
> }
> }
> @@ -169,12 +173,13 @@ static void npc_program_mkex_hash_tx(struct rvu *rvu, int blkaddr,
> for (lt = 0; lt < NPC_MAX_LT; lt++) {
> for (ld = 0; ld < NPC_MAX_LD; ld++)
> if (mkex_hash->lid_lt_ld_hash_en[intf][lid][lt][ld]) {
> - u64 cfg = npc_update_use_hash(lt, ld);
> + u64 cfg;
>
> - hash_cnt++;
> if (hash_cnt == NPC_MAX_HASH)
> return;
>
> + cfg = npc_update_use_hash(rvu, blkaddr,
> + intf, lid, lt, ld);
> /* Set updated KEX configuration */
> SET_KEX_LD(intf, lid, lt, ld, cfg);
> /* Set HASH configuration */
> @@ -187,8 +192,6 @@ static void npc_program_mkex_hash_tx(struct rvu *rvu, int blkaddr,
> SET_KEX_LD_HASH_CTRL(intf, ld,
> mkex_hash->hash_ctrl[intf][ld]);
> hash_cnt++;
> - if (hash_cnt == NPC_MAX_HASH)
> - return;
> }
> }
> }
> @@ -238,8 +241,8 @@ void npc_update_field_hash(struct rvu *rvu, u8 intf,
> struct flow_msg *omask)
> {
> struct npc_mcam_kex_hash *mkex_hash = rvu->kpu.mkex_hash;
> - struct npc_get_secret_key_req req;
> - struct npc_get_secret_key_rsp rsp;
> + struct npc_get_field_hash_info_req req;
> + struct npc_get_field_hash_info_rsp rsp;
> u64 ldata[2], cfg;
> u32 field_hash;
> u8 hash_idx;
> @@ -250,7 +253,7 @@ void npc_update_field_hash(struct rvu *rvu, u8 intf,
> }
>
> req.intf = intf;
> - rvu_mbox_handler_npc_get_secret_key(rvu, &req, &rsp);
> + rvu_mbox_handler_npc_get_field_hash_info(rvu, &req, &rsp);
>
> for (hash_idx = 0; hash_idx < NPC_MAX_HASH; hash_idx++) {
> cfg = rvu_read64(rvu, blkaddr, NPC_AF_INTFX_HASHX_CFG(intf, hash_idx));
> @@ -266,44 +269,45 @@ void npc_update_field_hash(struct rvu *rvu, u8 intf,
> * is hashed to 32 bit value.
> */
> case NPC_LT_LC_IP6:
> - if (features & BIT_ULL(NPC_SIP_IPV6)) {
> + /* ld[0] == hash_idx[0] == Source IPv6
> + * ld[1] == hash_idx[1] == Destination IPv6
> + */
> + if ((features & BIT_ULL(NPC_SIP_IPV6)) && !hash_idx) {
> u32 src_ip[IPV6_WORDS];
>
> be32_to_cpu_array(src_ip, pkt->ip6src, IPV6_WORDS);
> - ldata[0] = (u64)src_ip[0] << 32 | src_ip[1];
> - ldata[1] = (u64)src_ip[2] << 32 | src_ip[3];
> + ldata[1] = (u64)src_ip[0] << 32 | src_ip[1];
> + ldata[0] = (u64)src_ip[2] << 32 | src_ip[3];
> field_hash = npc_field_hash_calc(ldata,
> - mkex_hash,
> - rsp.secret_key,
> + rsp,
> intf,
> hash_idx);
> npc_update_entry(rvu, NPC_SIP_IPV6, entry,
> - field_hash, 0, 32, 0, intf);
> + field_hash, 0,
> + GENMASK(31, 0), 0, intf);
> memcpy(&opkt->ip6src, &pkt->ip6src,
> sizeof(pkt->ip6src));
> memcpy(&omask->ip6src, &mask->ip6src,
> sizeof(mask->ip6src));
> - break;
> - }
> -
> - if (features & BIT_ULL(NPC_DIP_IPV6)) {
> + } else if ((features & BIT_ULL(NPC_DIP_IPV6)) && hash_idx) {
> u32 dst_ip[IPV6_WORDS];
>
> be32_to_cpu_array(dst_ip, pkt->ip6dst, IPV6_WORDS);
> - ldata[0] = (u64)dst_ip[0] << 32 | dst_ip[1];
> - ldata[1] = (u64)dst_ip[2] << 32 | dst_ip[3];
> + ldata[1] = (u64)dst_ip[0] << 32 | dst_ip[1];
> + ldata[0] = (u64)dst_ip[2] << 32 | dst_ip[3];
> field_hash = npc_field_hash_calc(ldata,
> - mkex_hash,
> - rsp.secret_key,
> + rsp,
> intf,
> hash_idx);
> npc_update_entry(rvu, NPC_DIP_IPV6, entry,
> - field_hash, 0, 32, 0, intf);
> + field_hash, 0,
> + GENMASK(31, 0), 0, intf);
> memcpy(&opkt->ip6dst, &pkt->ip6dst,
> sizeof(pkt->ip6dst));
> memcpy(&omask->ip6dst, &mask->ip6dst,
> sizeof(mask->ip6dst));
> }
> +
> break;
> }
> }
> @@ -311,13 +315,13 @@ void npc_update_field_hash(struct rvu *rvu, u8 intf,
> }
> }
>
> -int rvu_mbox_handler_npc_get_secret_key(struct rvu *rvu,
> - struct npc_get_secret_key_req *req,
> - struct npc_get_secret_key_rsp *rsp)
> +int rvu_mbox_handler_npc_get_field_hash_info(struct rvu *rvu,
> + struct npc_get_field_hash_info_req *req,
> + struct npc_get_field_hash_info_rsp *rsp)
> {
> u64 *secret_key = rsp->secret_key;
> u8 intf = req->intf;
> - int blkaddr;
> + int i, j, blkaddr;
>
> blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPC, 0);
> if (blkaddr < 0) {
> @@ -329,6 +333,19 @@ int rvu_mbox_handler_npc_get_secret_key(struct rvu *rvu,
> secret_key[1] = rvu_read64(rvu, blkaddr, NPC_AF_INTFX_SECRET_KEY1(intf));
> secret_key[2] = rvu_read64(rvu, blkaddr, NPC_AF_INTFX_SECRET_KEY2(intf));
>
> + for (i = 0; i < NPC_MAX_HASH; i++) {
> + for (j = 0; j < NPC_MAX_HASH_MASK; j++) {
> + rsp->hash_mask[NIX_INTF_RX][i][j] =
> + GET_KEX_LD_HASH_MASK(NIX_INTF_RX, i, j);
> + rsp->hash_mask[NIX_INTF_TX][i][j] =
> + GET_KEX_LD_HASH_MASK(NIX_INTF_TX, i, j);
> + }
> + }
> +
> + for (i = 0; i < NPC_MAX_INTF; i++)
> + for (j = 0; j < NPC_MAX_HASH; j++)
> + rsp->hash_ctrl[i][j] = GET_KEX_LD_HASH_CTRL(i, j);
> +
> return 0;
> }
>
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
> index 3efeb09c58de..a1c3d987b804 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
> @@ -31,6 +31,12 @@
> rvu_write64(rvu, blkaddr, \
> NPC_AF_INTFX_HASHX_MASKX(intf, ld, mask_idx), cfg)
>
> +#define GET_KEX_LD_HASH_CTRL(intf, ld) \
> + rvu_read64(rvu, blkaddr, NPC_AF_INTFX_HASHX_RESULT_CTRL(intf, ld))
> +
> +#define GET_KEX_LD_HASH_MASK(intf, ld, mask_idx) \
> + rvu_read64(rvu, blkaddr, NPC_AF_INTFX_HASHX_MASKX(intf, ld, mask_idx))
> +
> #define SET_KEX_LD_HASH_CTRL(intf, ld, cfg) \
> rvu_write64(rvu, blkaddr, \
> NPC_AF_INTFX_HASHX_RESULT_CTRL(intf, ld), cfg)
> @@ -56,8 +62,8 @@ void npc_update_field_hash(struct rvu *rvu, u8 intf,
> struct flow_msg *omask);
> void npc_config_secret_key(struct rvu *rvu, int blkaddr);
> void npc_program_mkex_hash(struct rvu *rvu, int blkaddr);
> -u32 npc_field_hash_calc(u64 *ldata, struct npc_mcam_kex_hash *mkex_hash,
> - u64 *secret_key, u8 intf, u8 hash_idx);
> +u32 npc_field_hash_calc(u64 *ldata, struct npc_get_field_hash_info_rsp rsp,
> + u8 intf, u8 hash_idx);
>
> static struct npc_mcam_kex_hash npc_mkex_hash_default __maybe_unused = {
> .lid_lt_ld_hash_en = {
> --
> 2.25.1
>

2023-03-30 06:31:55

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [net PATCH 7/7] octeontx2-pf: Disable packet I/O for graceful exit

On Wed, Mar 29, 2023 at 10:36:19PM +0530, Sai Krishna wrote:
> From: Subbaraya Sundeep <[email protected]>
>
> At the stage of enabling packet I/O in otx2_open, If mailbox
> timeout occurs then interface ends up in down state where as
> hardware packet I/O is enabled. Hence disable packet I/O also
> before bailing out. This patch also free the LMTST per cpu structure
> on teardown, if the lmt_info pointer is not NULL.
>
> Fixes: 1ea0166da050 ("octeontx2-pf: Fix the device state on error")
> Signed-off-by: Subbaraya Sundeep <[email protected]>
> Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
> Signed-off-by: Sai Krishna <[email protected]>
> ---
> drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 11 ++++++++++-
> drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c | 8 +++++---
> 2 files changed, 15 insertions(+), 4 deletions(-)
>

Thanks,
Reviewed-by: Leon Romanovsky <[email protected]>

2023-03-30 06:31:57

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [net PATCH 4/7] octeontx2-af: Update correct mask to filter IPv4 fragments

On Wed, Mar 29, 2023 at 10:36:16PM +0530, Sai Krishna wrote:
> From: Suman Ghosh <[email protected]>
>
> During the initial design, the IPv4 ip_flag mask was set to 0xff.
> Which results to filter only fragmets with (fragment_offset == 0).
> As part of the fix, updated the mask to 0x20 to filter all the
> fragmented packets irrespective of the fragment_offset value.
>
> Fixes: c672e3727989 ("octeontx2-pf: Add support to filter packet based on IP fragment")
> Signed-off-by: Suman Ghosh <[email protected]>
> Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
> Signed-off-by: Sai Krishna <[email protected]>
> ---
> drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>

Thanks,
Reviewed-by: Leon Romanovsky <[email protected]>

2023-03-30 06:32:01

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [net PATCH 3/7] octeontx2-af: Add validation for lmac type

On Wed, Mar 29, 2023 at 10:36:15PM +0530, Sai Krishna wrote:
> From: Hariprasad Kelam <[email protected]>
>
> Upon physical link change, firmware reports to the kernel about the
> change along with the details like speed, lmac_type_id, etc.
> Kernel derives lmac_type based on lmac_type_id received from firmware.
>
> In a few scenarios, firmware returns an invalid lmac_type_id, which
> is resulting in below kernel panic. This patch adds the missing
> validation of the lmac_type_id field.
>
> Internal error: Oops: 96000005 [#1] PREEMPT SMP
> [ 35.321595] Modules linked in:
> [ 35.328982] CPU: 0 PID: 31 Comm: kworker/0:1 Not tainted
> 5.4.210-g2e3169d8e1bc-dirty #17
> [ 35.337014] Hardware name: Marvell CN103XX board (DT)
> [ 35.344297] Workqueue: events work_for_cpu_fn
> [ 35.352730] pstate: 40400089 (nZcv daIf +PAN -UAO)
> [ 35.360267] pc : strncpy+0x10/0x30
> [ 35.366595] lr : cgx_link_change_handler+0x90/0x180
>
> Fixes: 61071a871ea6 ("octeontx2-af: Forward CGX link notifications to PFs")
> Signed-off-by: Hariprasad Kelam <[email protected]>
> Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
> Signed-off-by: Sai Krishna <[email protected]>
> ---
> drivers/net/ethernet/marvell/octeontx2/af/cgx.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> index 724df6398bbe..180aa84cf1c3 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> @@ -1231,6 +1231,13 @@ static inline void link_status_user_format(u64 lstat,
> linfo->an = FIELD_GET(RESP_LINKSTAT_AN, lstat);
> linfo->fec = FIELD_GET(RESP_LINKSTAT_FEC, lstat);
> linfo->lmac_type_id = FIELD_GET(RESP_LINKSTAT_LMAC_TYPE, lstat);
> +
> + if (linfo->lmac_type_id >= LMAC_MODE_MAX) {
> + dev_err(&cgx->pdev->dev, "Unknown lmac_type_id %d reported by firmware on cgx port%d:%d",
> + linfo->lmac_type_id, cgx->cgx_id, lmac_id);
> + return;

You are keeping old lmac_type, which is out-of-sync now.
Why don't you do something like that?

if (linfo->lmac_type_id >= LMAC_MODE_MAX) {
strncpy(linfo->lmac_type, "Unknown", LMACTYPE_STR_LEN - 1);
return;
}


> + }
> +
> lmac_string = cgx_lmactype_string[linfo->lmac_type_id];
> strncpy(linfo->lmac_type, lmac_string, LMACTYPE_STR_LEN - 1);
> }
> --
> 2.25.1
>

2023-03-30 07:03:58

by Geetha sowjanya

[permalink] [raw]
Subject: RE: [EXT] Re: [net PATCH 1/7] octeontx2-af: Secure APR table update with the lock


>-----Original Message-----
>From: Leon Romanovsky <[email protected]>
>Sent: Thursday, March 30, 2023 11:26 AM
>To: Sai Krishna Gajula <[email protected]>
>Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Sunil Kovvuri Goutham <[email protected]>; >[email protected]; Geethasowjanya Akula <[email protected]>
>Subject: [EXT] Re: [net PATCH 1/7] octeontx2-af: Secure APR table update with the lock

>External Email

>----------------------------------------------------------------------
>On Wed, Mar 29, 2023 at 10:36:13PM +0530, Sai Krishna wrote:
>> From: Geetha sowjanya <[email protected]>
>>
>> APR table contains the lmtst base address of PF/VFs.
>> These entries are updated by the PF/VF during the device probe. Due to
>> race condition while updating the entries are getting corrupted. Hence
>> secure the APR table update with the lock.

>However, I don't see rsrc_lock in probe path.
>otx2_probe()
>-> cn10k_lmtst_init()
> -> lmt_base/lmstst is updated with and without mbox.lock.

>Where did you take rsrc_lock in probe flow?

rsrc_lock is initialized in AF driver. PF/VF driver in cn10k_lmtst_init() send a mbox request to AF to update the lmtst table.
mbox handler in AF takes rsrc_lock to update lmtst table.

Thanks,
Geetha.

>Thanks

>>
>> Fixes: 893ae97214c3 ("octeontx2-af: cn10k: Support configurable LMTST
>> regions")
>> Signed-off-by: Geetha sowjanya <[email protected]>
>> Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
>> Signed-off-by: Sai Krishna <[email protected]>
>> ---
>> drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
>> b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
>> index 4ad9ff025c96..8530250f6fba 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
>> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
>> @@ -142,16 +142,17 @@ int rvu_mbox_handler_lmtst_tbl_setup(struct rvu *rvu,
>> * region, if so, convert that IOVA to physical address and
>> * populate LMT table with that address
>> */
>> + mutex_lock(&rvu->rsrc_lock);
>> if (req->use_local_lmt_region) {
>> err = rvu_get_lmtaddr(rvu, req->hdr.pcifunc,
>> req->lmt_iova, &lmt_addr);
>> if (err < 0)
>> - return err;
>> + goto error;
>>
>> /* Update the lmt addr for this PFFUNC in the LMT table */
>> err = rvu_update_lmtaddr(rvu, req->hdr.pcifunc, lmt_addr);
>> if (err)
>> - return err;
>> + goto error;
>> }
>>
>> /* Reconfiguring lmtst map table in lmt region shared mode i.e. make
>> @@ -181,7 +182,7 @@ int rvu_mbox_handler_lmtst_tbl_setup(struct rvu *rvu,
>> */
>> err = rvu_update_lmtaddr(rvu, req->hdr.pcifunc, val);
>> if (err)
>> - return err;
>> + goto error;
>> }
>>
>> /* This mailbox can also be used to update word1 of
>> APR_LMT_MAP_ENTRY_S @@ -230,6 +231,7 @@ int rvu_mbox_handler_lmtst_tbl_setup(struct rvu *rvu,
>> }
>>
>> error:
>> + mutex_unlock(&rvu->rsrc_lock);
>> return err;
>> }
>>
>> --
>> 2.25.1
>>

2023-03-30 07:18:36

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [EXT] Re: [net PATCH 1/7] octeontx2-af: Secure APR table update with the lock

On Thu, Mar 30, 2023 at 06:56:54AM +0000, Geethasowjanya Akula wrote:
>
> >-----Original Message-----
> >From: Leon Romanovsky <[email protected]>
> >Sent: Thursday, March 30, 2023 11:26 AM
> >To: Sai Krishna Gajula <[email protected]>
> >Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Sunil Kovvuri Goutham <[email protected]>; >[email protected]; Geethasowjanya Akula <[email protected]>
> >Subject: [EXT] Re: [net PATCH 1/7] octeontx2-af: Secure APR table update with the lock
>
> >External Email
>
> >----------------------------------------------------------------------
> >On Wed, Mar 29, 2023 at 10:36:13PM +0530, Sai Krishna wrote:
> >> From: Geetha sowjanya <[email protected]>
> >>
> >> APR table contains the lmtst base address of PF/VFs.
> >> These entries are updated by the PF/VF during the device probe. Due to
> >> race condition while updating the entries are getting corrupted. Hence
> >> secure the APR table update with the lock.
>
> >However, I don't see rsrc_lock in probe path.
> >otx2_probe()
> >-> cn10k_lmtst_init()
> > -> lmt_base/lmstst is updated with and without mbox.lock.
>
> >Where did you take rsrc_lock in probe flow?
>
> rsrc_lock is initialized in AF driver. PF/VF driver in cn10k_lmtst_init() send a mbox request to AF to update the lmtst table.
> mbox handler in AF takes rsrc_lock to update lmtst table.

Can you please present the stack trace of such flow? What are the actual variables/struct rsrc_lock
is protecting?

Thanks

>
> Thanks,
> Geetha.
>
> >Thanks
>
> >>
> >> Fixes: 893ae97214c3 ("octeontx2-af: cn10k: Support configurable LMTST
> >> regions")
> >> Signed-off-by: Geetha sowjanya <[email protected]>
> >> Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
> >> Signed-off-by: Sai Krishna <[email protected]>
> >> ---
> >> drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c | 8 +++++---
> >> 1 file changed, 5 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
> >> b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
> >> index 4ad9ff025c96..8530250f6fba 100644
> >> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
> >> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
> >> @@ -142,16 +142,17 @@ int rvu_mbox_handler_lmtst_tbl_setup(struct rvu *rvu,
> >> * region, if so, convert that IOVA to physical address and
> >> * populate LMT table with that address
> >> */
> >> + mutex_lock(&rvu->rsrc_lock);
> >> if (req->use_local_lmt_region) {
> >> err = rvu_get_lmtaddr(rvu, req->hdr.pcifunc,
> >> req->lmt_iova, &lmt_addr);
> >> if (err < 0)
> >> - return err;
> >> + goto error;
> >>
> >> /* Update the lmt addr for this PFFUNC in the LMT table */
> >> err = rvu_update_lmtaddr(rvu, req->hdr.pcifunc, lmt_addr);
> >> if (err)
> >> - return err;
> >> + goto error;
> >> }
> >>
> >> /* Reconfiguring lmtst map table in lmt region shared mode i.e. make
> >> @@ -181,7 +182,7 @@ int rvu_mbox_handler_lmtst_tbl_setup(struct rvu *rvu,
> >> */
> >> err = rvu_update_lmtaddr(rvu, req->hdr.pcifunc, val);
> >> if (err)
> >> - return err;
> >> + goto error;
> >> }
> >>
> >> /* This mailbox can also be used to update word1 of
> >> APR_LMT_MAP_ENTRY_S @@ -230,6 +231,7 @@ int rvu_mbox_handler_lmtst_tbl_setup(struct rvu *rvu,
> >> }
> >>
> >> error:
> >> + mutex_unlock(&rvu->rsrc_lock);
> >> return err;
> >> }
> >>
> >> --
> >> 2.25.1
> >>

2023-03-30 10:17:15

by Sai Krishna Gajula

[permalink] [raw]
Subject: RE: [EXT] Re: [net PATCH 3/7] octeontx2-af: Add validation for lmac type

Please see inline.

> -----Original Message-----
> From: Leon Romanovsky <[email protected]>
> Sent: Thursday, March 30, 2023 11:49 AM
> To: Sai Krishna Gajula <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> Sunil Kovvuri Goutham <[email protected]>;
> [email protected]; Hariprasad Kelam <[email protected]>
> Subject: [EXT] Re: [net PATCH 3/7] octeontx2-af: Add validation for lmac type
>
> External Email
>
> ----------------------------------------------------------------------
> On Wed, Mar 29, 2023 at 10:36:15PM +0530, Sai Krishna wrote:
> > From: Hariprasad Kelam <[email protected]>
> >
> > Upon physical link change, firmware reports to the kernel about the
> > change along with the details like speed, lmac_type_id, etc.
> > Kernel derives lmac_type based on lmac_type_id received from firmware.
> >
> > In a few scenarios, firmware returns an invalid lmac_type_id, which is
> > resulting in below kernel panic. This patch adds the missing
> > validation of the lmac_type_id field.
> >
> > Internal error: Oops: 96000005 [#1] PREEMPT SMP
> > [ 35.321595] Modules linked in:
> > [ 35.328982] CPU: 0 PID: 31 Comm: kworker/0:1 Not tainted
> > 5.4.210-g2e3169d8e1bc-dirty #17
> > [ 35.337014] Hardware name: Marvell CN103XX board (DT)
> > [ 35.344297] Workqueue: events work_for_cpu_fn
> > [ 35.352730] pstate: 40400089 (nZcv daIf +PAN -UAO)
> > [ 35.360267] pc : strncpy+0x10/0x30
> > [ 35.366595] lr : cgx_link_change_handler+0x90/0x180
> >
> > Fixes: 61071a871ea6 ("octeontx2-af: Forward CGX link notifications to
> > PFs")
> > Signed-off-by: Hariprasad Kelam <[email protected]>
> > Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
> > Signed-off-by: Sai Krishna <[email protected]>
> > ---
> > drivers/net/ethernet/marvell/octeontx2/af/cgx.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> > b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> > index 724df6398bbe..180aa84cf1c3 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> > @@ -1231,6 +1231,13 @@ static inline void link_status_user_format(u64
> lstat,
> > linfo->an = FIELD_GET(RESP_LINKSTAT_AN, lstat);
> > linfo->fec = FIELD_GET(RESP_LINKSTAT_FEC, lstat);
> > linfo->lmac_type_id = FIELD_GET(RESP_LINKSTAT_LMAC_TYPE, lstat);
> > +
> > + if (linfo->lmac_type_id >= LMAC_MODE_MAX) {
> > + dev_err(&cgx->pdev->dev, "Unknown lmac_type_id %d
> reported by firmware on cgx port%d:%d",
> > + linfo->lmac_type_id, cgx->cgx_id, lmac_id);
> > + return;
>
> You are keeping old lmac_type, which is out-of-sync now.
> Why don't you do something like that?
>
> if (linfo->lmac_type_id >= LMAC_MODE_MAX) {
> strncpy(linfo->lmac_type, "Unknown", LMACTYPE_STR_LEN - 1);
> return;
> }
>
>
We will add the proposed change (Unknown). Since we need to know the firmware reported lmac type ID is proper or not, we will keep dev_err also.
> > + }
> > +
> > lmac_string = cgx_lmactype_string[linfo->lmac_type_id];
> > strncpy(linfo->lmac_type, lmac_string, LMACTYPE_STR_LEN - 1); }
> > --
> > 2.25.1
> >
Thanks,
Sai

2023-03-30 10:20:20

by Sai Krishna Gajula

[permalink] [raw]
Subject: Re: [net PATCH 3/7] octeontx2-af: Add validation for lmac type

Please see inline,

> -----Original Message-----
> From: Leon Romanovsky <[email protected]>
> Sent: Thursday, March 30, 2023 11:49 AM
> To: Sai Krishna Gajula <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> Sunil Kovvuri Goutham <[email protected]>;
> [email protected]; Hariprasad Kelam <[email protected]>
> Subject: Re: [net PATCH 3/7] octeontx2-af: Add validation for lmac type

> > From: Hariprasad Kelam <[email protected]>
> >
> > Upon physical link change, firmware reports to the kernel about the
> > change along with the details like speed, lmac_type_id, etc.
> > Kernel derives lmac_type based on lmac_type_id received from firmware.
> >
> > In a few scenarios, firmware returns an invalid lmac_type_id, which is
> > resulting in below kernel panic. This patch adds the missing
> > validation of the lmac_type_id field.
> >
> > Internal error: Oops: 96000005 [#1] PREEMPT SMP
> > [ 35.321595] Modules linked in:
> > [ 35.328982] CPU: 0 PID: 31 Comm: kworker/0:1 Not tainted
> > 5.4.210-g2e3169d8e1bc-dirty #17
> > [ 35.337014] Hardware name: Marvell CN103XX board (DT)
> > [ 35.344297] Workqueue: events work_for_cpu_fn
> > [ 35.352730] pstate: 40400089 (nZcv daIf +PAN -UAO)
> > [ 35.360267] pc : strncpy+0x10/0x30
> > [ 35.366595] lr : cgx_link_change_handler+0x90/0x180
> >
> > Fixes: 61071a871ea6 ("octeontx2-af: Forward CGX link notifications to
> > PFs")
> > Signed-off-by: Hariprasad Kelam <[email protected]>
> > Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
> > Signed-off-by: Sai Krishna <[email protected]>
> > ---
> > drivers/net/ethernet/marvell/octeontx2/af/cgx.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> > b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> > index 724df6398bbe..180aa84cf1c3 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
> > @@ -1231,6 +1231,13 @@ static inline void link_status_user_format(u64
> lstat,
> > linfo->an = FIELD_GET(RESP_LINKSTAT_AN, lstat);
> > linfo->fec = FIELD_GET(RESP_LINKSTAT_FEC, lstat);
> > linfo->lmac_type_id = FIELD_GET(RESP_LINKSTAT_LMAC_TYPE, lstat);
> > +
> > + if (linfo->lmac_type_id >= LMAC_MODE_MAX) {
> > + dev_err(&cgx->pdev->dev, "Unknown lmac_type_id %d
> reported by firmware on cgx port%d:%d",
> > + linfo->lmac_type_id, cgx->cgx_id, lmac_id);
> > + return;
>
> You are keeping old lmac_type, which is out-of-sync now.
> Why don't you do something like that?
>
> if (linfo->lmac_type_id >= LMAC_MODE_MAX) {
> strncpy(linfo->lmac_type, "Unknown", LMACTYPE_STR_LEN - 1);
> return;
> }
>
>
We will add the proposed change (Unknown). Since we need to know the firmware reported lmac type ID is proper or not, we will keep dev_err also.

Thanks,
Sai

> > + }
> > +
> > lmac_string = cgx_lmactype_string[linfo->lmac_type_id];
> > strncpy(linfo->lmac_type, lmac_string, LMACTYPE_STR_LEN - 1); }
> > --
> > 2.25.1
> >

2023-03-30 18:00:07

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [net PATCH 0/7] octeontx2: Miscellaneous fixes

On Wed, 29 Mar 2023 22:36:12 +0530 Sai Krishna wrote:
> From: Sai Krishna <[email protected]>
> To: <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>
> CC: Sai Krishna <[email protected]>

First of all, does the maintainers file need to be updated?

This driver has a crazy number of maintainers:

MARVELL OCTEONTX2 RVU ADMIN FUNCTION DRIVER
M: Sunil Goutham <[email protected]>
M: Linu Cherian <[email protected]>
M: Geetha sowjanya <[email protected]>
M: Jerin Jacob <[email protected]>
M: hariprasad <[email protected]>
M: Subbaraya Sundeep <[email protected]>
L: [email protected]
S: Supported
F: Documentation/networking/device_drivers/ethernet/marvell/octeontx2.rst
F: drivers/net/ethernet/marvell/octeontx2/af/

And yet the person posting patches for the company is not on that list?!
Please clean this up, or CC authors of patches on the fixes.

2023-03-30 18:04:06

by Sai Krishna Gajula

[permalink] [raw]
Subject: Re: [net PATCH 5/7] octeontx2-af: Fix issues with NPC field hash extract

Please see inline,

> -----Original Message-----
> From: Leon Romanovsky <[email protected]>
> Sent: Thursday, March 30, 2023 11:50 AM
> To: Sai Krishna Gajula <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> Sunil Kovvuri Goutham <[email protected]>;
> [email protected]; Ratheesh Kannoth <[email protected]>
> Subject: Re: [net PATCH 5/7] octeontx2-af: Fix issues with NPC field
> hash extract

> On Wed, Mar 29, 2023 at 10:36:17PM +0530, Sai Krishna wrote:
> > From: Ratheesh Kannoth <[email protected]>
> >
> > 1. Update secret key mbox to provide hash mask and hash control as well.
> > 2. Allow field hash configuration for both source and destination IPv6
> > 3. Fix internal logic for IPv6 source/destination address hash
> > reduction via ntuple rule 4. Configure hardware parser based on hash
> > extract feature enable flag
> > for IPv6.
>
> This commit message explained what you did, but not what was the
> problem.
>
We will update the commit message with the problem statement along with fix description in V2 patch.

Thanks,
Sai

> Thanks
>
> >
> > Fixes: 56d9f5fd2246 ("octeontx2-af: Use hashed field in MCAM key")
> > Signed-off-by: Ratheesh Kannoth <[email protected]>
> > Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
> > Signed-off-by: Sai Krishna <[email protected]>
> > ---
> > .../net/ethernet/marvell/octeontx2/af/mbox.h | 16 ++-
> > .../marvell/octeontx2/af/rvu_npc_fs.c | 23 +++-
> > .../marvell/octeontx2/af/rvu_npc_fs.h | 4 +
> > .../marvell/octeontx2/af/rvu_npc_hash.c | 121 ++++++++++--------
> > .../marvell/octeontx2/af/rvu_npc_hash.h | 10 +-
> > 5 files changed, 108 insertions(+), 66 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> > b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> > index 5727d67e0259..0ce533848536 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> > @@ -245,9 +245,9 @@ M(NPC_MCAM_READ_BASE_RULE, 0x6011,
> npc_read_base_steer_rule, \
> > M(NPC_MCAM_GET_STATS, 0x6012, npc_mcam_entry_stats, \
> > npc_mcam_get_stats_req, \
> > npc_mcam_get_stats_rsp) \
> > -M(NPC_GET_SECRET_KEY, 0x6013, npc_get_secret_key, \
> > - npc_get_secret_key_req, \
> > - npc_get_secret_key_rsp) \
> > +M(NPC_GET_FIELD_HASH_INFO, 0x6013, npc_get_field_hash_info,
> \
> > + npc_get_field_hash_info_req, \
> > + npc_get_field_hash_info_rsp) \
> > M(NPC_GET_FIELD_STATUS, 0x6014, npc_get_field_status, \
> > npc_get_field_status_req, \
> > npc_get_field_status_rsp) \
> > @@ -1524,14 +1524,20 @@ struct npc_mcam_get_stats_rsp {
> > u8 stat_ena; /* enabled */
> > };
> >
> > -struct npc_get_secret_key_req {
> > +struct npc_get_field_hash_info_req {
> > struct mbox_msghdr hdr;
> > u8 intf;
> > };
> >
> > -struct npc_get_secret_key_rsp {
> > +struct npc_get_field_hash_info_rsp {
> > struct mbox_msghdr hdr;
> > u64 secret_key[3];
> > +#define NPC_MAX_HASH 2
> > +#define NPC_MAX_HASH_MASK 2
> > + /* NPC_AF_INTF(0..1)_HASH(0..1)_MASK(0..1) */
> > + u64
> hash_mask[NPC_MAX_INTF][NPC_MAX_HASH][NPC_MAX_HASH_MASK];
> > + /* NPC_AF_INTF(0..1)_HASH(0..1)_RESULT_CTRL */
> > + u64 hash_ctrl[NPC_MAX_INTF][NPC_MAX_HASH];
> > };
> >
> > enum ptp_op {
> > 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 27603078689a..6d63a0ef6d9c 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
> > @@ -13,11 +13,6 @@
> > #include "rvu_npc_fs.h"
> > #include "rvu_npc_hash.h"
> >
> > -#define NPC_BYTESM GENMASK_ULL(19, 16)
> > -#define NPC_HDR_OFFSET GENMASK_ULL(15, 8)
> > -#define NPC_KEY_OFFSET GENMASK_ULL(5, 0)
> > -#define NPC_LDATA_EN BIT_ULL(7)
> > -
> > static const char * const npc_flow_names[] = {
> > [NPC_DMAC] = "dmac",
> > [NPC_SMAC] = "smac",
> > @@ -442,6 +437,7 @@ static void npc_handle_multi_layer_fields(struct
> > rvu *rvu, int blkaddr, u8 intf) static void npc_scan_ldata(struct rvu *rvu, int
> blkaddr, u8 lid,
> > u8 lt, u64 cfg, u8 intf)
> > {
> > + struct npc_mcam_kex_hash *mkex_hash = rvu->kpu.mkex_hash;
> > struct npc_mcam *mcam = &rvu->hw->mcam;
> > u8 hdr, key, nr_bytes, bit_offset;
> > u8 la_ltype, la_start;
> > @@ -490,8 +486,21 @@ do {
> \
> > NPC_SCAN_HDR(NPC_SIP_IPV4, NPC_LID_LC, NPC_LT_LC_IP, 12, 4);
> > NPC_SCAN_HDR(NPC_DIP_IPV4, NPC_LID_LC, NPC_LT_LC_IP, 16, 4);
> > NPC_SCAN_HDR(NPC_IPFRAG_IPV6, NPC_LID_LC,
> NPC_LT_LC_IP6_EXT, 6, 1);
> > - NPC_SCAN_HDR(NPC_SIP_IPV6, NPC_LID_LC, NPC_LT_LC_IP6, 8, 16);
> > - NPC_SCAN_HDR(NPC_DIP_IPV6, NPC_LID_LC, NPC_LT_LC_IP6, 24,
> 16);
> > + if (rvu->hw->cap.npc_hash_extract) {
> > + if (mkex_hash->lid_lt_ld_hash_en[intf][lid][lt][0])
> > + NPC_SCAN_HDR(NPC_SIP_IPV6, NPC_LID_LC,
> NPC_LT_LC_IP6, 8, 4);
> > + else
> > + NPC_SCAN_HDR(NPC_SIP_IPV6, NPC_LID_LC,
> NPC_LT_LC_IP6, 8, 16);
> > +
> > + if (mkex_hash->lid_lt_ld_hash_en[intf][lid][lt][1])
> > + NPC_SCAN_HDR(NPC_DIP_IPV6, NPC_LID_LC,
> NPC_LT_LC_IP6, 24, 4);
> > + else
> > + NPC_SCAN_HDR(NPC_DIP_IPV6, NPC_LID_LC,
> NPC_LT_LC_IP6, 24, 16);
> > + } else {
> > + NPC_SCAN_HDR(NPC_SIP_IPV6, NPC_LID_LC, NPC_LT_LC_IP6,
> 8, 16);
> > + NPC_SCAN_HDR(NPC_DIP_IPV6, NPC_LID_LC,
> NPC_LT_LC_IP6, 24, 16);
> > + }
> > +
> > NPC_SCAN_HDR(NPC_SPORT_UDP, NPC_LID_LD, NPC_LT_LD_UDP, 0,
> 2);
> > NPC_SCAN_HDR(NPC_DPORT_UDP, NPC_LID_LD, NPC_LT_LD_UDP, 2,
> 2);
> > NPC_SCAN_HDR(NPC_SPORT_TCP, NPC_LID_LD, NPC_LT_LD_TCP, 0,
> 2); diff
> > --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.h
> > b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.h
> > index bdd65ce56a32..3f5c9042d10e 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.h
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.h
> > @@ -9,6 +9,10 @@
> > #define __RVU_NPC_FS_H
> >
> > #define IPV6_WORDS 4
> > +#define NPC_BYTESM GENMASK_ULL(19, 16)
> > +#define NPC_HDR_OFFSET GENMASK_ULL(15, 8)
> > +#define NPC_KEY_OFFSET GENMASK_ULL(5, 0)
> > +#define NPC_LDATA_EN BIT_ULL(7)
> >
> > void npc_update_entry(struct rvu *rvu, enum key_fields type,
> > struct mcam_entry *entry, u64 val_lo, diff --git
> > a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
> > b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
> > index 6597af84aa36..51209119f0f2 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
> > @@ -78,42 +78,43 @@ static u32 rvu_npc_toeplitz_hash(const u64 *data,
> u64 *key, size_t data_bit_len,
> > return hash_out;
> > }
> >
> > -u32 npc_field_hash_calc(u64 *ldata, struct npc_mcam_kex_hash
> *mkex_hash,
> > - u64 *secret_key, u8 intf, u8 hash_idx)
> > +u32 npc_field_hash_calc(u64 *ldata, struct npc_get_field_hash_info_rsp
> rsp,
> > + u8 intf, u8 hash_idx)
> > {
> > u64 hash_key[3];
> > u64 data_padded[2];
> > u32 field_hash;
> >
> > - hash_key[0] = secret_key[1] << 31;
> > - hash_key[0] |= secret_key[2];
> > - hash_key[1] = secret_key[1] >> 33;
> > - hash_key[1] |= secret_key[0] << 31;
> > - hash_key[2] = secret_key[0] >> 33;
> > + hash_key[0] = rsp.secret_key[1] << 31;
> > + hash_key[0] |= rsp.secret_key[2];
> > + hash_key[1] = rsp.secret_key[1] >> 33;
> > + hash_key[1] |= rsp.secret_key[0] << 31;
> > + hash_key[2] = rsp.secret_key[0] >> 33;
> >
> > - data_padded[0] = mkex_hash->hash_mask[intf][hash_idx][0] &
> ldata[0];
> > - data_padded[1] = mkex_hash->hash_mask[intf][hash_idx][1] &
> ldata[1];
> > + data_padded[0] = rsp.hash_mask[intf][hash_idx][0] & ldata[0];
> > + data_padded[1] = rsp.hash_mask[intf][hash_idx][1] & ldata[1];
> > field_hash = rvu_npc_toeplitz_hash(data_padded, hash_key, 128,
> 159);
> >
> > - field_hash &= mkex_hash->hash_ctrl[intf][hash_idx] >> 32;
> > - field_hash |= mkex_hash->hash_ctrl[intf][hash_idx];
> > + field_hash &= FIELD_GET(GENMASK(63, 32),
> rsp.hash_ctrl[intf][hash_idx]);
> > + field_hash += FIELD_GET(GENMASK(31, 0),
> > +rsp.hash_ctrl[intf][hash_idx]);
> > return field_hash;
> > }
> >
> > -static u64 npc_update_use_hash(int lt, int ld)
> > +static u64 npc_update_use_hash(struct rvu *rvu, int blkaddr,
> > + u8 intf, int lid, int lt, int ld)
> > {
> > - u64 cfg = 0;
> > -
> > - switch (lt) {
> > - case NPC_LT_LC_IP6:
> > - /* Update use_hash(bit-20) and bytesm1 (bit-16:19)
> > - * in KEX_LD_CFG
> > - */
> > - cfg = KEX_LD_CFG_USE_HASH(0x1, 0x03,
> > - ld ? 0x8 : 0x18,
> > - 0x1, 0x0, 0x10);
> > - break;
> > - }
> > + u8 hdr, key;
> > + u64 cfg;
> > +
> > + cfg = rvu_read64(rvu, blkaddr,
> NPC_AF_INTFX_LIDX_LTX_LDX_CFG(intf, lid, lt, ld));
> > + hdr = FIELD_GET(NPC_HDR_OFFSET, cfg);
> > + key = FIELD_GET(NPC_KEY_OFFSET, cfg);
> > +
> > + /* Update use_hash(bit-20) to 'true' and
> > + * bytesm1(bit-16:19) to '0x3' in KEX_LD_CFG
> > + */
> > + cfg = KEX_LD_CFG_USE_HASH(0x1, 0x03,
> > + hdr, 0x1, 0x0, key);
> >
> > return cfg;
> > }
> > @@ -132,12 +133,13 @@ static void npc_program_mkex_hash_rx(struct
> rvu *rvu, int blkaddr,
> > for (lt = 0; lt < NPC_MAX_LT; lt++) {
> > for (ld = 0; ld < NPC_MAX_LD; ld++) {
> > if (mkex_hash-
> >lid_lt_ld_hash_en[intf][lid][lt][ld]) {
> > - u64 cfg = npc_update_use_hash(lt,
> ld);
> > + u64 cfg;
> >
> > - hash_cnt++;
> > if (hash_cnt == NPC_MAX_HASH)
> > return;
> >
> > + cfg = npc_update_use_hash(rvu,
> blkaddr,
> > + intf, lid, lt,
> ld);
> > /* Set updated KEX configuration */
> > SET_KEX_LD(intf, lid, lt, ld, cfg);
> > /* Set HASH configuration */
> > @@ -149,6 +151,8 @@ static void npc_program_mkex_hash_rx(struct rvu
> *rvu, int blkaddr,
> > mkex_hash-
> >hash_mask[intf][ld][1]);
> > SET_KEX_LD_HASH_CTRL(intf, ld,
> > mkex_hash-
> >hash_ctrl[intf][ld]);
> > +
> > + hash_cnt++;
> > }
> > }
> > }
> > @@ -169,12 +173,13 @@ static void npc_program_mkex_hash_tx(struct
> rvu *rvu, int blkaddr,
> > for (lt = 0; lt < NPC_MAX_LT; lt++) {
> > for (ld = 0; ld < NPC_MAX_LD; ld++)
> > if (mkex_hash-
> >lid_lt_ld_hash_en[intf][lid][lt][ld]) {
> > - u64 cfg = npc_update_use_hash(lt,
> ld);
> > + u64 cfg;
> >
> > - hash_cnt++;
> > if (hash_cnt == NPC_MAX_HASH)
> > return;
> >
> > + cfg = npc_update_use_hash(rvu,
> blkaddr,
> > + intf, lid, lt,
> ld);
> > /* Set updated KEX configuration */
> > SET_KEX_LD(intf, lid, lt, ld, cfg);
> > /* Set HASH configuration */
> > @@ -187,8 +192,6 @@ static void npc_program_mkex_hash_tx(struct rvu
> *rvu, int blkaddr,
> > SET_KEX_LD_HASH_CTRL(intf, ld,
> > mkex_hash-
> >hash_ctrl[intf][ld]);
> > hash_cnt++;
> > - if (hash_cnt == NPC_MAX_HASH)
> > - return;
> > }
> > }
> > }
> > @@ -238,8 +241,8 @@ void npc_update_field_hash(struct rvu *rvu, u8
> intf,
> > struct flow_msg *omask)
> > {
> > struct npc_mcam_kex_hash *mkex_hash = rvu->kpu.mkex_hash;
> > - struct npc_get_secret_key_req req;
> > - struct npc_get_secret_key_rsp rsp;
> > + struct npc_get_field_hash_info_req req;
> > + struct npc_get_field_hash_info_rsp rsp;
> > u64 ldata[2], cfg;
> > u32 field_hash;
> > u8 hash_idx;
> > @@ -250,7 +253,7 @@ void npc_update_field_hash(struct rvu *rvu, u8
> intf,
> > }
> >
> > req.intf = intf;
> > - rvu_mbox_handler_npc_get_secret_key(rvu, &req, &rsp);
> > + rvu_mbox_handler_npc_get_field_hash_info(rvu, &req, &rsp);
> >
> > for (hash_idx = 0; hash_idx < NPC_MAX_HASH; hash_idx++) {
> > cfg = rvu_read64(rvu, blkaddr,
> NPC_AF_INTFX_HASHX_CFG(intf,
> > hash_idx)); @@ -266,44 +269,45 @@ void npc_update_field_hash(struct
> rvu *rvu, u8 intf,
> > * is hashed to 32 bit value.
> > */
> > case NPC_LT_LC_IP6:
> > - if (features & BIT_ULL(NPC_SIP_IPV6))
> {
> > + /* ld[0] == hash_idx[0] == Source IPv6
> > + * ld[1] == hash_idx[1] == Destination
> IPv6
> > + */
> > + if ((features &
> BIT_ULL(NPC_SIP_IPV6)) && !hash_idx) {
> > u32 src_ip[IPV6_WORDS];
> >
> > be32_to_cpu_array(src_ip,
> pkt->ip6src, IPV6_WORDS);
> > - ldata[0] = (u64)src_ip[0] << 32
> | src_ip[1];
> > - ldata[1] = (u64)src_ip[2] << 32
> | src_ip[3];
> > + ldata[1] = (u64)src_ip[0] << 32
> | src_ip[1];
> > + ldata[0] = (u64)src_ip[2] << 32
> | src_ip[3];
> > field_hash =
> npc_field_hash_calc(ldata,
> > -
> mkex_hash,
> > -
> rsp.secret_key,
> > +
> rsp,
> >
> intf,
> >
> hash_idx);
> > npc_update_entry(rvu,
> NPC_SIP_IPV6, entry,
> > - field_hash, 0,
> 32, 0, intf);
> > + field_hash, 0,
> > +
> GENMASK(31, 0), 0, intf);
> > memcpy(&opkt->ip6src,
> &pkt->ip6src,
> > sizeof(pkt->ip6src));
> > memcpy(&omask->ip6src,
> &mask->ip6src,
> > sizeof(mask->ip6src));
> > - break;
> > - }
> > -
> > - if (features &
> BIT_ULL(NPC_DIP_IPV6)) {
> > + } else if ((features &
> BIT_ULL(NPC_DIP_IPV6)) && hash_idx) {
> > u32 dst_ip[IPV6_WORDS];
> >
> > be32_to_cpu_array(dst_ip,
> pkt->ip6dst, IPV6_WORDS);
> > - ldata[0] = (u64)dst_ip[0] <<
> 32 | dst_ip[1];
> > - ldata[1] = (u64)dst_ip[2] <<
> 32 | dst_ip[3];
> > + ldata[1] = (u64)dst_ip[0] <<
> 32 | dst_ip[1];
> > + ldata[0] = (u64)dst_ip[2] <<
> 32 | dst_ip[3];
> > field_hash =
> npc_field_hash_calc(ldata,
> > -
> mkex_hash,
> > -
> rsp.secret_key,
> > +
> rsp,
> >
> intf,
> >
> hash_idx);
> > npc_update_entry(rvu,
> NPC_DIP_IPV6, entry,
> > - field_hash, 0,
> 32, 0, intf);
> > + field_hash, 0,
> > +
> GENMASK(31, 0), 0, intf);
> > memcpy(&opkt->ip6dst,
> &pkt->ip6dst,
> > sizeof(pkt->ip6dst));
> > memcpy(&omask->ip6dst,
> &mask->ip6dst,
> > sizeof(mask->ip6dst));
> > }
> > +
> > break;
> > }
> > }
> > @@ -311,13 +315,13 @@ void npc_update_field_hash(struct rvu *rvu, u8
> intf,
> > }
> > }
> >
> > -int rvu_mbox_handler_npc_get_secret_key(struct rvu *rvu,
> > - struct npc_get_secret_key_req *req,
> > - struct npc_get_secret_key_rsp *rsp)
> > +int rvu_mbox_handler_npc_get_field_hash_info(struct rvu *rvu,
> > + struct
> npc_get_field_hash_info_req *req,
> > + struct
> npc_get_field_hash_info_rsp *rsp)
> > {
> > u64 *secret_key = rsp->secret_key;
> > u8 intf = req->intf;
> > - int blkaddr;
> > + int i, j, blkaddr;
> >
> > blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPC, 0);
> > if (blkaddr < 0) {
> > @@ -329,6 +333,19 @@ int rvu_mbox_handler_npc_get_secret_key(struct
> rvu *rvu,
> > secret_key[1] = rvu_read64(rvu, blkaddr,
> NPC_AF_INTFX_SECRET_KEY1(intf));
> > secret_key[2] = rvu_read64(rvu, blkaddr,
> > NPC_AF_INTFX_SECRET_KEY2(intf));
> >
> > + for (i = 0; i < NPC_MAX_HASH; i++) {
> > + for (j = 0; j < NPC_MAX_HASH_MASK; j++) {
> > + rsp->hash_mask[NIX_INTF_RX][i][j] =
> > + GET_KEX_LD_HASH_MASK(NIX_INTF_RX, i, j);
> > + rsp->hash_mask[NIX_INTF_TX][i][j] =
> > + GET_KEX_LD_HASH_MASK(NIX_INTF_TX, i, j);
> > + }
> > + }
> > +
> > + for (i = 0; i < NPC_MAX_INTF; i++)
> > + for (j = 0; j < NPC_MAX_HASH; j++)
> > + rsp->hash_ctrl[i][j] = GET_KEX_LD_HASH_CTRL(i, j);
> > +
> > return 0;
> > }
> >
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
> > b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
> > index 3efeb09c58de..a1c3d987b804 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.h
> > @@ -31,6 +31,12 @@
> > rvu_write64(rvu, blkaddr, \
> > NPC_AF_INTFX_HASHX_MASKX(intf, ld, mask_idx), cfg)
> >
> > +#define GET_KEX_LD_HASH_CTRL(intf, ld) \
> > + rvu_read64(rvu, blkaddr, NPC_AF_INTFX_HASHX_RESULT_CTRL(intf,
> ld))
> > +
> > +#define GET_KEX_LD_HASH_MASK(intf, ld, mask_idx) \
> > + rvu_read64(rvu, blkaddr, NPC_AF_INTFX_HASHX_MASKX(intf, ld,
> > +mask_idx))
> > +
> > #define SET_KEX_LD_HASH_CTRL(intf, ld, cfg) \
> > rvu_write64(rvu, blkaddr, \
> > NPC_AF_INTFX_HASHX_RESULT_CTRL(intf, ld), cfg) @@ -
> 56,8 +62,8
> > @@ void npc_update_field_hash(struct rvu *rvu, u8 intf,
> > struct flow_msg *omask);
> > void npc_config_secret_key(struct rvu *rvu, int blkaddr); void
> > npc_program_mkex_hash(struct rvu *rvu, int blkaddr);
> > -u32 npc_field_hash_calc(u64 *ldata, struct npc_mcam_kex_hash
> *mkex_hash,
> > - u64 *secret_key, u8 intf, u8 hash_idx);
> > +u32 npc_field_hash_calc(u64 *ldata, struct npc_get_field_hash_info_rsp
> rsp,
> > + u8 intf, u8 hash_idx);
> >
> > static struct npc_mcam_kex_hash npc_mkex_hash_default
> __maybe_unused = {
> > .lid_lt_ld_hash_en = {
> > --
> > 2.25.1
> >

2023-03-31 09:02:04

by Geetha sowjanya

[permalink] [raw]
Subject: RE: [EXT] Re: [net PATCH 1/7] octeontx2-af: Secure APR table update with the lock



>-----Original Message-----
>From: Leon Romanovsky <[email protected]>
>Sent: Thursday, March 30, 2023 12:46 PM
>To: Geethasowjanya Akula <[email protected]>
>Cc: Sai Krishna Gajula <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Sunil >Kovvuri Goutham <[email protected]>; [email protected]
>Subject: Re: [EXT] Re: [net PATCH 1/7] octeontx2-af: Secure APR table update with the lock
>
>On Thu, Mar 30, 2023 at 06:56:54AM +0000, Geethasowjanya Akula wrote:
>>
>> >-----Original Message-----
>> >From: Leon Romanovsky <[email protected]>
>> >Sent: Thursday, March 30, 2023 11:26 AM
>> >To: Sai Krishna Gajula <[email protected]>
>> >Cc: [email protected]; [email protected]; [email protected];
>> >[email protected]; [email protected];
>> >[email protected]; Sunil Kovvuri Goutham
>> ><[email protected]>; >[email protected]; Geethasowjanya
>> >Akula <[email protected]>
>> >Subject: [EXT] Re: [net PATCH 1/7] octeontx2-af: Secure APR table
>> >update with the lock
>>
>> >External Email
>>
>> >---------------------------------------------------------------------
>> >- On Wed, Mar 29, 2023 at 10:36:13PM +0530, Sai Krishna wrote:
>> >> From: Geetha sowjanya <[email protected]>
>> >>
>> >> APR table contains the lmtst base address of PF/VFs.
>> >> These entries are updated by the PF/VF during the device probe. Due
>> >> to race condition while updating the entries are getting corrupted.
>> >> Hence secure the APR table update with the lock.
>>
>> >However, I don't see rsrc_lock in probe path.
>> >otx2_probe()
>> >-> cn10k_lmtst_init()
>> > -> lmt_base/lmstst is updated with and without mbox.lock.
>>
>> >Where did you take rsrc_lock in probe flow?
>>
>> rsrc_lock is initialized in AF driver. PF/VF driver in cn10k_lmtst_init() send a mbox request to AF to update the lmtst table.
>> mbox handler in AF takes rsrc_lock to update lmtst table.

>Can you please present the stack trace of such flow? What are the actual variables/struct rsrc_lock is protecting?

The lock tries to protect the request and response register at line#73 and line#83 in below function, from getting overwritten when
Multiple PFs invokes rvu_get_lmtaddr() simultaneously.
For example, if PF1 submit the request at line#73 and got permitted before it reads the response at line#80.
PF2 got scheduled submit the request then the response of PF1 is overwritten by the PF2 response.
When PF1 gets reschedule, it reads wrong data.

#static int rvu_get_lmtaddr(struct rvu *rvu, u16 pcifunc,
59 u64 iova, u64 *lmt_addr)
60 {
61 [...]
68
69 rvu_write64(rvu, BLKADDR_RVUM, RVU_AF_SMMU_ADDR_REQ, iova);
70 pf = rvu_get_pf(pcifunc) & 0x1F;
71 val = BIT_ULL(63) | BIT_ULL(14) | BIT_ULL(13) | pf << 8 |
72 ((pcifunc & RVU_PFVF_FUNC_MASK) & 0xFF);
73 rvu_write64(rvu, BLKADDR_RVUM, RVU_AF_SMMU_TXN_REQ, val);
74
75 err = rvu_poll_reg(rvu, BLKADDR_RVUM, RVU_AF_SMMU_ADDR_RSP_STS, BIT_ULL(0), false);
76 if (err) {
77 dev_err(rvu->dev, "%s LMTLINE iova transulation failed\n", __func__);
78 return err;
79 }
80 val = rvu_read64(rvu, BLKADDR_RVUM, RVU_AF_SMMU_ADDR_RSP_STS);
81 if (val & ~0x1ULL) {
82 dev_err(rvu->dev, "%s LMTLINE iova transulation failed err:%llx\n", __func__, val);
83 return -EIO;
84 }
85

Thanks.
>>Thanks

>>
>> Thanks,
>> Geetha.
>>
>> >Thanks
>>
>> >>
>> >> Fixes: 893ae97214c3 ("octeontx2-af: cn10k: Support configurable
>> >> LMTST
>> >> regions")
>> >> Signed-off-by: Geetha sowjanya <[email protected]>
>> >> Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
>> >> Signed-off-by: Sai Krishna <[email protected]>
>> >> ---
>> >> drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c | 8 +++++---
>> >> 1 file changed, 5 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
>> >> b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
>> >> index 4ad9ff025c96..8530250f6fba 100644
>> >> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
>> >> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
>> >> @@ -142,16 +142,17 @@ int rvu_mbox_handler_lmtst_tbl_setup(struct rvu *rvu,
>> >> * region, if so, convert that IOVA to physical address and
>> >> * populate LMT table with that address
>> >> */
>> >> + mutex_lock(&rvu->rsrc_lock);
>> >> if (req->use_local_lmt_region) {
>> >> err = rvu_get_lmtaddr(rvu, req->hdr.pcifunc,
>> >> req->lmt_iova, &lmt_addr);
>> >> if (err < 0)
>> >> - return err;
>> >> + goto error;
>> >>
>> >> /* Update the lmt addr for this PFFUNC in the LMT table */
>> >> err = rvu_update_lmtaddr(rvu, req->hdr.pcifunc, lmt_addr);
>> >> if (err)
>> >> - return err;
>> >> + goto error;
>> >> }
>> >>
>> >> /* Reconfiguring lmtst map table in lmt region shared mode i.e.
>> >> make @@ -181,7 +182,7 @@ int rvu_mbox_handler_lmtst_tbl_setup(struct rvu *rvu,
>> >> */
>> >> err = rvu_update_lmtaddr(rvu, req->hdr.pcifunc, val);
>> >> if (err)
>> >> - return err;
>> >> + goto error;
>> >> }
>> >>
>> >> /* This mailbox can also be used to update word1 of
>> >> APR_LMT_MAP_ENTRY_S @@ -230,6 +231,7 @@ int rvu_mbox_handler_lmtst_tbl_setup(struct rvu *rvu,
>> >> }
>> >>
>> >> error:
>> >> + mutex_unlock(&rvu->rsrc_lock);
>> >> return err;
>> >> }
>> >>
>> >> --
>> >> 2.25.1
>> >>

2023-04-03 13:08:56

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [EXT] Re: [net PATCH 1/7] octeontx2-af: Secure APR table update with the lock

On Fri, Mar 31, 2023 at 08:56:59AM +0000, Geethasowjanya Akula wrote:
>
>
> >-----Original Message-----
> >From: Leon Romanovsky <[email protected]>
> >Sent: Thursday, March 30, 2023 12:46 PM
> >To: Geethasowjanya Akula <[email protected]>
> >Cc: Sai Krishna Gajula <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Sunil >Kovvuri Goutham <[email protected]>; [email protected]
> >Subject: Re: [EXT] Re: [net PATCH 1/7] octeontx2-af: Secure APR table update with the lock
> >
> >On Thu, Mar 30, 2023 at 06:56:54AM +0000, Geethasowjanya Akula wrote:
> >>
> >> >-----Original Message-----
> >> >From: Leon Romanovsky <[email protected]>
> >> >Sent: Thursday, March 30, 2023 11:26 AM
> >> >To: Sai Krishna Gajula <[email protected]>
> >> >Cc: [email protected]; [email protected]; [email protected];
> >> >[email protected]; [email protected];
> >> >[email protected]; Sunil Kovvuri Goutham
> >> ><[email protected]>; >[email protected]; Geethasowjanya
> >> >Akula <[email protected]>
> >> >Subject: [EXT] Re: [net PATCH 1/7] octeontx2-af: Secure APR table
> >> >update with the lock
> >>
> >> >External Email
> >>
> >> >---------------------------------------------------------------------
> >> >- On Wed, Mar 29, 2023 at 10:36:13PM +0530, Sai Krishna wrote:
> >> >> From: Geetha sowjanya <[email protected]>
> >> >>
> >> >> APR table contains the lmtst base address of PF/VFs.
> >> >> These entries are updated by the PF/VF during the device probe. Due
> >> >> to race condition while updating the entries are getting corrupted.
> >> >> Hence secure the APR table update with the lock.
> >>
> >> >However, I don't see rsrc_lock in probe path.
> >> >otx2_probe()
> >> >-> cn10k_lmtst_init()
> >> > -> lmt_base/lmstst is updated with and without mbox.lock.
> >>
> >> >Where did you take rsrc_lock in probe flow?
> >>
> >> rsrc_lock is initialized in AF driver. PF/VF driver in cn10k_lmtst_init() send a mbox request to AF to update the lmtst table.
> >> mbox handler in AF takes rsrc_lock to update lmtst table.
>
> >Can you please present the stack trace of such flow? What are the actual variables/struct rsrc_lock is protecting?
>
> The lock tries to protect the request and response register at line#73 and line#83 in below function, from getting overwritten when
> Multiple PFs invokes rvu_get_lmtaddr() simultaneously.
> For example, if PF1 submit the request at line#73 and got permitted before it reads the response at line#80.
> PF2 got scheduled submit the request then the response of PF1 is overwritten by the PF2 response.
> When PF1 gets reschedule, it reads wrong data.

I see, thanks

2023-04-05 06:36:59

by Sai Krishna Gajula

[permalink] [raw]
Subject: Re: [net PATCH 0/7] octeontx2: Miscellaneous fixes

Will check and submit a patch, if needed

> -----Original Message-----
> From: Jakub Kicinski <[email protected]>
> Sent: Thursday, March 30, 2023 11:18 PM
> To: Sai Krishna Gajula <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Sunil Kovvuri
> Goutham <[email protected]>; [email protected]
> Subject: Re: [net PATCH 0/7] octeontx2: Miscellaneous fixes
>
> On Wed, 29 Mar 2023 22:36:12 +0530 Sai Krishna wrote:
> > From: Sai Krishna <[email protected]>
> > To: <[email protected]>, <[email protected]>,
> <[email protected]>, <[email protected]>,
> <[email protected]>, <[email protected]>,
> <[email protected]>, <[email protected]>
> > CC: Sai Krishna <[email protected]>
>
> First of all, does the maintainers file need to be updated?
>
> This driver has a crazy number of maintainers:
>
> MARVELL OCTEONTX2 RVU ADMIN FUNCTION DRIVER
> M: Sunil Goutham <[email protected]>
> M: Linu Cherian <[email protected]>
> M: Geetha sowjanya <[email protected]>
> M: Jerin Jacob <[email protected]>
> M: hariprasad <[email protected]>
> M: Subbaraya Sundeep <[email protected]>
> L: [email protected]
> S: Supported
> F:
> Documentation/networking/device_drivers/ethernet/marvell/octeo
> ntx2.rst
> F: drivers/net/ethernet/marvell/octeontx2/af/
>
> And yet the person posting patches for the company is not on that list?!
> Please clean this up, or CC authors of patches on the fixes.