selftests: mptcp: pm_nl_ctl: always look for errors

If a Netlink command for the MPTCP path-managers is not valid, it is
important to check if there are errors. If yes, they need to be reported
instead of being ignored and exiting without errors.

Now if no replies are expected, an ACK from the kernelspace is asked by
the userspace in order to always expect a reply. We can use the same
buffer that is currently always >1024 bytes. Then we can check if there
is an error (err->error), print it if any and report the error.

After this modification, it is required to mute expected errors in
mptcp_join.sh and pm_netlink.sh selftests:

- when trying to add a bad endpoint, e.g. duplicated
- when trying to set the two limits above the hard limit

Reviewed-by: Mat Martineau <martineau@kernel.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Link: https://lore.kernel.org/r/20230730-upstream-net-next-20230728-mptcp-selftests-misc-v1-3-7e9cc530a9cd@tessares.net
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 136d20641fceacb203c32775b91d402b07de9467..63658b0416fb6f71b4e9c1ee7598c1787bb8f933 100755 (executable)
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -3452,7 +3452,7 @@ endpoint_tests()
                chk_mptcp_info subflows 1 subflows 1
                chk_mptcp_info add_addr_signal 1 add_addr_accepted 1
 
                chk_mptcp_info subflows 1 subflows 1
                chk_mptcp_info add_addr_signal 1 add_addr_accepted 1
 

-               pm_nl_add_endpoint $ns2 10.0.2.2 id 33
+               pm_nl_add_endpoint $ns2 10.0.2.2 id 33 2>/dev/null

                pm_nl_check_endpoint "ID change is prevented" \
                        $ns2 10.0.2.2 id 1 flags implicit
 
                pm_nl_check_endpoint "ID change is prevented" \
                        $ns2 10.0.2.2 id 1 flags implicit
 

diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
index f32038fe1ee5b2b4fcec3bf6780fb27c3ae5e011..8f4ff123a7eb92646845a5dea4caf28483057085 100755 (executable)
--- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
+++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
@@ -99,7 +99,7 @@ check "ip netns exec $ns1 ./pm_nl_ctl dump" \
 "id 1 flags  10.0.1.1
 id 3 flags signal,backup 10.0.1.3" "dump addrs after del"
 
 "id 1 flags  10.0.1.1
 id 3 flags signal,backup 10.0.1.3" "dump addrs after del"
 

-ip netns exec $ns1 ./pm_nl_ctl add 10.0.1.3
+ip netns exec $ns1 ./pm_nl_ctl add 10.0.1.3 2>/dev/null

 check "ip netns exec $ns1 ./pm_nl_ctl get 4" "" "duplicate addr"
 
 ip netns exec $ns1 ./pm_nl_ctl add 10.0.1.4 flags signal
 check "ip netns exec $ns1 ./pm_nl_ctl get 4" "" "duplicate addr"
 
 ip netns exec $ns1 ./pm_nl_ctl add 10.0.1.4 flags signal


@@ -127,10 +127,10 @@ id 8 flags signal 10.0.1.8" "id limit"
 ip netns exec $ns1 ./pm_nl_ctl flush
 check "ip netns exec $ns1 ./pm_nl_ctl dump" "" "flush addrs"
 
 ip netns exec $ns1 ./pm_nl_ctl flush
 check "ip netns exec $ns1 ./pm_nl_ctl dump" "" "flush addrs"
 

-ip netns exec $ns1 ./pm_nl_ctl limits 9 1
+ip netns exec $ns1 ./pm_nl_ctl limits 9 1 2>/dev/null

 check "ip netns exec $ns1 ./pm_nl_ctl limits" "$default_limits" "rcv addrs above hard limit"
 
 check "ip netns exec $ns1 ./pm_nl_ctl limits" "$default_limits" "rcv addrs above hard limit"
 

-ip netns exec $ns1 ./pm_nl_ctl limits 1 9
+ip netns exec $ns1 ./pm_nl_ctl limits 1 9 2>/dev/null

 check "ip netns exec $ns1 ./pm_nl_ctl limits" "$default_limits" "subflows above hard limit"
 
 ip netns exec $ns1 ./pm_nl_ctl limits 8 8
 check "ip netns exec $ns1 ./pm_nl_ctl limits" "$default_limits" "subflows above hard limit"
 
 ip netns exec $ns1 ./pm_nl_ctl limits 8 8

diff --git a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
index 1887bd61bd9a5f5818a7e06e47ecb627bac35205..49369c4a5f261f73c51f7e598f9d53f9b5970d66 100644 (file)
--- a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
+++ b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
@@ -66,20 +66,25 @@ static int init_genl_req(char *data, int family, int cmd, int version)
        return off;
 }
 
        return off;
 }
 

