| Message ID | 20240505214716.62304-1-jhubbard@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] selftests/netfilter: use socklen_t, not a signed int, for len | expand |
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,
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,
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";
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(-) base-commit: f462ae0edd3703edd6f22fe41d336369c38b884b prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27