Re: [PATCH 2/2] selftests/netfilter: return a value for several "int" functions

[John Hubbard <jhubbard@xxxxxxxxxx>]

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