From: Kishore Padmanabha Date: Mon, 4 May 2020 17:25:02 +0000 (-0400) Subject: net/bnxt: modify mark manager validity checks X-Git-Url: http://git.droids-corp.org/?a=commitdiff_plain;h=8a32d5c49207b6be78da9e965c9ab6472ae237ce;p=dpdk.git net/bnxt: modify mark manager validity checks 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 Reviewed-by: Mike Baucom Reviewed-by: Ajit Khaparde --- diff --git a/drivers/net/bnxt/tf_ulp/ulp_mapper.c b/drivers/net/bnxt/tf_ulp/ulp_mapper.c index 9ea6fdba00..938b88e229 100644 --- a/drivers/net/bnxt/tf_ulp/ulp_mapper.c +++ b/drivers/net/bnxt/tf_ulp/ulp_mapper.c @@ -459,18 +459,9 @@ static inline int32_t 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 @@ ulp_mapper_em_tbl_process(struct bnxt_ulp_mapper_parms *parms, 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 @@ ulp_mapper_em_tbl_process(struct bnxt_ulp_mapper_parms *parms, 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 ad8353159b..9e8b81e4c2 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 @@ ulp_mark_db_idx_get(bool is_gfid, uint32_t fid, struct bnxt_ulp_mark_tbl *mtbl) /* 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 @@ ulp_mark_db_init(struct bnxt_ulp_context *ctxt) 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 @@ ulp_mark_db_mark_get(struct bnxt_ulp_context *ctxt, 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 @@ ulp_mark_db_mark_get(struct bnxt_ulp_context *ctxt, *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 @@ ulp_mark_db_mark_get(struct bnxt_ulp_context *ctxt, * * 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 @@ ulp_mark_db_mark_get(struct bnxt_ulp_context *ctxt, */ 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 @@ ulp_mark_db_mark_add(struct bnxt_ulp_context *ctxt, * * 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 0f8a5e5f4c..fd0d84011b 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 @@ ulp_mark_db_mark_get(struct bnxt_ulp_context *ctxt, * * 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 @@ ulp_mark_db_mark_get(struct bnxt_ulp_context *ctxt, */ 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 @@ ulp_mark_db_mark_add(struct bnxt_ulp_context *ctxt, * * 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_ */