From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id A2B85A04B2; Mon, 4 May 2020 19:25:46 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 3928E1D172; Mon, 4 May 2020 19:25:46 +0200 (CEST) Received: from mail-wr1-f44.google.com (mail-wr1-f44.google.com [209.85.221.44]) by dpdk.org (Postfix) with ESMTP id 7ED0F1D157 for ; Mon, 4 May 2020 19:25:44 +0200 (CEST) Received: by mail-wr1-f44.google.com with SMTP id d15so62512wrx.3 for ; Mon, 04 May 2020 10:25:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=from:to:cc:subject:date:message-id; bh=6U10C9ENtP7h1s7tkWz9pvFullXxLKEhV4NHr+Do8dk=; b=eKzoFCFYAWNpITPkxRTR9/vBPpDGSYI0Rhkq2Lgh1hFP+6Zbeuh0ukqquXBc1md1ho Wj/l+9qevZ2YNGkBBY/+0/iiAs29bEnCxrOAQ7ogtRH80VZg4zhOKeskc8vv1trdizG4 RWRjZ7mqSZUtum6xe9o9zIWXmzvU0v3iBHNuE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=6U10C9ENtP7h1s7tkWz9pvFullXxLKEhV4NHr+Do8dk=; b=tv2QEOcxJY7b31KnXSOoskRusF9FCgssaLk5pKsZP/xb4IWbsEH9y6vyHH8keX+LCP 21Ncw/6X1jkyZ+XGvxzcwhpRZjzBjQZhsQ4BMwGVqOKHRw87Y8Un5EPhJln2xtCGtCdZ +d4TzE/Fj9jfW18X4vKBo17OMH3hmiZvl6XI1hJ/7K1eYc6Bz3EfegUP+QVTFPIs2UCX 7+/a9KFhxC2YiBtQllDbYvV9p2s/7B99kAyiAEhFn8gUb2JPXvXqREzjQm4ucbnn2z1E xzQP7np5Umo/79QOO/l3UblX0ehE5uod/tvhc2LDiiHojyyionRrpVayE1nq4/34N8CC LCrA== X-Gm-Message-State: AGi0PuaJg6HtJzWA+iO57kIrGoeBgmepfO7plC0yHlCZ2X4+3gNV3lUr l0oAJTr8gsnTYjJl6Q/O+AsSySKwXDEJyH8B8fhb0Y6v6RbExJ59i60QAO2gqbXmfOukAk21vEr kISn3fuB51ctLeufm/pCIPpIghBvl7GHwhcuXZJXsGxRpI4eTcQcxFAUP1SL+DA== X-Google-Smtp-Source: APiQypIDxUVt80ylkzxzkjS5ifIuzKq/plBVfliHdtkck1BTDk4foaa9uqvlG3T60hctXp+mpOUc+Q== X-Received: by 2002:adf:e408:: with SMTP id g8mr106416wrm.363.1588613143685; Mon, 04 May 2020 10:25:43 -0700 (PDT) Received: from lab-rtp-b05-01.rtp.broadcom.com ([192.19.231.250]) by smtp.gmail.com with ESMTPSA id s24sm138900wmj.28.2020.05.04.10.25.42 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 04 May 2020 10:25:43 -0700 (PDT) From: Mike Baucom To: dev@dpdk.org Cc: Kishore Padmanabha Date: Mon, 4 May 2020 13:25:02 -0400 Message-Id: <1588613102-46399-1-git-send-email-michael.baucom@broadcom.com> X-Mailer: git-send-email 1.9.1 Subject: [dpdk-dev] [PATCH] net/bnxt: modify mark manager validity checks X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" From: Kishore Padmanabha 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 --- 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