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 91232A04B4; Tue, 19 Nov 2019 06:41:53 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B80411C0B; Tue, 19 Nov 2019 06:41:52 +0100 (CET) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id A18CFB62 for ; Tue, 19 Nov 2019 06:41:51 +0100 (CET) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Nov 2019 21:41:50 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,322,1569308400"; d="scan'208";a="209082460" Received: from yexl-server.sh.intel.com (HELO localhost) ([10.67.117.17]) by orsmga003.jf.intel.com with ESMTP; 18 Nov 2019 21:41:49 -0800 Date: Tue, 19 Nov 2019 13:38:17 +0800 From: Ye Xiaolong To: Qi Zhang Cc: beilei.xing@intel.com, yahui.cao@intel.com, dev@dpdk.org Message-ID: <20191119053817.GK69793@intel.com> References: <20191118063436.37570-1-qi.z.zhang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191118063436.37570-1-qi.z.zhang@intel.com> User-Agent: Mutt/1.9.4 (2018-02-28) Subject: Re: [dpdk-dev] [PATCH v3] net/ice: fix FDIR flow type conflict 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" On 11/18, Qi Zhang wrote: >Flow type "IPv4 + UDP" or "IPv4 + TCP" is conflict with "IPv4 + any" >flow type. If a rule for IPv4 + any is created, we should reject any rule >for IPv4 + UDP otherwise the first rule may be impacted, same decision >should be made on a reverse order. >For IPv6 and IPv4 GTPU inner case, we have the same limitation. > >Fixes: 109e8e06249e ("net/ice: configure HW flow director rule") > >Signed-off-by: Qi Zhang >--- > >v3: >- replace EAGAIN with EEXIST when a profile already exist. > >v2: >- fix check patch warning. > > drivers/net/ice/ice_fdir_filter.c | 174 ++++++++++++++++++++++++++++++++++---- > 1 file changed, 157 insertions(+), 17 deletions(-) > >diff --git a/drivers/net/ice/ice_fdir_filter.c b/drivers/net/ice/ice_fdir_filter.c >index 7876f4bbc..e5a35dfe8 100644 >--- a/drivers/net/ice/ice_fdir_filter.c >+++ b/drivers/net/ice/ice_fdir_filter.c >@@ -646,6 +646,153 @@ ice_fdir_teardown(struct ice_pf *pf) > } > } > >+static int ice_fdir_cur_prof_conflict(struct ice_pf *pf, Please put the return type in a separate line, same comments for the rest of functions introduced in this patch. >+ enum ice_fltr_ptype ptype, >+ struct ice_flow_seg_info *seg, >+ bool is_tunnel) >+{ >+ struct ice_hw *hw = ICE_PF_TO_HW(pf); >+ struct ice_flow_seg_info *ori_seg; >+ struct ice_fd_hw_prof *hw_prof; >+ >+ hw_prof = hw->fdir_prof[ptype]; >+ ori_seg = hw_prof->fdir_seg[is_tunnel]; >+ >+ /* profile does not exist */ >+ if (!ori_seg) >+ return 0; >+ >+ /* if no input set conflict, return -EEXIST */ >+ if ((!is_tunnel && !memcmp(ori_seg, seg, sizeof(*seg))) || >+ (is_tunnel && !memcmp(&ori_seg[1], &seg[1], sizeof(*seg)))) { >+ PMD_DRV_LOG(DEBUG, "Profile already exist for flow type %d.", s/exist/exists/ >+ ptype); >+ return -EEXIST; >+ } >+ >+ /* a rule with input set conflict already exist, so give up */ >+ if (pf->fdir_fltr_cnt[ptype][is_tunnel]) { >+ PMD_DRV_LOG(DEBUG, "Failed to create profile for flow type %d due to conflict with exist rule.", >+ ptype); s/exist/existing Shall we use ERR level for this case? >+ return -EINVAL; >+ } >+ >+ /* it's safe to delete an empty profile */ >+ ice_fdir_prof_rm(pf, ptype, is_tunnel); >+ return 0; >+} >+ >+static bool ice_fdir_prof_resolve_conflict(struct ice_pf *pf, >+ enum ice_fltr_ptype ptype, >+ bool is_tunnel) >+{ >+ struct ice_hw *hw = ICE_PF_TO_HW(pf); >+ struct ice_fd_hw_prof *hw_prof; >+ struct ice_flow_seg_info *seg; >+ >+ hw_prof = hw->fdir_prof[ptype]; >+ seg = hw_prof->fdir_seg[is_tunnel]; >+ >+ /* profile does not exist */ >+ if (!seg) >+ return true; >+ >+ /* profile exist and rule exist, fail to resolve the conflict */ s/exist/exists >+ if (pf->fdir_fltr_cnt[ptype][is_tunnel] != 0) >+ return false; >+ >+ /* it's safe to delete an empty profile */ >+ ice_fdir_prof_rm(pf, ptype, is_tunnel); >+ >+ return true; >+} >+ >+static int ice_fdir_cross_prof_conflict(struct ice_pf *pf, >+ enum ice_fltr_ptype ptype, >+ bool is_tunnel) >+{ >+ enum ice_fltr_ptype cflct_ptype; >+ >+ switch (ptype) { >+ /* IPv4 */ >+ case ICE_FLTR_PTYPE_NONF_IPV4_UDP: >+ case ICE_FLTR_PTYPE_NONF_IPV4_TCP: >+ case ICE_FLTR_PTYPE_NONF_IPV4_SCTP: >+ cflct_ptype = ICE_FLTR_PTYPE_NONF_IPV4_OTHER; >+ if (!ice_fdir_prof_resolve_conflict >+ (pf, cflct_ptype, is_tunnel)) >+ goto err; >+ break; >+ case ICE_FLTR_PTYPE_NONF_IPV4_OTHER: >+ cflct_ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP; >+ if (!ice_fdir_prof_resolve_conflict >+ (pf, cflct_ptype, is_tunnel)) >+ goto err; >+ cflct_ptype = ICE_FLTR_PTYPE_NONF_IPV4_TCP; >+ if (!ice_fdir_prof_resolve_conflict >+ (pf, cflct_ptype, is_tunnel)) >+ goto err; >+ cflct_ptype = ICE_FLTR_PTYPE_NONF_IPV4_SCTP; >+ if (!ice_fdir_prof_resolve_conflict >+ (pf, cflct_ptype, is_tunnel)) >+ goto err; >+ break; >+ /* IPv4 GTPU */ >+ case ICE_FLTR_PTYPE_NONF_IPV4_GTPU_IPV4_UDP: >+ case ICE_FLTR_PTYPE_NONF_IPV4_GTPU_IPV4_TCP: >+ case ICE_FLTR_PTYPE_NONF_IPV4_GTPU_IPV4_ICMP: >+ cflct_ptype = ICE_FLTR_PTYPE_NONF_IPV4_GTPU_IPV4_OTHER; >+ if (!ice_fdir_prof_resolve_conflict >+ (pf, cflct_ptype, is_tunnel)) >+ goto err; >+ break; >+ case ICE_FLTR_PTYPE_NONF_IPV4_GTPU_IPV4_OTHER: >+ cflct_ptype = ICE_FLTR_PTYPE_NONF_IPV4_GTPU_IPV4_OTHER; >+ if (!ice_fdir_prof_resolve_conflict >+ (pf, cflct_ptype, is_tunnel)) >+ goto err; >+ cflct_ptype = ICE_FLTR_PTYPE_NONF_IPV4_GTPU_IPV4_OTHER; >+ if (!ice_fdir_prof_resolve_conflict >+ (pf, cflct_ptype, is_tunnel)) >+ goto err; >+ cflct_ptype = ICE_FLTR_PTYPE_NONF_IPV4_GTPU_IPV4_OTHER; >+ if (!ice_fdir_prof_resolve_conflict >+ (pf, cflct_ptype, is_tunnel)) >+ goto err; >+ break; >+ /* IPv6 */ >+ case ICE_FLTR_PTYPE_NONF_IPV6_UDP: >+ case ICE_FLTR_PTYPE_NONF_IPV6_TCP: >+ case ICE_FLTR_PTYPE_NONF_IPV6_SCTP: >+ cflct_ptype = ICE_FLTR_PTYPE_NONF_IPV6_OTHER; >+ if (!ice_fdir_prof_resolve_conflict >+ (pf, cflct_ptype, is_tunnel)) >+ goto err; >+ break; >+ case ICE_FLTR_PTYPE_NONF_IPV6_OTHER: >+ cflct_ptype = ICE_FLTR_PTYPE_NONF_IPV6_UDP; >+ if (!ice_fdir_prof_resolve_conflict >+ (pf, cflct_ptype, is_tunnel)) >+ goto err; >+ cflct_ptype = ICE_FLTR_PTYPE_NONF_IPV6_TCP; >+ if (!ice_fdir_prof_resolve_conflict >+ (pf, cflct_ptype, is_tunnel)) >+ goto err; >+ cflct_ptype = ICE_FLTR_PTYPE_NONF_IPV6_SCTP; >+ if (!ice_fdir_prof_resolve_conflict >+ (pf, cflct_ptype, is_tunnel)) >+ goto err; >+ break; >+ default: >+ break; >+ } >+ return 0; >+err: >+ PMD_DRV_LOG(DEBUG, "Failed to create profile for flow type %d due to conflict with exist rule of flow type %d.", >+ ptype, cflct_ptype); Use ERR? >+ return -EINVAL; >+} >+ > static int > ice_fdir_hw_tbl_conf(struct ice_pf *pf, struct ice_vsi *vsi, > struct ice_vsi *ctrl_vsi, >@@ -655,7 +802,6 @@ ice_fdir_hw_tbl_conf(struct ice_pf *pf, struct ice_vsi *vsi, > { > struct ice_hw *hw = ICE_PF_TO_HW(pf); > enum ice_flow_dir dir = ICE_FLOW_RX; >- struct ice_flow_seg_info *ori_seg; > struct ice_fd_hw_prof *hw_prof; > struct ice_flow_prof *prof; > uint64_t entry_1 = 0; >@@ -664,22 +810,15 @@ ice_fdir_hw_tbl_conf(struct ice_pf *pf, struct ice_vsi *vsi, > int ret; > uint64_t prof_id; > >- hw_prof = hw->fdir_prof[ptype]; >- ori_seg = hw_prof->fdir_seg[is_tunnel]; >- if (ori_seg) { >- if (!is_tunnel) { >- if (!memcmp(ori_seg, seg, sizeof(*seg))) >- return -EAGAIN; >- } else { >- if (!memcmp(&ori_seg[1], &seg[1], sizeof(*seg))) >- return -EAGAIN; >- } >- >- if (pf->fdir_fltr_cnt[ptype][is_tunnel]) >- return -EINVAL; >+ /* check if have input set conflict on current profile. */ >+ ret = ice_fdir_cur_prof_conflict(pf, ptype, seg, is_tunnel); >+ if (ret) >+ return ret; > >- ice_fdir_prof_rm(pf, ptype, is_tunnel); >- } >+ /* check if the profile is conflict with other profile. */ >+ ret = ice_fdir_cross_prof_conflict(pf, ptype, is_tunnel); >+ if (ret) >+ return ret; > > prof_id = ptype + is_tunnel * ICE_FLTR_PTYPE_MAX; > ret = ice_flow_add_prof(hw, ICE_BLK_FD, dir, prof_id, seg, >@@ -703,6 +842,7 @@ ice_fdir_hw_tbl_conf(struct ice_pf *pf, struct ice_vsi *vsi, > goto err_add_entry; > } > >+ hw_prof = hw->fdir_prof[ptype]; > pf->hw_prof_cnt[ptype][is_tunnel] = 0; > hw_prof->cnt = 0; > hw_prof->fdir_seg[is_tunnel] = seg; >@@ -866,7 +1006,7 @@ ice_fdir_input_set_conf(struct ice_pf *pf, enum ice_fltr_ptype flow, > rte_free(seg); > if (is_tunnel) > rte_free(seg_tun); >- return (ret == -EAGAIN) ? 0 : ret; >+ return (ret == -EEXIST) ? 0 : ret; > } else { > return ret; > } >-- >2.13.6 >