Re: [PATCH v5] acl: support custom memory allocators in rte_acl_build

[fengchengwen <fengchengwen@huawei.com>]

commit header could simply as: acl: support custom memory allocator

On 12/1/2025 8:45 PM, mannywang(王永峰) wrote:
> Allow users to provide custom
> memory allocation callbacks for runtime memory in rte_acl_ctx, via
> struct rte_acl_mem_cb.
> 
> Key changes:
> - Added struct rte_acl_mem_cb with zalloc, free, and udata.
> - Added rte_acl_set_mem_cb() / rte_acl_get_mem_cb() to set/get callbacks.
> - Default allocation uses existing rte_zmalloc_socket/rte_free.
> - Modified ACL code to call callbacks for runtime allocations instead
>   of rte_zmalloc_socket/rte_free directly.
> 
> v5:
> - Remove temporary memory allocation callback for build stage.
> - Introduce new API (rte_acl_set_mem_cb / rte_acl_get_mem_cb) instead of
>   modifying existing rte_acl_config to preserve ABI compatibility.
> 
> Signed-off-by: YongFeng Wang <mannywang@tencent.com>
> ---
>  app/test/test_acl.c | 121 ++++++++++++++++++++++++++++++++++++++++++++
>  lib/acl/acl.h       |   1 +
>  lib/acl/acl_bld.c   |   2 +-
>  lib/acl/acl_gen.c   |   4 +-
>  lib/acl/rte_acl.c   |  45 +++++++++++++++-
>  lib/acl/rte_acl.h   |  42 +++++++++++++++
>  6 files changed, 211 insertions(+), 4 deletions(-)
> 
> diff --git a/app/test/test_acl.c b/app/test/test_acl.c
> index 43d13b5b0f..930fdf8362 100644
> --- a/app/test/test_acl.c
> +++ b/app/test/test_acl.c
> @@ -1721,6 +1721,125 @@ test_u32_range(void)
>  	return rc;
>  }
>  
> +struct acl_ctx_wrapper_t {

no need add post-fix _t

> +	struct rte_acl_ctx *ctx;
> +	void *running_buf;
> +	bool running_buf_using;
> +};
> +
> +#define ACL_RUNNING_BUF_SIZE (10 * 1024 * 1024)
> +
> +static void *running_alloc(void *udata, char *name, size_t size,
> +	size_t align, int32_t socket_id)
> +{
> +	(void)align;
> +	(void)name;
> +	(void)socket_id;
> +	if (size > ACL_RUNNING_BUF_SIZE)
> +		return NULL;
> +	struct acl_ctx_wrapper_t *gwlb_acl_ctx = (struct acl_ctx_wrapper_t *)udata;

gwlb - what the meaning? suggest simple acl_ctx or just ctx.

> +	if (gwlb_acl_ctx->running_buf_using)
> +		return NULL;
> +	printf("running memory alloc for acl context, size=%zu, pointer=%p\n",
> +		size,
> +		gwlb_acl_ctx->running_buf);
> +	memset(gwlb_acl_ctx->running_buf, 0, size);
> +	gwlb_acl_ctx->running_buf_using = true;

this allocator don't support conti alloc, it will has dependent for acl-library,
how about use Dmitry Kozlyuk's comment for [PATCH v3], that is use rte_malloc_heap_create

> +	return gwlb_acl_ctx->running_buf;
> +}
> +
> +static void running_free(void *udata, void *ptr)
> +{
> +	if (!ptr)
> +		return;
> +	struct acl_ctx_wrapper_t *gwlb_acl_ctx = (struct acl_ctx_wrapper_t *)udata;
> +	printf("running memory free, pointer=%p\n", ptr);
> +	gwlb_acl_ctx->running_buf_using = false;
> +}
> +
> +static int
> +test_mem_cb(void)
> +{
> +	int i, ret;
> +	struct acl_ctx_wrapper_t g_acl_ctx_wrapper;

no need add g_acl_ prefix, and suggest zero default (struct acl_ctx_wrapper ctx_wrap = {0})

> +	g_acl_ctx_wrapper.ctx = rte_acl_create(&acl_param);
> +	if (g_acl_ctx_wrapper.ctx == NULL) {
> +		printf("Line %i: Error creating ACL context!\n", __LINE__);
> +		return -1;
> +	}
> +	g_acl_ctx_wrapper.running_buf = rte_zmalloc_socket(
> +		"test_acl",
> +		ACL_RUNNING_BUF_SIZE,
> +		RTE_CACHE_LINE_SIZE,
> +		SOCKET_ID_ANY);
> +	if (!g_acl_ctx_wrapper.running_buf) {
> +		printf("Line %i: Error allocing running buf for acl context!\n", __LINE__);

please add resource rollback, the same as following.

> +		return 1;
> +	}
> +	g_acl_ctx_wrapper.running_buf_using = false;
> +
> +	struct rte_acl_mem_cb mcb = {
> +		.zalloc = running_alloc,
> +		.free = running_free,
> +		.udata = &g_acl_ctx_wrapper
> +	};
> +	ret = rte_acl_set_mem_cb(g_acl_ctx_wrapper.ctx, &mcb);
> +	if (ret) {

please use explicit exp, e.g. if (ret != 0), the same as other place.

> +		printf("Line %i: Error set mem cb for acl context!\n", __LINE__);
> +		return 1;
> +	}
> +	struct rte_acl_mem_cb new_mcb;
> +	memset(&new_mcb, 0, sizeof(struct rte_acl_mem_cb));
> +	ret = rte_acl_get_mem_cb(g_acl_ctx_wrapper.ctx, &new_mcb);
> +	if (ret) {
> +		printf("Line %i: Error get mem cb for acl context!\n", __LINE__);
> +		return 1;
> +	}
> +	if (memcmp(&mcb, &new_mcb, sizeof(struct rte_acl_mem_cb)) != 0) {
> +		printf("Line %i: Error get mem cb for acl context!\n", __LINE__);
> +		return 1;
> +	}
> +	ret = 0;
> +	for (i = 0; i != TEST_CLASSIFY_ITER; i++) {

why not i < TEST_CLASSIFY_ITER ?

> +
> +		if ((i & 1) == 0)
> +			rte_acl_reset(g_acl_ctx_wrapper.ctx);
> +		else
> +			rte_acl_reset_rules(g_acl_ctx_wrapper.ctx);
> +
> +		ret = test_classify_buid(g_acl_ctx_wrapper.ctx, acl_test_rules,
> +			RTE_DIM(acl_test_rules));
> +		if (ret != 0) {
> +			printf("Line %i, iter: %d: "
> +				"Adding rules to ACL context failed!\n",

no need extra line for log-string.

> +				__LINE__, i);
> +			break;
> +		}
> +
> +		ret = test_classify_run(g_acl_ctx_wrapper.ctx, acl_test_data,
> +			RTE_DIM(acl_test_data));
> +		if (ret != 0) {
> +			printf("Line %i, iter: %d: %s failed!\n",
> +				__LINE__, i, __func__);
> +			break;
> +		}
> +
> +		/* reset rules and make sure that classify still works ok. */
> +		rte_acl_reset_rules(g_acl_ctx_wrapper.ctx);
> +		ret = test_classify_run(g_acl_ctx_wrapper.ctx, acl_test_data,
> +			RTE_DIM(acl_test_data));
> +		if (ret != 0) {
> +			printf("Line %i, iter: %d: %s failed!\n",
> +				__LINE__, i, __func__);
> +			break;
> +		}
> +	}
> +
> +	rte_acl_free(g_acl_ctx_wrapper.ctx);
> +	rte_free(g_acl_ctx_wrapper.running_buf);
> +	return ret;
> +}
> +
>  static int
>  test_acl(void)
>  {
> @@ -1742,6 +1861,8 @@ test_acl(void)
>  		return -1;
>  	if (test_u32_range() < 0)
>  		return -1;
> +	if (test_mem_cb() < 0)
> +		return -1;
>  
>  	return 0;
>  }
> diff --git a/lib/acl/acl.h b/lib/acl/acl.h
> index c8e4e72fab..3acfc0cb9f 100644
> --- a/lib/acl/acl.h
> +++ b/lib/acl/acl.h
> @@ -174,6 +174,7 @@ struct rte_acl_ctx {
>  	uint32_t            max_rules;
>  	uint32_t            rule_sz;
>  	uint32_t            num_rules;
> +	struct rte_acl_mem_cb mem_cb;

why not place near *mem/mem_sz field ?

>  	uint32_t            num_categories;
>  	uint32_t            num_tries;
>  	uint32_t            match_index;
> diff --git a/lib/acl/acl_bld.c b/lib/acl/acl_bld.c
> index 7056b1c117..e3d342bd79 100644
> --- a/lib/acl/acl_bld.c
> +++ b/lib/acl/acl_bld.c
> @@ -779,7 +779,7 @@ acl_merge_trie(struct acl_build_context *context,
>  static void
>  acl_build_reset(struct rte_acl_ctx *ctx)
>  {
> -	rte_free(ctx->mem);
> +	ctx->mem_cb.free(ctx->mem_cb.udata, ctx->mem);
>  	memset(&ctx->num_categories, 0,
>  		sizeof(*ctx) - offsetof(struct rte_acl_ctx, num_categories));
>  }
> diff --git a/lib/acl/acl_gen.c b/lib/acl/acl_gen.c
> index 3c53d24056..f482317884 100644
> --- a/lib/acl/acl_gen.c
> +++ b/lib/acl/acl_gen.c
> @@ -478,8 +478,8 @@ rte_acl_gen(struct rte_acl_ctx *ctx, struct rte_acl_trie *trie,
>  		return -ERANGE;
>  	}
>  
> -	mem = rte_zmalloc_socket(ctx->name, total_size, RTE_CACHE_LINE_SIZE,
> -			ctx->socket_id);
> +	mem = ctx->mem_cb.zalloc(ctx->mem_cb.udata, ctx->name, total_size,
> +			RTE_CACHE_LINE_SIZE, ctx->socket_id);
>  	if (mem == NULL) {
>  		ACL_LOG(ERR,
>  			"allocation of %zu bytes on socket %d for %s failed",
> diff --git a/lib/acl/rte_acl.c b/lib/acl/rte_acl.c
> index 8c0ca29618..3c1a118c46 100644
> --- a/lib/acl/rte_acl.c
> +++ b/lib/acl/rte_acl.c
> @@ -264,6 +264,20 @@ acl_get_best_alg(void)
>  	return alg[i];
>  }
>  
> +static void *
> +zalloc_dft(void *udata, char *name, size_t size, size_t align, int32_t socket_id)

how about acl_mem_default_zalloc?

> +{
> +	(void)udata;
> +	return rte_zmalloc_socket(name, size, align, socket_id);
> +}
> +
> +static void
> +free_dft(void *udata, void *ptr)

how about acl_mem_default_free?

> +{
> +	(void)udata;
> +	rte_free(ptr);
> +}
> +
>  RTE_EXPORT_SYMBOL(rte_acl_set_ctx_classify)
>  extern int
>  rte_acl_set_ctx_classify(struct rte_acl_ctx *ctx, enum rte_acl_classify_alg alg)
> @@ -362,7 +376,7 @@ rte_acl_free(struct rte_acl_ctx *ctx)
>  
>  	rte_mcfg_tailq_write_unlock();
>  
> -	rte_free(ctx->mem);
> +	ctx->mem_cb.free(ctx->mem_cb.udata, ctx->mem);
>  	rte_free(ctx);
>  	rte_free(te);
>  }
> @@ -425,6 +439,9 @@ rte_acl_create(const struct rte_acl_param *param)
>  		ctx->rule_sz = param->rule_size;
>  		ctx->socket_id = param->socket_id;
>  		ctx->alg = acl_get_best_alg();
> +		ctx->mem_cb.zalloc = zalloc_dft;
> +		ctx->mem_cb.free = free_dft;
> +		ctx->mem_cb.udata = NULL;
>  		strlcpy(ctx->name, param->name, sizeof(ctx->name));
>  
>  		te->data = (void *) ctx;
> @@ -555,3 +572,29 @@ rte_acl_list_dump(void)
>  	}
>  	rte_mcfg_tailq_read_unlock();
>  }
> +
> +/*
> + * Set memory allocation callbacks for a given ACL context.
> + */
> +RTE_EXPORT_SYMBOL(rte_acl_set_mem_cb)

