Re: [PATCH bpf-next v2 2/2] selftests/bpf: Add pairs_redir_to_connected helper

On 10/5/23 12:21 AM, Geliang Tang wrote:

--- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c @@ -1336,32 +1336,22 @@ static void test_redir(struct test_sockmap_listen *skel, struct bpf_map *map, } } -static void unix_redir_to_connected(int sotype, int sock_mapfd,

• 	       int verd_mapfd, enum redir_mode mode)
  

+static void pairs_redir_to_connected(int cli0, int peer0, int cli1, int peer1,

• 		     int sock_mapfd, int verd_mapfd, enum redir_mode mode)
  

  { const char *log_prefix = redir_mode_str(mode);

• int c0, c1, p0, p1; unsigned int pass; int err, n;
• int sfd[2]; u32 key; char b;

zero_verdict_count(verd_mapfd);

• if (socketpair(AF_UNIX, sotype | SOCK_NONBLOCK, 0, sfd))

• return;
  

• c0 = sfd[0], p0 = sfd[1];
• if (socketpair(AF_UNIX, sotype | SOCK_NONBLOCK, 0, sfd))

• goto close0;
  

• c1 = sfd[0], p1 = sfd[1];
• err = add_to_sockmap(sock_mapfd, p0, p1);

• err = add_to_sockmap(sock_mapfd, peer0, peer1); if (err) goto close;

• n = write(c1, "a", 1);

• n = write(cli1, "a", 1); if (n < 0) FAIL_ERRNO("%s: write", log_prefix); if (n == 0)

@@ -1376,16 +1366,34 @@ static void unix_redir_to_connected(int sotype, int sock_mapfd, if (pass != 1) FAIL("%s: want pass count 1, have %d", log_prefix, pass);

• n = recv_timeout(mode == REDIR_INGRESS ? p0 : c0, &b, 1, 0, IO_TIMEOUT_SEC);

• n = recv_timeout(mode == REDIR_INGRESS ? peer0 : cli0, &b, 1, 0, IO_TIMEOUT_SEC); if (n < 0) FAIL_ERRNO("%s: recv_timeout", log_prefix); if (n == 0) FAIL("%s: incomplete recv", log_prefix);

close:

• xclose(c1);
• xclose(p1);

-close0:

• xclose(cli1);
• xclose(peer1);

+}

+static void unix_redir_to_connected(int sotype, int sock_mapfd,

• 	       int verd_mapfd, enum redir_mode mode)
  

+{

• int c0, c1, p0, p1;
• int sfd[2];
• if (socketpair(AF_UNIX, sotype | SOCK_NONBLOCK, 0, sfd))

• return;
  

• c0 = sfd[0], p0 = sfd[1];
• if (socketpair(AF_UNIX, sotype | SOCK_NONBLOCK, 0, sfd))

• goto close;
  

• c1 = sfd[0], p1 = sfd[1];
• pairs_redir_to_connected(c0, p0, c1, p1, sock_mapfd, verd_mapfd, mode);

+close: xclose(c0); xclose(p0); } @@ -1661,51 +1669,19 @@ static int inet_socketpair(int family, int type, int *s, int *c) static void udp_redir_to_connected(int family, int sock_mapfd, int verd_mapfd, enum redir_mode mode) {

• const char *log_prefix = redir_mode_str(mode); int c0, c1, p0, p1;
• unsigned int pass;
• int err, n;
• u32 key;
• char b;
• zero_verdict_count(verd_mapfd);

• int err;

err = inet_socketpair(family, SOCK_DGRAM, &p0, &c0); if (err) return; err = inet_socketpair(family, SOCK_DGRAM, &p1, &c1); if (err)

• goto close_cli0;
  

• err = add_to_sockmap(sock_mapfd, p0, p1);
• if (err)

• goto close_cli1;
  

• n = write(c1, "a", 1);
• if (n < 0)

• FAIL_ERRNO("%s: write", log_prefix);
  

• if (n == 0)

• FAIL("%s: incomplete write", log_prefix);
  

• if (n < 1)

• goto close_cli1;
  

• key = SK_PASS;
• err = xbpf_map_lookup_elem(verd_mapfd, &key, &pass);
• if (err)

• goto close_cli1;
  

• if (pass != 1)

• FAIL("%s: want pass count 1, have %d", log_prefix, pass);
  

• goto close;
  

• n = recv_timeout(mode == REDIR_INGRESS ? p0 : c0, &b, 1, 0, IO_TIMEOUT_SEC);
• if (n < 0)

• FAIL_ERRNO("%s: recv_timeout", log_prefix);
  

• if (n == 0)

• FAIL("%s: incomplete recv", log_prefix);
  

• pairs_redir_to_connected(c0, p0, c1, p1, sock_mapfd, verd_mapfd, mode);

-close_cli1:

• xclose(c1);
• xclose(p1);

-close_cli0: +close: xclose(c0); xclose(p0);

Patch 1 is applied. Thanks.

In patch 2, the xclose() here is confusing after this change. It is also inconsistent from how other tests in sockmap_listen.c is doing it. c0/p0 and c1/p1 are opened here but only c0/p0 is closed here and c1/p1 is closed in the pairs_redir_to_connected() above instead.