[PATCH v2] selftests: mptcp: Fix incorrect file descriptor check in main_loop

Cong Liu posted 1 patch 5 days, 5 hours ago
Failed in applying to current master (apply log)
tools/testing/selftests/net/mptcp/mptcp_connect.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH v2] selftests: mptcp: Fix incorrect file descriptor check in main_loop
Posted by Cong Liu 5 days, 5 hours ago
Fix a bug where the code was checking the wrong file descriptor
when opening the input file. The code was checking 'fd' instead
of 'fd_in', which could lead to incorrect error handling.

Fixes: ca7ae8916043 ("selftests: mptcp: mptfo Initiator/Listener")
Signed-off-by: Cong Liu <liucong2@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_connect.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index 4209b9569039..31f4c5618569 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
@@ -1249,7 +1249,7 @@ int main_loop(void)
 
 	if (cfg_input && cfg_sockopt_types.mptfo) {
 		fd_in = open(cfg_input, O_RDONLY);
-		if (fd < 0)
+		if (fd_in < 0)
 			xerror("can't open %s:%d", cfg_input, errno);
 	}
 
@@ -1272,7 +1272,7 @@ int main_loop(void)
 
 	if (cfg_input && !cfg_sockopt_types.mptfo) {
 		fd_in = open(cfg_input, O_RDONLY);
-		if (fd < 0)
+		if (fd_in < 0)
 			xerror("can't open %s:%d", cfg_input, errno);
 	}
 
-- 
2.43.0


When I tried to solve the problem where fd_in was closed in the again
code block but not reopened in certain scenarios, I encountered some 
issues. Here's my code, but it causes the disconnect test to fail and I'm
not sure how to fix this. 

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index 31f4c5618569..fbcb6bf6500c 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
@@ -1247,7 +1247,7 @@ int main_loop(void)
        struct addrinfo *peer;
        struct wstate winfo;
 
-       if (cfg_input && cfg_sockopt_types.mptfo) {
+       if (cfg_input) {
                fd_in = open(cfg_input, O_RDONLY);
                if (fd_in < 0)
                        xerror("can't open %s:%d", cfg_input, errno);
@@ -1270,12 +1270,6 @@ int main_loop(void)
        if (cfg_cmsg_types.cmsg_enabled)
                apply_cmsg_types(fd, &cfg_cmsg_types);
 
-       if (cfg_input && !cfg_sockopt_types.mptfo) {
-               fd_in = open(cfg_input, O_RDONLY);
-               if (fd_in < 0)
-                       xerror("can't open %s:%d", cfg_input, errno);
-       }
-
        ret = copyfd_io(fd_in, fd, 1, 0, &winfo);
        if (ret)
                return ret;
@@ -1291,8 +1285,6 @@ int main_loop(void)
                set_nonblock(fd, false);
                if (connect(fd, peer->ai_addr, peer->ai_addrlen))
                        xerror("can't reconnect: %d", errno);
-               if (cfg_input)
-                       close(fd_in);
                memset(&winfo, 0, sizeof(winfo));
                goto again;
        } else {




INFO: disconnect
63 ns1 MPTCP -> ns1 (10.0.1.1:20001      ) MPTCP     (duration   103ms) [FAIL] file received by server does not match (in, out):
-rw-r--r-- 1 root root 18548982  1月 16 16:18 /tmp/tmp.mBbh37L7rj.disconnect
Trailing bytes are: 
�ǫ��r�_���m:�!d��v��Pv��-rw------- 1 root root 6182994  1月 16 16:18 /tmp/tmp.mJRvCLPpUi
Trailing bytes are: 
�ǫ��r�_���m:�!d��v��Pv��64 ns1 MPTCP -> ns1 (dead:beef:1::1:20002) MPTCP     (duration   149ms) [FAIL] file received by server does not match (in, out):
-rw-r--r-- 1 root root 18548982  1月 16 16:18 /tmp/tmp.mBbh37L7rj.disconnect
Trailing bytes are: 
�ǫ��r�_���m:�!d��v��Pv��-rw------- 1 root root 6182994  1月 16 16:18 /tmp/tmp.mJRvCLPpUi
Trailing bytes are: 
�ǫ��r�_���m:�!d��v��Pv��[FAIL] Tests of the full disconnection have failed
Time: 49 seconds

TAP version 13
Re: [PATCH v2] selftests: mptcp: Fix incorrect file descriptor check in main_loop
Posted by Matthieu Baerts 4 days, 21 hours ago
Hi Cong Liu,

(-Cc everybody, except the MPTCP ML)

Thank you for the v2.

On 16/01/2025 09:54, Cong Liu wrote:
> Fix a bug where the code was checking the wrong file descriptor
> when opening the input file. The code was checking 'fd' instead
> of 'fd_in', which could lead to incorrect error handling.
> 
> Fixes: ca7ae8916043 ("selftests: mptcp: mptfo Initiator/Listener")

I think the first issue has been introduced in 05be5e273c84 ("selftests:
mptcp: add disconnect tests"), then in the one you mentioned. Because it
is just in the selftests, feel free to add the older one, or both.

> Signed-off-by: Cong Liu <liucong2@kylinos.cn>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_connect.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index 4209b9569039..31f4c5618569 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> @@ -1249,7 +1249,7 @@ int main_loop(void)
>  
>  	if (cfg_input && cfg_sockopt_types.mptfo) {
>  		fd_in = open(cfg_input, O_RDONLY);
> -		if (fd < 0)
> +		if (fd_in < 0)
>  			xerror("can't open %s:%d", cfg_input, errno);
>  	}
>  
> @@ -1272,7 +1272,7 @@ int main_loop(void)
>  
>  	if (cfg_input && !cfg_sockopt_types.mptfo) {
>  		fd_in = open(cfg_input, O_RDONLY);
> -		if (fd < 0)
> +		if (fd_in < 0)
>  			xerror("can't open %s:%d", cfg_input, errno);
>  	}

The CI is not able to apply your patch. On which base are you?
Do you mind using our 'export' branch from our GitHub repository?

> When I tried to solve the problem where fd_in was closed in the again
> code block but not reopened in certain scenarios, I encountered some 
> issues. Here's my code, but it causes the disconnect test to fail and I'm
> not sure how to fix this. 

Some fixes have been recently applied around the 'disconnect' tests,
maybe this issue has already been fixed?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.