[v4,1/2] net/ice: fix DCF ACL flow engine
Checks
Commit Message
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(-)
Comments
> -----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
@@ -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!");
+ return ret;
}
static void
@@ -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,
+ * 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