From: Kishore Padmanabha <kishore.padmana...@broadcom.com> 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 <michael.wi...@broadcom.com> Signed-off-by: Kishore Padmanabha <kishore.padmana...@broadcom.com> Reviewed-by: Randy Schacher <stuart.schac...@broadcom.com> Reviewed-by: Ajit Kumar Khaparde <ajit.khapa...@broadcom.com> Signed-off-by: Somnath Kotur <somnath.ko...@broadcom.com> --- 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 <assert.h> #include <inttypes.h> #include <stdbool.h> #include <stdlib.h> @@ -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