On Mon, Oct 20, 2025 at 5:32 AM Mehdi Ben Hadj Khelifa mehdi.benhadjkhelifa@gmail.com wrote:

This is a follow up patch for commit 495d2d8133fd("selftests/bpf: Attempt to build BPF programs with -Wsign-compare") from Alexei Starovoitov[1] to be able to enable -Wsign-compare C compilation flag for clang since -Wall doesn't add it and BPF programs are built with clang.This has the benefit to catch problematic comparisons in future tests as quoted from the commit message:" int i = -1; unsigned int j = 1; if (i < j) // this is false.

long i = -1; unsigned int j = 1; if (i < j) // this is true.

C standard for reference:

• If either operand is unsigned long the other shall be converted to

unsigned long.

• Otherwise, if one operand is a long int and the other unsigned int,

then if a long int can represent all the values of an unsigned int, the unsigned int shall be converted to a long int; otherwise both operands shall be converted to unsigned long int.

• Otherwise, if either operand is long, the other shall be

converted to long.

• Otherwise, if either operand is unsigned, the other shall be

converted to unsigned.

Unfortunately clang's -Wsign-compare is very noisy. It complains about (s32)a == (u32)b which is safe and doen't have surprising behavior."

This specific patch supresses the following warnings when -Wsign-compare is enabled:

1 warning generated.

progs/bpf_iter_bpf_percpu_array_map.c:35:16: warning: comparison of integers of different signs: 'int' and 'const volatile __u32' (aka 'const volatile unsigned int') [-Wsign-compare] 35 | for (i = 0; i < num_cpus; i++) { | ~ ^ ~~~~~~~~

1 warning generated.

progs/bpf_qdisc_fifo.c:93:2: warning: comparison of integers of different signs: 'int' and '__u32' (aka 'unsigned int') [-Wsign-compare] 93 | bpf_for(i, 0, sch->q.qlen) { | ^ ~ ~~~~~~~~~~~

Should be noted that many more similar changes are still needed in order to be able to enable the -Wsign-compare flag since -Werror is enabled and would cause compilation of bpf selftests to fail.

[1]. Link:https://github.com/torvalds/linux/commit/495d2d8133fd1407519170a5238f455abbd...

Signed-off-by: Mehdi Ben Hadj Khelifa mehdi.benhadjkhelifa@gmail.com

Changelog:

Changes from v3:

-Downsized the patch as suggested by vivek yadav[2].

-Changed the commit message as suggested by Daniel Borkmann[3].

Link:https://lore.kernel.org/all/20250925103559.14876-1-mehdi.benhadjkhelifa@gmai...

Changes from v2:

-Split up the patch into a patch series as suggested by vivek

-Include only changes to variable types with no casting by my mentor david

-Removed the -Wsign-compare in Makefile to avoid compilation errors until adding casting for rest of comparisons.

Link:https://lore.kernel.org/bpf/20250924195731.6374-1-mehdi.benhadjkhelifa@gmail...

Changes from v1:

• Fix CI failed builds where it failed due to do missing .c and

.h files in my patch for working in mainline.

Link:https://lore.kernel.org/bpf/20250924162408.815137-1-mehdi.benhadjkhelifa@gma...

.../testing/selftests/bpf/progs/bpf_iter_bpf_percpu_array_map.c | 2 +- tools/testing/selftests/bpf/progs/bpf_qdisc_fifo.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_bpf_percpu_array_map.c b/tools/testing/selftests/bpf/progs/bpf_iter_bpf_percpu_array_map.c index 9fdea8cd4c6f..0baf00463f35 100644 --- a/tools/testing/selftests/bpf/progs/bpf_iter_bpf_percpu_array_map.c +++ b/tools/testing/selftests/bpf/progs/bpf_iter_bpf_percpu_array_map.c @@ -24,7 +24,7 @@ int dump_bpf_percpu_array_map(struct bpf_iter__bpf_map_elem *ctx) __u32 *key = ctx->key; void *pptr = ctx->value; __u32 step;

•   int i;
  

•   __u32 i;
  
    if (key == (void *)0 || pptr == (void *)0)
            return 0;
  

diff --git a/tools/testing/selftests/bpf/progs/bpf_qdisc_fifo.c b/tools/testing/selftests/bpf/progs/bpf_qdisc_fifo.c index 1de2be3e370b..7a639dcb23a9 100644 --- a/tools/testing/selftests/bpf/progs/bpf_qdisc_fifo.c +++ b/tools/testing/selftests/bpf/progs/bpf_qdisc_fifo.c @@ -88,7 +88,7 @@ void BPF_PROG(bpf_fifo_reset, struct Qdisc *sch) { struct bpf_list_node *node; struct skb_node *skbn;

•   int i;
  

•   __u32 i;
  
  

this is wrong, i is coming from bpf_for() and is signed int

I'd suggest dropping this patch altogether, it's not helpful and doesn't fix any real bugs.

pw-bot: cr

    bpf_for(i, 0, sch->q.qlen) {
            struct sk_buff *skb = NULL;

-- 2.51.1.dirty