* [PATCH v3 1/3] selftests/bpf: Prepare to add -Wsign-compare for bpf tests
2025-09-25 10:35 [PATCH v3 0/3] selftests/bpf: Prepare to add -Wsign-compare for bpf selftests Mehdi Ben Hadj Khelifa
@ 2025-09-25 10:35 ` Mehdi Ben Hadj Khelifa
2025-09-25 10:35 ` [PATCH v3 2/3] " Mehdi Ben Hadj Khelifa
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Mehdi Ben Hadj Khelifa @ 2025-09-25 10:35 UTC (permalink / raw)
To: andrii, eddyz87, ast, daniel, martin.lau, song, yonghong.song,
john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah, matttbe,
martineau, geliang, davem, kuba, hawk, linux, ameryhung, toke,
houtao1, emil, yatsenko, isolodrai, a.s.protopopov, dxu, memxor,
vmalik, bigeasy, tj, gregkh, paul, bboscaccy, James.Bottomley,
mrpre, jakub
Cc: bpf, linux-kernel, linux-kselftest, netdev, mptcp,
linux-kernel-mentees, skhan, david.hunter.linux,
Mehdi Ben Hadj Khelifa
-Change only variable types for correct sign comparisons
Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
---
tools/testing/selftests/bpf/progs/test_global_func11.c | 2 +-
tools/testing/selftests/bpf/progs/test_global_func12.c | 2 +-
tools/testing/selftests/bpf/progs/test_global_func13.c | 2 +-
tools/testing/selftests/bpf/progs/test_global_func9.c | 2 +-
tools/testing/selftests/bpf/progs/test_map_init.c | 2 +-
tools/testing/selftests/bpf/progs/test_parse_tcp_hdr_opt.c | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/test_global_func11.c b/tools/testing/selftests/bpf/progs/test_global_func11.c
index 283e036dc401..2ad72bf0e07b 100644
--- a/tools/testing/selftests/bpf/progs/test_global_func11.c
+++ b/tools/testing/selftests/bpf/progs/test_global_func11.c
@@ -5,7 +5,7 @@
#include "bpf_misc.h"
struct S {
- int x;
+ __u32 x;
};
__noinline int foo(const struct S *s)
diff --git a/tools/testing/selftests/bpf/progs/test_global_func12.c b/tools/testing/selftests/bpf/progs/test_global_func12.c
index 6e03d42519a6..53eab8ec6772 100644
--- a/tools/testing/selftests/bpf/progs/test_global_func12.c
+++ b/tools/testing/selftests/bpf/progs/test_global_func12.c
@@ -5,7 +5,7 @@
#include "bpf_misc.h"
struct S {
- int x;
+ __u32 x;
};
__noinline int foo(const struct S *s)
diff --git a/tools/testing/selftests/bpf/progs/test_global_func13.c b/tools/testing/selftests/bpf/progs/test_global_func13.c
index 02ea80da75b5..c4afdfc9d92e 100644
--- a/tools/testing/selftests/bpf/progs/test_global_func13.c
+++ b/tools/testing/selftests/bpf/progs/test_global_func13.c
@@ -5,7 +5,7 @@
#include "bpf_misc.h"
struct S {
- int x;
+ __u32 x;
};
__noinline int foo(const struct S *s)
diff --git a/tools/testing/selftests/bpf/progs/test_global_func9.c b/tools/testing/selftests/bpf/progs/test_global_func9.c
index 1f2cb0159b8d..9138d9bd08fc 100644
--- a/tools/testing/selftests/bpf/progs/test_global_func9.c
+++ b/tools/testing/selftests/bpf/progs/test_global_func9.c
@@ -5,7 +5,7 @@
#include "bpf_misc.h"
struct S {
- int x;
+ __u32 x;
};
struct C {
diff --git a/tools/testing/selftests/bpf/progs/test_map_init.c b/tools/testing/selftests/bpf/progs/test_map_init.c
index c89d28ead673..311e6ac64588 100644
--- a/tools/testing/selftests/bpf/progs/test_map_init.c
+++ b/tools/testing/selftests/bpf/progs/test_map_init.c
@@ -22,7 +22,7 @@ int sysenter_getpgid(const void *ctx)
/* Just do it for once, when called from our own test prog. This
* ensures the map value is only updated for a single CPU.
*/
- int cur_pid = bpf_get_current_pid_tgid() >> 32;
+ __u32 cur_pid = bpf_get_current_pid_tgid() >> 32;
if (cur_pid == inPid)
bpf_map_update_elem(&hashmap1, &inKey, &inValue, BPF_NOEXIST);
diff --git a/tools/testing/selftests/bpf/progs/test_parse_tcp_hdr_opt.c b/tools/testing/selftests/bpf/progs/test_parse_tcp_hdr_opt.c
index d9b2ba7ac340..4b8ab8716246 100644
--- a/tools/testing/selftests/bpf/progs/test_parse_tcp_hdr_opt.c
+++ b/tools/testing/selftests/bpf/progs/test_parse_tcp_hdr_opt.c
@@ -102,7 +102,7 @@ int xdp_ingress_v6(struct xdp_md *xdp)
opt_state.byte_offset = sizeof(struct tcphdr) + tcp_offset;
/* max number of bytes of options in tcp header is 40 bytes */
- for (int i = 0; i < tcp_hdr_opt_max_opt_checks; i++) {
+ for (__u32 i = 0; i < tcp_hdr_opt_max_opt_checks; i++) {
err = parse_hdr_opt(xdp, &opt_state);
if (err || !opt_state.hdr_bytes_remaining)
--
2.51.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 2/3] selftests/bpf: Prepare to add -Wsign-compare for bpf tests
2025-09-25 10:35 [PATCH v3 0/3] selftests/bpf: Prepare to add -Wsign-compare for bpf selftests Mehdi Ben Hadj Khelifa
2025-09-25 10:35 ` [PATCH v3 1/3] selftests/bpf: Prepare to add -Wsign-compare for bpf tests Mehdi Ben Hadj Khelifa
@ 2025-09-25 10:35 ` Mehdi Ben Hadj Khelifa
2025-09-25 10:35 ` [PATCH v3 3/3] " Mehdi Ben Hadj Khelifa
2025-09-25 23:32 ` [PATCH v3 0/3] selftests/bpf: Prepare to add -Wsign-compare for bpf selftests Andrii Nakryiko
3 siblings, 0 replies; 9+ messages in thread
From: Mehdi Ben Hadj Khelifa @ 2025-09-25 10:35 UTC (permalink / raw)
To: andrii, eddyz87, ast, daniel, martin.lau, song, yonghong.song,
john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah, matttbe,
martineau, geliang, davem, kuba, hawk, linux, ameryhung, toke,
houtao1, emil, yatsenko, isolodrai, a.s.protopopov, dxu, memxor,
vmalik, bigeasy, tj, gregkh, paul, bboscaccy, James.Bottomley,
mrpre, jakub
Cc: bpf, linux-kernel, linux-kselftest, netdev, mptcp,
linux-kernel-mentees, skhan, david.hunter.linux,
Mehdi Ben Hadj Khelifa
-Change only variable types for correct sign comparisons
Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
---
.../testing/selftests/bpf/progs/test_parse_tcp_hdr_opt_dynptr.c | 2 +-
tools/testing/selftests/bpf/progs/test_skb_ctx.c | 2 +-
tools/testing/selftests/bpf/progs/test_snprintf.c | 2 +-
tools/testing/selftests/bpf/progs/test_sockmap_strp.c | 2 +-
tools/testing/selftests/bpf/progs/test_tc_tunnel.c | 2 +-
tools/testing/selftests/bpf/progs/test_xdp.c | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/test_parse_tcp_hdr_opt_dynptr.c b/tools/testing/selftests/bpf/progs/test_parse_tcp_hdr_opt_dynptr.c
index dc6e43bc6a62..bf3ac5c2938c 100644
--- a/tools/testing/selftests/bpf/progs/test_parse_tcp_hdr_opt_dynptr.c
+++ b/tools/testing/selftests/bpf/progs/test_parse_tcp_hdr_opt_dynptr.c
@@ -100,7 +100,7 @@ int xdp_ingress_v6(struct xdp_md *xdp)
off += sizeof(struct tcphdr);
/* max number of bytes of options in tcp header is 40 bytes */
- for (int i = 0; i < tcp_hdr_opt_max_opt_checks; i++) {
+ for (__u32 i = 0; i < tcp_hdr_opt_max_opt_checks; i++) {
err = parse_hdr_opt(&ptr, &off, &hdr_bytes_remaining, &server_id);
if (err || !hdr_bytes_remaining)
diff --git a/tools/testing/selftests/bpf/progs/test_skb_ctx.c b/tools/testing/selftests/bpf/progs/test_skb_ctx.c
index a724a70c6700..7939a2edc414 100644
--- a/tools/testing/selftests/bpf/progs/test_skb_ctx.c
+++ b/tools/testing/selftests/bpf/progs/test_skb_ctx.c
@@ -11,7 +11,7 @@ SEC("tc")
int process(struct __sk_buff *skb)
{
__pragma_loop_unroll_full
- for (int i = 0; i < 5; i++) {
+ for (__u32 i = 0; i < 5; i++) {
if (skb->cb[i] != i + 1)
return 1;
skb->cb[i]++;
diff --git a/tools/testing/selftests/bpf/progs/test_snprintf.c b/tools/testing/selftests/bpf/progs/test_snprintf.c
index 8fda07544023..1aa4835da71a 100644
--- a/tools/testing/selftests/bpf/progs/test_snprintf.c
+++ b/tools/testing/selftests/bpf/progs/test_snprintf.c
@@ -4,7 +4,7 @@
#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>
-__u32 pid = 0;
+int pid = 0;
char num_out[64] = {};
long num_ret = 0;
diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_strp.c b/tools/testing/selftests/bpf/progs/test_sockmap_strp.c
index dde3d5bec515..e9675c45d8ef 100644
--- a/tools/testing/selftests/bpf/progs/test_sockmap_strp.c
+++ b/tools/testing/selftests/bpf/progs/test_sockmap_strp.c
@@ -2,7 +2,7 @@
#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_endian.h>
-int verdict_max_size = 10000;
+__u32 verdict_max_size = 10000;
struct {
__uint(type, BPF_MAP_TYPE_SOCKMAP);
__uint(max_entries, 20);
diff --git a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
index 404124a93892..c7e2d4571a2b 100644
--- a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
+++ b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
@@ -80,7 +80,7 @@ static __always_inline void set_ipv4_csum(struct iphdr *iph)
{
__u16 *iph16 = (__u16 *)iph;
__u32 csum;
- int i;
+ size_t i;
iph->check = 0;
diff --git a/tools/testing/selftests/bpf/progs/test_xdp.c b/tools/testing/selftests/bpf/progs/test_xdp.c
index 8caf58be5818..ce2a9ae26088 100644
--- a/tools/testing/selftests/bpf/progs/test_xdp.c
+++ b/tools/testing/selftests/bpf/progs/test_xdp.c
@@ -89,7 +89,7 @@ static __always_inline int handle_ipv4(struct xdp_md *xdp)
struct vip vip = {};
int dport;
__u32 csum = 0;
- int i;
+ size_t i;
if (iph + 1 > data_end)
return XDP_DROP;
--
2.51.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 3/3] selftests/bpf: Prepare to add -Wsign-compare for bpf tests
2025-09-25 10:35 [PATCH v3 0/3] selftests/bpf: Prepare to add -Wsign-compare for bpf selftests Mehdi Ben Hadj Khelifa
2025-09-25 10:35 ` [PATCH v3 1/3] selftests/bpf: Prepare to add -Wsign-compare for bpf tests Mehdi Ben Hadj Khelifa
2025-09-25 10:35 ` [PATCH v3 2/3] " Mehdi Ben Hadj Khelifa
@ 2025-09-25 10:35 ` Mehdi Ben Hadj Khelifa
2025-09-25 15:04 ` Daniel Borkmann
2025-09-25 23:32 ` [PATCH v3 0/3] selftests/bpf: Prepare to add -Wsign-compare for bpf selftests Andrii Nakryiko
3 siblings, 1 reply; 9+ messages in thread
From: Mehdi Ben Hadj Khelifa @ 2025-09-25 10:35 UTC (permalink / raw)
To: andrii, eddyz87, ast, daniel, martin.lau, song, yonghong.song,
john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah, matttbe,
martineau, geliang, davem, kuba, hawk, linux, ameryhung, toke,
houtao1, emil, yatsenko, isolodrai, a.s.protopopov, dxu, memxor,
vmalik, bigeasy, tj, gregkh, paul, bboscaccy, James.Bottomley,
mrpre, jakub
Cc: bpf, linux-kernel, linux-kselftest, netdev, mptcp,
linux-kernel-mentees, skhan, david.hunter.linux,
Mehdi Ben Hadj Khelifa
-Change only variable types for correct sign comparisons
Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
---
tools/testing/selftests/bpf/progs/test_xdp_dynptr.c | 2 +-
tools/testing/selftests/bpf/progs/test_xdp_loop.c | 2 +-
tools/testing/selftests/bpf/progs/test_xdp_noinline.c | 4 ++--
tools/testing/selftests/bpf/progs/uprobe_multi.c | 4 ++--
.../selftests/bpf/progs/uprobe_multi_session_recursive.c | 5 +++--
.../selftests/bpf/progs/verifier_iterating_callbacks.c | 2 +-
6 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c b/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c
index 67a77944ef29..12ad0ec91021 100644
--- a/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c
+++ b/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c
@@ -89,7 +89,7 @@ static __always_inline int handle_ipv4(struct xdp_md *xdp, struct bpf_dynptr *xd
struct vip vip = {};
int dport;
__u32 csum = 0;
- int i;
+ size_t i;
__builtin_memset(eth_buffer, 0, sizeof(eth_buffer));
__builtin_memset(iph_buffer_tcp, 0, sizeof(iph_buffer_tcp));
diff --git a/tools/testing/selftests/bpf/progs/test_xdp_loop.c b/tools/testing/selftests/bpf/progs/test_xdp_loop.c
index 93267a68825b..e9b7bbff5c23 100644
--- a/tools/testing/selftests/bpf/progs/test_xdp_loop.c
+++ b/tools/testing/selftests/bpf/progs/test_xdp_loop.c
@@ -85,7 +85,7 @@ static __always_inline int handle_ipv4(struct xdp_md *xdp)
struct vip vip = {};
int dport;
__u32 csum = 0;
- int i;
+ size_t i;
if (iph + 1 > data_end)
return XDP_DROP;
diff --git a/tools/testing/selftests/bpf/progs/test_xdp_noinline.c b/tools/testing/selftests/bpf/progs/test_xdp_noinline.c
index fad94e41cef9..85ef3c0a3e20 100644
--- a/tools/testing/selftests/bpf/progs/test_xdp_noinline.c
+++ b/tools/testing/selftests/bpf/progs/test_xdp_noinline.c
@@ -372,7 +372,7 @@ bool encap_v4(struct xdp_md *xdp, struct ctl_value *cval,
next_iph_u16 = (__u16 *) iph;
__pragma_loop_unroll_full
- for (int i = 0; i < sizeof(struct iphdr) >> 1; i++)
+ for (size_t i = 0; i < sizeof(struct iphdr) >> 1; i++)
csum += *next_iph_u16++;
iph->check = ~((csum & 0xffff) + (csum >> 16));
if (bpf_xdp_adjust_head(xdp, (int)sizeof(struct iphdr)))
@@ -423,7 +423,7 @@ int send_icmp_reply(void *data, void *data_end)
iph->check = 0;
next_iph_u16 = (__u16 *) iph;
__pragma_loop_unroll_full
- for (int i = 0; i < sizeof(struct iphdr) >> 1; i++)
+ for (size_t i = 0; i < sizeof(struct iphdr) >> 1; i++)
csum += *next_iph_u16++;
iph->check = ~((csum & 0xffff) + (csum >> 16));
return swap_mac_and_send(data, data_end);
diff --git a/tools/testing/selftests/bpf/progs/uprobe_multi.c b/tools/testing/selftests/bpf/progs/uprobe_multi.c
index 44190efcdba2..f99957773c3a 100644
--- a/tools/testing/selftests/bpf/progs/uprobe_multi.c
+++ b/tools/testing/selftests/bpf/progs/uprobe_multi.c
@@ -20,13 +20,13 @@ __u64 uretprobe_multi_func_3_result = 0;
__u64 uprobe_multi_sleep_result = 0;
-int pid = 0;
+__u32 pid = 0;
int child_pid = 0;
int child_tid = 0;
int child_pid_usdt = 0;
int child_tid_usdt = 0;
-int expect_pid = 0;
+__u32 expect_pid = 0;
bool bad_pid_seen = false;
bool bad_pid_seen_usdt = false;
diff --git a/tools/testing/selftests/bpf/progs/uprobe_multi_session_recursive.c b/tools/testing/selftests/bpf/progs/uprobe_multi_session_recursive.c
index 8fbcd69fae22..017f1859ebe8 100644
--- a/tools/testing/selftests/bpf/progs/uprobe_multi_session_recursive.c
+++ b/tools/testing/selftests/bpf/progs/uprobe_multi_session_recursive.c
@@ -3,6 +3,7 @@
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>
#include <stdbool.h>
+#include <stddef.h>
#include "bpf_kfuncs.h"
#include "bpf_misc.h"
@@ -10,8 +11,8 @@ char _license[] SEC("license") = "GPL";
int pid = 0;
-int idx_entry = 0;
-int idx_return = 0;
+size_t idx_entry = 0;
+size_t idx_return = 0;
__u64 test_uprobe_cookie_entry[6];
__u64 test_uprobe_cookie_return[3];
diff --git a/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c b/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c
index 75dd922e4e9f..72f9f8c23c93 100644
--- a/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c
+++ b/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c
@@ -593,7 +593,7 @@ int loop_inside_iter_volatile_limit(const void *ctx)
{
struct bpf_iter_num it;
int *v, sum = 0;
- __u64 i = 0;
+ __s32 i = 0;
bpf_iter_num_new(&it, 0, ARR2_SZ);
while ((v = bpf_iter_num_next(&it))) {
--
2.51.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3 3/3] selftests/bpf: Prepare to add -Wsign-compare for bpf tests
2025-09-25 10:35 ` [PATCH v3 3/3] " Mehdi Ben Hadj Khelifa
@ 2025-09-25 15:04 ` Daniel Borkmann
2025-09-25 16:26 ` Mehdi Ben Hadj Khelifa
0 siblings, 1 reply; 9+ messages in thread
From: Daniel Borkmann @ 2025-09-25 15:04 UTC (permalink / raw)
To: Mehdi Ben Hadj Khelifa, andrii, eddyz87, ast, martin.lau, song,
yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah,
matttbe, martineau, geliang, davem, kuba, hawk, linux, ameryhung,
toke, houtao1, emil, yatsenko, isolodrai, a.s.protopopov, dxu,
memxor, vmalik, bigeasy, tj, gregkh, paul, bboscaccy,
James.Bottomley, mrpre, jakub
Cc: bpf, linux-kernel, linux-kselftest, netdev, mptcp,
linux-kernel-mentees, skhan, david.hunter.linux
On 9/25/25 12:35 PM, Mehdi Ben Hadj Khelifa wrote:
> -Change only variable types for correct sign comparisons
>
> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
Pls write some better commit messages and not just copy/paste the same $subj/
message every time; proper sentences w/o the weird " -" indent. Also say why
this is needed in the commit message, and add a reference to the commit which
initially added this as a TODO, i.e. 495d2d8133fd ("selftests/bpf: Attempt to
build BPF programs with -Wsign-compare"). If you group these, then maybe also
include the parts of the compiler-emitted warnings during build which are
relevant to the code changes you do here.
> ---
> tools/testing/selftests/bpf/progs/test_xdp_dynptr.c | 2 +-
> tools/testing/selftests/bpf/progs/test_xdp_loop.c | 2 +-
> tools/testing/selftests/bpf/progs/test_xdp_noinline.c | 4 ++--
> tools/testing/selftests/bpf/progs/uprobe_multi.c | 4 ++--
> .../selftests/bpf/progs/uprobe_multi_session_recursive.c | 5 +++--
> .../selftests/bpf/progs/verifier_iterating_callbacks.c | 2 +-
> 6 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c b/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c
> index 67a77944ef29..12ad0ec91021 100644
> --- a/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c
> +++ b/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c
> @@ -89,7 +89,7 @@ static __always_inline int handle_ipv4(struct xdp_md *xdp, struct bpf_dynptr *xd
> struct vip vip = {};
> int dport;
> __u32 csum = 0;
> - int i;
> + size_t i;
>
> __builtin_memset(eth_buffer, 0, sizeof(eth_buffer));
> __builtin_memset(iph_buffer_tcp, 0, sizeof(iph_buffer_tcp));
> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_loop.c b/tools/testing/selftests/bpf/progs/test_xdp_loop.c
> index 93267a68825b..e9b7bbff5c23 100644
> --- a/tools/testing/selftests/bpf/progs/test_xdp_loop.c
> +++ b/tools/testing/selftests/bpf/progs/test_xdp_loop.c
> @@ -85,7 +85,7 @@ static __always_inline int handle_ipv4(struct xdp_md *xdp)
> struct vip vip = {};
> int dport;
> __u32 csum = 0;
> - int i;
> + size_t i;
>
> if (iph + 1 > data_end)
> return XDP_DROP;
> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_noinline.c b/tools/testing/selftests/bpf/progs/test_xdp_noinline.c
> index fad94e41cef9..85ef3c0a3e20 100644
> --- a/tools/testing/selftests/bpf/progs/test_xdp_noinline.c
> +++ b/tools/testing/selftests/bpf/progs/test_xdp_noinline.c
> @@ -372,7 +372,7 @@ bool encap_v4(struct xdp_md *xdp, struct ctl_value *cval,
>
> next_iph_u16 = (__u16 *) iph;
> __pragma_loop_unroll_full
> - for (int i = 0; i < sizeof(struct iphdr) >> 1; i++)
> + for (size_t i = 0; i < sizeof(struct iphdr) >> 1; i++)
> csum += *next_iph_u16++;
> iph->check = ~((csum & 0xffff) + (csum >> 16));
> if (bpf_xdp_adjust_head(xdp, (int)sizeof(struct iphdr)))
> @@ -423,7 +423,7 @@ int send_icmp_reply(void *data, void *data_end)
> iph->check = 0;
> next_iph_u16 = (__u16 *) iph;
> __pragma_loop_unroll_full
> - for (int i = 0; i < sizeof(struct iphdr) >> 1; i++)
> + for (size_t i = 0; i < sizeof(struct iphdr) >> 1; i++)
> csum += *next_iph_u16++;
> iph->check = ~((csum & 0xffff) + (csum >> 16));
> return swap_mac_and_send(data, data_end);
> diff --git a/tools/testing/selftests/bpf/progs/uprobe_multi.c b/tools/testing/selftests/bpf/progs/uprobe_multi.c
> index 44190efcdba2..f99957773c3a 100644
> --- a/tools/testing/selftests/bpf/progs/uprobe_multi.c
> +++ b/tools/testing/selftests/bpf/progs/uprobe_multi.c
> @@ -20,13 +20,13 @@ __u64 uretprobe_multi_func_3_result = 0;
>
> __u64 uprobe_multi_sleep_result = 0;
>
> -int pid = 0;
> +__u32 pid = 0;
> int child_pid = 0;
> int child_tid = 0;
> int child_pid_usdt = 0;
> int child_tid_usdt = 0;
>
> -int expect_pid = 0;
> +__u32 expect_pid = 0;
> bool bad_pid_seen = false;
> bool bad_pid_seen_usdt = false;
>
> diff --git a/tools/testing/selftests/bpf/progs/uprobe_multi_session_recursive.c b/tools/testing/selftests/bpf/progs/uprobe_multi_session_recursive.c
> index 8fbcd69fae22..017f1859ebe8 100644
> --- a/tools/testing/selftests/bpf/progs/uprobe_multi_session_recursive.c
> +++ b/tools/testing/selftests/bpf/progs/uprobe_multi_session_recursive.c
> @@ -3,6 +3,7 @@
> #include <bpf/bpf_helpers.h>
> #include <bpf/bpf_tracing.h>
> #include <stdbool.h>
> +#include <stddef.h>
> #include "bpf_kfuncs.h"
> #include "bpf_misc.h"
>
> @@ -10,8 +11,8 @@ char _license[] SEC("license") = "GPL";
>
> int pid = 0;
>
> -int idx_entry = 0;
> -int idx_return = 0;
> +size_t idx_entry = 0;
> +size_t idx_return = 0;
>
> __u64 test_uprobe_cookie_entry[6];
> __u64 test_uprobe_cookie_return[3];
> diff --git a/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c b/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c
> index 75dd922e4e9f..72f9f8c23c93 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c
> @@ -593,7 +593,7 @@ int loop_inside_iter_volatile_limit(const void *ctx)
> {
> struct bpf_iter_num it;
> int *v, sum = 0;
> - __u64 i = 0;
> + __s32 i = 0;
>
> bpf_iter_num_new(&it, 0, ARR2_SZ);
> while ((v = bpf_iter_num_next(&it))) {
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 3/3] selftests/bpf: Prepare to add -Wsign-compare for bpf tests
2025-09-25 15:04 ` Daniel Borkmann
@ 2025-09-25 16:26 ` Mehdi Ben Hadj Khelifa
2025-09-25 16:18 ` vivek yadav
0 siblings, 1 reply; 9+ messages in thread
From: Mehdi Ben Hadj Khelifa @ 2025-09-25 16:26 UTC (permalink / raw)
To: Daniel Borkmann, andrii, eddyz87, ast, martin.lau, song,
yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah,
matttbe, martineau, geliang, davem, kuba, hawk, linux, ameryhung,
toke, houtao1, emil, yatsenko, isolodrai, a.s.protopopov, dxu,
memxor, vmalik, bigeasy, tj, gregkh, paul, bboscaccy,
James.Bottomley, mrpre, jakub
Cc: bpf, linux-kernel, linux-kselftest, netdev, mptcp,
linux-kernel-mentees, skhan, david.hunter.linux
On 9/25/25 4:04 PM, Daniel Borkmann wrote:
> On 9/25/25 12:35 PM, Mehdi Ben Hadj Khelifa wrote:
>> -Change only variable types for correct sign comparisons
>>
>> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
>
> Pls write some better commit messages and not just copy/paste the same
> $subj/
> message every time; proper sentences w/o the weird " -" indent.
Understood, though the changes are very similar / are the same with the
same goal that's why it made sense to me to do that and I will remove
the - in future commits.> Also say
> why
> this is needed in the commit message, and add a reference to the commit
> which
> initially added this as a TODO, i.e. 495d2d8133fd ("selftests/bpf:
> Attempt to
> build BPF programs with -Wsign-compare").
I will do that in the upcoming version.
> If you group these, then maybe
> also
> include the parts of the compiler-emitted warnings during build which are
> relevant to the code changes you do here.
Okay. I will do that. Should i send a v4 with the recommended changes
but not including the rest of the files meaning the ones that I haven't
uploaded in this patch series which contain type casting or should i
just make changes for these files in this series?
Also will it be better if dropped these versions and made a new patch
with v1?
Thank you for your review and time Daniel.
Regards,
Mehdi
>> ---
>> tools/testing/selftests/bpf/progs/test_xdp_dynptr.c | 2 +-
>> tools/testing/selftests/bpf/progs/test_xdp_loop.c | 2 +-
>> tools/testing/selftests/bpf/progs/test_xdp_noinline.c | 4 ++--
>> tools/testing/selftests/bpf/progs/uprobe_multi.c | 4 ++--
>> .../selftests/bpf/progs/uprobe_multi_session_recursive.c | 5 +++--
>> .../selftests/bpf/progs/verifier_iterating_callbacks.c | 2 +-
>> 6 files changed, 10 insertions(+), 9 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c b/
>> tools/testing/selftests/bpf/progs/test_xdp_dynptr.c
>> index 67a77944ef29..12ad0ec91021 100644
>> --- a/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c
>> +++ b/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c
>> @@ -89,7 +89,7 @@ static __always_inline int handle_ipv4(struct xdp_md
>> *xdp, struct bpf_dynptr *xd
>> struct vip vip = {};
>> int dport;
>> __u32 csum = 0;
>> - int i;
>> + size_t i;
>> __builtin_memset(eth_buffer, 0, sizeof(eth_buffer));
>> __builtin_memset(iph_buffer_tcp, 0, sizeof(iph_buffer_tcp));
>> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_loop.c b/
>> tools/testing/selftests/bpf/progs/test_xdp_loop.c
>> index 93267a68825b..e9b7bbff5c23 100644
>> --- a/tools/testing/selftests/bpf/progs/test_xdp_loop.c
>> +++ b/tools/testing/selftests/bpf/progs/test_xdp_loop.c
>> @@ -85,7 +85,7 @@ static __always_inline int handle_ipv4(struct xdp_md
>> *xdp)
>> struct vip vip = {};
>> int dport;
>> __u32 csum = 0;
>> - int i;
>> + size_t i;
>> if (iph + 1 > data_end)
>> return XDP_DROP;
>> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_noinline.c b/
>> tools/testing/selftests/bpf/progs/test_xdp_noinline.c
>> index fad94e41cef9..85ef3c0a3e20 100644
>> --- a/tools/testing/selftests/bpf/progs/test_xdp_noinline.c
>> +++ b/tools/testing/selftests/bpf/progs/test_xdp_noinline.c
>> @@ -372,7 +372,7 @@ bool encap_v4(struct xdp_md *xdp, struct ctl_value
>> *cval,
>> next_iph_u16 = (__u16 *) iph;
>> __pragma_loop_unroll_full
>> - for (int i = 0; i < sizeof(struct iphdr) >> 1; i++)
>> + for (size_t i = 0; i < sizeof(struct iphdr) >> 1; i++)
>> csum += *next_iph_u16++;
>> iph->check = ~((csum & 0xffff) + (csum >> 16));
>> if (bpf_xdp_adjust_head(xdp, (int)sizeof(struct iphdr)))
>> @@ -423,7 +423,7 @@ int send_icmp_reply(void *data, void *data_end)
>> iph->check = 0;
>> next_iph_u16 = (__u16 *) iph;
>> __pragma_loop_unroll_full
>> - for (int i = 0; i < sizeof(struct iphdr) >> 1; i++)
>> + for (size_t i = 0; i < sizeof(struct iphdr) >> 1; i++)
>> csum += *next_iph_u16++;
>> iph->check = ~((csum & 0xffff) + (csum >> 16));
>> return swap_mac_and_send(data, data_end);
>> diff --git a/tools/testing/selftests/bpf/progs/uprobe_multi.c b/tools/
>> testing/selftests/bpf/progs/uprobe_multi.c
>> index 44190efcdba2..f99957773c3a 100644
>> --- a/tools/testing/selftests/bpf/progs/uprobe_multi.c
>> +++ b/tools/testing/selftests/bpf/progs/uprobe_multi.c
>> @@ -20,13 +20,13 @@ __u64 uretprobe_multi_func_3_result = 0;
>> __u64 uprobe_multi_sleep_result = 0;
>> -int pid = 0;
>> +__u32 pid = 0;
>> int child_pid = 0;
>> int child_tid = 0;
>> int child_pid_usdt = 0;
>> int child_tid_usdt = 0;
>> -int expect_pid = 0;
>> +__u32 expect_pid = 0;
>> bool bad_pid_seen = false;
>> bool bad_pid_seen_usdt = false;
>> diff --git a/tools/testing/selftests/bpf/progs/
>> uprobe_multi_session_recursive.c b/tools/testing/selftests/bpf/progs/
>> uprobe_multi_session_recursive.c
>> index 8fbcd69fae22..017f1859ebe8 100644
>> --- a/tools/testing/selftests/bpf/progs/uprobe_multi_session_recursive.c
>> +++ b/tools/testing/selftests/bpf/progs/uprobe_multi_session_recursive.c
>> @@ -3,6 +3,7 @@
>> #include <bpf/bpf_helpers.h>
>> #include <bpf/bpf_tracing.h>
>> #include <stdbool.h>
>> +#include <stddef.h>
>> #include "bpf_kfuncs.h"
>> #include "bpf_misc.h"
>> @@ -10,8 +11,8 @@ char _license[] SEC("license") = "GPL";
>> int pid = 0;
>> -int idx_entry = 0;
>> -int idx_return = 0;
>> +size_t idx_entry = 0;
>> +size_t idx_return = 0;
>> __u64 test_uprobe_cookie_entry[6];
>> __u64 test_uprobe_cookie_return[3];
>> diff --git a/tools/testing/selftests/bpf/progs/
>> verifier_iterating_callbacks.c b/tools/testing/selftests/bpf/progs/
>> verifier_iterating_callbacks.c
>> index 75dd922e4e9f..72f9f8c23c93 100644
>> --- a/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c
>> +++ b/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c
>> @@ -593,7 +593,7 @@ int loop_inside_iter_volatile_limit(const void *ctx)
>> {
>> struct bpf_iter_num it;
>> int *v, sum = 0;
>> - __u64 i = 0;
>> + __s32 i = 0;
>> bpf_iter_num_new(&it, 0, ARR2_SZ);
>> while ((v = bpf_iter_num_next(&it))) {
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 3/3] selftests/bpf: Prepare to add -Wsign-compare for bpf tests
2025-09-25 16:26 ` Mehdi Ben Hadj Khelifa
@ 2025-09-25 16:18 ` vivek yadav
0 siblings, 0 replies; 9+ messages in thread
From: vivek yadav @ 2025-09-25 16:18 UTC (permalink / raw)
To: Mehdi Ben Hadj Khelifa
Cc: Daniel Borkmann, andrii, eddyz87, ast, martin.lau, song,
yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah,
matttbe, martineau, geliang, davem, kuba, hawk, linux, ameryhung,
toke, houtao1, emil, yatsenko, isolodrai, a.s.protopopov, dxu,
memxor, vmalik, bigeasy, tj, gregkh, paul, bboscaccy,
James.Bottomley, mrpre, jakub, bpf, linux-kernel, linux-kselftest,
netdev, mptcp, linux-kernel-mentees, skhan, david.hunter.linux
Hi Mehdi,
You are trying to do much with the patch series. I don't think it will
help much as reviewers will give comments and you will revise the
patches. This loop will continue forever.
I totally agree with Daniel. Please write a proper commit message.
While writing a commit message or creating a patch.Please try to give
the answers of the following questions.
1) What is the issue which your patch resolves?
2) Are you trying to do more than one thing at a time? Please don't.
3) if a patch modifies more than one file then either these changes
inter link with each other or they have similar kind of work.
~~Vivek
On Thu, Sep 25, 2025 at 9:04 PM Mehdi Ben Hadj Khelifa
<mehdi.benhadjkhelifa@gmail.com> wrote:
>
> On 9/25/25 4:04 PM, Daniel Borkmann wrote:
> > On 9/25/25 12:35 PM, Mehdi Ben Hadj Khelifa wrote:
> >> -Change only variable types for correct sign comparisons
> >>
> >> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
> >
> > Pls write some better commit messages and not just copy/paste the same
> > $subj/
> > message every time; proper sentences w/o the weird " -" indent.
>
> Understood, though the changes are very similar / are the same with the
> same goal that's why it made sense to me to do that and I will remove
> the - in future commits.> Also say
> > why
> > this is needed in the commit message, and add a reference to the commit
> > which
> > initially added this as a TODO, i.e. 495d2d8133fd ("selftests/bpf:
> > Attempt to
> > build BPF programs with -Wsign-compare").
> I will do that in the upcoming version.
>
> > If you group these, then maybe
> > also
> > include the parts of the compiler-emitted warnings during build which are
> > relevant to the code changes you do here.
>
> Okay. I will do that. Should i send a v4 with the recommended changes
> but not including the rest of the files meaning the ones that I haven't
> uploaded in this patch series which contain type casting or should i
> just make changes for these files in this series?
> Also will it be better if dropped these versions and made a new patch
> with v1?
>
> Thank you for your review and time Daniel.
> Regards,
> Mehdi
> >> ---
> >> tools/testing/selftests/bpf/progs/test_xdp_dynptr.c | 2 +-
> >> tools/testing/selftests/bpf/progs/test_xdp_loop.c | 2 +-
> >> tools/testing/selftests/bpf/progs/test_xdp_noinline.c | 4 ++--
> >> tools/testing/selftests/bpf/progs/uprobe_multi.c | 4 ++--
> >> .../selftests/bpf/progs/uprobe_multi_session_recursive.c | 5 +++--
> >> .../selftests/bpf/progs/verifier_iterating_callbacks.c | 2 +-
> >> 6 files changed, 10 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c b/
> >> tools/testing/selftests/bpf/progs/test_xdp_dynptr.c
> >> index 67a77944ef29..12ad0ec91021 100644
> >> --- a/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c
> >> +++ b/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c
> >> @@ -89,7 +89,7 @@ static __always_inline int handle_ipv4(struct xdp_md
> >> *xdp, struct bpf_dynptr *xd
> >> struct vip vip = {};
> >> int dport;
> >> __u32 csum = 0;
> >> - int i;
> >> + size_t i;
> >> __builtin_memset(eth_buffer, 0, sizeof(eth_buffer));
> >> __builtin_memset(iph_buffer_tcp, 0, sizeof(iph_buffer_tcp));
> >> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_loop.c b/
> >> tools/testing/selftests/bpf/progs/test_xdp_loop.c
> >> index 93267a68825b..e9b7bbff5c23 100644
> >> --- a/tools/testing/selftests/bpf/progs/test_xdp_loop.c
> >> +++ b/tools/testing/selftests/bpf/progs/test_xdp_loop.c
> >> @@ -85,7 +85,7 @@ static __always_inline int handle_ipv4(struct xdp_md
> >> *xdp)
> >> struct vip vip = {};
> >> int dport;
> >> __u32 csum = 0;
> >> - int i;
> >> + size_t i;
> >> if (iph + 1 > data_end)
> >> return XDP_DROP;
> >> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_noinline.c b/
> >> tools/testing/selftests/bpf/progs/test_xdp_noinline.c
> >> index fad94e41cef9..85ef3c0a3e20 100644
> >> --- a/tools/testing/selftests/bpf/progs/test_xdp_noinline.c
> >> +++ b/tools/testing/selftests/bpf/progs/test_xdp_noinline.c
> >> @@ -372,7 +372,7 @@ bool encap_v4(struct xdp_md *xdp, struct ctl_value
> >> *cval,
> >> next_iph_u16 = (__u16 *) iph;
> >> __pragma_loop_unroll_full
> >> - for (int i = 0; i < sizeof(struct iphdr) >> 1; i++)
> >> + for (size_t i = 0; i < sizeof(struct iphdr) >> 1; i++)
> >> csum += *next_iph_u16++;
> >> iph->check = ~((csum & 0xffff) + (csum >> 16));
> >> if (bpf_xdp_adjust_head(xdp, (int)sizeof(struct iphdr)))
> >> @@ -423,7 +423,7 @@ int send_icmp_reply(void *data, void *data_end)
> >> iph->check = 0;
> >> next_iph_u16 = (__u16 *) iph;
> >> __pragma_loop_unroll_full
> >> - for (int i = 0; i < sizeof(struct iphdr) >> 1; i++)
> >> + for (size_t i = 0; i < sizeof(struct iphdr) >> 1; i++)
> >> csum += *next_iph_u16++;
> >> iph->check = ~((csum & 0xffff) + (csum >> 16));
> >> return swap_mac_and_send(data, data_end);
> >> diff --git a/tools/testing/selftests/bpf/progs/uprobe_multi.c b/tools/
> >> testing/selftests/bpf/progs/uprobe_multi.c
> >> index 44190efcdba2..f99957773c3a 100644
> >> --- a/tools/testing/selftests/bpf/progs/uprobe_multi.c
> >> +++ b/tools/testing/selftests/bpf/progs/uprobe_multi.c
> >> @@ -20,13 +20,13 @@ __u64 uretprobe_multi_func_3_result = 0;
> >> __u64 uprobe_multi_sleep_result = 0;
> >> -int pid = 0;
> >> +__u32 pid = 0;
> >> int child_pid = 0;
> >> int child_tid = 0;
> >> int child_pid_usdt = 0;
> >> int child_tid_usdt = 0;
> >> -int expect_pid = 0;
> >> +__u32 expect_pid = 0;
> >> bool bad_pid_seen = false;
> >> bool bad_pid_seen_usdt = false;
> >> diff --git a/tools/testing/selftests/bpf/progs/
> >> uprobe_multi_session_recursive.c b/tools/testing/selftests/bpf/progs/
> >> uprobe_multi_session_recursive.c
> >> index 8fbcd69fae22..017f1859ebe8 100644
> >> --- a/tools/testing/selftests/bpf/progs/uprobe_multi_session_recursive.c
> >> +++ b/tools/testing/selftests/bpf/progs/uprobe_multi_session_recursive.c
> >> @@ -3,6 +3,7 @@
> >> #include <bpf/bpf_helpers.h>
> >> #include <bpf/bpf_tracing.h>
> >> #include <stdbool.h>
> >> +#include <stddef.h>
> >> #include "bpf_kfuncs.h"
> >> #include "bpf_misc.h"
> >> @@ -10,8 +11,8 @@ char _license[] SEC("license") = "GPL";
> >> int pid = 0;
> >> -int idx_entry = 0;
> >> -int idx_return = 0;
> >> +size_t idx_entry = 0;
> >> +size_t idx_return = 0;
> >> __u64 test_uprobe_cookie_entry[6];
> >> __u64 test_uprobe_cookie_return[3];
> >> diff --git a/tools/testing/selftests/bpf/progs/
> >> verifier_iterating_callbacks.c b/tools/testing/selftests/bpf/progs/
> >> verifier_iterating_callbacks.c
> >> index 75dd922e4e9f..72f9f8c23c93 100644
> >> --- a/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c
> >> +++ b/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c
> >> @@ -593,7 +593,7 @@ int loop_inside_iter_volatile_limit(const void *ctx)
> >> {
> >> struct bpf_iter_num it;
> >> int *v, sum = 0;
> >> - __u64 i = 0;
> >> + __s32 i = 0;
> >> bpf_iter_num_new(&it, 0, ARR2_SZ);
> >> while ((v = bpf_iter_num_next(&it))) {
> >
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/3] selftests/bpf: Prepare to add -Wsign-compare for bpf selftests
2025-09-25 10:35 [PATCH v3 0/3] selftests/bpf: Prepare to add -Wsign-compare for bpf selftests Mehdi Ben Hadj Khelifa
` (2 preceding siblings ...)
2025-09-25 10:35 ` [PATCH v3 3/3] " Mehdi Ben Hadj Khelifa
@ 2025-09-25 23:32 ` Andrii Nakryiko
2025-09-26 8:00 ` Mehdi Ben Hadj Khelifa
3 siblings, 1 reply; 9+ messages in thread
From: Andrii Nakryiko @ 2025-09-25 23:32 UTC (permalink / raw)
To: Mehdi Ben Hadj Khelifa
Cc: andrii, eddyz87, ast, daniel, martin.lau, song, yonghong.song,
john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah, matttbe,
martineau, geliang, davem, kuba, hawk, linux, ameryhung, toke,
houtao1, emil, yatsenko, isolodrai, a.s.protopopov, dxu, memxor,
vmalik, bigeasy, tj, gregkh, paul, bboscaccy, James.Bottomley,
mrpre, jakub, bpf, linux-kernel, linux-kselftest, netdev, mptcp,
linux-kernel-mentees, skhan, david.hunter.linux
On Thu, Sep 25, 2025 at 2:36 AM Mehdi Ben Hadj Khelifa
<mehdi.benhadjkhelifa@gmail.com> wrote:
>
> This series is preparing to add the -Wsign-compare C compilation flag to
> the Makefile for bpf selftests as requested by a TODO to help avoid
> implicit type conversions and have predictable behavior.
>
> Changelog:
>
> 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.com/T/#u
>
> 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@gmail.com/T/#u
>
> Mehdi Ben Hadj Khelifa (3):
> selftests/bpf: Prepare to add -Wsign-compare for bpf tests
> selftests/bpf: Prepare to add -Wsign-compare for bpf tests
> selftests/bpf: Prepare to add -Wsign-compare for bpf tests
>
I see little value in these transformations. Did we catch any real
issue here? All this type casting and replacement is just churn.
I certainly don't want such churn in libbpf, and I'd leave BPF
selftests as is as well. int vs u64 can have subtle and non-obvious
implications for BPF code generation (for no-alu32 variants
especially), and I think BPF CI already exposed some of those already.
I think we can live without -Wsign-compare just fine.
> tools/testing/selftests/bpf/progs/test_global_func11.c | 2 +-
> tools/testing/selftests/bpf/progs/test_global_func12.c | 2 +-
> tools/testing/selftests/bpf/progs/test_global_func13.c | 2 +-
> tools/testing/selftests/bpf/progs/test_global_func9.c | 2 +-
> tools/testing/selftests/bpf/progs/test_map_init.c | 2 +-
> tools/testing/selftests/bpf/progs/test_parse_tcp_hdr_opt.c | 2 +-
> .../selftests/bpf/progs/test_parse_tcp_hdr_opt_dynptr.c | 2 +-
> tools/testing/selftests/bpf/progs/test_skb_ctx.c | 2 +-
> tools/testing/selftests/bpf/progs/test_snprintf.c | 2 +-
> tools/testing/selftests/bpf/progs/test_sockmap_strp.c | 2 +-
> tools/testing/selftests/bpf/progs/test_tc_tunnel.c | 2 +-
> tools/testing/selftests/bpf/progs/test_xdp.c | 2 +-
> tools/testing/selftests/bpf/progs/test_xdp_dynptr.c | 2 +-
> tools/testing/selftests/bpf/progs/test_xdp_loop.c | 2 +-
> tools/testing/selftests/bpf/progs/test_xdp_noinline.c | 4 ++--
> tools/testing/selftests/bpf/progs/uprobe_multi.c | 4 ++--
> .../selftests/bpf/progs/uprobe_multi_session_recursive.c | 5 +++--
> .../selftests/bpf/progs/verifier_iterating_callbacks.c | 2 +-
> 18 files changed, 22 insertions(+), 21 deletions(-)
>
> --
> 2.51.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 0/3] selftests/bpf: Prepare to add -Wsign-compare for bpf selftests
2025-09-25 23:32 ` [PATCH v3 0/3] selftests/bpf: Prepare to add -Wsign-compare for bpf selftests Andrii Nakryiko
@ 2025-09-26 8:00 ` Mehdi Ben Hadj Khelifa
0 siblings, 0 replies; 9+ messages in thread
From: Mehdi Ben Hadj Khelifa @ 2025-09-26 8:00 UTC (permalink / raw)
To: Andrii Nakryiko, daniel
Cc: andrii, eddyz87, ast, daniel, martin.lau, song, yonghong.song,
john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah, matttbe,
martineau, geliang, davem, kuba, hawk, linux, ameryhung, toke,
houtao1, emil, yatsenko, isolodrai, a.s.protopopov, dxu, memxor,
vmalik, bigeasy, tj, gregkh, paul, bboscaccy, James.Bottomley,
mrpre, jakub, bpf, linux-kernel, linux-kselftest, netdev, mptcp,
linux-kernel-mentees, skhan, david.hunter.linux
> I see little value in these transformations. Did we catch any real
> issue here? All this type casting and replacement is just churn.
>
> I certainly don't want such churn in libbpf, and I'd leave BPF
> selftests as is as well. int vs u64 can have subtle and non-obvious
> implications for BPF code generation (for no-alu32 variants
> especially), and I think BPF CI already exposed some of those already.
>
> I think we can live without -Wsign-compare just fine.
>
I was convinced by [1] that this needed to be done not just for current
version of the code but for future code being more robust initially.I
have already done all the work and I can follow daniel's suggestion for
the next version.Otherwise,This means then that the TODO comment to add
that compiler warning in the makefile needs to be removed.
Also I wanted to ask since the CI bot had success with my patch.What
does this [2] mean exactly.
Thank you for your time reviewing and helping.
Regards,
Mehdi
[1]:https://github.com/kernel-patches/bpf/commit/495d2d8133fd1407519170a5238f455abbd9ec9b
[2]:https://github.com/kernel-patches/bpf/actions/runs/18006172526
^ permalink raw reply [flat|nested] 9+ messages in thread