RE: [PATCH v4 1/2] net/ice: fix DCF ACL flow engine

["Zhang, Qi Z" <qi.z.zhang@intel.com>]

> -----Original Message-----
> From: Liu, KevinX <kevinx.liu@intel.com>
> Sent: Wednesday, April 20, 2022 12:02 AM
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Yang, SteveX <stevex.yang@intel.com>; Alvin Zhang
> <alvinx.zhang@intel.com>; Liu, KevinX <kevinx.liu@intel.com>
> Subject: [PATCH v4 1/2] net/ice: fix DCF ACL flow engine
> 
> From: Alvin Zhang <alvinx.zhang@intel.com>
> 
> ACL is not a necessary feature for DCF, it may not be supported by the ice
> kernel driver, so in this patch the program does not return the ACL initiation
> fails to high level functions, as substitute it prints some error logs, cleans the
> related resources and unregisters the ACL engine.
> 
> Fixes: 40d466fa9f76 ("net/ice: support ACL filter in DCF")
> 
> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
> ---
>  drivers/net/ice/ice_acl_filter.c   | 20 ++++++++++++++----
>  drivers/net/ice/ice_generic_flow.c | 34 +++++++++++++++++++++++-------
>  2 files changed, 42 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_acl_filter.c b/drivers/net/ice/ice_acl_filter.c
> index 8fe6f5aeb0..20a1f86c43 100644
> --- a/drivers/net/ice/ice_acl_filter.c
> +++ b/drivers/net/ice/ice_acl_filter.c
> @@ -56,6 +56,8 @@ ice_pattern_match_item ice_acl_pattern[] = {
>  	{pattern_eth_ipv4_sctp,	ICE_ACL_INSET_ETH_IPV4_SCTP,
> 	ICE_INSET_NONE,	ICE_INSET_NONE},
>  };
> 
> +static void ice_acl_prof_free(struct ice_hw *hw);
> +
>  static int
>  ice_acl_prof_alloc(struct ice_hw *hw)
>  {
> @@ -1007,17 +1009,27 @@ ice_acl_init(struct ice_adapter *ad)
> 
>  	ret = ice_acl_setup(pf);
>  	if (ret)
> -		return ret;
> +		goto deinit_acl;
> 
>  	ret = ice_acl_bitmap_init(pf);
>  	if (ret)
> -		return ret;
> +		goto deinit_acl;
> 
>  	ret = ice_acl_prof_init(pf);
>  	if (ret)
> -		return ret;
> +		goto deinit_acl;
> 
> -	return ice_register_parser(parser, ad);
> +	ret = ice_register_parser(parser, ad);
> +	if (ret)
> +		goto deinit_acl;
> +
> +	return 0;
> +
> +deinit_acl:
> +	ice_deinit_acl(pf);
> +	ice_acl_prof_free(hw);
> +	PMD_DRV_LOG(ERR, "ACL init failed, may not supported!");

Better to print the error message at the place where the error happens, for easy debugging.

> +	return ret;
>  }
> 
>  static void
> diff --git a/drivers/net/ice/ice_generic_flow.c
> b/drivers/net/ice/ice_generic_flow.c
> index 57eb002bde..cfdc4bd697 100644
> --- a/drivers/net/ice/ice_generic_flow.c
> +++ b/drivers/net/ice/ice_generic_flow.c
> @@ -1817,6 +1817,12 @@ ice_register_flow_engine(struct ice_flow_engine
> *engine)
>  	TAILQ_INSERT_TAIL(&engine_list, engine, node);  }
> 
> +static void
> +ice_unregister_flow_engine(struct ice_flow_engine *engine) {
> +	TAILQ_REMOVE(&engine_list, engine, node); }
> +
>  int
>  ice_flow_init(struct ice_adapter *ad)
>  {
> @@ -1843,9 +1849,18 @@ ice_flow_init(struct ice_adapter *ad)
> 
>  		ret = engine->init(ad);
>  		if (ret) {
> -			PMD_INIT_LOG(ERR, "Failed to initialize engine %d",
> -					engine->type);
> -			return ret;
> +			/**
> +			 * ACL may not supported in kernel driver,


may not be supported

> +			 * so just unregister the engine.
> +			 */
> +			if (engine->type == ICE_FLOW_ENGINE_ACL) {
> +				ice_unregister_flow_engine(engine);
> +			} else {
> +				PMD_INIT_LOG(ERR,
> +					     "Failed to initialize engine %d",
> +					     engine->type);
> +				return ret;
> +			}
>  		}
>  	}
>  	return 0;
> @@ -1937,7 +1952,7 @@ ice_register_parser(struct ice_flow_parser *parser,
> 
>  	list = ice_get_parser_list(parser, ad);
>  	if (list == NULL)
> -		return -EINVAL;
> +		goto err;
> 
>  	if (ad->devargs.pipe_mode_support) {
>  		TAILQ_INSERT_TAIL(list, parser_node, node); @@ -1949,7
> +1964,7 @@ ice_register_parser(struct ice_flow_parser *parser,
>  				    ICE_FLOW_ENGINE_ACL) {
>  					TAILQ_INSERT_AFTER(list,
> existing_node,
>  							   parser_node, node);
> -					goto DONE;
> +					return 0;
>  				}
>  			}
>  			TAILQ_INSERT_HEAD(list, parser_node, node); @@ -
> 1960,7 +1975,7 @@ ice_register_parser(struct ice_flow_parser *parser,
>  				    ICE_FLOW_ENGINE_SWITCH) {
>  					TAILQ_INSERT_AFTER(list,
> existing_node,
>  							   parser_node, node);
> -					goto DONE;
> +					return 0;
>  				}
>  			}
>  			TAILQ_INSERT_HEAD(list, parser_node, node); @@ -
> 1969,11 +1984,14 @@ ice_register_parser(struct ice_flow_parser *parser,
>  		} else if (parser->engine->type == ICE_FLOW_ENGINE_ACL) {
>  			TAILQ_INSERT_HEAD(list, parser_node, node);
>  		} else {
> -			return -EINVAL;
> +			goto err;
>  		}
>  	}
> -DONE:
>  	return 0;
> +err:
> +	rte_free(parser_node);
> +	PMD_DRV_LOG(ERR, "%s failed.", __func__);
> +	return -EINVAL;
>  }
> 
>  void
> --
> 2.33.1