Re: [dpdk-dev] [PATCH v3 1/2] net/ice: refactor FDIR set conf function

["Cao, Yahui" <yahui.cao@intel.com>]

> -----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