OSDN Git Service

bpf: selftests: A few improvements to network_helpers.c
authorMartin KaFai Lau <kafai@fb.com>
Thu, 2 Jul 2020 00:48:52 +0000 (17:48 -0700)
committerDaniel Borkmann <daniel@iogearbox.net>
Thu, 2 Jul 2020 14:09:01 +0000 (16:09 +0200)
This patch makes a few changes to the network_helpers.c

1) Enforce SO_RCVTIMEO and SO_SNDTIMEO
   This patch enforces timeout to the network fds through setsockopt
   SO_RCVTIMEO and SO_SNDTIMEO.

   It will remove the need for SOCK_NONBLOCK that requires a more demanding
   timeout logic with epoll/select, e.g. epoll_create, epoll_ctrl, and
   then epoll_wait for timeout.

   That removes the need for connect_wait() from the
   cgroup_skb_sk_lookup.c. The needed change is made in
   cgroup_skb_sk_lookup.c.

2) start_server():
   Add optional addr_str and port to start_server().
   That removes the need of the start_server_with_port().  The caller
   can pass addr_str==NULL and/or port==0.

   I have a future tcp-hdr-opt test that will pass a non-NULL addr_str
   and it is in general useful for other future tests.

   "int timeout_ms" is also added to control the timeout
   on the "accept(listen_fd)".

3) connect_to_fd(): Fully use the server_fd.
   The server sock address has already been obtained from
   getsockname(server_fd).  The sockaddr includes the family,
   so the "int family" arg is redundant.

   Since the server address is obtained from server_fd,  there
   is little reason not to get the server's socket type from the
   server_fd also.  getsockopt(server_fd) can be used to do that,
   so "int type" arg is also removed.

   "int timeout_ms" is added.

4) connect_fd_to_fd():
   "int timeout_ms" is added.
   Some code is also refactored to connect_fd_to_addr() which is
   shared with connect_to_fd().

5) Preserve errno:
   Some callers need to check errno, e.g. cgroup_skb_sk_lookup.c.
   Make changes to do it more consistently in save_errno_close()
   and log_err().

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Yonghong Song <yhs@fb.com>
Link: https://lore.kernel.org/bpf/20200702004852.2103003-1-kafai@fb.com
tools/testing/selftests/bpf/network_helpers.c
tools/testing/selftests/bpf/network_helpers.h
tools/testing/selftests/bpf/prog_tests/cgroup_skb_sk_lookup.c
tools/testing/selftests/bpf/prog_tests/connect_force_port.c
tools/testing/selftests/bpf/prog_tests/load_bytes_relative.c
tools/testing/selftests/bpf/prog_tests/tcp_rtt.c

index e36dd1a..acd0871 100644 (file)
@@ -7,8 +7,6 @@
 
 #include <arpa/inet.h>
 
-#include <sys/epoll.h>
-
 #include <linux/err.h>
 #include <linux/in.h>
 #include <linux/in6.h>
 #include "network_helpers.h"
 
 #define clean_errno() (errno == 0 ? "None" : strerror(errno))
