Re: [dpdk-dev] [PATCH] acl: fix classify with no rules in ctx

Re: [dpdk-dev] [PATCH] acl: fix classify with no rules in ctx

[Ananyev, Konstantin]

Hi Filip,

> 
> Executing classify with no context or a context without any rules added
> to it leads to a crash. This patch introduces checks for acl to ensure
> possibility to defer creation of rules until a later point. A new
> return value is introduced to allow the user of the API to decide how
> to treat the specific scenario when the context is "empty" of rules.

I don't think the patch is really needed.
classify() is a data-path function, so obviously we try to avoid extra checks here.
from rte_acl.h:
"... Note, that it is a caller's responsibility to ensure that input parameters
 * are valid and point to correct memory locations."
Again if ctx->num_rules != 0 it doesn't guarantee that build already completed
successfully and it is ok to call classify(). 
If you need - you can always do these checks just before calling rte_acl_classify(). 
Konstantin

> 
> Signed-off-by: Filip Pudak <filip.pudak@ericsson.com>
> ---
>  lib/librte_acl/rte_acl.c | 10 ++++++++--
>  lib/librte_acl/rte_acl.h |  2 ++
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c
> index 2f1243c..4a6a894 100644
> --- a/lib/librte_acl/rte_acl.c
> +++ b/lib/librte_acl/rte_acl.c
> @@ -121,10 +121,16 @@ rte_acl_classify_alg(const struct rte_acl_ctx *ctx, const uint8_t **data,
>  	uint32_t *results, uint32_t num, uint32_t categories,
>  	enum rte_acl_classify_alg alg)
>  {
> -	if (categories != 1 &&
> -			((RTE_ACL_RESULTS_MULTIPLIER - 1) & categories) != 0)
> +	if (ctx == NULL || (categories != 1 &&
> +			((RTE_ACL_RESULTS_MULTIPLIER - 1) & categories) != 0))
>  		return -EINVAL;
> 
> +        if (ctx->num_rules == 0) {
> +                RTE_LOG(INFO, ACL, "Cannot perform acl classify with no rules added!\n");
> +                memset(results, 0, categories * num * sizeof(uint32_t));
> +                return -ENODATA;
> +        }
> +
>  	return classify_fns[alg](ctx, data, results, num, categories);
>  }
> 
> diff --git a/lib/librte_acl/rte_acl.h b/lib/librte_acl/rte_acl.h
> index aa22e70..3e56fe5 100644
> --- a/lib/librte_acl/rte_acl.h
> +++ b/lib/librte_acl/rte_acl.h
> @@ -272,6 +272,7 @@ enum rte_acl_classify_alg {
>   * @return
>   *   zero on successful completion.
>   *   -EINVAL for incorrect arguments.
> + *   -ENODATA if the context does not contain any rules to classify against.
>   */
>  extern int
>  rte_acl_classify(const struct rte_acl_ctx *ctx,
> @@ -312,6 +313,7 @@ rte_acl_classify(const struct rte_acl_ctx *ctx,
>   * @return
>   *   zero on successful completion.
>   *   -EINVAL for incorrect arguments.
> + *   -ENODATA if the context does not contain any rules to classify against.
>   */
>  extern int
>  rte_acl_classify_alg(const struct rte_acl_ctx *ctx,
> --
> 2.9.0