* [PATCH 1/2] selftests/netfilter: use socklen_t, not a signed int, for len
@ 2024-05-05 21:47 John Hubbard
2024-05-05 21:47 ` [PATCH 2/2] selftests/netfilter: return a value for several "int" functions John Hubbard
0 siblings, 1 reply; 4+ messages in thread
From: John Hubbard @ 2024-05-05 21:47 UTC (permalink / raw
To: Shuah Khan
Cc: Phil Sutter, Pablo Neira Ayuso, Florian Westphal, Felix Huettner,
Max Lamprecht, Luca Czesla, Xin Long, David S . Miller,
Paul Moore, Richard Guy Briggs, Boris Sukholitko, Valentin Obst,
linux-kselftest, LKML, llvm, John Hubbard
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@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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] selftests/netfilter: return a value for several "int" functions
2024-05-05 21:47 [PATCH 1/2] selftests/netfilter: use socklen_t, not a signed int, for len John Hubbard
@ 2024-05-05 21:47 ` John Hubbard
[not found] ` <ZjjsGW314qCgpTKs@felix.runs.onstackit.cloud>
0 siblings, 1 reply; 4+ messages in thread
From: John Hubbard @ 2024-05-05 21:47 UTC (permalink / raw
To: Shuah Khan
Cc: Phil Sutter, Pablo Neira Ayuso, Florian Westphal, Felix Huettner,
Max Lamprecht, Luca Czesla, Xin Long, David S . Miller,
Paul Moore, Richard Guy Briggs, Boris Sukholitko, Valentin Obst,
linux-kselftest, LKML, llvm, John Hubbard
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 0 for each function. (For these functions,
non-zero values indicate failure.)
Signed-off-by: John Hubbard <jhubbard@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..2513221ae5e6 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 0;
}
static int conntracK_count_zone(struct mnl_socket *sock, uint16_t zone)
--
2.45.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] selftests/netfilter: return a value for several "int" functions
[not found] ` <ZjjsGW314qCgpTKs@felix.runs.onstackit.cloud>
@ 2024-05-06 18:29 ` John Hubbard
[not found] ` <ZjnoFfkyREHWUPtq@felix.runs.onstackit.cloud>
0 siblings, 1 reply; 4+ messages in thread
From: John Hubbard @ 2024-05-06 18:29 UTC (permalink / raw
To: Felix Huettner
Cc: Shuah Khan, Phil Sutter, Pablo Neira Ayuso, Florian Westphal,
Max Lamprecht, Luca Czesla, Xin Long, David S . Miller,
Paul Moore, Richard Guy Briggs, Boris Sukholitko, Valentin Obst,
linux-kselftest, LKML, llvm
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] selftests/netfilter: return a value for several "int" functions
[not found] ` <ZjnoFfkyREHWUPtq@felix.runs.onstackit.cloud>
@ 2024-05-07 17:05 ` John Hubbard
0 siblings, 0 replies; 4+ messages in thread
From: John Hubbard @ 2024-05-07 17:05 UTC (permalink / raw
To: Felix Huettner
Cc: Shuah Khan, Phil Sutter, Pablo Neira Ayuso, Florian Westphal,
Max Lamprecht, Luca Czesla, Xin Long, David S . Miller,
Paul Moore, Richard Guy Briggs, Boris Sukholitko, Valentin Obst,
linux-kselftest, LKML, llvm
On 5/7/24 1:36 AM, Felix Huettner wrote:
> >
> > 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
> >
> > ?
>
> lgtm
>
OK, I'll send that out, appreciate your looking at it.
thanks,
--
John Hubbard
NVIDIA
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-05-07 17:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-05 21:47 [PATCH 1/2] selftests/netfilter: use socklen_t, not a signed int, for len John Hubbard
2024-05-05 21:47 ` [PATCH 2/2] selftests/netfilter: return a value for several "int" functions John Hubbard
[not found] ` <ZjjsGW314qCgpTKs@felix.runs.onstackit.cloud>
2024-05-06 18:29 ` John Hubbard
[not found] ` <ZjnoFfkyREHWUPtq@felix.runs.onstackit.cloud>
2024-05-07 17:05 ` John Hubbard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).