[v3,1/2] net/ice: refactor FDIR set conf function

Message ID 20200915052702.3194061-2-zhirun.yan@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Qi Zhang
Headers
Series refactor and clean FDIR |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Performance-Testing fail build patch failure

Commit Message

Yan, Zhirun Sept. 15, 2020, 5:27 a.m. UTC
  The original set conf function in FDIR was very long. Refactor to
increase readability to make it clearer and allow for more convenient
further changes.

No functional change here.

Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
---
 drivers/net/ice/ice_fdir_filter.c | 53 +++++++++++++++++--------------
 1 file changed, 30 insertions(+), 23 deletions(-)
  

Comments

Cao, Yahui Sept. 18, 2020, 9:28 a.m. UTC | #1
> -----Original Message-----
> From: Yan, Zhirun <zhirun.yan@intel.com>
> Sent: Tuesday, September 15, 2020 1:27 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> Cc: Cao, Yahui <yahui.cao@intel.com>; Wang, Xiao W
> <xiao.w.wang@intel.com>; Su, Simei <simei.su@intel.com>; Guo, Junfeng
> <junfeng.guo@intel.com>; Yan, Zhirun <zhirun.yan@intel.com>
> Subject: [PATCH v3 1/2] net/ice: refactor FDIR set conf function
> 
> The original set conf function in FDIR was very long. Refactor to increase
> readability to make it clearer and allow for more convenient further changes.
> 
> No functional change here.
> 
> Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
> ---
>  drivers/net/ice/ice_fdir_filter.c | 53 +++++++++++++++++--------------
>  1 file changed, 30 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_fdir_filter.c b/drivers/net/ice/ice_fdir_filter.c
> index c4b7265ce..a65523781 100644
> --- a/drivers/net/ice/ice_fdir_filter.c
> +++ b/drivers/net/ice/ice_fdir_filter.c
> @@ -943,30 +943,9 @@ ice_fdir_input_set_parse(uint64_t inset, enum
> ice_flow_field *field)
>  	}
>  }
> 
> -static int
> -ice_fdir_input_set_conf(struct ice_pf *pf, enum ice_fltr_ptype flow,
> -			uint64_t input_set, enum ice_fdir_tunnel_type ttype)
> +static void
> +ice_fdir_input_set_hdrs(enum ice_fltr_ptype flow, struct ice_flow_seg_info *seg)
[Cao, Yahui] 
suggest to change function name from ice_fdir_input_set_hdrs to ice_fdir_input_set_segs,
and all seg related variable can be inited inside this function, so that this function init
seg header and seg field. And below function can be simplified as(see next comments)

