Series comparison

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch series addresses several resource management issues in the MPTCP
sockopt selftests:

Patch 1 fixes a resource leak by ensuring the server's listening socket
is properly closed after processing clients, rather than leaving it open.

Patch 2 eliminates double-closing of pipe descriptors by removing redundant
close operations in functions where these descriptors are already managed
by the main function.

Patch 3 corrects an error reporting issue where the wrong variable was
being checked in debug output, ensuring accurate debugging information
when assertions fail.

These changes improve the robustness and correctness of the MPTCP selftests
by ensuring proper resource cleanup and accurate error reporting.

Geliang Tang (3):
  selftests: mptcp: close server file descriptor
  selftests: mptcp: avoid double closing pipe descriptor
  selftests: mptcp: sockopt: fix variable check in error reporting

tools/testing/selftests/net/mptcp/mptcp_inq.c     | 3 +--
 tools/testing/selftests/net/mptcp/mptcp_sockopt.c | 6 +++---
 2 files changed, 4 insertions(+), 5 deletions(-)

-- 
2.48.1

From: Geliang Tang <tanggeliang@kylinos.cn>

The server file descriptor ('fd') is opened in server() but never closed.
While accepted connections are properly closed in process_one_client(),
the main listening socket remains open, causing a resource leak.

This patch ensures the server fd is properly closed after processing
clients, bringing the sockopt and inq test cases in line with proper
resource cleanup practices.

Fixes: ce9979129a0b ("selftests: mptcp: add mptcp getsockopt test cases")
Fixes: b51880568f20 ("selftests: mptcp: add inq test case")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_inq.c     | 1 +
 tools/testing/selftests/net/mptcp/mptcp_sockopt.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_inq.c b/tools/testing/selftests/net/mptcp/mptcp_inq.c
index XXXXXXX..XXXXXXX 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_inq.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_inq.c
@@ -XXX,XX +XXX,XX @@ static int server(int unixfd)
 
 	process_one_client(r, unixfd);
 
+	close(fd);
 	return 0;
 }
 
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
index XXXXXXX..XXXXXXX 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
@@ -XXX,XX +XXX,XX @@ static int server(int pipefd)
 
 	process_one_client(r, pipefd);
 
+	close(fd);
 	return 0;
 }
 
-- 
2.48.1

From: Geliang Tang <tanggeliang@kylinos.cn>

The pipe descriptor (pipefds[0]) is already closed in main() after
the client() function completes, making the explicit close in
connect_one_server() redundant and potentially harmful.

This patch removes the unnecessary close operations in both the sockopt
and inq test cases to prevent double-closing of file descriptors, which
could lead to undefined behavior if the descriptor is reused.

Fixes: ce9979129a0b ("selftests: mptcp: add mptcp getsockopt test cases")
Fixes: b51880568f20 ("selftests: mptcp: add inq test case")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_inq.c     | 2 --
 tools/testing/selftests/net/mptcp/mptcp_sockopt.c | 1 -
 2 files changed, 3 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_inq.c b/tools/testing/selftests/net/mptcp/mptcp_inq.c
index XXXXXXX..XXXXXXX 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_inq.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_inq.c
@@ -XXX,XX +XXX,XX @@ static void connect_one_server(int fd, int unixfd)
 	close(fd);
 	ret = write(unixfd, "closed", 6);
 	assert(ret == 6);
-
-	close(unixfd);
 }
 
 static void get_tcp_inq(struct msghdr *msgh, unsigned int *inqv)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
index XXXXXXX..XXXXXXX 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
@@ -XXX,XX +XXX,XX @@ static void connect_one_server(int fd, int pipefd)
 	/* un-block server */
 	ret = read(pipefd, buf2, 4);
 	assert(ret == 4);
-	close(pipefd);
 
 	assert(strncmp(buf2, "xmit", 4) == 0);
 
-- 
2.48.1

From: Geliang Tang <tanggeliang@kylinos.cn>

The error message for mptcpi_bytes_acked comparison incorrectly
references 'ret2' instead of 'ret'. Since the comparison is between
mptcpi_bytes_acked and 'ret', the error output should consistently
use 'ret' for both the expected value and difference calculation.

This patch corrects the variable usage in the error output to ensure
accurate debugging information when the assertion fails.

Fixes: 5dcff89e1455 ("selftests: mptcp: explicitly tests aggregate counters")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_sockopt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch ensures the server fd is properly closed after processing
clients, bringing the sockopt and inq test cases in line with proper
resource cleanup practices.

Fixes: ce9979129a0b ("selftests: mptcp: add mptcp getsockopt test cases")
Fixes: b51880568f20 ("selftests: mptcp: add inq test case")
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_inq.c     | 1 +
 tools/testing/selftests/net/mptcp/mptcp_sockopt.c | 1 +
 2 files changed, 2 insertions(+)

