| Message ID | 20230809225918.21523-1-kuniyu@amazon.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | 769fb24aa39cc320062d993b51e16283ed28b54e |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | [v1,mptcp-next] mptcp: Remove unnecessary test for __mptcp_init_sock(). | expand |
| Context | Check | Description |
|---|---|---|
| matttbe/build | fail | Build error with: -Werror |
| matttbe/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 7 lines checked |
| matttbe/KVM_Validation__normal__except_selftest_mptcp_join_ | fail | Script error! ❓ |
| matttbe/KVM_Validation__normal__only_selftest_mptcp_join_ | fail | Script error! ❓ |
| matttbe/KVM_Validation__debug__except_selftest_mptcp_join_ | fail | Script error! ❓ |
| matttbe/KVM_Validation__debug__only_selftest_mptcp_join_ | fail | Script error! ❓ |
Hi Kuniyuki, Thank you for your modifications, that's great! But sadly, our CI spotted some issues with it when trying to build it. You can find more details there: https://patchwork.kernel.org/project/mptcp/patch/20230809225918.21523-1-kuniyu@amazon.com/ https://github.com/multipath-tcp/mptcp_net-next/actions/runs/5815022877 Status: failure Initiator: MPTCPimporter Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/e7b0047cdbb4 Feel free to reply to this email if you cannot access logs, if you need some support to fix the error, if this doesn't seem to be caused by your modifications or if the error is a false positive one. Cheers, MPTCP GH Action bot Bot operated by Matthieu Baerts (Tessares)
Hi Kuniyuki,
Thank you for your modifications, that's great!
Our CI did some validations and here is its report:
- KVM Validation: normal (except selftest_mptcp_join):
- Script error! ❓:
- Task: https://cirrus-ci.com/task/5907693383188480
- Summary: https://api.cirrus-ci.com/v1/artifact/task/5907693383188480/summary/summary.txt
- {"code":404,"message":
- "Can't find artifacts containing file conclusion.txt"}:
- Task: https://cirrus-ci.com/task/6470643336609792
- Summary: https://api.cirrus-ci.com/v1/artifact/task/6470643336609792/summary/summary.txt
- KVM Validation: debug (only selftest_mptcp_join):
- Script error! ❓:
- Task: https://cirrus-ci.com/task/5063268453056512
- Summary: https://api.cirrus-ci.com/v1/artifact/task/5063268453056512/summary/summary.txt
- KVM Validation: normal (only selftest_mptcp_join):
- Script error! ❓:
- Task: https://cirrus-ci.com/task/5344743429767168
- Summary: https://api.cirrus-ci.com/v1/artifact/task/5344743429767168/summary/summary.txt
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/e7b0047cdbb4
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-debug
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 (Tessares)
From: MPTCP CI <wpasupplicant.patchew@gmail.com> Date: Wed, 09 Aug 2023 23:20:04 +0000 > Hi Kuniyuki, > > Thank you for your modifications, that's great! > > But sadly, our CI spotted some issues with it when trying to build it. > > You can find more details there: > > https://patchwork.kernel.org/project/mptcp/patch/20230809225918.21523-1-kuniyu@amazon.com/ > https://github.com/multipath-tcp/mptcp_net-next/actions/runs/5815022877 > > Status: failure > Initiator: MPTCPimporter > Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/e7b0047cdbb4 Oops, I used net-next and missed f2b529788d80 ("mptcp: add sched in mptcp_sock"). I'll rebase on the mptcp tree.
Hi Kuniyuki, Thank you for the patch! On 10/08/2023 01:39, Kuniyuki Iwashima wrote: > From: MPTCP CI <wpasupplicant.patchew@gmail.com> > Date: Wed, 09 Aug 2023 23:20:04 +0000 >> Hi Kuniyuki, >> >> Thank you for your modifications, that's great! >> >> But sadly, our CI spotted some issues with it when trying to build it. >> >> You can find more details there: >> >> https://patchwork.kernel.org/project/mptcp/patch/20230809225918.21523-1-kuniyu@amazon.com/ >> https://github.com/multipath-tcp/mptcp_net-next/actions/runs/5815022877 >> >> Status: failure >> Initiator: MPTCPimporter >> Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/e7b0047cdbb4 > > Oops, I used net-next and missed f2b529788d80 ("mptcp: add sched in > mptcp_sock"). > > I'll rebase on the mptcp tree. That's OK, I can apply your patch on our side before "mptcp: add sched in mptcp_sock" (we will not send it to netdev before next week anyway). When doing that, I will also adapt this other commit not to have the compilation issue. That way, your patch can be applied in net-next directly and our daily automatic rebase will handle that without issue. I will send a reply on your email including the patch where netdev ML was cced (the CI only sends emails to MPTCP ML). Cheers, Matt
On 10/08/2023 10:30, Matthieu Baerts wrote: > On 10/08/2023 01:39, Kuniyuki Iwashima wrote: >> From: MPTCP CI <wpasupplicant.patchew@gmail.com> >> Date: Wed, 09 Aug 2023 23:20:04 +0000 >>> Hi Kuniyuki, >>> >>> Thank you for your modifications, that's great! >>> >>> But sadly, our CI spotted some issues with it when trying to build it. >>> >>> You can find more details there: >>> >>> https://patchwork.kernel.org/project/mptcp/patch/20230809225918.21523-1-kuniyu@amazon.com/ >>> https://github.com/multipath-tcp/mptcp_net-next/actions/runs/5815022877 >>> >>> Status: failure >>> Initiator: MPTCPimporter >>> Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/e7b0047cdbb4 >> >> Oops, I used net-next and missed f2b529788d80 ("mptcp: add sched in >> mptcp_sock"). >> >> I'll rebase on the mptcp tree. > > That's OK, I can apply your patch on our side before "mptcp: add sched > in mptcp_sock" (we will not send it to netdev before next week anyway). > When doing that, I will also adapt this other commit not to have the > compilation issue. > > That way, your patch can be applied in net-next directly and our daily > automatic rebase will handle that without issue. I will send a reply on > your email including the patch where netdev ML was cced (the CI only > sends emails to MPTCP ML). I just noticed your patch was addressed to us with netdev ML in CC and not addressed to the Net maintainers. That's fine of course but I guess the Net maintainers will expect us to apply the patch on our side and send it later with other ones. I will just make it clean to the Net maintainers who is doing what :) Cheers, Matt
Hi Kuniyuki, On 10/08/2023 00:59, Kuniyuki Iwashima wrote: > __mptcp_init_sock() always returns 0 because mptcp_init_sock() used > to return the value directly. > > But after commit 18b683bff89d ("mptcp: queue data for mptcp level > retransmission"), __mptcp_init_sock() need not return value anymore. > > Let's remove the unnecessary test for __mptcp_init_sock() and make > it return void. Thank you for your patch, it looks good to me! Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net> As the CI said (on MPTCP ML only), there is a conflict with another patch already in MPTCP tree that is going to be sent later. But that's fine, no need to rebase, it is fine to apply your patch as it is in our tree before the other patch and modify the latter one to avoid compilation issue. So just to be clear, I just applied this patch in MPTCP tree and we will send it to Netdev later. I hope it is OK if I change the Patchwork status to "Deferred". (If it is the correct status because it looks like the "Handled Elsewhere" status is not used in Netdev PW and the bot doesn't support it). pw-bot: defer New patches for t/upstream: - 769fb24aa39c: mptcp: Remove unnecessary test for __mptcp_init_sock() - 1ba457522bce: mptcp: fix compilation issue in "mptcp: add sched in mptcp_sock" - Results: 3a7cf9f5d51c..649448c9126d (export) Tests are now in progress: https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20230810T092803 Cheers, Matt
Hi Kuniyuki, Thank you for your modifications, that's great! But sadly, our CI spotted some issues with it when trying to build it. You can find more details there: https://patchwork.kernel.org/project/mptcp/patch/20230809225918.21523-1-kuniyu@amazon.com/ https://github.com/multipath-tcp/mptcp_net-next/actions/runs/5819664427 Status: failure Initiator: MPTCPimporter Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/d9cabde674a5 Feel free to reply to this email if you cannot access logs, if you need some support to fix the error, if this doesn't seem to be caused by your modifications or if the error is a false positive one. Cheers, MPTCP GH Action bot Bot operated by Matthieu Baerts (Tessares)
Hi Kuniyuki,
Thank you for your modifications, that's great!
Our CI did some validations and here is its report:
- KVM Validation: normal (except selftest_mptcp_join):
- Script error! ❓:
- Task: https://cirrus-ci.com/task/4953412383539200
- Summary: https://api.cirrus-ci.com/v1/artifact/task/4953412383539200/summary/summary.txt
- KVM Validation: normal (only selftest_mptcp_join):
- Script error! ❓:
- Task: https://cirrus-ci.com/task/6079312290381824
- Summary: https://api.cirrus-ci.com/v1/artifact/task/6079312290381824/summary/summary.txt
- KVM Validation: debug (except selftest_mptcp_join):
- Script error! ❓:
- Task: https://cirrus-ci.com/task/5516362336960512
- Summary: https://api.cirrus-ci.com/v1/artifact/task/5516362336960512/summary/summary.txt
- KVM Validation: debug (only selftest_mptcp_join):
- Script error! ❓:
- Task: https://cirrus-ci.com/task/6642262243803136
- Summary: https://api.cirrus-ci.com/v1/artifact/task/6642262243803136/summary/summary.txt
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/d9cabde674a5
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-debug
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 (Tessares)
Hi Kuniyuki, On 10/08/2023 12:04, MPTCP CI wrote: > Thank you for your modifications, that's great! > > But sadly, our CI spotted some issues with it when trying to build it. Sorry for that, Patchew[1] automatically re-applies patches when tags (Reviewed-By, etc.) are sent on the ML and we cannot control that. So here this report is exactly the same as the one you got with your original patch, just before I applied your patch and did the modifications in the other commit. So you can safely ignore this email and the next one. [1] https://patchew.org/MPTCP/20230809225918.21523-1-kuniyu@amazon.com/ Cheers, Matt
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index 65ee949a8a44..ddafb095fc3d 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -2662,7 +2662,7 @@ static void mptcp_worker(struct work_struct *work) sock_put(sk); } -static int __mptcp_init_sock(struct sock *sk) +static void __mptcp_init_sock(struct sock *sk) { struct mptcp_sock *msk = mptcp_sk(sk); @@ -2689,8 +2689,6 @@ static int __mptcp_init_sock(struct sock *sk) /* re-use the csk retrans timer for MPTCP-level retrans */ timer_setup(&msk->sk.icsk_retransmit_timer, mptcp_retransmit_timer, 0); timer_setup(&sk->sk_timer, mptcp_timeout_timer, 0); - - return 0; } static void mptcp_ca_reset(struct sock *sk) @@ -2708,11 +2706,8 @@ static void mptcp_ca_reset(struct sock *sk) static int mptcp_init_sock(struct sock *sk) { struct net *net = sock_net(sk); - int ret; - ret = __mptcp_init_sock(sk); - if (ret) - return ret; + __mptcp_init_sock(sk); if (!mptcp_is_enabled(net)) return -ENOPROTOOPT;
__mptcp_init_sock() always returns 0 because mptcp_init_sock() used to return the value directly. But after commit 18b683bff89d ("mptcp: queue data for mptcp level retransmission"), __mptcp_init_sock() need not return value anymore. Let's remove the unnecessary test for __mptcp_init_sock() and make it return void. Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com> --- net/mptcp/protocol.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)