-static void nl_error(struct nlmsghdr *nh)
+static int nl_error(struct nlmsghdr *nh)

 {
        struct nlmsgerr *err = (struct nlmsgerr *)NLMSG_DATA(nh);
        int len = nh->nlmsg_len - sizeof(*nh);
        uint32_t off;
 
 {
        struct nlmsgerr *err = (struct nlmsgerr *)NLMSG_DATA(nh);
        int len = nh->nlmsg_len - sizeof(*nh);
        uint32_t off;
 

-       if (len < sizeof(struct nlmsgerr))
+       if (len < sizeof(struct nlmsgerr)) {

                error(1, 0, "netlink error message truncated %d min %ld", len,
                      sizeof(struct nlmsgerr));
                error(1, 0, "netlink error message truncated %d min %ld", len,
                      sizeof(struct nlmsgerr));

+               return -1;
+       }

 
 

-       if (!err->error) {
+       if (err->error) {

                /* check messages from kernel */
                struct rtattr *attrs = (struct rtattr *)NLMSG_DATA(nh);
 
                /* check messages from kernel */
                struct rtattr *attrs = (struct rtattr *)NLMSG_DATA(nh);
 

+               fprintf(stderr, "netlink error %d (%s)\n",
+                       err->error, strerror(-err->error));
+

                while (RTA_OK(attrs, len)) {
                        if (attrs->rta_type == NLMSGERR_ATTR_MSG)
                                fprintf(stderr, "netlink ext ack msg: %s\n",
                while (RTA_OK(attrs, len)) {
                        if (attrs->rta_type == NLMSGERR_ATTR_MSG)
                                fprintf(stderr, "netlink ext ack msg: %s\n",


@@ -91,9 +96,10 @@ static void nl_error(struct nlmsghdr *nh)
                        }
                        attrs = RTA_NEXT(attrs, len);
                }
                        }
                        attrs = RTA_NEXT(attrs, len);
                }

-       } else {
-               fprintf(stderr, "netlink error %d", err->error);
+               return -1;

        }
        }

+
+       return 0;

 }
 
 static int capture_events(int fd, int event_group)
 }
 
 static int capture_events(int fd, int event_group)


@@ -198,7 +204,7 @@ static int capture_events(int fd, int event_group)
        return 0;
 }
 
        return 0;
 }
 

-/* do a netlink command and, if max > 0, fetch the reply  */
+/* do a netlink command and, if max > 0, fetch the reply ; nh's size >1024B */

 static int do_nl_req(int fd, struct nlmsghdr *nh, int len, int max)
 {
        struct sockaddr_nl nladdr = { .nl_family = AF_NETLINK };
 static int do_nl_req(int fd, struct nlmsghdr *nh, int len, int max)
 {
        struct sockaddr_nl nladdr = { .nl_family = AF_NETLINK };


@@ -207,12 +213,16 @@ static int do_nl_req(int fd, struct nlmsghdr *nh, int len, int max)
        int rem, ret;
        int err = 0;
 
        int rem, ret;
        int err = 0;
 

+       /* If no expected answer, ask for an ACK to look for errors if any */
+       if (max == 0) {
+               nh->nlmsg_flags |= NLM_F_ACK;
+               max = 1024;
+       }
+

        nh->nlmsg_len = len;
        ret = sendto(fd, data, len, 0, (void *)&nladdr, sizeof(nladdr));
        if (ret != len)
                error(1, errno, "send netlink: %uB != %uB\n", ret, len);
        nh->nlmsg_len = len;
        ret = sendto(fd, data, len, 0, (void *)&nladdr, sizeof(nladdr));
        if (ret != len)
                error(1, errno, "send netlink: %uB != %uB\n", ret, len);

-       if (max == 0)
-               return 0;

 
        addr_len = sizeof(nladdr);
        rem = ret = recvfrom(fd, data, max, 0, (void *)&nladdr, &addr_len);
 
        addr_len = sizeof(nladdr);
        rem = ret = recvfrom(fd, data, max, 0, (void *)&nladdr, &addr_len);


@@ -221,10 +231,11 @@ static int do_nl_req(int fd, struct nlmsghdr *nh, int len, int max)
 
        /* Beware: the NLMSG_NEXT macro updates the 'rem' argument */
        for (; NLMSG_OK(nh, rem); nh = NLMSG_NEXT(nh, rem)) {
 
        /* Beware: the NLMSG_NEXT macro updates the 'rem' argument */
        for (; NLMSG_OK(nh, rem); nh = NLMSG_NEXT(nh, rem)) {

-               if (nh->nlmsg_type == NLMSG_ERROR) {
-                       nl_error(nh);
+               if (nh->nlmsg_type == NLMSG_DONE)
+                       break;
+
+               if (nh->nlmsg_type == NLMSG_ERROR && nl_error(nh))

                        err = 1;
                        err = 1;

-               }

        }
        if (err)
                error(1, 0, "bailing out due to netlink error[s]");
        }
        if (err)
                error(1, 0, "bailing out due to netlink error[s]");