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 12B35A00C5; Thu, 11 Jun 2020 10:39:45 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 4210614581; Thu, 11 Jun 2020 10:39:39 +0200 (CEST) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id 68D0C49E0 for ; Thu, 11 Jun 2020 10:39:35 +0200 (CEST) IronPort-SDR: 2g0Fy2Uf1f3+hpY5cDrm/1cBC/ocMYRna7+9hAShSa28noZEALemsfKOWjjt/ZzUo1tZ6mgrKn 3nvf+D5JmsDw== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Jun 2020 01:39:35 -0700 IronPort-SDR: TxSEnkOiehr/llAdJ8r6SKwRd7BLA6WNlhYcz+GY906iZI9/QUHzNBrr/UNBMe4f+XgvoHZ4vG h84zyKORNPeQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,499,1583222400"; d="scan'208";a="419039122" Received: from dpdk51.sh.intel.com ([10.67.111.82]) by orsmga004.jf.intel.com with ESMTP; 11 Jun 2020 01:39:33 -0700 From: Qi Zhang To: qiming.yang@intel.com Cc: dev@dpdk.org, xiaolong.ye@intel.com, Qi Zhang , Victor Raj , Paul M Stillwell Jr Date: Thu, 11 Jun 2020 16:43:21 +0800 Message-Id: <20200611084330.18301-2-qi.z.zhang@intel.com> X-Mailer: git-send-email 2.13.6 In-Reply-To: <20200611084330.18301-1-qi.z.zhang@intel.com> References: <20200611084330.18301-1-qi.z.zhang@intel.com> Subject: [dpdk-dev] [PATCH 01/10] net/ice/base: adjust profile id map locks 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" The profile id map lock should be held till the caller completes all references of that profile entries. The current code releases the lock right after the match search. This caused a driver issue when the profile map entries were referenced after it was freed in other thread after the lock was released earlier. Also return type of get/set profile functions were changed to return the ice status instead of the profile entry pointer. This will prevent the caller referencing the profile fields outside the lock. Signed-off-by: Victor Raj Signed-off-by: Paul M Stillwell Jr Signed-off-by: Qi Zhang --- drivers/net/ice/base/ice_flex_pipe.c | 95 ++++++++++++++++++------------------ drivers/net/ice/base/ice_flow.c | 16 ++++-- 2 files changed, 58 insertions(+), 53 deletions(-) diff --git a/drivers/net/ice/base/ice_flex_pipe.c b/drivers/net/ice/base/ice_flex_pipe.c index f953d891d..016dc2b39 100644 --- a/drivers/net/ice/base/ice_flex_pipe.c +++ b/drivers/net/ice/base/ice_flex_pipe.c @@ -4710,22 +4710,21 @@ ice_add_prof(struct ice_hw *hw, enum ice_block blk, u64 id, u8 ptypes[], } /** - * ice_search_prof_id_low - Search for a profile tracking ID low level + * ice_search_prof_id - Search for a profile tracking ID * @hw: pointer to the HW struct * @blk: hardware block * @id: profile tracking ID * - * This will search for a profile tracking ID which was previously added. This - * version assumes that the caller has already acquired the prof map lock. + * This will search for a profile tracking ID which was previously added. + * The profile map lock should be held before calling this function. */ -static struct ice_prof_map * -ice_search_prof_id_low(struct ice_hw *hw, enum ice_block blk, u64 id) +struct ice_prof_map * +ice_search_prof_id(struct ice_hw *hw, enum ice_block blk, u64 id) { struct ice_prof_map *entry = NULL; struct ice_prof_map *map; - LIST_FOR_EACH_ENTRY(map, &hw->blk[blk].es.prof_map, ice_prof_map, - list) + LIST_FOR_EACH_ENTRY(map, &hw->blk[blk].es.prof_map, ice_prof_map, list) if (map->profile_cookie == id) { entry = map; break; @@ -4735,26 +4734,6 @@ ice_search_prof_id_low(struct ice_hw *hw, enum ice_block blk, u64 id) } /** - * ice_search_prof_id - Search for a profile tracking ID - * @hw: pointer to the HW struct - * @blk: hardware block - * @id: profile tracking ID - * - * This will search for a profile tracking ID which was previously added. - */ -struct ice_prof_map * -ice_search_prof_id(struct ice_hw *hw, enum ice_block blk, u64 id) -{ - struct ice_prof_map *entry; - - ice_acquire_lock(&hw->blk[blk].es.prof_map_lock); - entry = ice_search_prof_id_low(hw, blk, id); - ice_release_lock(&hw->blk[blk].es.prof_map_lock); - - return entry; -} - -/** * ice_vsig_prof_id_count - count profiles in a VSIG * @hw: pointer to the HW struct * @blk: hardware block @@ -4969,7 +4948,7 @@ enum ice_status ice_rem_prof(struct ice_hw *hw, enum ice_block blk, u64 id) ice_acquire_lock(&hw->blk[blk].es.prof_map_lock); - pmap = ice_search_prof_id_low(hw, blk, id); + pmap = ice_search_prof_id(hw, blk, id); if (!pmap) { status = ICE_ERR_DOES_NOT_EXIST; goto err_ice_rem_prof; @@ -5002,21 +4981,27 @@ static enum ice_status ice_get_prof(struct ice_hw *hw, enum ice_block blk, u64 hdl, struct LIST_HEAD_TYPE *chg) { + enum ice_status status = ICE_SUCCESS; struct ice_prof_map *map; struct ice_chs_chg *p; u16 i; + ice_acquire_lock(&hw->blk[blk].es.prof_map_lock); /* Get the details on the profile specified by the handle ID */ map = ice_search_prof_id(hw, blk, hdl); - if (!map) - return ICE_ERR_DOES_NOT_EXIST; + if (!map) { + status = ICE_ERR_DOES_NOT_EXIST; + goto err_ice_get_prof; + } for (i = 0; i < map->ptg_cnt; i++) if (!hw->blk[blk].es.written[map->prof_id]) { /* add ES to change list */ p = (struct ice_chs_chg *)ice_malloc(hw, sizeof(*p)); - if (!p) + if (!p) { + status = ICE_ERR_NO_MEMORY; goto err_ice_get_prof; + } p->type = ICE_PTG_ES_ADD; p->ptype = 0; @@ -5032,11 +5017,10 @@ ice_get_prof(struct ice_hw *hw, enum ice_block blk, u64 hdl, LIST_ADD(&p->list_entry, chg); } - return ICE_SUCCESS; - err_ice_get_prof: + ice_release_lock(&hw->blk[blk].es.prof_map_lock); /* let caller clean up the change list */ - return ICE_ERR_NO_MEMORY; + return status; } /** @@ -5090,17 +5074,23 @@ static enum ice_status ice_add_prof_to_lst(struct ice_hw *hw, enum ice_block blk, struct LIST_HEAD_TYPE *lst, u64 hdl) { + enum ice_status status = ICE_SUCCESS; struct ice_prof_map *map; struct ice_vsig_prof *p; u16 i; + ice_acquire_lock(&hw->blk[blk].es.prof_map_lock); map = ice_search_prof_id(hw, blk, hdl); - if (!map) - return ICE_ERR_DOES_NOT_EXIST; + if (!map) { + status = ICE_ERR_DOES_NOT_EXIST; + goto err_ice_add_prof_to_lst; + } p = (struct ice_vsig_prof *)ice_malloc(hw, sizeof(*p)); - if (!p) - return ICE_ERR_NO_MEMORY; + if (!p) { + status = ICE_ERR_NO_MEMORY; + goto err_ice_add_prof_to_lst; + } p->profile_cookie = map->profile_cookie; p->prof_id = map->prof_id; @@ -5115,7 +5105,9 @@ ice_add_prof_to_lst(struct ice_hw *hw, enum ice_block blk, LIST_ADD(&p->list, lst); - return ICE_SUCCESS; +err_ice_add_prof_to_lst: + ice_release_lock(&hw->blk[blk].es.prof_map_lock); + return status; } /** @@ -5399,16 +5391,12 @@ ice_add_prof_id_vsig(struct ice_hw *hw, enum ice_block blk, u16 vsig, u64 hdl, u8 vl_msk[ICE_TCAM_KEY_VAL_SZ] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF }; u8 dc_msk[ICE_TCAM_KEY_VAL_SZ] = { 0xFF, 0xFF, 0x00, 0x00, 0x00 }; u8 nm_msk[ICE_TCAM_KEY_VAL_SZ] = { 0x00, 0x00, 0x00, 0x00, 0x00 }; + enum ice_status status = ICE_SUCCESS; struct ice_prof_map *map; struct ice_vsig_prof *t; struct ice_chs_chg *p; u16 vsig_idx, i; - /* Get the details on the profile specified by the handle ID */ - map = ice_search_prof_id(hw, blk, hdl); - if (!map) - return ICE_ERR_DOES_NOT_EXIST; - /* Error, if this VSIG already has this profile */ if (ice_has_prof_vsig(hw, blk, vsig, hdl)) return ICE_ERR_ALREADY_EXISTS; @@ -5418,19 +5406,28 @@ ice_add_prof_id_vsig(struct ice_hw *hw, enum ice_block blk, u16 vsig, u64 hdl, if (!t) return ICE_ERR_NO_MEMORY; + ice_acquire_lock(&hw->blk[blk].es.prof_map_lock); + /* Get the details on the profile specified by the handle ID */ + map = ice_search_prof_id(hw, blk, hdl); + if (!map) { + status = ICE_ERR_DOES_NOT_EXIST; + goto err_ice_add_prof_id_vsig; + } + t->profile_cookie = map->profile_cookie; t->prof_id = map->prof_id; t->tcam_count = map->ptg_cnt; /* create TCAM entries */ for (i = 0; i < map->ptg_cnt; i++) { - enum ice_status status; u16 tcam_idx; /* add TCAM to change list */ p = (struct ice_chs_chg *)ice_malloc(hw, sizeof(*p)); - if (!p) + if (!p) { + status = ICE_ERR_NO_MEMORY; goto err_ice_add_prof_id_vsig; + } /* allocate the TCAM entry index */ /* for entries with empty attribute masks, allocate entry from @@ -5484,12 +5481,14 @@ ice_add_prof_id_vsig(struct ice_hw *hw, enum ice_block blk, u16 vsig, u64 hdl, LIST_ADD(&t->list, &hw->blk[blk].xlt2.vsig_tbl[vsig_idx].prop_lst); - return ICE_SUCCESS; + ice_release_lock(&hw->blk[blk].es.prof_map_lock); + return status; err_ice_add_prof_id_vsig: + ice_release_lock(&hw->blk[blk].es.prof_map_lock); /* let caller clean up the change list */ ice_free(hw, t); - return ICE_ERR_NO_MEMORY; + return status; } /** diff --git a/drivers/net/ice/base/ice_flow.c b/drivers/net/ice/base/ice_flow.c index 1e944bf52..fb0e34e5f 100644 --- a/drivers/net/ice/base/ice_flow.c +++ b/drivers/net/ice/base/ice_flow.c @@ -2215,15 +2215,17 @@ enum ice_status ice_flow_get_hw_prof(struct ice_hw *hw, enum ice_block blk, u64 prof_id, u8 *hw_prof_id) { + enum ice_status status = ICE_ERR_DOES_NOT_EXIST; struct ice_prof_map *map; + ice_acquire_lock(&hw->blk[blk].es.prof_map_lock); map = ice_search_prof_id(hw, blk, prof_id); if (map) { *hw_prof_id = map->prof_id; - return ICE_SUCCESS; + status = ICE_SUCCESS; } - - return ICE_ERR_DOES_NOT_EXIST; + ice_release_lock(&hw->blk[blk].es.prof_map_lock); + return status; } /** @@ -3456,9 +3458,13 @@ ice_rss_update_symm(struct ice_hw *hw, struct ice_prof_map *map; u8 prof_id, m; + ice_acquire_lock(&hw->blk[ICE_BLK_RSS].es.prof_map_lock); map = ice_search_prof_id(hw, ICE_BLK_RSS, prof->id); - prof_id = map->prof_id; - + if (map) + prof_id = map->prof_id; + ice_release_lock(&hw->blk[ICE_BLK_RSS].es.prof_map_lock); + if (!map) + return; /* clear to default */ for (m = 0; m < 6; m++) wr32(hw, GLQF_HSYMM(prof_id, m), 0); -- 2.13.6