> -----Original Message----- > From: Maxime Coquelin [mailto:maxime.coque...@redhat.com] > Sent: Wednesday, June 5, 2019 8:05 PM > To: Rong, Leyi <leyi.r...@intel.com>; Zhang, Qi Z <qi.z.zh...@intel.com> > Cc: dev@dpdk.org; Stillwell Jr, Paul M <paul.m.stillwell...@intel.com> > Subject: Re: [dpdk-dev] [PATCH 39/49] net/ice/base: slightly code update > > > > On 6/4/19 7:42 AM, Leyi Rong wrote: > > Mainly update below functions: > > > > ice_flow_proc_seg_hdrs > > ice_flow_find_prof_conds > > ice_dealloc_flow_entry > > ice_add_rule_internal > > > It seems that some of the changes are bug fixes. > So IMO, these changes should be in dedicated patches, with Fixes tag in the > commit message. > > Overall, these changes should be split by kind of changes. There are > functions reworks, minor cleanups, robustness > changes, ...
Will try to split this commit into multiple ones as your suggestion, thanks! > > > Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell...@intel.com> > > Signed-off-by: Leyi Rong <leyi.r...@intel.com> > > --- > > drivers/net/ice/base/ice_flex_pipe.c | 13 +++---- > > drivers/net/ice/base/ice_flow.c | 47 +++++++++++++++++------- > > drivers/net/ice/base/ice_nvm.c | 4 +- > > drivers/net/ice/base/ice_protocol_type.h | 1 + > > drivers/net/ice/base/ice_switch.c | 24 +++++++----- > > drivers/net/ice/base/ice_switch.h | 14 +++---- > > drivers/net/ice/base/ice_type.h | 13 ++++++- > > 7 files changed, 73 insertions(+), 43 deletions(-) > > > > diff --git a/drivers/net/ice/base/ice_flex_pipe.c > > b/drivers/net/ice/base/ice_flex_pipe.c > > index 5864cbf3e..2a310b6e1 100644 > > --- a/drivers/net/ice/base/ice_flex_pipe.c > > +++ b/drivers/net/ice/base/ice_flex_pipe.c > > @@ -134,7 +134,7 @@ static struct ice_buf_table *ice_find_buf_table(struct > > ice_seg *ice_seg) > > nvms = (struct ice_nvm_table *)(ice_seg->device_table + > > LE32_TO_CPU(ice_seg->device_table_count)); > > > > - return (struct ice_buf_table *) > > + return (_FORCE_ struct ice_buf_table *) > > (nvms->vers + LE32_TO_CPU(nvms->table_count)); > > } > > > > @@ -1005,9 +1005,8 @@ ice_dwnld_cfg_bufs(struct ice_hw *hw, struct > > ice_buf *bufs, u32 count) > > > > bh = (struct ice_buf_hdr *)(bufs + i); > > > > - status = ice_aq_download_pkg(hw, bh, LE16_TO_CPU(bh->data_end), > > - last, &offset, &info, NULL); > > - > > + status = ice_aq_download_pkg(hw, bh, ICE_PKG_BUF_SIZE, last, > > + &offset, &info, NULL); > > if (status) { > > ice_debug(hw, ICE_DBG_PKG, > > "Pkg download failed: err %d off %d inf > > %d\n", @@ -2937,7 > > +2936,7 @@ static void ice_fill_tbl(struct ice_hw *hw, enum ice_block > > block_id, u32 sid) > > case ICE_SID_XLT2_ACL: > > case ICE_SID_XLT2_PE: > > xlt2 = (struct ice_xlt2_section *)sect; > > - src = (u8 *)xlt2->value; > > + src = (_FORCE_ u8 *)xlt2->value; > > sect_len = LE16_TO_CPU(xlt2->count) * > > sizeof(*hw->blk[block_id].xlt2.t); > > dst = (u8 *)hw->blk[block_id].xlt2.t; @@ -3889,7 > > +3888,7 @@ > > ice_update_fd_swap(struct ice_hw *hw, u16 prof_id, struct ice_fv_word > > *es) > > > > /* fill in the swap array */ > > si = hw->blk[ICE_BLK_FD].es.fvw - 1; > > - do { > > + while (si >= 0) { > > u8 indexes_used = 1; > > > > /* assume flat at this index */ > > @@ -3921,7 +3920,7 @@ ice_update_fd_swap(struct ice_hw *hw, u16 prof_id, > > struct ice_fv_word *es) > > } > > > > si -= indexes_used; > > - } while (si >= 0); > > + } > > > > /* for each set of 4 swap indexes, write the appropriate register */ > > for (j = 0; j < hw->blk[ICE_BLK_FD].es.fvw / 4; j++) { diff --git > > a/drivers/net/ice/base/ice_flow.c b/drivers/net/ice/base/ice_flow.c > > index 795abe98f..f31557eac 100644 > > --- a/drivers/net/ice/base/ice_flow.c > > +++ b/drivers/net/ice/base/ice_flow.c > > @@ -415,9 +415,6 @@ ice_flow_proc_seg_hdrs(struct ice_flow_prof_params > > *params) > > const ice_bitmap_t *src; > > u32 hdrs; > > > > - if (i > 0 && (i + 1) < prof->segs_cnt) > > - continue; > > - > > hdrs = prof->segs[i].hdrs; > > > > if (hdrs & ICE_FLOW_SEG_HDR_ETH) { @@ -847,6 +844,7 @@ > > ice_flow_proc_segs(struct ice_hw *hw, struct ice_flow_prof_params > > *params) > > > > #define ICE_FLOW_FIND_PROF_CHK_FLDS 0x00000001 > > #define ICE_FLOW_FIND_PROF_CHK_VSI 0x00000002 > > +#define ICE_FLOW_FIND_PROF_NOT_CHK_DIR 0x00000004 > > > > /** > > * ice_flow_find_prof_conds - Find a profile matching headers and > > conditions @@ -866,7 +864,8 @@ ice_flow_find_prof_conds(struct ice_hw *hw, > > enum ice_block blk, > > struct ice_flow_prof *p; > > > > LIST_FOR_EACH_ENTRY(p, &hw->fl_profs[blk], ice_flow_prof, l_entry) { > > - if (p->dir == dir && segs_cnt && segs_cnt == p->segs_cnt) { > > + if ((p->dir == dir || conds & ICE_FLOW_FIND_PROF_NOT_CHK_DIR) && > > + segs_cnt && segs_cnt == p->segs_cnt) { > > u8 i; > > > > /* Check for profile-VSI association if specified */ @@ > > -935,17 > > +934,15 @@ ice_flow_find_prof_id(struct ice_hw *hw, enum ice_block blk, u64 > > prof_id) > > } > > > > /** > > - * ice_flow_rem_entry_sync - Remove a flow entry > > + * ice_dealloc_flow_entry - Deallocate flow entry memory > > * @hw: pointer to the HW struct > > * @entry: flow entry to be removed > > */ > > -static enum ice_status > > -ice_flow_rem_entry_sync(struct ice_hw *hw, struct ice_flow_entry > > *entry) > > +static void > > +ice_dealloc_flow_entry(struct ice_hw *hw, struct ice_flow_entry > > +*entry) > > { > > if (!entry) > > - return ICE_ERR_BAD_PTR; > > - > > - LIST_DEL(&entry->l_entry); > > + return; > > > > if (entry->entry) > > ice_free(hw, entry->entry); > > @@ -957,6 +954,22 @@ ice_flow_rem_entry_sync(struct ice_hw *hw, struct > > ice_flow_entry *entry) > > } > > > > ice_free(hw, entry); > > +} > > + > > +/** > > + * ice_flow_rem_entry_sync - Remove a flow entry > > + * @hw: pointer to the HW struct > > + * @entry: flow entry to be removed > > + */ > > +static enum ice_status > > +ice_flow_rem_entry_sync(struct ice_hw *hw, struct ice_flow_entry > > +*entry) { > > + if (!entry) > > + return ICE_ERR_BAD_PTR; > > + > > + LIST_DEL(&entry->l_entry); > > + > > + ice_dealloc_flow_entry(hw, entry); > > > > return ICE_SUCCESS; > > } > > @@ -1395,9 +1408,12 @@ ice_flow_add_entry(struct ice_hw *hw, enum ice_block > > blk, u64 prof_id, > > goto out; > > } > > > > - ice_acquire_lock(&prof->entries_lock); > > - LIST_ADD(&e->l_entry, &prof->entries); > > - ice_release_lock(&prof->entries_lock); > > + if (blk != ICE_BLK_ACL) { > > + /* ACL will handle the entry management */ > > + ice_acquire_lock(&prof->entries_lock); > > + LIST_ADD(&e->l_entry, &prof->entries); > > + ice_release_lock(&prof->entries_lock); > > + } > > > > *entry_h = ICE_FLOW_ENTRY_HNDL(e); > > > > @@ -1425,7 +1441,7 @@ enum ice_status ice_flow_rem_entry(struct ice_hw *hw, > > u64 entry_h) > > if (entry_h == ICE_FLOW_ENTRY_HANDLE_INVAL) > > return ICE_ERR_PARAM; > > > > - entry = ICE_FLOW_ENTRY_PTR((unsigned long)entry_h); > > + entry = ICE_FLOW_ENTRY_PTR(entry_h); > > > > /* Retain the pointer to the flow profile as the entry will be freed */ > > prof = entry->prof; > > @@ -1676,6 +1692,9 @@ enum ice_status ice_rem_vsi_rss_cfg(struct ice_hw > > *hw, u16 vsi_handle) > > if (!ice_is_vsi_valid(hw, vsi_handle)) > > return ICE_ERR_PARAM; > > > > + if (LIST_EMPTY(&hw->fl_profs[blk])) > > + return ICE_SUCCESS; > > + > > It should be useless as LIST_FOR_EACH_ENTRY_SAFE handles that case properly > IIUC. > > > ice_acquire_lock(&hw->fl_profs_locks[blk]); > > LIST_FOR_EACH_ENTRY_SAFE(p, t, &hw->fl_profs[blk], ice_flow_prof, > > l_entry) { > > diff --git a/drivers/net/ice/base/ice_nvm.c > > b/drivers/net/ice/base/ice_nvm.c index fa9c348ce..76cfedb29 100644 > > --- a/drivers/net/ice/base/ice_nvm.c > > +++ b/drivers/net/ice/base/ice_nvm.c > > @@ -127,7 +127,7 @@ ice_read_sr_word_aq(struct ice_hw *hw, u16 offset, > > u16 *data) > > > > status = ice_read_sr_aq(hw, offset, 1, data, true); > > if (!status) > > - *data = LE16_TO_CPU(*(__le16 *)data); > > + *data = LE16_TO_CPU(*(_FORCE_ __le16 *)data); > > > > return status; > > } > > @@ -185,7 +185,7 @@ ice_read_sr_buf_aq(struct ice_hw *hw, u16 offset, u16 > > *words, u16 *data) > > } while (words_read < *words); > > > > for (i = 0; i < *words; i++) > > - data[i] = LE16_TO_CPU(((__le16 *)data)[i]); > > + data[i] = LE16_TO_CPU(((_FORCE_ __le16 *)data)[i]); > > > > read_nvm_buf_aq_exit: > > *words = words_read; > > diff --git a/drivers/net/ice/base/ice_protocol_type.h > > b/drivers/net/ice/base/ice_protocol_type.h > > index e572dd320..82822fb74 100644 > > --- a/drivers/net/ice/base/ice_protocol_type.h > > +++ b/drivers/net/ice/base/ice_protocol_type.h > > @@ -189,6 +189,7 @@ struct ice_udp_tnl_hdr { > > u16 field; > > u16 proto_type; > > u16 vni; > > + u16 reserved; > > }; > > > > struct ice_nvgre { > > diff --git a/drivers/net/ice/base/ice_switch.c > > b/drivers/net/ice/base/ice_switch.c > > index faaedd4c8..373acb7a6 100644 > > --- a/drivers/net/ice/base/ice_switch.c > > +++ b/drivers/net/ice/base/ice_switch.c > > @@ -279,6 +279,7 @@ enum ice_status ice_init_def_sw_recp(struct ice_hw *hw) > > recps[i].root_rid = i; > > INIT_LIST_HEAD(&recps[i].filt_rules); > > INIT_LIST_HEAD(&recps[i].filt_replay_rules); > > + INIT_LIST_HEAD(&recps[i].rg_list); > > That looks like a bug fix, isn't it? > > > ice_init_lock(&recps[i].filt_rule_lock); > > } > > > > @@ -859,7 +860,7 @@ ice_aq_add_update_mir_rule(struct ice_hw *hw, u16 > > rule_type, u16 dest_vsi, > > return ICE_ERR_PARAM; > > > > buf_size = count * sizeof(__le16); > > - mr_list = (__le16 *)ice_malloc(hw, buf_size); > > + mr_list = (_FORCE_ __le16 *)ice_malloc(hw, buf_size); > > if (!mr_list) > > return ICE_ERR_NO_MEMORY; > > break; > > @@ -1459,7 +1460,6 @@ static int ice_ilog2(u64 n) > > return -1; > > } > > > > - > > /** > > * ice_fill_sw_rule - Helper function to fill switch rule structure > > * @hw: pointer to the hardware structure @@ -1479,7 +1479,6 @@ > > ice_fill_sw_rule(struct ice_hw *hw, struct ice_fltr_info *f_info, > > __be16 *off; > > u8 q_rgn; > > > > - > > if (opc == ice_aqc_opc_remove_sw_rules) { > > s_rule->pdata.lkup_tx_rx.act = 0; > > s_rule->pdata.lkup_tx_rx.index = > > @@ -1555,7 +1554,7 @@ ice_fill_sw_rule(struct ice_hw *hw, struct > > ice_fltr_info *f_info, > > daddr = f_info->l_data.ethertype_mac.mac_addr; > > /* fall-through */ > > case ICE_SW_LKUP_ETHERTYPE: > > - off = (__be16 *)(eth_hdr + ICE_ETH_ETHTYPE_OFFSET); > > + off = (_FORCE_ __be16 *)(eth_hdr + ICE_ETH_ETHTYPE_OFFSET); > > *off = CPU_TO_BE16(f_info->l_data.ethertype_mac.ethertype); > > break; > > case ICE_SW_LKUP_MAC_VLAN: > > @@ -1586,7 +1585,7 @@ ice_fill_sw_rule(struct ice_hw *hw, struct > > ice_fltr_info *f_info, > > ICE_NONDMA_TO_NONDMA); > > > > if (!(vlan_id > ICE_MAX_VLAN_ID)) { > > - off = (__be16 *)(eth_hdr + ICE_ETH_VLAN_TCI_OFFSET); > > + off = (_FORCE_ __be16 *)(eth_hdr + ICE_ETH_VLAN_TCI_OFFSET); > > *off = CPU_TO_BE16(vlan_id); > > } > > > > @@ -2289,14 +2288,15 @@ ice_add_rule_internal(struct ice_hw *hw, u8 > > recp_id, > > > > m_entry = ice_find_rule_entry(hw, recp_id, new_fltr); > > if (!m_entry) { > > - ice_release_lock(rule_lock); > > - return ice_create_pkt_fwd_rule(hw, f_entry); > > + status = ice_create_pkt_fwd_rule(hw, f_entry); > > + goto exit_add_rule_internal; > > } > > > > cur_fltr = &m_entry->fltr_info; > > status = ice_add_update_vsi_list(hw, m_entry, cur_fltr, new_fltr); > > - ice_release_lock(rule_lock); > > > > +exit_add_rule_internal: > > + ice_release_lock(rule_lock); > > return status; > > } > > > > @@ -2975,12 +2975,19 @@ ice_add_mac_vlan(struct ice_hw *hw, struct > > LIST_HEAD_TYPE *mv_list) > > * ice_add_eth_mac - Add ethertype and MAC based filter rule > > * @hw: pointer to the hardware structure > > * @em_list: list of ether type MAC filter, MAC is optional > > + * > > + * This function requires the caller to populate the entries in > > + * the filter list with the necessary fields (including flags to > > + * indicate Tx or Rx rules). > > */ > > enum ice_status > > ice_add_eth_mac(struct ice_hw *hw, struct LIST_HEAD_TYPE *em_list) > > { > > struct ice_fltr_list_entry *em_list_itr; > > > > + if (!em_list || !hw) > > + return ICE_ERR_PARAM; > > + > > LIST_FOR_EACH_ENTRY(em_list_itr, em_list, ice_fltr_list_entry, > > list_entry) { > > enum ice_sw_lkup_type l_type = > > @@ -2990,7 +2997,6 @@ ice_add_eth_mac(struct ice_hw *hw, struct > > LIST_HEAD_TYPE *em_list) > > l_type != ICE_SW_LKUP_ETHERTYPE) > > return ICE_ERR_PARAM; > > > > - em_list_itr->fltr_info.flag = ICE_FLTR_TX; > > em_list_itr->status = ice_add_rule_internal(hw, l_type, > > em_list_itr); > > if (em_list_itr->status) > > diff --git a/drivers/net/ice/base/ice_switch.h > > b/drivers/net/ice/base/ice_switch.h > > index 2f140a86d..05b1170c9 100644 > > --- a/drivers/net/ice/base/ice_switch.h > > +++ b/drivers/net/ice/base/ice_switch.h > > @@ -11,6 +11,9 @@ > > #define ICE_SW_CFG_MAX_BUF_LEN 2048 > > #define ICE_MAX_SW 256 > > #define ICE_DFLT_VSI_INVAL 0xff > > +#define ICE_FLTR_RX BIT(0) > > +#define ICE_FLTR_TX BIT(1) > > +#define ICE_FLTR_TX_RX (ICE_FLTR_RX | ICE_FLTR_TX) > > > > > > /* Worst case buffer length for ice_aqc_opc_get_res_alloc */ @@ > > -77,9 +80,6 @@ struct ice_fltr_info { > > /* rule ID returned by firmware once filter rule is created */ > > u16 fltr_rule_id; > > u16 flag; > > -#define ICE_FLTR_RX BIT(0) > > -#define ICE_FLTR_TX BIT(1) > > -#define ICE_FLTR_TX_RX (ICE_FLTR_RX | ICE_FLTR_TX) > > > > /* Source VSI for LOOKUP_TX or source port for LOOKUP_RX */ > > u16 src; > > @@ -145,10 +145,6 @@ struct ice_sw_act_ctrl { > > /* Source VSI for LOOKUP_TX or source port for LOOKUP_RX */ > > u16 src; > > u16 flag; > > -#define ICE_FLTR_RX BIT(0) > > -#define ICE_FLTR_TX BIT(1) > > -#define ICE_FLTR_TX_RX (ICE_FLTR_RX | ICE_FLTR_TX) > > - > > enum ice_sw_fwd_act_type fltr_act; > > /* Depending on filter action */ > > union { > > @@ -368,6 +364,8 @@ ice_aq_get_res_descs(struct ice_hw *hw, u16 num_entries, > > struct ice_sq_cd *cd); > > enum ice_status > > ice_add_vlan(struct ice_hw *hw, struct LIST_HEAD_TYPE *m_list); > > +enum ice_status > > +ice_remove_vlan(struct ice_hw *hw, struct LIST_HEAD_TYPE *v_list); > > void ice_rem_all_sw_rules_info(struct ice_hw *hw); > > enum ice_status ice_add_mac(struct ice_hw *hw, struct LIST_HEAD_TYPE > > *m_lst); > > enum ice_status ice_remove_mac(struct ice_hw *hw, struct > > LIST_HEAD_TYPE *m_lst); @@ -375,8 +373,6 @@ enum ice_status > > ice_add_eth_mac(struct ice_hw *hw, struct LIST_HEAD_TYPE *em_list); > > enum ice_status > > ice_remove_eth_mac(struct ice_hw *hw, struct LIST_HEAD_TYPE > > *em_list); -enum ice_status -ice_remove_vlan(struct ice_hw *hw, struct > > LIST_HEAD_TYPE *v_list); > > #ifndef NO_MACVLAN_SUPPORT > > enum ice_status > > ice_add_mac_vlan(struct ice_hw *hw, struct LIST_HEAD_TYPE *m_list); > > diff --git a/drivers/net/ice/base/ice_type.h > > b/drivers/net/ice/base/ice_type.h index 919ca7fa8..f4e151c55 100644 > > --- a/drivers/net/ice/base/ice_type.h > > +++ b/drivers/net/ice/base/ice_type.h > > @@ -14,6 +14,10 @@ > > > > #define BITS_PER_BYTE 8 > > > > +#ifndef _FORCE_ > > +#define _FORCE_ > > +#endif > > + > > #define ICE_BYTES_PER_WORD 2 > > #define ICE_BYTES_PER_DWORD 4 > > #define ICE_MAX_TRAFFIC_CLASS 8 > > @@ -35,7 +39,7 @@ > > #endif > > > > #ifndef IS_ASCII > > -#define IS_ASCII(_ch) ((_ch) < 0x80) > > +#define IS_ASCII(_ch) ((_ch) < 0x80) > > #endif > > > > #include "ice_status.h" > > @@ -80,6 +84,7 @@ static inline u32 ice_round_to_num(u32 N, u32 R) > > #define ICE_HI_WORD(x) ((u16)(((x) >> 16) & 0xFFFF)) > > > > /* debug masks - set these bits in hw->debug_mask to control output > > */ > > +#define ICE_DBG_TRACE BIT_ULL(0) /* for function-trace only */ > > #define ICE_DBG_INIT BIT_ULL(1) > > #define ICE_DBG_RELEASE BIT_ULL(2) > > #define ICE_DBG_FW_LOG BIT_ULL(3) > > @@ -199,6 +204,7 @@ enum ice_vsi_type { > > #ifdef ADQ_SUPPORT > > ICE_VSI_CHNL = 4, > > #endif /* ADQ_SUPPORT */ > > + ICE_VSI_LB = 6, > > }; > > > > struct ice_link_status { > > @@ -718,6 +724,8 @@ struct ice_fw_log_cfg { > > #define ICE_FW_LOG_EVNT_INIT (ICE_AQC_FW_LOG_INIT_EN >> > > ICE_AQC_FW_LOG_EN_S) > > #define ICE_FW_LOG_EVNT_FLOW (ICE_AQC_FW_LOG_FLOW_EN >> > > ICE_AQC_FW_LOG_EN_S) > > #define ICE_FW_LOG_EVNT_ERR (ICE_AQC_FW_LOG_ERR_EN >> > > ICE_AQC_FW_LOG_EN_S) > > +#define ICE_FW_LOG_EVNT_ALL (ICE_FW_LOG_EVNT_INFO | > > ICE_FW_LOG_EVNT_INIT | \ > > + ICE_FW_LOG_EVNT_FLOW | ICE_FW_LOG_EVNT_ERR) > > struct ice_fw_log_evnt evnts[ICE_AQC_FW_LOG_ID_MAX]; > > }; > > > > @@ -745,6 +753,7 @@ struct ice_hw { > > u8 pf_id; /* device profile info */ > > > > u16 max_burst_size; /* driver sets this value */ > > + > > /* Tx Scheduler values */ > > u16 num_tx_sched_layers; > > u16 num_tx_sched_phys_layers; > > @@ -948,7 +957,6 @@ enum ice_sw_fwd_act_type { > > #define ICE_SR_CSR_PROTECTED_LIST_PTR 0x0D > > #define ICE_SR_MNG_CFG_PTR 0x0E > > #define ICE_SR_EMP_MODULE_PTR 0x0F > > -#define ICE_SR_PBA_FLAGS 0x15 > > #define ICE_SR_PBA_BLOCK_PTR 0x16 > > #define ICE_SR_BOOT_CFG_PTR 0x17 > > #define ICE_SR_NVM_WOL_CFG 0x19 > > @@ -994,6 +1002,7 @@ enum ice_sw_fwd_act_type { > > #define ICE_SR_EMP_SR_SETTINGS_PTR 0x48 > > #define ICE_SR_CONFIGURATION_METADATA_PTR 0x4D > > #define ICE_SR_IMMEDIATE_VALUES_PTR 0x4E > > +#define ICE_SR_POR_REGISTERS_AUTOLOAD_PTR 0x118 > > > > /* Auxiliary field, mask and shift definition for Shadow RAM and NVM > > Flash */ > > #define ICE_SR_VPD_SIZE_WORDS 512 > >