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 50687A00C5; Mon, 6 Jul 2020 10:31:59 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id AF71A1DAAB; Mon, 6 Jul 2020 10:30:19 +0200 (CEST) Received: from relay.smtp.broadcom.com (relay.smtp.broadcom.com [192.19.211.62]) by dpdk.org (Postfix) with ESMTP id 7C0BF1D944 for ; Mon, 6 Jul 2020 10:30:05 +0200 (CEST) Received: from dhcp-10-123-153-55.dhcp.broadcom.net (dhcp-10-123-153-55.dhcp.broadcom.net [10.123.153.55]) by relay.smtp.broadcom.com (Postfix) with ESMTP id A980429856E; Mon, 6 Jul 2020 01:30:04 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.10.3 relay.smtp.broadcom.com A980429856E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=broadcom.com; s=dkimrelay; t=1594024204; bh=zXI1tW+b4zqwscWDQ/SfDbNFSUPN0mY+/bCotgduDDg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=KywkMNKsf4OIUynSZp5HQNS3Nsvat8+wPaU7jEDPAWy4XsMtv2GblfqSYAHI5Gked /0YzGsYz6Fn/H5G5GcEidcyO8kflFALjVd4M+VF20KuV6ydQ7hRS84ArA+Hh8sxxDE KSGsd9rHC1rBuBYXAHcVQ5uPyX+HkHkg6CoQLrgU= From: Somnath Kotur To: dev@dpdk.org Cc: ferruh.yigit@intel.com Date: Mon, 6 Jul 2020 13:54:50 +0530 Message-Id: <20200706082502.26935-9-somnath.kotur@broadcom.com> X-Mailer: git-send-email 2.10.1.613.g2cc2e70 In-Reply-To: <20200706082502.26935-1-somnath.kotur@broadcom.com> References: <20200706082502.26935-1-somnath.kotur@broadcom.com> Subject: [dpdk-dev] [PATCH 08/20] net/bnxt: cleanup and refactoring 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 return value of the function is explicitly ignored in this case, since scope id may not be valid for internal EM entries. Additional minor refactoring and cleanups. Signed-off-by: Michael Wildt Signed-off-by: Kishore Padmanabha Reviewed-by: Randy Schacher Reviewed-by: Ajit Kumar Khaparde Signed-off-by: Somnath Kotur --- drivers/net/bnxt/tf_core/tf_em_host.c | 2 +- drivers/net/bnxt/tf_core/tf_msg.c | 75 +++++++++++++++++++++++++++++++++-- drivers/net/bnxt/tf_core/tf_session.c | 8 ++++ drivers/net/bnxt/tf_ulp/bnxt_ulp.c | 2 +- drivers/net/bnxt/tf_ulp/ulp_mapper.c | 4 +- 5 files changed, 84 insertions(+), 7 deletions(-) diff --git a/drivers/net/bnxt/tf_core/tf_em_host.c b/drivers/net/bnxt/tf_core/tf_em_host.c index 8cc92c4..24287c0 100644 --- a/drivers/net/bnxt/tf_core/tf_em_host.c +++ b/drivers/net/bnxt/tf_core/tf_em_host.c @@ -333,7 +333,7 @@ tf_em_ctx_reg(struct tf *tfp, { struct hcapi_cfa_em_ctx_mem_info *ctxp = &tbl_scope_cb->em_ctx_info[dir]; struct hcapi_cfa_em_table *tbl; - int rc; + int rc = 0; int i; for (i = TF_KEY0_TABLE; i < TF_MAX_TABLE; i++) { diff --git a/drivers/net/bnxt/tf_core/tf_msg.c b/drivers/net/bnxt/tf_core/tf_msg.c index 77c9659..1e14d92 100644 --- a/drivers/net/bnxt/tf_core/tf_msg.c +++ b/drivers/net/bnxt/tf_core/tf_msg.c @@ -3,6 +3,7 @@ * All rights reserved. */ +#include #include #include #include @@ -21,6 +22,40 @@ /* Logging defines */ #define TF_RM_MSG_DEBUG 0 +/* Specific msg size defines as we cannot use defines in tf.yaml. This + * means we have to manually sync hwrm with these defines if the + * tf.yaml changes. + */ +#define TF_MSG_SET_GLOBAL_CFG_DATA_SIZE 16 +#define TF_MSG_EM_INSERT_KEY_SIZE 8 +#define TF_MSG_TCAM_SET_DEV_DATA_SIZE 88 +#define TF_MSG_TBL_TYPE_SET_DATA_SIZE 88 + +/* Compile check - Catch any msg changes that we depend on, like the + * defines listed above for array size checking. + * + * Checking array size is dangerous in that the type could change and + * we wouldn't be able to catch it. Thus we check if the complete msg + * changed instead. Best we can do. + * + * If failure is observed then both msg size (defines below) and the + * array size (define above) should be checked and compared. + */ +#define TF_MSG_SIZE_HWRM_TF_GLOBAL_CFG_SET 56 +static_assert(sizeof(struct hwrm_tf_global_cfg_set_input) == + TF_MSG_SIZE_HWRM_TF_GLOBAL_CFG_SET, + "HWRM message size changed: hwrm_tf_global_cfg_set_input"); + +#define TF_MSG_SIZE_HWRM_TF_EM_INSERT 104 +static_assert(sizeof(struct hwrm_tf_em_insert_input) == + TF_MSG_SIZE_HWRM_TF_EM_INSERT, + "HWRM message size changed: hwrm_tf_em_insert_input"); + +#define TF_MSG_SIZE_HWRM_TF_TBL_TYPE_SET 128 +static_assert(sizeof(struct hwrm_tf_tbl_type_set_input) == + TF_MSG_SIZE_HWRM_TF_TBL_TYPE_SET, + "HWRM message size changed: hwrm_tf_tbl_type_set_input"); + /** * This is the MAX data we can transport across regular HWRM */ @@ -321,7 +356,7 @@ tf_msg_session_resc_qcaps(struct tf *tfp, TFP_DRV_LOG(ERR, "%s: QCAPS message size error, rc:%s\n", tf_dir_2_str(dir), - strerror(-EINVAL)); + strerror(EINVAL)); rc = -EINVAL; goto cleanup; } @@ -436,7 +471,7 @@ tf_msg_session_resc_alloc(struct tf *tfp, TFP_DRV_LOG(ERR, "%s: Alloc message size error, rc:%s\n", tf_dir_2_str(dir), - strerror(-EINVAL)); + strerror(EINVAL)); rc = -EINVAL; goto cleanup; } @@ -546,6 +581,7 @@ tf_msg_insert_em_internal_entry(struct tf *tfp, (struct tf_em_64b_entry *)em_parms->em_record; uint16_t flags; uint8_t fw_session_id; + uint8_t msg_key_size; rc = tf_session_get_fw_session_id(tfp, &fw_session_id); if (rc) { @@ -558,9 +594,21 @@ tf_msg_insert_em_internal_entry(struct tf *tfp, /* Populate the request */ req.fw_session_id = tfp_cpu_to_le_32(fw_session_id); + + /* Check for key size conformity */ + msg_key_size = (em_parms->key_sz_in_bits + 7) / 8; + if (msg_key_size > TF_MSG_EM_INSERT_KEY_SIZE) { + rc = -EINVAL; + TFP_DRV_LOG(ERR, + "%s: Invalid parameters for msg type, rc:%s\n", + tf_dir_2_str(em_parms->dir), + strerror(-rc)); + return rc; + } + tfp_memcpy(req.em_key, em_parms->key, - ((em_parms->key_sz_in_bits + 7) / 8)); + msg_key_size); flags = (em_parms->dir == TF_DIR_TX ? HWRM_TF_EM_INSERT_INPUT_FLAGS_DIR_TX : @@ -942,6 +990,16 @@ tf_msg_set_tbl_entry(struct tf *tfp, req.size = tfp_cpu_to_le_16(size); req.index = tfp_cpu_to_le_32(index); + /* Check for data size conformity */ + if (size > TF_MSG_TBL_TYPE_SET_DATA_SIZE) { + rc = -EINVAL; + TFP_DRV_LOG(ERR, + "%s: Invalid parameters for msg type, rc:%s\n", + tf_dir_2_str(dir), + strerror(-rc)); + return rc; + } + tfp_memcpy(&req.data, data, size); @@ -1102,6 +1160,17 @@ tf_msg_set_global_cfg(struct tf *tfp, req.flags = tfp_cpu_to_le_32(flags); req.type = tfp_cpu_to_le_32(params->type); req.offset = tfp_cpu_to_le_32(params->offset); + + /* Check for data size conformity */ + if (params->config_sz_in_bytes > TF_MSG_SET_GLOBAL_CFG_DATA_SIZE) { + rc = -EINVAL; + TFP_DRV_LOG(ERR, + "%s: Invalid parameters for msg type, rc:%s\n", + tf_dir_2_str(params->dir), + strerror(-rc)); + return rc; + } + tfp_memcpy(req.data, params->config, params->config_sz_in_bytes); req.size = tfp_cpu_to_le_32(params->config_sz_in_bytes); diff --git a/drivers/net/bnxt/tf_core/tf_session.c b/drivers/net/bnxt/tf_core/tf_session.c index 932a14a..ca46f9b 100644 --- a/drivers/net/bnxt/tf_core/tf_session.c +++ b/drivers/net/bnxt/tf_core/tf_session.c @@ -472,6 +472,14 @@ tf_session_close_session(struct tf *tfp, client = tf_session_find_session_client_by_fid(tfs, fid); + if (!client) { + rc = -EINVAL; + TFP_DRV_LOG(ERR, + "Client not part of the session, unable to close, rc:%s\n", + strerror(-rc)); + return rc; + } + /* In case multiple clients we chose to close those first */ if (tfs->ref_count > 1) { /* Linaro gcc can't static init this structure */ diff --git a/drivers/net/bnxt/tf_ulp/bnxt_ulp.c b/drivers/net/bnxt/tf_ulp/bnxt_ulp.c index fc29ff1..469ad36 100644 --- a/drivers/net/bnxt/tf_ulp/bnxt_ulp.c +++ b/drivers/net/bnxt/tf_ulp/bnxt_ulp.c @@ -594,7 +594,7 @@ bnxt_ulp_init(struct bnxt *bp) int rc; if (bp->ulp_ctx) { - BNXT_TF_DBG(ERR, "ulp ctx already allocated\n"); + BNXT_TF_DBG(DEBUG, "ulp ctx already allocated\n"); return -EINVAL; } diff --git a/drivers/net/bnxt/tf_ulp/ulp_mapper.c b/drivers/net/bnxt/tf_ulp/ulp_mapper.c index b0d31a8..dd99ea3 100644 --- a/drivers/net/bnxt/tf_ulp/ulp_mapper.c +++ b/drivers/net/bnxt/tf_ulp/ulp_mapper.c @@ -537,10 +537,10 @@ ulp_mapper_index_entry_free(struct bnxt_ulp_context *ulp, }; /* - * Just set the table scope, it will be ignored if not necessary + * Just get the table scope, it will be ignored if not necessary * by the tf_free_tbl_entry */ - bnxt_ulp_cntxt_tbl_scope_id_get(ulp, &fparms.tbl_scope_id); + (void)bnxt_ulp_cntxt_tbl_scope_id_get(ulp, &fparms.tbl_scope_id); return tf_free_tbl_entry(tfp, &fparms); } -- 2.7.4