-#define log_err(MSG, ...) fprintf(stderr, "(%s:%d: errno: %s) " MSG "\n", \
-       __FILE__, __LINE__, clean_errno(), ##__VA_ARGS__)
+#define log_err(MSG, ...) ({                                           \
+                       int __save = errno;                             \
+                       fprintf(stderr, "(%s:%d: errno: %s) " MSG "\n", \
+                               __FILE__, __LINE__, clean_errno(),      \
+                               ##__VA_ARGS__);                         \
+                       errno = __save;                                 \
+})
 
 struct ipv4_packet pkt_v4 = {
        .eth.h_proto = __bpf_constant_htons(ETH_P_IP),
@@ -37,7 +40,34 @@ struct ipv6_packet pkt_v6 = {
        .tcp.doff = 5,
 };
 
-int start_server_with_port(int family, int type, __u16 port)
+static int settimeo(int fd, int timeout_ms)
+{
+       struct timeval timeout = { .tv_sec = 3 };
+
+       if (timeout_ms > 0) {
+               timeout.tv_sec = timeout_ms / 1000;
+               timeout.tv_usec = (timeout_ms % 1000) * 1000;
+       }
+
+       if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeout,
+                      sizeof(timeout))) {
+               log_err("Failed to set SO_RCVTIMEO");
+               return -1;
+       }
+
+       if (setsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, &timeout,
+                      sizeof(timeout))) {
+               log_err("Failed to set SO_SNDTIMEO");
+               return -1;
+       }
+
+       return 0;
+}
+
+#define save_errno_close(fd) ({ int __save = errno; close(fd); errno = __save; })
+
+int start_server(int family, int type, const char *addr_str, __u16 port,
+                int timeout_ms)
 {
        struct sockaddr_storage addr = {};
        socklen_t len;
@@ -48,120 +78,119 @@ int start_server_with_port(int family, int type, __u16 port)
 
                sin->sin_family = AF_INET;
                sin->sin_port = htons(port);
+               if (addr_str &&
+                   inet_pton(AF_INET, addr_str, &sin->sin_addr) != 1) {
+                       log_err("inet_pton(AF_INET, %s)", addr_str);
+                       return -1;
+               }
                len = sizeof(*sin);
        } else {
                struct sockaddr_in6 *sin6 = (void *)&addr;
 
                sin6->sin6_family = AF_INET6;
                sin6->sin6_port = htons(port);
+               if (addr_str &&
+                   inet_pton(AF_INET6, addr_str, &sin6->sin6_addr) != 1) {
+                       log_err("inet_pton(AF_INET6, %s)", addr_str);
+                       return -1;
+               }
                len = sizeof(*sin6);
        }
 
-       fd = socket(family, type | SOCK_NONBLOCK, 0);
+       fd = socket(family, type, 0);
        if (fd < 0) {
                log_err("Failed to create server socket");
                return -1;
        }
 
+       if (settimeo(fd, timeout_ms))
+               goto error_close;
+
        if (bind(fd, (const struct sockaddr *)&addr, len) < 0) {
                log_err("Failed to bind socket");
-               close(fd);
-               return -1;
+               goto error_close;
        }
 
        if (type == SOCK_STREAM) {
                if (listen(fd, 1) < 0) {
                        log_err("Failed to listed on socket");
-                       close(fd);
-                       return -1;
+                       goto error_close;
                }
        }
 
        return fd;
-}
 
-int start_server(int family, int type)
-{
-       return start_server_with_port(family, type, 0);
+error_close:
+       save_errno_close(fd);
+       return -1;
 }
 
