Re: [PATCH 6.1 54/61] selftests: mptcp: connect: catch IO errors on listen side

[Matthieu Baerts <matttbe@kernel.org>]

On 01/10/2025 18:43, Kenta Akagi wrote:
> Hi Matthieu,
> 
> On 2025/10/01 16:56, Matthieu Baerts wrote:
>> Hi Kenta,
>>
>> On 30/09/2025 17:30, Kenta Akagi wrote:
>>> Hi,
>>>
>>> On 2025/09/23 4:29, Greg Kroah-Hartman wrote:
>>>> 6.1-stable review patch.  If anyone has any objections, please let me know.
>>>>
>>>> ------------------
>>>>
>>>> From: "Matthieu Baerts (NGI0)" <matttbe@kernel.org>
>>>>
>>>> [ Upstream commit 14e22b43df25dbd4301351b882486ea38892ae4f ]
>>>>
>>>> IO errors were correctly printed to stderr, and propagated up to the
>>>> main loop for the server side, but the returned value was ignored. As a
>>>> consequence, the program for the listener side was no longer exiting
>>>> with an error code in case of IO issues.
>>>>
>>>> Because of that, some issues might not have been seen. But very likely,
>>>> most issues either had an effect on the client side, or the file
>>>> transfer was not the expected one, e.g. the connection got reset before
>>>> the end. Still, it is better to fix this.
>>>>
>>>> The main consequence of this issue is the error that was reported by the
>>>> selftests: the received and sent files were different, and the MIB
>>>> counters were not printed. Also, when such errors happened during the
>>>> 'disconnect' tests, the program tried to continue until the timeout.
>>>>
>>>> Now when an IO error is detected, the program exits directly with an
>>>> error.
>>>>
>>>> Fixes: 05be5e273c84 ("selftests: mptcp: add disconnect tests")
>>>> Cc: stable@vger.kernel.org
>>>> Reviewed-by: Mat Martineau <martineau@kernel.org>
>>>> Reviewed-by: Geliang Tang <geliang@kernel.org>
>>>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>>>> Link: https://patch.msgid.link/20250912-net-mptcp-fix-sft-connect-v1-2-d40e77cbbf02@kernel.org
>>>> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
>>>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>>>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>> ---
>>>>  tools/testing/selftests/net/mptcp/mptcp_connect.c |    7 ++++---
>>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>>
>>>> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
>>>> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
>>>> @@ -1005,6 +1005,7 @@ int main_loop_s(int listensock)
>>>>  	struct pollfd polls;
>>>>  	socklen_t salen;
>>>>  	int remotesock;
>>>> +	int err = 0;
>>>>  	int fd = 0;
>>>>  
>>>>  again:
>>>> @@ -1036,7 +1037,7 @@ again:
>>>>  
>>>>  		SOCK_TEST_TCPULP(remotesock, 0);
>>>>  
>>>> -		copyfd_io(fd, remotesock, 1, true);
>>>> +		err = copyfd_io(fd, remotesock, 1, true, &winfo);
>>>
>>> The winfo in function main_loop_s was added in
>>> commit ca7ae8916043 ("selftests: mptcp: mptfo Initiator/Listener")
>>> but not present in v6.1.y.
>>> As a result, mptcp selftests will fail to compile from v6.1.154.
>>> I'm not sure whether I should send a revert patch, a patch that removes &winfo,
>>> or ask for the prereq patch to be applied. So, I'm reporting it for now.
>>
>> Thank you for reporting the error!
>>
>> I think the best is a patch removing "&winfo": the goal of 14e22b43df25
>> ("selftests: mptcp: connect: catch IO errors on listen side") is to stop
>> in case of errors with copyfd_io(). No need to add commit ca7ae8916043
>> ("selftests: mptcp: mptfo Initiator/Listener") as prereq.
>>
>> Do you plan to send such patch for v6.1, or do you prefer if I do it?
> 
> I would like to send it, but I don't know the following, so could you please tell me?
> 
> In a case like this, should I send a patch series of Revert that, and adjusted backport 
> patch that compilable?
> Or can I fix it directly like below?
> 
> -               err = copyfd_io(fd, remotesock, 1, true, &winfo);
> +               err = copyfd_io(fd, remotesock, 1, true);
> 
> I checked stable-kernel-rules.html but I couldn't find it.

In such case, I would fix it directly. Simply create a commit with this
diff, mention the issue and add a "Fixes" tag referencing the commit in
v6.1. Then send a patch to Greg and Sasha, with the stable and MPTCP ML
in cc, and use "PATCH 6.1.y" as prefix, e.g.

  $ git commit -s
  $ git send-email \
        --to 'gregkh@linuxfoundation.org' \
        --to 'sashal@kernel.org' \
        --cc 'stable@vger.kernel.org' \
        --cc 'MPTCP Upstream <mptcp@lists.linux.dev>' \
        --subject-prefix 'PATCH 6.1.y' \
        -1 HEAD

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