From: Geliang Tang <tanggeliang@kylinos.cn>

The client-side function 'connect_one_server()' correctly closes the IPC
descriptor (a pipe or UNIX socket) after use. However, the server-side
functions 'process_one_client()' in both 'mptcp_sockopt.c' and 'mptcp_inq.c'
were missing the corresponding 'close()' call for their IPC descriptors.

This omission could lead to resource leaks (file descriptors) in the test
server processes over time.

This patch adds the missing 'close(pipefd)' and 'close(unixfd)' calls in the
server-side code, ensuring symmetric and correct resource cleanup.

diff --git a/tools/testing/selftests/net/mptcp/mptcp_inq.c b/tools/testing/selftests/net/mptcp/mptcp_inq.c
index XXXXXXX..XXXXXXX 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_inq.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_inq.c
@@ -XXX,XX +XXX,XX @@ static void process_one_client(int fd, int unixfd)
 	get_tcp_inq(&msg, &tcp_inq);
 	assert(tcp_inq == 1);
 
+	close(unixfd);
 	close(fd);
 }
 
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
index XXXXXXX..XXXXXXX 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
@@ -XXX,XX +XXX,XX @@ static void process_one_client(int fd, int pipefd)
 			       s.last_sample.mptcpi_bytes_acked - ret2);
 	}
 
+	close(pipefd);
 	close(fd);
 }
 
-- 
2.48.1

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch fixes several issues in the error reporting of the MPTCP sockopt
selftest:

1. Add diff calculation: The error messages for counter mismatches now
   include the actual difference ('diff') between the expected and received
   values, making debugging significantly easier.

2. Fix variable usage: The error check for 'mptcpi_bytes_acked' incorrectly
   used 'ret2' (sent bytes) for both the expected value and the difference
   calculation. It now correctly uses 'ret' (received bytes), which is the
   expected value for bytes_acked.

3. Fix off-by-one in diff: The calculation for the 'mptcpi_rcv_delta' diff
   was 's.mptcpi_rcv_delta - ret', which is off-by-one. It has been corrected
   to 's.mptcpi_rcv_delta - (ret + 1)' to match the expected value in the
   condition above it.

Fixes: 5dcff89e1455 ("selftests: mptcp: explicitly tests aggregate counters")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 .../testing/selftests/net/mptcp/mptcp_sockopt.c  | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
index XXXXXXX..XXXXXXX 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
@@ -XXX,XX +XXX,XX @@ static void process_one_client(int fd, int pipefd)
 
 	do_getsockopts(&s, fd, ret, ret2);
 	if (s.mptcpi_rcv_delta != (uint64_t)ret + 1)
-		xerror("mptcpi_rcv_delta %" PRIu64 ", expect %" PRIu64, s.mptcpi_rcv_delta, ret + 1, s.mptcpi_rcv_delta - ret);
+		xerror("mptcpi_rcv_delta %" PRIu64 ", expect %" PRIu64 ", diff %" PRId64,
+		       s.mptcpi_rcv_delta, ret + 1, s.mptcpi_rcv_delta - (ret + 1));
 
 	/* be nice when running on top of older kernel */
 	if (s.pkt_stats_avail) {
 		if (s.last_sample.mptcpi_bytes_sent != ret2)
-			xerror("mptcpi_bytes_sent %" PRIu64 ", expect %" PRIu64,
+			xerror("mptcpi_bytes_sent %" PRIu64 ", expect %" PRIu64
+			       ", diff %" PRId64,
 			       s.last_sample.mptcpi_bytes_sent, ret2,
 			       s.last_sample.mptcpi_bytes_sent - ret2);
 		if (s.last_sample.mptcpi_bytes_received != ret)
-			xerror("mptcpi_bytes_received %" PRIu64 ", expect %" PRIu64,
+			xerror("mptcpi_bytes_received %" PRIu64 ", expect %" PRIu64
+			       ", diff %" PRId64,
 			       s.last_sample.mptcpi_bytes_received, ret,
 			       s.last_sample.mptcpi_bytes_received - ret);
 		if (s.last_sample.mptcpi_bytes_acked != ret)
-			xerror("mptcpi_bytes_acked %" PRIu64 ", expect %" PRIu64,
-			       s.last_sample.mptcpi_bytes_acked, ret2,
-			       s.last_sample.mptcpi_bytes_acked - ret2);
+			xerror("mptcpi_bytes_acked %" PRIu64 ", expect %" PRIu64
+			       ", diff %" PRId64,
+			       s.last_sample.mptcpi_bytes_acked, ret,
+			       s.last_sample.mptcpi_bytes_acked - ret);
 	}
 
 	close(pipefd);
-- 
2.48.1