-static const struct timeval timeo_sec = { .tv_sec = 3 };
-static const size_t timeo_optlen = sizeof(timeo_sec);
-
-int connect_to_fd(int family, int type, int server_fd)
+static int connect_fd_to_addr(int fd,
+                             const struct sockaddr_storage *addr,
+                             socklen_t addrlen)
 {
-       int fd, save_errno;
-
-       fd = socket(family, type, 0);
-       if (fd < 0) {
-               log_err("Failed to create client socket");
+       if (connect(fd, (const struct sockaddr *)addr, addrlen)) {
+               log_err("Failed to connect to server");
                return -1;
        }
 
-       if (connect_fd_to_fd(fd, server_fd) < 0 && errno != EINPROGRESS) {
-               save_errno = errno;
-               close(fd);
-               errno = save_errno;
-               return -1;
-       }
-
-       return fd;
+       return 0;
 }
 
-int connect_fd_to_fd(int client_fd, int server_fd)
+int connect_to_fd(int server_fd, int timeout_ms)
 {
        struct sockaddr_storage addr;
-       socklen_t len = sizeof(addr);
-       int save_errno;
+       struct sockaddr_in *addr_in;
+       socklen_t addrlen, optlen;
+       int fd, type;
 
-       if (setsockopt(client_fd, SOL_SOCKET, SO_RCVTIMEO, &timeo_sec,
-                      timeo_optlen)) {
-               log_err("Failed to set SO_RCVTIMEO");
+       optlen = sizeof(type);
+       if (getsockopt(server_fd, SOL_SOCKET, SO_TYPE, &type, &optlen)) {
+               log_err("getsockopt(SOL_TYPE)");
                return -1;
        }
 
-       if (getsockname(server_fd, (struct sockaddr *)&addr, &len)) {
+       addrlen = sizeof(addr);
+       if (getsockname(server_fd, (struct sockaddr *)&addr, &addrlen)) {
                log_err("Failed to get server addr");
                return -1;
        }
 
-       if (connect(client_fd, (const struct sockaddr *)&addr, len) < 0) {
-               if (errno != EINPROGRESS) {
-                       save_errno = errno;
-                       log_err("Failed to connect to server");
-                       errno = save_errno;
-               }
+       addr_in = (struct sockaddr_in *)&addr;
+       fd = socket(addr_in->sin_family, type, 0);
+       if (fd < 0) {
+               log_err("Failed to create client socket");
                return -1;
        }
 
-       return 0;
+       if (settimeo(fd, timeout_ms))
+               goto error_close;
+
+       if (connect_fd_to_addr(fd, &addr, addrlen))
+               goto error_close;
+
+       return fd;
+
+error_close:
+       save_errno_close(fd);
+       return -1;
 }
 
-int connect_wait(int fd)
+int connect_fd_to_fd(int client_fd, int server_fd, int timeout_ms)
 {
-       struct epoll_event ev = {}, events[2];
-       int timeout_ms = 1000;
-       int efd, nfd;
+       struct sockaddr_storage addr;
+       socklen_t len = sizeof(addr);
 
-       efd = epoll_create1(EPOLL_CLOEXEC);
-       if (efd < 0) {
-               log_err("Failed to open epoll fd");
+       if (settimeo(client_fd, timeout_ms))
                return -1;
-       }
-
-       ev.events = EPOLLRDHUP | EPOLLOUT;
-       ev.data.fd = fd;
 
-       if (epoll_ctl(efd, EPOLL_CTL_ADD, fd, &ev) < 0) {
-               log_err("Failed to register fd=%d on epoll fd=%d", fd, efd);
-               close(efd);
+       if (getsockname(server_fd, (struct sockaddr *)&addr, &len)) {
+               log_err("Failed to get server addr");
                return -1;
        }
 
-       nfd = epoll_wait(efd, events, ARRAY_SIZE(events), timeout_ms);
-       if (nfd < 0)
-               log_err("Failed to wait for I/O event on epoll fd=%d", efd);
+       if (connect_fd_to_addr(client_fd, &addr, len))
+               return -1;
 
-       close(efd);
-       return nfd;
+       return 0;
 }
index 6a80096..f580e82 100644 (file)
@@ -33,10 +33,9 @@ struct ipv6_packet {
 } __packed;
 extern struct ipv6_packet pkt_v6;
 
-int start_server(int family, int type);
-int start_server_with_port(int family, int type, __u16 port);
-int connect_to_fd(int family, int type, int server_fd);
-int connect_fd_to_fd(int client_fd, int server_fd);
-int connect_wait(int client_fd);
+int start_server(int family, int type, const char *addr, __u16 port,
+                int timeout_ms);
+int connect_to_fd(int server_fd, int timeout_ms);
+int connect_fd_to_fd(int client_fd, int server_fd, int timeout_ms);
 
 #endif
index 059047a..464edc1 100644 (file)
@@ -13,7 +13,7 @@ static void run_lookup_test(__u16 *g_serv_port, int out_sk)
        socklen_t addr_len = sizeof(addr);
        __u32 duration = 0;
 
-       serv_sk = start_server(AF_INET6, SOCK_STREAM);
+       serv_sk = start_server(AF_INET6, SOCK_STREAM, NULL, 0, 0);
        if (CHECK(serv_sk < 0, "start_server", "failed to start server\n"))
                return;
 
@@ -24,17 +24,13 @@ static void run_lookup_test(__u16 *g_serv_port, int out_sk)
        *g_serv_port = addr.sin6_port;
 
        /* Client outside of test cgroup should fail to connect by timeout. */
-       err = connect_fd_to_fd(out_sk, serv_sk);
+       err = connect_fd_to_fd(out_sk, serv_sk, 1000);
        if (CHECK(!err || errno != EINPROGRESS, "connect_fd_to_fd",
                  "unexpected result err %d errno %d\n", err, errno))
                goto cleanup;
 
-       err = connect_wait(out_sk);
-       if (CHECK(err, "connect_wait", "unexpected result %d\n", err))
-               goto cleanup;
-
        /* Client inside test cgroup should connect just fine. */
-       in_sk = connect_to_fd(AF_INET6, SOCK_STREAM, serv_sk);
+       in_sk = connect_to_fd(serv_sk, 0);
        if (CHECK(in_sk < 0, "connect_to_fd", "errno %d\n", errno))
                goto cleanup;
 
@@ -85,7 +81,7 @@ void test_cgroup_skb_sk_lookup(void)
         * differs from that of testing cgroup. Moving selftests process to
         * testing cgroup won't change cgroup id of an already created socket.
         */
-       out_sk = socket(AF_INET6, SOCK_STREAM | SOCK_NONBLOCK, 0);
+       out_sk = socket(AF_INET6, SOCK_STREAM, 0);
        if (CHECK_FAIL(out_sk < 0))
                return;
 
index 17bbf76..9229db2 100644 (file)
@@ -114,7 +114,7 @@ static int run_test(int cgroup_fd, int server_fd, int family, int type)
                goto close_bpf_object;
        }
 
-       fd = connect_to_fd(family, type, server_fd);
+       fd = connect_to_fd(server_fd, 0);
        if (fd < 0) {
                err = -1;
                goto close_bpf_object;
@@ -137,25 +137,25 @@ void test_connect_force_port(void)
        if (CHECK_FAIL(cgroup_fd < 0))
                return;
 
-       server_fd = start_server_with_port(AF_INET, SOCK_STREAM, 60123);
+       server_fd = start_server(AF_INET, SOCK_STREAM, NULL, 60123, 0);
        if (CHECK_FAIL(server_fd < 0))
                goto close_cgroup_fd;
        CHECK_FAIL(run_test(cgroup_fd, server_fd, AF_INET, SOCK_STREAM));
        close(server_fd);
 
-       server_fd = start_server_with_port(AF_INET6, SOCK_STREAM, 60124);
+       server_fd = start_server(AF_INET6, SOCK_STREAM, NULL, 60124, 0);
        if (CHECK_FAIL(server_fd < 0))
                goto close_cgroup_fd;
        CHECK_FAIL(run_test(cgroup_fd, server_fd, AF_INET6, SOCK_STREAM));
        close(server_fd);
 
-       server_fd = start_server_with_port(AF_INET, SOCK_DGRAM, 60123);
+       server_fd = start_server(AF_INET, SOCK_DGRAM, NULL, 60123, 0);
        if (CHECK_FAIL(server_fd < 0))
                goto close_cgroup_fd;
        CHECK_FAIL(run_test(cgroup_fd, server_fd, AF_INET, SOCK_DGRAM));
        close(server_fd);
 
-       server_fd = start_server_with_port(AF_INET6, SOCK_DGRAM, 60124);
+       server_fd = start_server(AF_INET6, SOCK_DGRAM, NULL, 60124, 0);
        if (CHECK_FAIL(server_fd < 0))
                goto close_cgroup_fd;
        CHECK_FAIL(run_test(cgroup_fd, server_fd, AF_INET6, SOCK_DGRAM));
index c1168e4..5a2a689 100644 (file)
@@ -23,7 +23,7 @@ void test_load_bytes_relative(void)
        if (CHECK_FAIL(cgroup_fd < 0))
                return;
 
-       server_fd = start_server(AF_INET, SOCK_STREAM);
+       server_fd = start_server(AF_INET, SOCK_STREAM, NULL, 0, 0);
        if (CHECK_FAIL(server_fd < 0))
                goto close_cgroup_fd;
 
@@ -49,7 +49,7 @@ void test_load_bytes_relative(void)
        if (CHECK_FAIL(err))
                goto close_bpf_object;
 
-       client_fd = connect_to_fd(AF_INET, SOCK_STREAM, server_fd);
+       client_fd = connect_to_fd(server_fd, 0);
        if (CHECK_FAIL(client_fd < 0))
                goto close_bpf_object;
        close(client_fd);
index 9013a0c..d207e96 100644 (file)
@@ -118,7 +118,7 @@ static int run_test(int cgroup_fd, int server_fd)
                goto close_bpf_object;
        }
 
-       client_fd = connect_to_fd(AF_INET, SOCK_STREAM, server_fd);
+       client_fd = connect_to_fd(server_fd, 0);
        if (client_fd < 0) {
                err = -1;
                goto close_bpf_object;
@@ -161,7 +161,7 @@ void test_tcp_rtt(void)
        if (CHECK_FAIL(cgroup_fd < 0))
                return;
 
-       server_fd = start_server(AF_INET, SOCK_STREAM);
+       server_fd = start_server(AF_INET, SOCK_STREAM, NULL, 0, 0);
        if (CHECK_FAIL(server_fd < 0))
                goto close_cgroup_fd;