[dpdk-dev] [PATCH] net/bnxt: modify mark manager validity checks
Mike Baucom
michael.baucom at broadcom.com
Mon May 4 19:25:02 CEST 2020
From: Kishore Padmanabha <kishore.padmanabha at broadcom.com>
The ulp mark manager originally assumed that zero was an invalid
mark and used it for invalidation and deletion. The mark manager
now supports adding zero as a mark, flags for validity and type,
and adds explicit bounds checking instead of relying on mask.
Signed-off-by: Kishore Padmanabha <kishore.padmanabha at broadcom.com>
Reviewed-by: Mike Baucom <michael.baucom at broadcom.com>
---
drivers/net/bnxt/tf_ulp/ulp_mapper.c | 22 ++---
drivers/net/bnxt/tf_ulp/ulp_mark_mgr.c | 176 ++++++++++++++++++++-------------
drivers/net/bnxt/tf_ulp/ulp_mark_mgr.h | 25 ++---
3 files changed, 127 insertions(+), 96 deletions(-)
diff --git a/drivers/net/bnxt/tf_ulp/ulp_mapper.c b/drivers/net/bnxt/tf_ulp/ulp_mapper.c
index 9ea6fdb..938b88e 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_mapper.c
+++ b/drivers/net/bnxt/tf_ulp/ulp_mapper.c
@@ -459,18 +459,9 @@
ulp_mapper_mark_free(struct bnxt_ulp_context *ulp,
struct ulp_flow_db_res_params *res)
{
- uint32_t flag;
- uint32_t fid;
- uint32_t gfid;
-
- fid = (uint32_t)res->resource_hndl;
- TF_GET_FLAG_FROM_FLOW_ID(fid, flag);
- TF_GET_GFID_FROM_FLOW_ID(fid, gfid);
-
return ulp_mark_db_mark_del(ulp,
- (flag == TF_GFID_TABLE_EXTERNAL),
- gfid,
- 0);
+ res->resource_type,
+ res->resource_hndl);
}
/*
@@ -1318,10 +1309,9 @@
mark = tfp_be_to_cpu_32(val);
TF_GET_GFID_FROM_FLOW_ID(iparms.flow_id, gfid);
- TF_GET_FLAG_FROM_FLOW_ID(iparms.flow_id, flag);
-
+ flag = BNXT_ULP_MARK_GLOBAL_HW_FID;
rc = ulp_mark_db_mark_add(parms->ulp_ctx,
- (flag == TF_GFID_TABLE_EXTERNAL),
+ flag,
gfid,
mark);
if (rc) {
@@ -1336,8 +1326,8 @@
memset(&fid_parms, 0, sizeof(fid_parms));
fid_parms.direction = tbl->direction;
fid_parms.resource_func = BNXT_ULP_RESOURCE_FUNC_HW_FID;
- fid_parms.resource_type = tbl->table_type;
- fid_parms.resource_hndl = iparms.flow_id;
+ fid_parms.resource_type = flag;
+ fid_parms.resource_hndl = gfid;
fid_parms.critical_resource = 0;
rc = ulp_flow_db_resource_add(parms->ulp_ctx,
diff --git a/drivers/net/bnxt/tf_ulp/ulp_mark_mgr.c b/drivers/net/bnxt/tf_ulp/ulp_mark_mgr.c
index ad83531..9e8b81e 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_mark_mgr.c
+++ b/drivers/net/bnxt/tf_ulp/ulp_mark_mgr.c
@@ -14,6 +14,13 @@
#include "ulp_template_db.h"
#include "ulp_template_struct.h"
+#define ULP_MARK_DB_ENTRY_SET_VALID(mark_info) ((mark_info)->flags |=\
+ BNXT_ULP_MARK_VALID)
+#define ULP_MARK_DB_ENTRY_IS_INVALID(mark_info) (!((mark_info)->flags &\
+ BNXT_ULP_MARK_VALID))
+#define ULP_MARK_DB_ENTRY_IS_GLOBAL_HW_FID(mark_info) ((mark_info)->flags &\
+ BNXT_ULP_MARK_GLOBAL_HW_FID)
+
static inline uint32_t
ulp_mark_db_idx_get(bool is_gfid, uint32_t fid, struct bnxt_ulp_mark_tbl *mtbl)
{
@@ -25,52 +32,14 @@
/* Need to truncate anything beyond supported flows */
idx &= mtbl->gfid_mask;
-
if (hashtype)
idx |= mtbl->gfid_type_bit;
} else {
idx = fid;
}
-
return idx;
}
-static int32_t
-ulp_mark_db_mark_set(struct bnxt_ulp_context *ctxt,
- bool is_gfid,
- uint32_t fid,
- uint32_t mark)
-{
- struct bnxt_ulp_mark_tbl *mtbl;
- uint32_t idx = 0;
-
- if (!ctxt) {
- BNXT_TF_DBG(ERR, "Invalid ulp context\n");
- return -EINVAL;
- }
-
- mtbl = bnxt_ulp_cntxt_ptr2_mark_db_get(ctxt);
- if (!mtbl) {
- BNXT_TF_DBG(ERR, "Unable to get Mark DB\n");
- return -EINVAL;
- }
-
- idx = ulp_mark_db_idx_get(is_gfid, fid, mtbl);
-
- if (is_gfid) {
- BNXT_TF_DBG(DEBUG, "Set GFID[0x%0x] = 0x%0x\n", idx, mark);
-
- mtbl->gfid_tbl[idx].mark_id = mark;
- mtbl->gfid_tbl[idx].valid = true;
- } else {
- /* For the LFID, the FID is used as the index */
- mtbl->lfid_tbl[fid].mark_id = mark;
- mtbl->lfid_tbl[fid].valid = true;
- }
-
- return 0;
-}
-
/*
* Allocate and Initialize all Mark Manager resources for this ulp context.
*
@@ -105,48 +74,48 @@
if (!mark_tbl)
goto mem_error;
- /* Need to allocate 2 * Num flows to account for hash type bit. */
+ /* Need to allocate 2 * Num flows to account for hash type bit.*/
+ mark_tbl->lfid_num_entries = dparms->lfid_entries;
mark_tbl->lfid_tbl = rte_zmalloc("ulp_rx_em_flow_mark_table",
- dparms->lfid_entries *
- sizeof(struct bnxt_lfid_mark_info),
+ mark_tbl->lfid_num_entries *
+ sizeof(struct bnxt_lfid_mark_info),
0);
-
if (!mark_tbl->lfid_tbl)
goto mem_error;
- /* Need to allocate 2 * Num flows to account for hash type bit. */
+ /* Need to allocate 2 * Num flows to account for hash type bit */
+ mark_tbl->gfid_num_entries = dparms->gfid_entries;
mark_tbl->gfid_tbl = rte_zmalloc("ulp_rx_eem_flow_mark_table",
- 2 * dparms->num_flows *
- sizeof(struct bnxt_gfid_mark_info),
+ mark_tbl->gfid_num_entries *
+ sizeof(struct bnxt_gfid_mark_info),
0);
if (!mark_tbl->gfid_tbl)
goto mem_error;
/*
- * TBD: This needs to be generalized for better mark handling
* These values are used to compress the FID to the allowable index
- * space. The FID from hw may be the full hash.
+ * space. The FID from hw may be the full hash which may be a big
+ * value to allocate and so allocate only needed hash values.
+ * gfid mask is the number of flow entries for the each left/right
+ * hash The gfid type bit is used to get to the higher or lower hash
+ * entries.
*/
- mark_tbl->gfid_max = dparms->gfid_entries - 1;
- mark_tbl->gfid_mask = (dparms->gfid_entries / 2) - 1;
- mark_tbl->gfid_type_bit = (dparms->gfid_entries / 2);
+ mark_tbl->gfid_mask = (mark_tbl->gfid_num_entries / 2) - 1;
+ mark_tbl->gfid_type_bit = (mark_tbl->gfid_num_entries / 2);
BNXT_TF_DBG(DEBUG, "GFID Max = 0x%08x\nGFID MASK = 0x%08x\n",
- mark_tbl->gfid_max,
+ mark_tbl->gfid_num_entries - 1,
mark_tbl->gfid_mask);
/* Add the mark tbl to the ulp context. */
bnxt_ulp_cntxt_ptr2_mark_db_set(ctxt, mark_tbl);
-
return 0;
mem_error:
rte_free(mark_tbl->gfid_tbl);
rte_free(mark_tbl->lfid_tbl);
rte_free(mark_tbl);
- BNXT_TF_DBG(DEBUG,
- "Failed to allocate memory for mark mgr\n");
-
+ BNXT_TF_DBG(DEBUG, "Failed to allocate memory for mark mgr\n");
return -ENOMEM;
}
@@ -208,7 +177,8 @@
idx = ulp_mark_db_idx_get(is_gfid, fid, mtbl);
if (is_gfid) {
- if (!mtbl->gfid_tbl[idx].valid)
+ if (idx >= mtbl->gfid_num_entries ||
+ ULP_MARK_DB_ENTRY_IS_INVALID(&mtbl->gfid_tbl[idx]))
return -EINVAL;
BNXT_TF_DBG(DEBUG, "Get GFID[0x%0x] = 0x%0x\n",
@@ -216,7 +186,8 @@
*mark = mtbl->gfid_tbl[idx].mark_id;
} else {
- if (!mtbl->gfid_tbl[idx].valid)
+ if (idx >= mtbl->lfid_num_entries ||
+ ULP_MARK_DB_ENTRY_IS_INVALID(&mtbl->lfid_tbl[idx]))
return -EINVAL;
BNXT_TF_DBG(DEBUG, "Get LFID[0x%0x] = 0x%0x\n",
@@ -233,7 +204,7 @@
*
* ctxt [in] The ulp context for the mark manager
*
- * is_gfid [in] The type of fid (GFID or LFID)
+ * mark_flag [in] mark flags.
*
* fid [in] The flow id that is returned by HW in BD
*
@@ -242,11 +213,47 @@
*/
int32_t
ulp_mark_db_mark_add(struct bnxt_ulp_context *ctxt,
- bool is_gfid,
- uint32_t gfid,
+ uint32_t mark_flag,
+ uint32_t fid,
uint32_t mark)
{
- return ulp_mark_db_mark_set(ctxt, is_gfid, gfid, mark);
+ struct bnxt_ulp_mark_tbl *mtbl;
+ uint32_t idx = 0;
+ bool is_gfid;
+
+ if (!ctxt) {
+ BNXT_TF_DBG(ERR, "Invalid ulp context\n");
+ return -EINVAL;
+ }
+
+ mtbl = bnxt_ulp_cntxt_ptr2_mark_db_get(ctxt);
+ if (!mtbl) {
+ BNXT_TF_DBG(ERR, "Unable to get Mark DB\n");
+ return -EINVAL;
+ }
+
+ is_gfid = (mark_flag & BNXT_ULP_MARK_GLOBAL_HW_FID);
+ if (is_gfid) {
+ idx = ulp_mark_db_idx_get(is_gfid, fid, mtbl);
+ if (idx >= mtbl->gfid_num_entries) {
+ BNXT_TF_DBG(ERR, "Mark index greater than allocated\n");
+ return -EINVAL;
+ }
+ BNXT_TF_DBG(DEBUG, "Set GFID[0x%0x] = 0x%0x\n", idx, mark);
+ mtbl->gfid_tbl[idx].mark_id = mark;
+ ULP_MARK_DB_ENTRY_SET_VALID(&mtbl->gfid_tbl[idx]);
+
+ } else {
+ /* For the LFID, the FID is used as the index */
+ if (fid >= mtbl->lfid_num_entries) {
+ BNXT_TF_DBG(ERR, "Mark index greater than allocated\n");
+ return -EINVAL;
+ }
+ mtbl->lfid_tbl[fid].mark_id = mark;
+ ULP_MARK_DB_ENTRY_SET_VALID(&mtbl->lfid_tbl[fid]);
+ }
+
+ return 0;
}
/*
@@ -254,18 +261,51 @@
*
* ctxt [in] The ulp context for the mark manager
*
- * is_gfid [in] The type of fid (GFID or LFID)
+ * mark_flag [in] mark flags.
*
* fid [in] The flow id that is returned by HW in BD
*
- * mark [in] The mark to be associated with the FID
- *
*/
int32_t
ulp_mark_db_mark_del(struct bnxt_ulp_context *ctxt,
- bool is_gfid,
- uint32_t gfid,
- uint32_t mark __rte_unused)
+ uint32_t mark_flag,
+ uint32_t fid)
{
- return ulp_mark_db_mark_set(ctxt, is_gfid, gfid, ULP_MARK_INVALID);
+ struct bnxt_ulp_mark_tbl *mtbl;
+ uint32_t idx = 0;
+ bool is_gfid;
+
+ if (!ctxt) {
+ BNXT_TF_DBG(ERR, "Invalid ulp context\n");
+ return -EINVAL;
+ }
+
+ mtbl = bnxt_ulp_cntxt_ptr2_mark_db_get(ctxt);
+ if (!mtbl) {
+ BNXT_TF_DBG(ERR, "Unable to get Mark DB\n");
+ return -EINVAL;
+ }
+
+ is_gfid = (mark_flag & BNXT_ULP_MARK_GLOBAL_HW_FID);
+ if (is_gfid) {
+ idx = ulp_mark_db_idx_get(is_gfid, fid, mtbl);
+ if (idx >= mtbl->gfid_num_entries) {
+ BNXT_TF_DBG(ERR, "Mark index greater than allocated\n");
+ return -EINVAL;
+ }
+ BNXT_TF_DBG(DEBUG, "Reset GFID[0x%0x]\n", idx);
+ memset(&mtbl->gfid_tbl[idx], 0,
+ sizeof(struct bnxt_gfid_mark_info));
+
+ } else {
+ /* For the LFID, the FID is used as the index */
+ if (fid >= mtbl->lfid_num_entries) {
+ BNXT_TF_DBG(ERR, "Mark index greater than allocated\n");
+ return -EINVAL;
+ }
+ memset(&mtbl->lfid_tbl[fid], 0,
+ sizeof(struct bnxt_lfid_mark_info));
+ }
+
+ return 0;
}
diff --git a/drivers/net/bnxt/tf_ulp/ulp_mark_mgr.h b/drivers/net/bnxt/tf_ulp/ulp_mark_mgr.h
index 0f8a5e5..fd0d840 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_mark_mgr.h
+++ b/drivers/net/bnxt/tf_ulp/ulp_mark_mgr.h
@@ -8,23 +8,27 @@
#include "bnxt_ulp.h"
-#define ULP_MARK_INVALID (0)
+#define BNXT_ULP_MARK_VALID 0x1
+#define BNXT_ULP_MARK_GLOBAL_HW_FID 0x4
+#define BNXT_ULP_MARK_LOCAL_HW_FID 0x8
+
struct bnxt_lfid_mark_info {
uint16_t mark_id;
- bool valid;
+ uint16_t flags;
};
struct bnxt_gfid_mark_info {
uint32_t mark_id;
- bool valid;
+ uint16_t flags;
};
struct bnxt_ulp_mark_tbl {
struct bnxt_lfid_mark_info *lfid_tbl;
struct bnxt_gfid_mark_info *gfid_tbl;
+ uint32_t lfid_num_entries;
+ uint32_t gfid_num_entries;
uint32_t gfid_mask;
uint32_t gfid_type_bit;
- uint32_t gfid_max;
};
/*
@@ -77,7 +81,7 @@ struct bnxt_ulp_mark_tbl {
*
* ctxt [in] The ulp context for the mark manager
*
- * is_gfid [in] The type of fid (GFID or LFID)
+ * mark_flag [in] mark flags.
*
* fid [in] The flow id that is returned by HW in BD
*
@@ -86,7 +90,7 @@ struct bnxt_ulp_mark_tbl {
*/
int32_t
ulp_mark_db_mark_add(struct bnxt_ulp_context *ctxt,
- bool is_gfid,
+ uint32_t mark_flag,
uint32_t gfid,
uint32_t mark);
@@ -95,17 +99,14 @@ struct bnxt_ulp_mark_tbl {
*
* ctxt [in] The ulp context for the mark manager
*
- * is_gfid [in] The type of fid (GFID or LFID)
+ * mark_flag [in] mark flags
*
* fid [in] The flow id that is returned by HW in BD
*
- * mark [in] The mark to be associated with the FID
- *
*/
int32_t
ulp_mark_db_mark_del(struct bnxt_ulp_context *ctxt,
- bool is_gfid,
- uint32_t gfid,
- uint32_t mark);
+ uint32_t mark_flag,
+ uint32_t gfid);
#endif /* _ULP_MARK_MGR_H_ */
--
1.9.1
More information about the dev
mailing list