* [PATCH mptcp-next] Revert "mptcp: add data lock for sk timers"
@ 2022-05-05 17:12 Paolo Abeni
2022-05-05 18:38 ` Revert "mptcp: add data lock for sk timers": Tests Results MPTCP CI
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Paolo Abeni @ 2022-05-05 17:12 UTC (permalink / raw)
To: Geliang Tang; +Cc: mptcp
This reverts commit 4293248c6704b854bf816aa1967e433402bee11c.
Additional locks are not needed, all the touched sections
are already under mptcp socket lock protection.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/protocol.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 5243c58789a4..ff567e9d0b1f 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1605,10 +1605,8 @@ void __mptcp_push_pending(struct sock *sk, unsigned int flags)
out:
/* ensure the rtx timer is running */
- mptcp_data_lock(sk);
if (!mptcp_timer_pending(sk))
mptcp_reset_timer(sk);
- mptcp_data_unlock(sk);
if (copied)
__mptcp_check_send_data_fin(sk);
}
@@ -2516,10 +2514,8 @@ static void __mptcp_retrans(struct sock *sk)
reset_timer:
mptcp_check_and_set_pending(sk);
- mptcp_data_lock(sk);
if (!mptcp_timer_pending(sk))
mptcp_reset_timer(sk);
- mptcp_data_unlock(sk);
}
static void mptcp_mp_fail_no_response(struct mptcp_sock *msk)
@@ -2707,10 +2703,8 @@ void mptcp_subflow_shutdown(struct sock *sk, struct sock *ssk, int how)
} else {
pr_debug("Sending DATA_FIN on subflow %p", ssk);
tcp_send_ack(ssk);
- mptcp_data_lock(sk);
if (!mptcp_timer_pending(sk))
mptcp_reset_timer(sk);
- mptcp_data_unlock(sk);
}
break;
}
@@ -2811,10 +2805,8 @@ static void __mptcp_destroy_sock(struct sock *sk)
/* join list will be eventually flushed (with rst) at sock lock release time*/
list_splice_init(&msk->conn_list, &conn_list);
- mptcp_data_lock(sk);
mptcp_stop_timer(sk);
sk_stop_timer(sk, &sk->sk_timer);
- mptcp_data_unlock(sk);
msk->pm.status = 0;
mptcp_release_sched(msk);
@@ -2877,9 +2869,7 @@ static void mptcp_close(struct sock *sk, long timeout)
__mptcp_destroy_sock(sk);
do_cancel_work = true;
} else {
- mptcp_data_lock(sk);
sk_reset_timer(sk, &sk->sk_timer, jiffies + TCP_TIMEWAIT_LEN);
- mptcp_data_unlock(sk);
}
release_sock(sk);
if (do_cancel_work)
@@ -2924,10 +2914,8 @@ static int mptcp_disconnect(struct sock *sk, int flags)
__mptcp_close_ssk(sk, ssk, subflow, MPTCP_CF_FASTCLOSE);
}
- mptcp_data_lock(sk);
mptcp_stop_timer(sk);
sk_stop_timer(sk, &sk->sk_timer);
- mptcp_data_unlock(sk);
if (mptcp_sk(sk)->token)
mptcp_event(MPTCP_EVENT_CLOSED, mptcp_sk(sk), NULL, GFP_KERNEL);
--
2.35.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: Revert "mptcp: add data lock for sk timers": Tests Results
2022-05-05 17:12 [PATCH mptcp-next] Revert "mptcp: add data lock for sk timers" Paolo Abeni
@ 2022-05-05 18:38 ` MPTCP CI
2022-05-05 23:40 ` [PATCH mptcp-next] Revert "mptcp: add data lock for sk timers" Mat Martineau
2022-05-06 19:10 ` Matthieu Baerts
2 siblings, 0 replies; 9+ messages in thread
From: MPTCP CI @ 2022-05-05 18:38 UTC (permalink / raw)
To: Paolo Abeni; +Cc: mptcp
Hi Paolo,
Thank you for your modifications, that's great!
Our CI did some validations and here is its report:
- KVM Validation: normal:
- Unstable: 1 failed test(s): selftest_mptcp_join 🔴:
- Task: https://cirrus-ci.com/task/5032872854683648
- Summary: https://api.cirrus-ci.com/v1/artifact/task/5032872854683648/summary/summary.txt
- KVM Validation: debug:
- Unstable: 2 failed test(s): selftest_diag selftest_mptcp_join 🔴:
- Task: https://cirrus-ci.com/task/6158772761526272
- Summary: https://api.cirrus-ci.com/v1/artifact/task/6158772761526272/summary/summary.txt
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/641d7584c1de
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)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-next] Revert "mptcp: add data lock for sk timers"
2022-05-05 17:12 [PATCH mptcp-next] Revert "mptcp: add data lock for sk timers" Paolo Abeni
2022-05-05 18:38 ` Revert "mptcp: add data lock for sk timers": Tests Results MPTCP CI
@ 2022-05-05 23:40 ` Mat Martineau
2022-05-06 9:19 ` Paolo Abeni
2022-05-06 19:10 ` Matthieu Baerts
2 siblings, 1 reply; 9+ messages in thread
From: Mat Martineau @ 2022-05-05 23:40 UTC (permalink / raw)
To: Paolo Abeni; +Cc: Geliang Tang, mptcp
On Thu, 5 May 2022, Paolo Abeni wrote:
> This reverts commit 4293248c6704b854bf816aa1967e433402bee11c.
>
> Additional locks are not needed, all the touched sections
> are already under mptcp socket lock protection.
>
I agree that this needs to be reverted:
Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
But msk->sk_timer is *also* accessed in two places without the mptcp
socket lock (but with the data lock):
* mptcp_pm_mp_fail_received() (stop timer when mp_fail received)
* subflow_check_data_avail() (start timer on infinite mapping rx)
I think these were the reason the locking was added, because we don't want
the use of msk->sk_timer for MP_FAIL timeout to interfere with other use
of msk->sk_timer when a msk is closing.
Now I see that the data lock doesn't work for that purpose. In addition to
removing the data locks as this patch does, it looks like the two
functions I listed above need to use msk->cb_flags and deferred events to
safely check msk->sk_state and modify msk->sk_timer. What do you think? I
can send a patch on Friday.
- Mat
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> net/mptcp/protocol.c | 12 ------------
> 1 file changed, 12 deletions(-)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 5243c58789a4..ff567e9d0b1f 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1605,10 +1605,8 @@ void __mptcp_push_pending(struct sock *sk, unsigned int flags)
>
> out:
> /* ensure the rtx timer is running */
> - mptcp_data_lock(sk);
> if (!mptcp_timer_pending(sk))
> mptcp_reset_timer(sk);
> - mptcp_data_unlock(sk);
> if (copied)
> __mptcp_check_send_data_fin(sk);
> }
> @@ -2516,10 +2514,8 @@ static void __mptcp_retrans(struct sock *sk)
> reset_timer:
> mptcp_check_and_set_pending(sk);
>
> - mptcp_data_lock(sk);
> if (!mptcp_timer_pending(sk))
> mptcp_reset_timer(sk);
> - mptcp_data_unlock(sk);
> }
>
> static void mptcp_mp_fail_no_response(struct mptcp_sock *msk)
> @@ -2707,10 +2703,8 @@ void mptcp_subflow_shutdown(struct sock *sk, struct sock *ssk, int how)
> } else {
> pr_debug("Sending DATA_FIN on subflow %p", ssk);
> tcp_send_ack(ssk);
> - mptcp_data_lock(sk);
> if (!mptcp_timer_pending(sk))
> mptcp_reset_timer(sk);
> - mptcp_data_unlock(sk);
> }
> break;
> }
> @@ -2811,10 +2805,8 @@ static void __mptcp_destroy_sock(struct sock *sk)
> /* join list will be eventually flushed (with rst) at sock lock release time*/
> list_splice_init(&msk->conn_list, &conn_list);
>
> - mptcp_data_lock(sk);
> mptcp_stop_timer(sk);
> sk_stop_timer(sk, &sk->sk_timer);
> - mptcp_data_unlock(sk);
> msk->pm.status = 0;
> mptcp_release_sched(msk);
>
> @@ -2877,9 +2869,7 @@ static void mptcp_close(struct sock *sk, long timeout)
> __mptcp_destroy_sock(sk);
> do_cancel_work = true;
> } else {
> - mptcp_data_lock(sk);
> sk_reset_timer(sk, &sk->sk_timer, jiffies + TCP_TIMEWAIT_LEN);
> - mptcp_data_unlock(sk);
> }
> release_sock(sk);
> if (do_cancel_work)
> @@ -2924,10 +2914,8 @@ static int mptcp_disconnect(struct sock *sk, int flags)
> __mptcp_close_ssk(sk, ssk, subflow, MPTCP_CF_FASTCLOSE);
> }
>
> - mptcp_data_lock(sk);
> mptcp_stop_timer(sk);
> sk_stop_timer(sk, &sk->sk_timer);
> - mptcp_data_unlock(sk);
>
> if (mptcp_sk(sk)->token)
> mptcp_event(MPTCP_EVENT_CLOSED, mptcp_sk(sk), NULL, GFP_KERNEL);
> --
> 2.35.1
>
>
>
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-next] Revert "mptcp: add data lock for sk timers"
2022-05-05 23:40 ` [PATCH mptcp-next] Revert "mptcp: add data lock for sk timers" Mat Martineau
@ 2022-05-06 9:19 ` Paolo Abeni
0 siblings, 0 replies; 9+ messages in thread
From: Paolo Abeni @ 2022-05-06 9:19 UTC (permalink / raw)
To: Mat Martineau; +Cc: Geliang Tang, mptcp
On Thu, 2022-05-05 at 16:40 -0700, Mat Martineau wrote:
> On Thu, 5 May 2022, Paolo Abeni wrote:
>
> > This reverts commit 4293248c6704b854bf816aa1967e433402bee11c.
> >
> > Additional locks are not needed, all the touched sections
> > are already under mptcp socket lock protection.
> >
>
> I agree that this needs to be reverted:
>
> Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
>
>
> But msk->sk_timer is *also* accessed in two places without the mptcp
> socket lock (but with the data lock):
> * mptcp_pm_mp_fail_received() (stop timer when mp_fail received)
>
> * subflow_check_data_avail() (start timer on infinite mapping rx)
I'm reasonably no additional lock is required to call sk_stop_timer()
and/or sk_reset_timer(): they boil down to the timer_{del,mod}
primitives which in turns are irq safe.
The mptcp wrappers *could* require additional locking because they
additionally touch mptcp_sk(sk)->timer_ival. I *think* we could avoid
the lock even there with some additional barrier, but it looks every
caller is already under the lock.
I think we don't need to defer touching the timer. Eventully we could
remove the check on the msk socket status, which again looks not
needed.
Cheers,
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-next] Revert "mptcp: add data lock for sk timers"
2022-05-05 17:12 [PATCH mptcp-next] Revert "mptcp: add data lock for sk timers" Paolo Abeni
2022-05-05 18:38 ` Revert "mptcp: add data lock for sk timers": Tests Results MPTCP CI
2022-05-05 23:40 ` [PATCH mptcp-next] Revert "mptcp: add data lock for sk timers" Mat Martineau
@ 2022-05-06 19:10 ` Matthieu Baerts
2022-05-10 0:11 ` Mat Martineau
2 siblings, 1 reply; 9+ messages in thread
From: Matthieu Baerts @ 2022-05-06 19:10 UTC (permalink / raw)
To: Paolo Abeni, Geliang Tang; +Cc: mptcp
Hi Paolo, Mat,
On 05/05/2022 19:12, Paolo Abeni wrote:
> This reverts commit 4293248c6704b854bf816aa1967e433402bee11c.
>
> Additional locks are not needed, all the touched sections
> are already under mptcp socket lock protection.
Thank you for the patch and the review!
Now in our tree (fixes for net-next) with Mat's RvB and a Fixes tag.
New patches for t/upstream:
- 2ff5e4eba944: Revert "mptcp: add data lock for sk timers"
- Results: bc6528b2b1d7..22856348dac5 (export)
Builds and tests are now in progress:
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220506T190735
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-next] Revert "mptcp: add data lock for sk timers"
2022-05-06 19:10 ` Matthieu Baerts
@ 2022-05-10 0:11 ` Mat Martineau
2022-05-10 14:04 ` Matthieu Baerts
0 siblings, 1 reply; 9+ messages in thread
From: Mat Martineau @ 2022-05-10 0:11 UTC (permalink / raw)
To: Matthieu Baerts; +Cc: mptcp
On Fri, 6 May 2022, Matthieu Baerts wrote:
> Hi Paolo, Mat,
>
> On 05/05/2022 19:12, Paolo Abeni wrote:
>> This reverts commit 4293248c6704b854bf816aa1967e433402bee11c.
>>
>> Additional locks are not needed, all the touched sections
>> are already under mptcp socket lock protection.
>
> Thank you for the patch and the review!
>
> Now in our tree (fixes for net-next) with Mat's RvB and a Fixes tag.
>
> New patches for t/upstream:
> - 2ff5e4eba944: Revert "mptcp: add data lock for sk timers"
> - Results: bc6528b2b1d7..22856348dac5 (export)
This shows up in a different place in the export branch commit history
than I expected:
* 1238ef9dd5ce (HEAD, tag: export/20220509T115202, mptcp-nn/export) DO-NOT-MERGE: mptcp: enabled by default
* a89b2ebc4a26 DO-NOT-MERGE: mptcp: use kmalloc on kasan build
* 8adddf50a9e9 DO-NOT-MERGE: git markup: features other trees
* c0e05f5207ec selftests/bpf: add bpf_first test
* 90b2072ffc21 selftests/bpf: add bpf_first scheduler
* 2f547420d0f4 mptcp: add bpf_mptcp_sched_ops
* cf597ecbdb4b mptcp: add get_subflow wrappers
* 1b3d7a5f1f10 mptcp: add sched in mptcp_sock
* e31f54bd6183 mptcp: add a new sysctl scheduler
* 74d3c74494e6 mptcp: add struct mptcp_sched_ops
* 416cb433f1df selftests/bpf: verify first of struct mptcp_sock
* 57dcfb6f61ec selftests/bpf: verify ca_name of struct mptcp_sock
* 8996b41ffcd2 selftests/bpf: verify token of struct mptcp_sock
* f8f99d2ff6a7 selftests/bpf: test bpf_skc_to_mptcp_sock
* 3fb057749027 selftests/bpf: add MPTCP test base
* 1585ded1ef3d selftests/bpf: Enable CONFIG_IKCONFIG_PROC in config
* d4fc73d43d5b bpf: add bpf_skc_to_mptcp_sock_proto
* 581159326c14 bpf: expose is_mptcp flag to bpf_tcp_sock
* b20a348378e2 DO-NOT-MERGE: git markup: features net-next-next
* fa71bf0a2db6 selftests: mptcp: add MP_FAIL reset testcase
* fddb2da3de32 DO-NOT-MERGE: git markup: features net-next
Expected to see the patch here.
* 218186cd781d mptcp: sockopt: add TCP_DEFER_ACCEPT support
* 6d8839d159a7 DO-NOT-MERGE: git markup: fixes net-next
* f858740fef1e TopGit-driven merge of branches:
|\
| * baa752231c4d DO-NOT-MERGE: git markup: end common net net-next
| * cad34cf702bc DO-NOT-MERGE: mptcp: add CI support
| * d7945d0bf36f DO-NOT-MERGE: git markup: fixes net
| * d8e908a27426 selftests: mptcp: add subflow limits test-cases
| * 001ce634cb09 mptcp: fix subflow accounting on close
| * ca2debea5aa8 net/sched: act_pedit: really ensure the skb is writable
| * 945110b5a812 DO-NOT-MERGE: git markup: fixes other trees
| * 55775678c466 x86/pm: Fix false positive kmemleak report in msr_build_context()
| * 7a38d7853776 DO-NOT-MERGE: git markup: net
* | 4f43f697b6be Revert "mptcp: add data lock for sk timers"
^^^ but it's here instead
* | f57e72f418df selftests: mptcp: fix a mp_fail test warning
* | 49161dcf2772 DO-NOT-MERGE: git markup: net-next
* | c908565eecf2 (mptcp-nn/net-next) Merge tag 'batadv-next-pullrequest-202205
Matthieu, is that what you intended?
>
>
> Builds and tests are now in progress:
>
> https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220506T190735
> https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-next] Revert "mptcp: add data lock for sk timers"
2022-05-10 0:11 ` Mat Martineau
@ 2022-05-10 14:04 ` Matthieu Baerts
2022-05-11 20:40 ` Mat Martineau
0 siblings, 1 reply; 9+ messages in thread
From: Matthieu Baerts @ 2022-05-10 14:04 UTC (permalink / raw)
To: Mat Martineau; +Cc: mptcp
Hi Mat,
On 10/05/2022 02:11, Mat Martineau wrote:
> On Fri, 6 May 2022, Matthieu Baerts wrote:
>
>> Hi Paolo, Mat,
>>
>> On 05/05/2022 19:12, Paolo Abeni wrote:
>>> This reverts commit 4293248c6704b854bf816aa1967e433402bee11c.
>>>
>>> Additional locks are not needed, all the touched sections
>>> are already under mptcp socket lock protection.
>>
>> Thank you for the patch and the review!
>>
>> Now in our tree (fixes for net-next) with Mat's RvB and a Fixes tag.
>>
>> New patches for t/upstream:
>> - 2ff5e4eba944: Revert "mptcp: add data lock for sk timers"
>> - Results: bc6528b2b1d7..22856348dac5 (export)
>
> This shows up in a different place in the export branch commit history
> than I expected:
>
> * 1238ef9dd5ce (HEAD, tag: export/20220509T115202, mptcp-nn/export)
> DO-NOT-MERGE: mptcp: enabled by default
> * a89b2ebc4a26 DO-NOT-MERGE: mptcp: use kmalloc on kasan build
> * 8adddf50a9e9 DO-NOT-MERGE: git markup: features other trees
> * c0e05f5207ec selftests/bpf: add bpf_first test
> * 90b2072ffc21 selftests/bpf: add bpf_first scheduler
> * 2f547420d0f4 mptcp: add bpf_mptcp_sched_ops
> * cf597ecbdb4b mptcp: add get_subflow wrappers
> * 1b3d7a5f1f10 mptcp: add sched in mptcp_sock
> * e31f54bd6183 mptcp: add a new sysctl scheduler
> * 74d3c74494e6 mptcp: add struct mptcp_sched_ops
> * 416cb433f1df selftests/bpf: verify first of struct mptcp_sock
> * 57dcfb6f61ec selftests/bpf: verify ca_name of struct mptcp_sock
> * 8996b41ffcd2 selftests/bpf: verify token of struct mptcp_sock
> * f8f99d2ff6a7 selftests/bpf: test bpf_skc_to_mptcp_sock
> * 3fb057749027 selftests/bpf: add MPTCP test base
> * 1585ded1ef3d selftests/bpf: Enable CONFIG_IKCONFIG_PROC in config
> * d4fc73d43d5b bpf: add bpf_skc_to_mptcp_sock_proto
> * 581159326c14 bpf: expose is_mptcp flag to bpf_tcp_sock
> * b20a348378e2 DO-NOT-MERGE: git markup: features net-next-next
> * fa71bf0a2db6 selftests: mptcp: add MP_FAIL reset testcase
> * fddb2da3de32 DO-NOT-MERGE: git markup: features net-next
>
> Expected to see the patch here.
>
> * 218186cd781d mptcp: sockopt: add TCP_DEFER_ACCEPT support
> * 6d8839d159a7 DO-NOT-MERGE: git markup: fixes net-next
> * f858740fef1e TopGit-driven merge of branches:
> |\
> | * baa752231c4d DO-NOT-MERGE: git markup: end common net net-next
> | * cad34cf702bc DO-NOT-MERGE: mptcp: add CI support
> | * d7945d0bf36f DO-NOT-MERGE: git markup: fixes net
> | * d8e908a27426 selftests: mptcp: add subflow limits test-cases
> | * 001ce634cb09 mptcp: fix subflow accounting on close
> | * ca2debea5aa8 net/sched: act_pedit: really ensure the skb is writable
> | * 945110b5a812 DO-NOT-MERGE: git markup: fixes other trees
> | * 55775678c466 x86/pm: Fix false positive kmemleak report in
> msr_build_context()
> | * 7a38d7853776 DO-NOT-MERGE: git markup: net
> * | 4f43f697b6be Revert "mptcp: add data lock for sk timers"
>
> ^^^ but it's here instead
>
> * | f57e72f418df selftests: mptcp: fix a mp_fail test warning
> * | 49161dcf2772 DO-NOT-MERGE: git markup: net-next
> * | c908565eecf2 (mptcp-nn/net-next) Merge tag
> 'batadv-next-pullrequest-202205
>
>
> Matthieu, is that what you intended?
Yes, that was my initial intension: if there is an issue with a patch in
net-next, we probably don't want to wait for a sync with -net to send
the patch. So it is on top of net-next only. It is just in case we force
a sync with -net evenn if net and net-next have not been recently sync yet.
Do you prefer to have these patches on top of -net and net-next instead?
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-next] Revert "mptcp: add data lock for sk timers"
2022-05-10 14:04 ` Matthieu Baerts
@ 2022-05-11 20:40 ` Mat Martineau
2022-05-16 16:13 ` Matthieu Baerts
0 siblings, 1 reply; 9+ messages in thread
From: Mat Martineau @ 2022-05-11 20:40 UTC (permalink / raw)
To: Matthieu Baerts; +Cc: mptcp
[-- Attachment #1: Type: text/plain, Size: 4305 bytes --]
On Tue, 10 May 2022, Matthieu Baerts wrote:
> Hi Mat,
>
> On 10/05/2022 02:11, Mat Martineau wrote:
>> On Fri, 6 May 2022, Matthieu Baerts wrote:
>>
>>> Hi Paolo, Mat,
>>>
>>> On 05/05/2022 19:12, Paolo Abeni wrote:
>>>> This reverts commit 4293248c6704b854bf816aa1967e433402bee11c.
>>>>
>>>> Additional locks are not needed, all the touched sections
>>>> are already under mptcp socket lock protection.
>>>
>>> Thank you for the patch and the review!
>>>
>>> Now in our tree (fixes for net-next) with Mat's RvB and a Fixes tag.
>>>
>>> New patches for t/upstream:
>>> - 2ff5e4eba944: Revert "mptcp: add data lock for sk timers"
>>> - Results: bc6528b2b1d7..22856348dac5 (export)
>>
>> This shows up in a different place in the export branch commit history
>> than I expected:
>>
>> * 1238ef9dd5ce (HEAD, tag: export/20220509T115202, mptcp-nn/export)
>> DO-NOT-MERGE: mptcp: enabled by default
>> * a89b2ebc4a26 DO-NOT-MERGE: mptcp: use kmalloc on kasan build
>> * 8adddf50a9e9 DO-NOT-MERGE: git markup: features other trees
>> * c0e05f5207ec selftests/bpf: add bpf_first test
>> * 90b2072ffc21 selftests/bpf: add bpf_first scheduler
>> * 2f547420d0f4 mptcp: add bpf_mptcp_sched_ops
>> * cf597ecbdb4b mptcp: add get_subflow wrappers
>> * 1b3d7a5f1f10 mptcp: add sched in mptcp_sock
>> * e31f54bd6183 mptcp: add a new sysctl scheduler
>> * 74d3c74494e6 mptcp: add struct mptcp_sched_ops
>> * 416cb433f1df selftests/bpf: verify first of struct mptcp_sock
>> * 57dcfb6f61ec selftests/bpf: verify ca_name of struct mptcp_sock
>> * 8996b41ffcd2 selftests/bpf: verify token of struct mptcp_sock
>> * f8f99d2ff6a7 selftests/bpf: test bpf_skc_to_mptcp_sock
>> * 3fb057749027 selftests/bpf: add MPTCP test base
>> * 1585ded1ef3d selftests/bpf: Enable CONFIG_IKCONFIG_PROC in config
>> * d4fc73d43d5b bpf: add bpf_skc_to_mptcp_sock_proto
>> * 581159326c14 bpf: expose is_mptcp flag to bpf_tcp_sock
>> * b20a348378e2 DO-NOT-MERGE: git markup: features net-next-next
>> * fa71bf0a2db6 selftests: mptcp: add MP_FAIL reset testcase
>> * fddb2da3de32 DO-NOT-MERGE: git markup: features net-next
>>
>> Expected to see the patch here.
>>
>> * 218186cd781d mptcp: sockopt: add TCP_DEFER_ACCEPT support
>> * 6d8839d159a7 DO-NOT-MERGE: git markup: fixes net-next
>> * f858740fef1e TopGit-driven merge of branches:
>> |\
>> | * baa752231c4d DO-NOT-MERGE: git markup: end common net net-next
>> | * cad34cf702bc DO-NOT-MERGE: mptcp: add CI support
>> | * d7945d0bf36f DO-NOT-MERGE: git markup: fixes net
>> | * d8e908a27426 selftests: mptcp: add subflow limits test-cases
>> | * 001ce634cb09 mptcp: fix subflow accounting on close
>> | * ca2debea5aa8 net/sched: act_pedit: really ensure the skb is writable
>> | * 945110b5a812 DO-NOT-MERGE: git markup: fixes other trees
>> | * 55775678c466 x86/pm: Fix false positive kmemleak report in
>> msr_build_context()
>> | * 7a38d7853776 DO-NOT-MERGE: git markup: net
>> * | 4f43f697b6be Revert "mptcp: add data lock for sk timers"
>>
>> ^^^ but it's here instead
>>
>> * | f57e72f418df selftests: mptcp: fix a mp_fail test warning
>> * | 49161dcf2772 DO-NOT-MERGE: git markup: net-next
>> * | c908565eecf2 (mptcp-nn/net-next) Merge tag
>> 'batadv-next-pullrequest-202205
>>
>>
>> Matthieu, is that what you intended?
>
> Yes, that was my initial intension: if there is an issue with a patch in
> net-next, we probably don't want to wait for a sync with -net to send
> the patch. So it is on top of net-next only. It is just in case we force
> a sync with -net evenn if net and net-next have not been recently sync yet.
>
> Do you prefer to have these patches on top of -net and net-next instead?
>
That is my preference for future mptcp-next patches, yes. But you don't
need to re-arrange these two patches in the tree.
None of the patches staged for net-next (before or after the topgit-driven
merge) are waiting for a sync unless there's a known conflict with a
pending patch in export-net.
When I prepare a series to send to the netdev list I often "git rebase
--onto net-next/master <from-sha> <to-sha>" the relevant patches, and I
don't want a merge commit in the middle of the range. It's also helpful to
find out if there are any surprise conflicts at that stage so I can
consider that before sending.
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-next] Revert "mptcp: add data lock for sk timers"
2022-05-11 20:40 ` Mat Martineau
@ 2022-05-16 16:13 ` Matthieu Baerts
0 siblings, 0 replies; 9+ messages in thread
From: Matthieu Baerts @ 2022-05-16 16:13 UTC (permalink / raw)
To: Mat Martineau; +Cc: mptcp
Hi Mat,
On 11/05/2022 22:40, Mat Martineau wrote:
> On Tue, 10 May 2022, Matthieu Baerts wrote:
>
>> Hi Mat,
>>
>> On 10/05/2022 02:11, Mat Martineau wrote:
>>> On Fri, 6 May 2022, Matthieu Baerts wrote:
>>>
>>>> Hi Paolo, Mat,
>>>>
>>>> On 05/05/2022 19:12, Paolo Abeni wrote:
>>>>> This reverts commit 4293248c6704b854bf816aa1967e433402bee11c.
>>>>>
>>>>> Additional locks are not needed, all the touched sections
>>>>> are already under mptcp socket lock protection.
>>>>
>>>> Thank you for the patch and the review!
>>>>
>>>> Now in our tree (fixes for net-next) with Mat's RvB and a Fixes tag.
>>>>
>>>> New patches for t/upstream:
>>>> - 2ff5e4eba944: Revert "mptcp: add data lock for sk timers"
>>>> - Results: bc6528b2b1d7..22856348dac5 (export)
>>>
>>> This shows up in a different place in the export branch commit history
>>> than I expected:
>>>
>>> * 1238ef9dd5ce (HEAD, tag: export/20220509T115202, mptcp-nn/export)
>>> DO-NOT-MERGE: mptcp: enabled by default
>>> * a89b2ebc4a26 DO-NOT-MERGE: mptcp: use kmalloc on kasan build
>>> * 8adddf50a9e9 DO-NOT-MERGE: git markup: features other trees
>>> * c0e05f5207ec selftests/bpf: add bpf_first test
>>> * 90b2072ffc21 selftests/bpf: add bpf_first scheduler
>>> * 2f547420d0f4 mptcp: add bpf_mptcp_sched_ops
>>> * cf597ecbdb4b mptcp: add get_subflow wrappers
>>> * 1b3d7a5f1f10 mptcp: add sched in mptcp_sock
>>> * e31f54bd6183 mptcp: add a new sysctl scheduler
>>> * 74d3c74494e6 mptcp: add struct mptcp_sched_ops
>>> * 416cb433f1df selftests/bpf: verify first of struct mptcp_sock
>>> * 57dcfb6f61ec selftests/bpf: verify ca_name of struct mptcp_sock
>>> * 8996b41ffcd2 selftests/bpf: verify token of struct mptcp_sock
>>> * f8f99d2ff6a7 selftests/bpf: test bpf_skc_to_mptcp_sock
>>> * 3fb057749027 selftests/bpf: add MPTCP test base
>>> * 1585ded1ef3d selftests/bpf: Enable CONFIG_IKCONFIG_PROC in config
>>> * d4fc73d43d5b bpf: add bpf_skc_to_mptcp_sock_proto
>>> * 581159326c14 bpf: expose is_mptcp flag to bpf_tcp_sock
>>> * b20a348378e2 DO-NOT-MERGE: git markup: features net-next-next
>>> * fa71bf0a2db6 selftests: mptcp: add MP_FAIL reset testcase
>>> * fddb2da3de32 DO-NOT-MERGE: git markup: features net-next
>>>
>>> Expected to see the patch here.
>>>
>>> * 218186cd781d mptcp: sockopt: add TCP_DEFER_ACCEPT support
>>> * 6d8839d159a7 DO-NOT-MERGE: git markup: fixes net-next
>>> * f858740fef1e TopGit-driven merge of branches:
>>> |\
>>> | * baa752231c4d DO-NOT-MERGE: git markup: end common net net-next
>>> | * cad34cf702bc DO-NOT-MERGE: mptcp: add CI support
>>> | * d7945d0bf36f DO-NOT-MERGE: git markup: fixes net
>>> | * d8e908a27426 selftests: mptcp: add subflow limits test-cases
>>> | * 001ce634cb09 mptcp: fix subflow accounting on close
>>> | * ca2debea5aa8 net/sched: act_pedit: really ensure the skb is writable
>>> | * 945110b5a812 DO-NOT-MERGE: git markup: fixes other trees
>>> | * 55775678c466 x86/pm: Fix false positive kmemleak report in
>>> msr_build_context()
>>> | * 7a38d7853776 DO-NOT-MERGE: git markup: net
>>> * | 4f43f697b6be Revert "mptcp: add data lock for sk timers"
>>>
>>> ^^^ but it's here instead
>>>
>>> * | f57e72f418df selftests: mptcp: fix a mp_fail test warning
>>> * | 49161dcf2772 DO-NOT-MERGE: git markup: net-next
>>> * | c908565eecf2 (mptcp-nn/net-next) Merge tag
>>> 'batadv-next-pullrequest-202205
>>>
>>>
>>> Matthieu, is that what you intended?
>>
>> Yes, that was my initial intension: if there is an issue with a patch in
>> net-next, we probably don't want to wait for a sync with -net to send
>> the patch. So it is on top of net-next only. It is just in case we force
>> a sync with -net evenn if net and net-next have not been recently sync
>> yet.
>>
>> Do you prefer to have these patches on top of -net and net-next instead?
>>
>
> That is my preference for future mptcp-next patches, yes. But you don't
> need to re-arrange these two patches in the tree.
>
> None of the patches staged for net-next (before or after the
> topgit-driven merge) are waiting for a sync unless there's a known
> conflict with a pending patch in export-net.
>
> When I prepare a series to send to the netdev list I often "git rebase
> --onto net-next/master <from-sha> <to-sha>" the relevant patches, and I
> don't want a merge commit in the middle of the range. It's also helpful
> to find out if there are any surprise conflicts at that stage so I can
> consider that before sending.
Fine for me, I just did the modification to put all fixes for net-next
on top of -net as well!
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-05-16 16:13 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-05 17:12 [PATCH mptcp-next] Revert "mptcp: add data lock for sk timers" Paolo Abeni
2022-05-05 18:38 ` Revert "mptcp: add data lock for sk timers": Tests Results MPTCP CI
2022-05-05 23:40 ` [PATCH mptcp-next] Revert "mptcp: add data lock for sk timers" Mat Martineau
2022-05-06 9:19 ` Paolo Abeni
2022-05-06 19:10 ` Matthieu Baerts
2022-05-10 0:11 ` Mat Martineau
2022-05-10 14:04 ` Matthieu Baerts
2022-05-11 20:40 ` Mat Martineau
2022-05-16 16:13 ` Matthieu Baerts
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.