When building with clang, via:
make LLVM=1 -C tools/testing/selftest
...clang warns that "a variable sized type not at the end of a struct or
class is a GNU extension".
These cases are not easily changed, because they involve structs that
are part of the API. Fortunately, however, the tests seem to be doing
just fine (specifically, neither affected test runs any differently with
gcc vs. clang builds, on my test system) regardless of the warning. So,
all the warning is doing is preventing a clean build of selftests/net.
Fix this by suppressing this particular clang warning for the
selftests/net suite.
Signed-off-by: John Hubbard <jhubbard(a)nvidia.com>
---
tools/testing/selftests/net/Makefile | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 7b6918d5f4af..956481174783 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -6,6 +6,10 @@ CFLAGS += -I../../../../usr/include/ $(KHDR_INCLUDES)
# Additional include paths needed by kselftest.h
CFLAGS += -I../
+ifneq ($(LLVM),)
+ CFLAGS += -Wno-gnu-variable-sized-type-not-at-end
+endif
+
TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh \
rtnetlink.sh xfrm_policy.sh test_blackhole_dev.sh
TEST_PROGS += fib_tests.sh fib-onlink-tests.sh pmtu.sh udpgso.sh ip_defrag.sh
base-commit: f462ae0edd3703edd6f22fe41d336369c38b884b
prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27
--
2.45.0
When building with clang, via:
make LLVM=1 -C tools/testing/selftests
...clang warns, correctly, that several functions declared with an "int"
return type are not always returning values in all cases (or at least,
clang cannot prove that they always return a value).
Fix this by returning an appropriate value for each function. Thanks to
Felix Huettner for recommending MNL_CB_OK (which is non-zero) for the
return value of the count_entries() callback.
Cc: Felix Huettner <felix.huettner(a)mail.schwarz>
Signed-off-by: John Hubbard <jhubbard(a)nvidia.com>
---
tools/testing/selftests/netfilter/conntrack_dump_flush.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/netfilter/conntrack_dump_flush.c b/tools/testing/selftests/netfilter/conntrack_dump_flush.c
index b11ea8ee6719..e9df4ae14e16 100644
--- a/tools/testing/selftests/netfilter/conntrack_dump_flush.c
+++ b/tools/testing/selftests/netfilter/conntrack_dump_flush.c
@@ -43,6 +43,7 @@ static int build_cta_tuple_v4(struct nlmsghdr *nlh, int type,
mnl_attr_nest_end(nlh, nest_proto);
mnl_attr_nest_end(nlh, nest);
+ return 0;
}
static int build_cta_tuple_v6(struct nlmsghdr *nlh, int type,
@@ -71,6 +72,7 @@ static int build_cta_tuple_v6(struct nlmsghdr *nlh, int type,
mnl_attr_nest_end(nlh, nest_proto);
mnl_attr_nest_end(nlh, nest);
+ return 0;
}
static int build_cta_proto(struct nlmsghdr *nlh)
@@ -90,6 +92,7 @@ static int build_cta_proto(struct nlmsghdr *nlh)
mnl_attr_nest_end(nlh, nest_proto);
mnl_attr_nest_end(nlh, nest);
+ return 0;
}
static int conntrack_data_insert(struct mnl_socket *sock, struct nlmsghdr *nlh,
@@ -207,6 +210,7 @@ static int conntrack_data_generate_v6(struct mnl_socket *sock,
static int count_entries(const struct nlmsghdr *nlh, void *data)
{
reply_counter++;
+ return MNL_CB_OK;
}
static int conntracK_count_zone(struct mnl_socket *sock, uint16_t zone)
base-commit: f462ae0edd3703edd6f22fe41d336369c38b884b
prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27
prerequisite-patch-id: 9db2d20be98dc44731d8605a3da64ff118d2546d
--
2.45.0
On 5/6/24 7:41 AM, Felix Huettner wrote:
> On Sun, May 05, 2024 at 02:47:16PM -0700, John Hubbard wrote:
...
> > @@ -207,6 +210,7 @@ static int conntrack_data_generate_v6(struct
> mnl_socket *sock,
> > static int count_entries(const struct nlmsghdr *nlh, void *data)
> > {
> > reply_counter++;
> > + return 0;
>
> Hi John,
>
> This will need to return MNL_CB_OK.
> Otherwise mnl_cb_run below will abort early and the connection count
> will be wrong.
>
Thanks for catching that, I'm sending a v2 with that fix.
I was thinking about it, and expected that the pre-existing code
appeared to work because the return value was some non-zero garbage
value scrounged off of the stack (or %rax, for example on x86).
However, just a quick test showed that *any* value (O, 1==MNL_CB_OK,
or no value at all) allows the test to report success...oh, I see,
it's reporting PASSED when it really ought to say SKIPPED:
$ ./conntrack_dump_flush
TAP version 13
1..3
# Starting 3 tests from 1 test cases.
# RUN conntrack_dump_flush.test_dump_by_zone ...
mnl_socket_open: Protocol not supported
# OK conntrack_dump_flush.test_dump_by_zone
ok 1 conntrack_dump_flush.test_dump_by_zone
# RUN conntrack_dump_flush.test_flush_by_zone ...
mnl_socket_open: Protocol not supported
# OK conntrack_dump_flush.test_flush_by_zone
ok 2 conntrack_dump_flush.test_flush_by_zone
# RUN conntrack_dump_flush.test_flush_by_zone_default ...
mnl_socket_open: Protocol not supported
# OK conntrack_dump_flush.test_flush_by_zone_default
ok 3 conntrack_dump_flush.test_flush_by_zone_default
# PASSED: 3 / 3 tests passed.
# Totals: pass:3 fail:0 xfail:0 xpass:0 skip:0 error:0
As long as we are looking at this, what do you think about
this:
diff --git a/tools/testing/selftests/netfilter/conntrack_dump_flush.c
b/tools/testing/selftests/netfilter/conntrack_dump_flush.c
index e9df4ae14e16..4a73afad4de4 100644
--- a/tools/testing/selftests/netfilter/conntrack_dump_flush.c
+++ b/tools/testing/selftests/netfilter/conntrack_dump_flush.c
@@ -317,12 +317,12 @@ FIXTURE_SETUP(conntrack_dump_flush)
self->sock = mnl_socket_open(NETLINK_NETFILTER);
if (!self->sock) {
perror("mnl_socket_open");
- exit(EXIT_FAILURE);
+ SKIP(exit(EXIT_FAILURE), "mnl_socket_open() failed");
}
if (mnl_socket_bind(self->sock, 0, MNL_SOCKET_AUTOPID) < 0) {
perror("mnl_socket_bind");
- exit(EXIT_FAILURE);
+ SKIP(exit(EXIT_FAILURE), "mnl_socket_bind() failed");
}
ret = conntracK_count_zone(self->sock, TEST_ZONE_ID);
...which changes the above output, to:
$ ./conntrack_dump_flush
TAP version 13
1..3
# Starting 3 tests from 1 test cases.
# RUN conntrack_dump_flush.test_dump_by_zone ...
mnl_socket_open: Protocol not supported
# SKIP mnl_socket_open() failed
# OK conntrack_dump_flush.test_dump_by_zone
ok 1 conntrack_dump_flush.test_dump_by_zone # SKIP mnl_socket_open() failed
# RUN conntrack_dump_flush.test_flush_by_zone ...
mnl_socket_open: Protocol not supported
# SKIP mnl_socket_open() failed
# OK conntrack_dump_flush.test_flush_by_zone
ok 2 conntrack_dump_flush.test_flush_by_zone # SKIP mnl_socket_open() failed
# RUN conntrack_dump_flush.test_flush_by_zone_default ...
mnl_socket_open: Protocol not supported
# SKIP mnl_socket_open() failed
# OK conntrack_dump_flush.test_flush_by_zone_default
ok 3 conntrack_dump_flush.test_flush_by_zone_default # SKIP
mnl_socket_open() failed
# PASSED: 3 / 3 tests passed.
# Totals: pass:0 fail:0 xfail:0 xpass:0 skip:3 error:0
?
thanks,
--
John Hubbard
NVIDIA
Fixes clang compilation warnings by changing elf_hash's parameter type
to char * and casting to unsigned char * inside elf_hash:
parse_vdso.c:206:22: warning: passing 'const char *' to parameter of
type 'const unsigned char *' converts between pointers to integer types
where one is of the unique plain 'char' type and the other is not
[-Wpointer-sign]
ver_hash = elf_hash(version);
^~~~~~~
parse_vdso.c:59:52: note: passing argument to parameter 'name' here
static unsigned long elf_hash(const unsigned char *name)
^
parse_vdso.c:207:46: warning: passing 'const char *' to parameter of
type 'const unsigned char *' converts between pointers to integer types
where one is of the unique plain 'char' type and the other is not
[-Wpointer-sign]
ELF(Word) chain = vdso_info.bucket[elf_hash(name) % vdso_info.nbucket];
^~~~
parse_vdso.c:59:52: note: passing argument to parameter 'name' here
static unsigned long elf_hash(const unsigned char *name)
Fixes: 98eedc3a9dbf ("Document the vDSO and add a reference parser")
Signed-off-by: Edward Liaw <edliaw(a)google.com>
---
v2: updated commit message with correct compiler warning
v3: fixed checkpatch errors and indentation
https://lore.kernel.org/all/20240501180622.1676340-1-edliaw@google.com/
v4: moved the typecast into elf_hash based on libelf
https://sourceforge.net/p/elftoolchain/code/HEAD/tree/trunk/libelf/elf_hash…
---
tools/testing/selftests/vDSO/parse_vdso.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/vDSO/parse_vdso.c b/tools/testing/selftests/vDSO/parse_vdso.c
index 413f75620a35..33db8abd7d59 100644
--- a/tools/testing/selftests/vDSO/parse_vdso.c
+++ b/tools/testing/selftests/vDSO/parse_vdso.c
@@ -56,12 +56,15 @@ static struct vdso_info
} vdso_info;
/* Straight from the ELF specification. */
-static unsigned long elf_hash(const unsigned char *name)
+static unsigned long elf_hash(const char *name)
{
unsigned long h = 0, g;
- while (*name)
+ const unsigned char *s;
+
+ s = (const unsigned char *) name;
+ while (*s)
{
- h = (h << 4) + *name++;
+ h = (h << 4) + *s++;
if (g = h & 0xf0000000)
h ^= g >> 24;
h &= ~g;
--
2.45.0.rc1.225.g2a3ae87e7f-goog
dump_config_tree() is declared to return an int, but the compiler cannot
prove that it always returns any value at all. This leads to a clang
warning, when building via:
make LLVM=1 -C tools/testing/selftests
Furthermore, Mark Brown noticed that dump_config_tree() isn't even used
anymore, so just delete the entire function.
Cc: Mark Brown <broonie(a)kernel.org>
Signed-off-by: John Hubbard <jhubbard(a)nvidia.com>
---
tools/testing/selftests/alsa/conf.c | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/tools/testing/selftests/alsa/conf.c b/tools/testing/selftests/alsa/conf.c
index 89e3656a042d..357561c1759b 100644
--- a/tools/testing/selftests/alsa/conf.c
+++ b/tools/testing/selftests/alsa/conf.c
@@ -105,19 +105,6 @@ static struct card_cfg_data *conf_data_by_card(int card, bool msg)
return NULL;
}
-static int dump_config_tree(snd_config_t *top)
-{
- snd_output_t *out;
- int err;
-
- err = snd_output_stdio_attach(&out, stdout, 0);
- if (err < 0)
- ksft_exit_fail_msg("stdout attach\n");
- if (snd_config_save(top, out))
- ksft_exit_fail_msg("config save\n");
- snd_output_close(out);
-}
-
snd_config_t *conf_load_from_file(const char *filename)
{
snd_config_t *dst;
base-commit: f462ae0edd3703edd6f22fe41d336369c38b884b
prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27
--
2.45.0
First of all, in order to build with clang at all, one must first apply
Valentin Obst's build fix for LLVM [1]. Furthermore, for this particular
resctrl directory, my pending fix [2] must also be applied. Once those
fixes are in place, then when building with clang, via:
make LLVM=1 -C tools/testing/selftests
...two types of warnings occur:
warning: absolute value function 'abs' given an argument of type
'long' but has parameter of type 'int' which may cause truncation of
value
warning: taking the absolute value of unsigned type 'unsigned long'
has no effect
Fix these by:
a) using labs() in place of abs(), when long integers are involved, and
b) don't call labs() unnecessarily.
[1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1…
[2] https://lore.kernel.org/all/20240503021712.78601-1-jhubbard@nvidia.com/
Signed-off-by: John Hubbard <jhubbard(a)nvidia.com>
---
tools/testing/selftests/resctrl/cmt_test.c | 4 ++--
tools/testing/selftests/resctrl/mba_test.c | 2 +-
tools/testing/selftests/resctrl/mbm_test.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
index a81f91222a89..05a241519ae8 100644
--- a/tools/testing/selftests/resctrl/cmt_test.c
+++ b/tools/testing/selftests/resctrl/cmt_test.c
@@ -40,11 +40,11 @@ static int show_results_info(unsigned long sum_llc_val, int no_of_bits,
int ret;
avg_llc_val = sum_llc_val / num_of_runs;
- avg_diff = (long)abs(cache_span - avg_llc_val);
+ avg_diff = (long)(cache_span - avg_llc_val);
diff_percent = ((float)cache_span - avg_llc_val) / cache_span * 100;
ret = platform && abs((int)diff_percent) > max_diff_percent &&
- abs(avg_diff) > max_diff;
+ labs(avg_diff) > max_diff;
ksft_print_msg("%s Check cache miss rate within %lu%%\n",
ret ? "Fail:" : "Pass:", max_diff_percent);
diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c
index 7946e32e85c8..673b2bb800f7 100644
--- a/tools/testing/selftests/resctrl/mba_test.c
+++ b/tools/testing/selftests/resctrl/mba_test.c
@@ -77,7 +77,7 @@ static bool show_mba_info(unsigned long *bw_imc, unsigned long *bw_resc)
avg_bw_imc = sum_bw_imc / (NUM_OF_RUNS - 1);
avg_bw_resc = sum_bw_resc / (NUM_OF_RUNS - 1);
- avg_diff = (float)labs(avg_bw_resc - avg_bw_imc) / avg_bw_imc;
+ avg_diff = (float)(avg_bw_resc - avg_bw_imc) / avg_bw_imc;
avg_diff_per = (int)(avg_diff * 100);
ksft_print_msg("%s Check MBA diff within %d%% for schemata %u\n",
diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c
index d67ffa3ec63a..c873793d016d 100644
--- a/tools/testing/selftests/resctrl/mbm_test.c
+++ b/tools/testing/selftests/resctrl/mbm_test.c
@@ -33,7 +33,7 @@ show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, size_t span)
avg_bw_imc = sum_bw_imc / 4;
avg_bw_resc = sum_bw_resc / 4;
- avg_diff = (float)labs(avg_bw_resc - avg_bw_imc) / avg_bw_imc;
+ avg_diff = (float)(avg_bw_resc - avg_bw_imc) / avg_bw_imc;
avg_diff_per = (int)(avg_diff * 100);
ret = avg_diff_per > MAX_DIFF_PERCENT;
base-commit: f03359bca01bf4372cf2c118cd9a987a5951b1c8
prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27
prerequisite-patch-id: 8d96c4b8c3ed6d9ea2588ef7f594ae0f9f83c279
--
2.45.0
Hi,
This fifth series fixes _metadata reset and fixes the last patch to
handle code set with direct calls to _exit().
As reported by Kernel Test Robot [1], some pidfd tests fail. This is
due to the use of vfork() which introduced some side effects.
Similarly, while making it more generic, a previous commit made some
Landlock file system tests flaky, and subject to the host's file system
mount configuration.
This series fixes all these side effects by replacing vfork() with
clone3() and CLONE_VFORK, which is cleaner (no arbitrary shared memory)
and makes the Kselftest framework more robust.
I tried different approaches and I found this one to be the cleaner and
less invasive for current test cases.
I successfully ran the following tests (using TEST_F and
fork/clone/clone3, and KVM_ONE_VCPU_TEST) with this series:
- kvm:fix_hypercall_test
- kvm:sync_regs_test
- kvm:userspace_msr_exit_test
- kvm:vmx_pmu_caps_test
- landlock:fs_test
- landlock:net_test
- landlock:ptrace_test
- move_mount_set_group:move_mount_set_group_test
- net/af_unix:scm_pidfd
- perf_events:remove_on_exec
- pidfd:pidfd_getfd_test
- pidfd:pidfd_setns_test
- seccomp:seccomp_bpf
- user_events:abi_test
[1] https://lore.kernel.org/oe-lkp/202403291015.1fcfa957-oliver.sang@intel.com
Previous versions:
v1: https://lore.kernel.org/r/20240426172252.1862930-1-mic@digikod.net
v2: https://lore.kernel.org/r/20240429130931.2394118-1-mic@digikod.net
v3: https://lore.kernel.org/r/20240429191911.2552580-1-mic@digikod.net
v4: https://lore.kernel.org/r/20240502210926.145539-1-mic@digikod.net
Regards,
Mickaël Salaün (10):
selftests/pidfd: Fix config for pidfd_setns_test
selftests/landlock: Fix FS tests when run on a private mount point
selftests/harness: Fix fixture teardown
selftests/harness: Fix interleaved scheduling leading to race
conditions
selftests/landlock: Do not allocate memory in fixture data
selftests/harness: Constify fixture variants
selftests/pidfd: Fix wrong expectation
selftests/harness: Share _metadata between forked processes
selftests/harness: Fix vfork() side effects
selftests/harness: Handle TEST_F()'s explicit exit codes
tools/testing/selftests/kselftest_harness.h | 122 +++++++++++++-----
tools/testing/selftests/landlock/fs_test.c | 83 +++++++-----
tools/testing/selftests/pidfd/config | 2 +
.../selftests/pidfd/pidfd_setns_test.c | 2 +-
4 files changed, 143 insertions(+), 66 deletions(-)
base-commit: e67572cd2204894179d89bd7b984072f19313b03
--
2.45.0
When building either tools/bpf/bpftool, or tools/testing/selftests/hid,
(the same Makefile is used for these), clang generates many instances of
a warning that is useless here:
"clang: warning: -lLLVM-17: 'linker' input unused"
Silence this in both locations, by disabling that warning when building
with clang.
Signed-off-by: John Hubbard <jhubbard(a)nvidia.com>
---
tools/bpf/bpftool/Makefile | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index e9154ace80ff..c7457921d136 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -133,6 +133,10 @@ CFLAGS += -DUSE_LIBCAP
LIBS += -lcap
endif
+ifneq ($(LLVM),)
+ CFLAGS += -Wno-unused-command-line-argument
+endif
+
include $(wildcard $(OUTPUT)*.d)
all: $(OUTPUT)bpftool
base-commit: f462ae0edd3703edd6f22fe41d336369c38b884b
prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27
--
2.45.0
When building with clang, via:
make LLVM=1 -C tools/testing/selftest
...clang warns about using "int *" interchangeably with "socklen_t *".
clang is correct, so fix this by declaring len as a socklen_t, instead
of as an int.
Signed-off-by: John Hubbard <jhubbard(a)nvidia.com>
---
tools/testing/selftests/netfilter/sctp_collision.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/netfilter/sctp_collision.c b/tools/testing/selftests/netfilter/sctp_collision.c
index 21bb1cfd8a85..91df996367e9 100644
--- a/tools/testing/selftests/netfilter/sctp_collision.c
+++ b/tools/testing/selftests/netfilter/sctp_collision.c
@@ -9,7 +9,8 @@
int main(int argc, char *argv[])
{
struct sockaddr_in saddr = {}, daddr = {};
- int sd, ret, len = sizeof(daddr);
+ int sd, ret;
+ socklen_t len = sizeof(daddr);
struct timeval tv = {25, 0};
char buf[] = "hello";
base-commit: f462ae0edd3703edd6f22fe41d336369c38b884b
prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27
--
2.45.0