>  {
> -	struct ice_flow_seg_info *seg;
> -	struct ice_flow_seg_info *seg_tun = NULL;
> -	enum ice_flow_field field[ICE_FLOW_FIELD_IDX_MAX];
> -	bool is_tunnel;
> -	int i, ret;
> -
> -	if (!input_set)
> -		return -EINVAL;
> -
> -	seg = (struct ice_flow_seg_info *)
> -		ice_malloc(hw, sizeof(*seg));
> -	if (!seg) {
> -		PMD_DRV_LOG(ERR, "No memory can be allocated");
> -		return -ENOMEM;
> -	}
> -
> -	for (i = 0; i < ICE_FLOW_FIELD_IDX_MAX; i++)
> -		field[i] = ICE_FLOW_FIELD_IDX_MAX;
> -	ice_fdir_input_set_parse(input_set, field);
> -
>  	switch (flow) {
>  	case ICE_FLTR_PTYPE_NONF_IPV4_UDP:
>  		ICE_FLOW_SET_HDRS(seg, ICE_FLOW_SEG_HDR_UDP | @@ -
> 1038,6 +1017,34 @@ ice_fdir_input_set_conf(struct ice_pf *pf, enum
> ice_fltr_ptype flow,
>  		PMD_DRV_LOG(ERR, "not supported filter type.");
>  		break;
>  	}
> +}
> +
> +static int
> +ice_fdir_input_set_conf(struct ice_pf *pf, enum ice_fltr_ptype flow,
> +			uint64_t input_set, enum ice_fdir_tunnel_type ttype)
> {
> +	struct ice_flow_seg_info *seg;
> +	struct ice_flow_seg_info *seg_tun = NULL;
> +	enum ice_flow_field field[ICE_FLOW_FIELD_IDX_MAX];
> +	bool is_tunnel;
> +	int i, ret;
> +
> +	if (!input_set)
> +		return -EINVAL;
> +
> +	seg = (struct ice_flow_seg_info *)
> +		ice_malloc(hw, sizeof(*seg));
> +	if (!seg) {
> +		PMD_DRV_LOG(ERR, "No memory can be allocated");
> +		return -ENOMEM;
> +	}
> +

[Cao, Yahui] 
> +	for (i = 0; i < ICE_FLOW_FIELD_IDX_MAX; i++)
> +		field[i] = ICE_FLOW_FIELD_IDX_MAX;
> +
> +	ice_fdir_input_set_parse(input_set, field);
> +
> +	ice_fdir_input_set_hdrs(flow, seg);
[Cao, Yahui] 
Function above can be replaced by ice_fdir_input_set_segs(seg, input_set, flow)
> 
>  	for (i = 0; field[i] != ICE_FLOW_FIELD_IDX_MAX; i++) {
>  		ice_flow_set_fld(seg, field[i],
> --
> 2.25.1
  

Patch

diff --git a/drivers/net/ice/ice_fdir_filter.c b/drivers/net/ice/ice_fdir_filter.c
index c4b7265ce..a65523781 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -943,30 +943,9 @@  ice_fdir_input_set_parse(uint64_t inset, enum ice_flow_field *field)
 	}
 }
 
-static int
-ice_fdir_input_set_conf(struct ice_pf *pf, enum ice_fltr_ptype flow,
-			uint64_t input_set, enum ice_fdir_tunnel_type ttype)
+static void
+ice_fdir_input_set_hdrs(enum ice_fltr_ptype flow, struct ice_flow_seg_info *seg)
 {
-	struct ice_flow_seg_info *seg;
-	struct ice_flow_seg_info *seg_tun = NULL;
-	enum ice_flow_field field[ICE_FLOW_FIELD_IDX_MAX];
-	bool is_tunnel;
-	int i, ret;
-
-	if (!input_set)
-		return -EINVAL;
-
-	seg = (struct ice_flow_seg_info *)
-		ice_malloc(hw, sizeof(*seg));
-	if (!seg) {
-		PMD_DRV_LOG(ERR, "No memory can be allocated");
-		return -ENOMEM;
-	}
-
-	for (i = 0; i < ICE_FLOW_FIELD_IDX_MAX; i++)
-		field[i] = ICE_FLOW_FIELD_IDX_MAX;
-	ice_fdir_input_set_parse(input_set, field);
-
 	switch (flow) {
 	case ICE_FLTR_PTYPE_NONF_IPV4_UDP:
 		ICE_FLOW_SET_HDRS(seg, ICE_FLOW_SEG_HDR_UDP |
@@ -1038,6 +1017,34 @@  ice_fdir_input_set_conf(struct ice_pf *pf, enum ice_fltr_ptype flow,
 		PMD_DRV_LOG(ERR, "not supported filter type.");
 		break;
 	}
+}
+
+static int
+ice_fdir_input_set_conf(struct ice_pf *pf, enum ice_fltr_ptype flow,
+			uint64_t input_set, enum ice_fdir_tunnel_type ttype)
+{
+	struct ice_flow_seg_info *seg;
+	struct ice_flow_seg_info *seg_tun = NULL;
+	enum ice_flow_field field[ICE_FLOW_FIELD_IDX_MAX];
+	bool is_tunnel;
+	int i, ret;
+
+	if (!input_set)
+		return -EINVAL;
+
+	seg = (struct ice_flow_seg_info *)
+		ice_malloc(hw, sizeof(*seg));
+	if (!seg) {
+		PMD_DRV_LOG(ERR, "No memory can be allocated");
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < ICE_FLOW_FIELD_IDX_MAX; i++)
+		field[i] = ICE_FLOW_FIELD_IDX_MAX;
+
+	ice_fdir_input_set_parse(input_set, field);
+
+	ice_fdir_input_set_hdrs(flow, seg);
 
 	for (i = 0; field[i] != ICE_FLOW_FIELD_IDX_MAX; i++) {
 		ice_flow_set_fld(seg, field[i],