Should be RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_acl_set_mem_cb, 26.03)

> +int
> +rte_acl_set_mem_cb(struct rte_acl_ctx *acl, const struct rte_acl_mem_cb *mcb)
> +{
> +	if (!acl || !mcb || !mcb->zalloc || !mcb->free)

suggest acl == NULL || mcb == NULL and ...

> +		return -EINVAL;
> +	memcpy(&acl->mem_cb, mcb, sizeof(struct rte_acl_mem_cb));
> +	return 0;
> +}
> +
> +/*
> + * Retrieve the memory allocation callbacks assigned to the ACL context.
> + */
> +RTE_EXPORT_SYMBOL(rte_acl_get_mem_cb)

Should be RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_acl_get_mem_cb, 26.03)

> +int
> +rte_acl_get_mem_cb(const struct rte_acl_ctx *acl, struct rte_acl_mem_cb *mcb)
> +{
> +	if (!acl || !mcb)

suggest acl == NULL || mcb == NULL

> +		return -EINVAL;
> +	memcpy(mcb, &acl->mem_cb, sizeof(struct rte_acl_mem_cb));
> +	return 0;
> +}
> diff --git a/lib/acl/rte_acl.h b/lib/acl/rte_acl.h
> index 95354cabb8..e3273556e2 100644
> --- a/lib/acl/rte_acl.h
> +++ b/lib/acl/rte_acl.h
> @@ -136,6 +136,48 @@ struct rte_acl_param {
>  /** @internal opaque ACL handle */
>  struct rte_acl_ctx;
>  
> +/**
> + * Memory allocation callbacks for ACL runtime.
> + */
> +struct rte_acl_mem_cb {

How about rte_acl_mem_hook ?
I think hook is better suited to this semantics.

> +	/** Allocate zero-initialized memory used during runtime. */
> +	void *(*zalloc)(void *udata, char *name, size_t size, size_t align, int32_t socket_id);

Suggest move udata as the last one parameter.

> +
> +	/** Free memory previously allocated by zalloc(). */
> +	void (*free)(void *udata, void *ptr);

Suggest move udata as the last one parameter.

> +
> +	/** User-provided context passed to allocation/free callbacks. */
> +	void *udata;
> +};
> +
> +/**
> + * Set memory allocation callbacks for a given ACL context.
> + *
> + * @param acl
> + *   The ACL context.
> + * @param mcb
> + *   Pointer to the memory callback structure

Suggest add one note about when could invoke this API.

> + *
> + * @return
> + *   0 on success.
> + *   -EINVAL if parameters are invalid.
> + */

should add __rte_experimental

> +int rte_acl_set_mem_cb(struct rte_acl_ctx *acl, const struct rte_acl_mem_cb *mcb);
> +
> +/**
> + * Retrieve the memory allocation callbacks assigned to the ACL context.
> + *
> + * @param acl
> + *   The ACL context.
> + * @param mcb
> + *   Output location for the current memory callback structure
> + *
> + * @return
> + *   0 on success.
> + *   -EINVAL if parameters are invalid.
> + */

should add __rte_experimental

> +int rte_acl_get_mem_cb(const struct rte_acl_ctx *acl, struct rte_acl_mem_cb *mcb);
> +
>  /**
>   * De-allocate all memory used by ACL context.
>   *

please add descriptor section for packet_classif_access_ctrl.rst and release note.