|SUCCESS| [v4, 11/11] test/bpf: add tests for error handling contracts

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/164200

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-05-20

git commit-id: (not provided in patch format, series only)
Author: Marat Khalili <marat.khalili at huawei.com>
Subject: [PATCH v4 01/11] bpf: make logging prefixes more consistent

This patch series introduces an extensible BPF loading API and several
enhancements to the BPF library. The commit under review standardizes
logging prefixes across lib/bpf by introducing a new macro for
function-name-prefixed logging.

> diff --git a/lib/bpf/bpf_convert.c b/lib/bpf/bpf_convert.c
> index 86e703299d05..953ca80670c4 100644
> --- a/lib/bpf/bpf_convert.c
> +++ b/lib/bpf/bpf_convert.c
> @@ -247,8 +247,8 @@ static int bpf_convert_filter(const struct bpf_insn *prog, size_t len,

[ ... ]

> diff --git a/lib/bpf/bpf_impl.h b/lib/bpf/bpf_impl.h
> index f5fa22098489..fb5ec3c4d65f 100644
> --- a/lib/bpf/bpf_impl.h
> +++ b/lib/bpf/bpf_impl.h
> @@ -32,6 +32,9 @@ extern int rte_bpf_logtype;
>  #define RTE_BPF_LOG_LINE(lvl, ...) \
>  	RTE_LOG_LINE(lvl, BPF, __VA_ARGS__)
>  
> +#define RTE_BPF_LOG_FUNC_LINE(lvl, fmt, ...) \
> +	RTE_LOG_LINE(lvl, BPF, "%s(): " fmt, __func__, ##__VA_ARGS__)

In the macro definition RTE_BPF_LOG_FUNC_LINE, can the string
concatenation "%s(): " fmt work correctly in all cases when fmt contains
format specifiers? This appears safe, but should be verified to ensure
no edge cases exist with the preprocessor string concatenation when fmt
includes adjacent string literal tokens.

> diff --git a/lib/bpf/bpf_jit_arm64.c b/lib/bpf/bpf_jit_arm64.c
> index a04ef33a9c88..4bbb97da1b89 100644
> --- a/lib/bpf/bpf_jit_arm64.c
> +++ b/lib/bpf/bpf_jit_arm64.c
> @@ -98,8 +98,8 @@ check_invalid_args(struct a64_jit_ctx *ctx, uint32_t limit)
>  
>  	for (idx = 0; idx < limit; idx++) {
>  		if (rte_le_to_cpu_32(ctx->ins[idx]) == A64_INVALID_OP_CODE) {
> -			RTE_BPF_LOG_LINE(ERR,
> -				"%s: invalid opcode at %u;", __func__, idx);
> +			RTE_BPF_LOG_FUNC_LINE(ERR,
> +				"invalid opcode at %u;", idx);

This change is consistent with the stated goal of improving prefix
consistency.

> diff --git a/lib/bpf/bpf_load_elf.c b/lib/bpf/bpf_load_elf.c
> index 1d30ba17e25d..2390823cbf30 100644
> --- a/lib/bpf/bpf_load_elf.c
> +++ b/lib/bpf/bpf_load_elf.c
> @@ -122,7 +122,7 @@ check_elf_header(const Elf64_Ehdr *eh)
>  		err = "unexpected machine type";
>  
>  	if (err != NULL) {
> -		RTE_BPF_LOG_LINE(ERR, "%s(): %s", __func__, err);
> +		RTE_BPF_LOG_FUNC_LINE(ERR, "%s", err);

This correctly replaces the explicit "%s(): %s" pattern with the new
macro while keeping the err message parameter.

> diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
> index e8dbec282779..a7f4f576c9d6 100644
> --- a/lib/bpf/bpf_validate.c
> +++ b/lib/bpf/bpf_validate.c
> @@ -1838,16 +1838,16 @@ add_edge(struct bpf_verifier *bvf, struct inst_node *node, uint32_t nidx)

[ ... ]

> @@ -2005,8 +2005,7 @@ validate(struct bpf_verifier *bvf)
>  
>  		err = check_syntax(ins);
>  		if (err != 0) {
> -			RTE_BPF_LOG_LINE(ERR, "%s: %s at pc: %u",
> -				__func__, err, i);
> +			RTE_BPF_LOG_FUNC_LINE(ERR, "%s at pc: %u", err, i);

All the replacements here look correct. The macro handles prefixing with
function name automatically.

Overall this patch appears to be a straightforward refactor improving
logging consistency. No resource leaks, use-after-free, or race
conditions are introduced by these changes. The coding style change is
localized and correct.

However, the following point should be clarified:

In the macro definition, the string literal concatenation "%s(): " fmt
relies on the C preprocessor feature where adjacent string literals are
concatenated. Can you verify that when the fmt argument itself is a
string literal this does not create issues in all compiler versions
DPDK supports?