From 98487d729b4aecb1c7cd73641d894c3aa23fff29 Mon Sep 17 00:00:00 2001 From: Kishore Padmanabha Date: Mon, 6 Jul 2020 13:54:50 +0530 Subject: [PATCH] net/bnxt: cleanup and refactor session management The return value of some functions is explicitly ignored in cases where scope id may not be valid for internal EM entries. Additional minor refactoring and cleanups - Change log level for some log messages to DEBUG instead of ERR. - Check data size conformity and log appropriate message. Signed-off-by: Michael Wildt Signed-off-by: Kishore Padmanabha Signed-off-by: Somnath Kotur Reviewed-by: Randy Schacher Reviewed-by: Ajit Khaparde --- 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 3b8679ce9f..b5db94f3ef 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 77c965937f..1e14d9248b 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 6ab8088f0f..c95c4bdbd3 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 fc29ff1591..469ad36152 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 733766ce1e..30180d8ab8 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.20.1