From: Liu Jing <liujing@cmss.chinamobile.com>
in main_loop_s function, when open(cfg_input, O_RDONLY),When the open(cfg_input, O_RDONLY) function is run,
the last fd file descriptor is not closed if the if (--cfg_repeat > 0) branch is not taken
Signed-off-by: Liu Jing <liujing@cmss.chinamobile.com>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
tools/testing/selftests/net/mptcp/mptcp_connect.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index d2043ec3bf6d..48b7389ae75b 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
@@ -1119,7 +1119,8 @@ int main_loop_s(int listensock)
if (cfg_input)
close(fd);
goto again;
- }
+ } else
+ close(fd);
return 0;
}
--
2.45.2
Hi Liu, Thank you for your modifications, that's great! Our CI did some validations and here is its report: - KVM Validation: normal: Success! ✅ - KVM Validation: debug: Success! ✅ - KVM Validation: btf (only bpftest_all): Success! ✅ - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/9749324571 Initiator: Patchew Applier Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/27aea1e315d8 Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=867250 If there are some issues, you can reproduce them using the same environment as the one used by the CI thanks to a docker image, e.g.: $ cd [kernel source code] $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \ --pull always mptcp/mptcp-upstream-virtme-docker:latest \ auto-normal For more details: https://github.com/multipath-tcp/mptcp-upstream-virtme-docker Please note that despite all the efforts that have been already done to have a stable tests suite when executed on a public CI like here, it is possible some reported issues are not due to your modifications. Still, do not hesitate to help us improve that ;-) Cheers, MPTCP GH Action bot Bot operated by Matthieu Baerts (NGI0 Core)
Hi Liu Jing, Thank you for this patch. I have some comments, please see below. Can you send a v2 please? Please send such patches to MPTCP mailing list, as mentioned in the MAINTAINERS file: for us to be able to track and review these patches. And for our CI to validate them. On 01/07/2024 20:16, Matthieu Baerts (NGI0) wrote: > From: Liu Jing <liujing@cmss.chinamobile.com> > > in main_loop_s function, when open(cfg_input, O_RDONLY),When the open(cfg_input, O_RDONLY) function is run, "when open(cfg_input, O_RDONLY)" is duplicated. > the last fd file descriptor is not closed if the if (--cfg_repeat > 0) branch is not taken Could you limit the lines to max 72 chars for the description and around ~50 for the title please? selftests: mptcp: always close input's FD if opened Also, it looks like it is a bug-fix. Then please add a 'Fixes' tag, e.g.: Fixes: 05be5e273c84 ("selftests: mptcp: add disconnect tests") > Signed-off-by: Liu Jing <liujing@cmss.chinamobile.com> > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> (In your v2, please drop my SoB that has been automatically added when sending this patch to our ML) > --- > tools/testing/selftests/net/mptcp/mptcp_connect.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c > index d2043ec3bf6d..48b7389ae75b 100644 > --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c > +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c > @@ -1119,7 +1119,8 @@ int main_loop_s(int listensock) > if (cfg_input) > close(fd); > goto again; > - } > + } else > + close(fd); I think it would be clearer to move... if (cfg_input) close(fd); ... before 'if (--cfg_repeat > 0)' (with an empty line in between): so first we 'close(fd)' if needed, then we check if we need to do it again. Note: Please run 'checkpatch.pl': it was complaining about this line: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/9749324582 https://docs.kernel.org/process/5.Posting.html > > return 0; > } Cheers, Matt -- Sponsored by the NGI0 Core fund.
© 2016 - 2024 Red Hat, Inc.