* [PATCH mptcp-next] mptcp: update misleading comments.
@ 2022-09-26 15:27 Paolo Abeni
2022-09-26 16:51 ` mptcp: update misleading comments.: Tests Results MPTCP CI
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Paolo Abeni @ 2022-09-26 15:27 UTC (permalink / raw)
To: mptcp
The MPTCP data path is quite complex and hard to understend even
without some foggy comments referring to modified code and/or
completely misleading from the beginning.
Update a few of them to more accurately describing the current
status.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/protocol.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index c99eb4ce948e..8feb684408f7 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -662,9 +662,9 @@ static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
skb = skb_peek(&ssk->sk_receive_queue);
if (!skb) {
- /* if no data is found, a racing workqueue/recvmsg
- * already processed the new data, stop here or we
- * can enter an infinite loop
+ /* With racing move_skbs_to_msk() and __mptcp_move_skbs(),
+ * a different CPU can have already processed the pending
+ * data, stop here or we can enter an infinite loop
*/
if (!moved)
done = true;
@@ -672,9 +672,9 @@ static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
}
if (__mptcp_check_fallback(msk)) {
- /* if we are running under the workqueue, TCP could have
- * collapsed skbs between dummy map creation and now
- * be sure to adjust the size
+ /* Under fallback skbs have no MPTCP extension and TCP could
+ * collapse them between the dummy map creation and the
+ * current dequeue. Be sure to adjust the map size.
*/
map_remaining = skb->len;
subflow->map_data_len = skb->len;
@@ -3767,7 +3767,7 @@ static __poll_t mptcp_poll(struct file *file, struct socket *sock,
if (sk->sk_shutdown & RCV_SHUTDOWN)
mask |= EPOLLIN | EPOLLRDNORM | EPOLLRDHUP;
- /* This barrier is coupled with smp_wmb() in tcp_reset() */
+ /* This barrier is coupled with smp_wmb() in __mptcp_error_report() */
smp_rmb();
if (sk->sk_err)
mask |= EPOLLERR;
--
2.37.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: mptcp: update misleading comments.: Tests Results
2022-09-26 15:27 [PATCH mptcp-next] mptcp: update misleading comments Paolo Abeni
@ 2022-09-26 16:51 ` MPTCP CI
2022-09-26 21:14 ` [PATCH mptcp-next] mptcp: update misleading comments Mat Martineau
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: MPTCP CI @ 2022-09-26 16:51 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: 2 failed test(s): packetdrill_add_addr selftest_mptcp_join 🔴:
- Task: https://cirrus-ci.com/task/6420352380174336
- Summary: https://api.cirrus-ci.com/v1/artifact/task/6420352380174336/summary/summary.txt
- KVM Validation: debug:
- Success! ✅:
- Task: https://cirrus-ci.com/task/5012977496621056
- Summary: https://api.cirrus-ci.com/v1/artifact/task/5012977496621056/summary/summary.txt
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/d9dfe29d53ec
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] 6+ messages in thread
* Re: [PATCH mptcp-next] mptcp: update misleading comments.
2022-09-26 15:27 [PATCH mptcp-next] mptcp: update misleading comments Paolo Abeni
2022-09-26 16:51 ` mptcp: update misleading comments.: Tests Results MPTCP CI
@ 2022-09-26 21:14 ` Mat Martineau
2022-09-26 23:07 ` mptcp: update misleading comments.: Tests Results MPTCP CI
2022-09-27 8:02 ` [PATCH mptcp-next] mptcp: update misleading comments Matthieu Baerts
3 siblings, 0 replies; 6+ messages in thread
From: Mat Martineau @ 2022-09-26 21:14 UTC (permalink / raw)
To: Paolo Abeni; +Cc: mptcp
On Mon, 26 Sep 2022, Paolo Abeni wrote:
> The MPTCP data path is quite complex and hard to understend even
> without some foggy comments referring to modified code and/or
> completely misleading from the beginning.
>
> Update a few of them to more accurately describing the current
> status.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Thanks for updating the comments, Paolo. New comments look good to me:
Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> ---
> net/mptcp/protocol.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index c99eb4ce948e..8feb684408f7 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -662,9 +662,9 @@ static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
>
> skb = skb_peek(&ssk->sk_receive_queue);
> if (!skb) {
> - /* if no data is found, a racing workqueue/recvmsg
> - * already processed the new data, stop here or we
> - * can enter an infinite loop
> + /* With racing move_skbs_to_msk() and __mptcp_move_skbs(),
> + * a different CPU can have already processed the pending
> + * data, stop here or we can enter an infinite loop
> */
> if (!moved)
> done = true;
> @@ -672,9 +672,9 @@ static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
> }
>
> if (__mptcp_check_fallback(msk)) {
> - /* if we are running under the workqueue, TCP could have
> - * collapsed skbs between dummy map creation and now
> - * be sure to adjust the size
> + /* Under fallback skbs have no MPTCP extension and TCP could
> + * collapse them between the dummy map creation and the
> + * current dequeue. Be sure to adjust the map size.
> */
> map_remaining = skb->len;
> subflow->map_data_len = skb->len;
> @@ -3767,7 +3767,7 @@ static __poll_t mptcp_poll(struct file *file, struct socket *sock,
> if (sk->sk_shutdown & RCV_SHUTDOWN)
> mask |= EPOLLIN | EPOLLRDNORM | EPOLLRDHUP;
>
> - /* This barrier is coupled with smp_wmb() in tcp_reset() */
> + /* This barrier is coupled with smp_wmb() in __mptcp_error_report() */
> smp_rmb();
> if (sk->sk_err)
> mask |= EPOLLERR;
> --
> 2.37.3
>
>
>
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: mptcp: update misleading comments.: Tests Results
2022-09-26 15:27 [PATCH mptcp-next] mptcp: update misleading comments Paolo Abeni
2022-09-26 16:51 ` mptcp: update misleading comments.: Tests Results MPTCP CI
2022-09-26 21:14 ` [PATCH mptcp-next] mptcp: update misleading comments Mat Martineau
@ 2022-09-26 23:07 ` MPTCP CI
2022-09-27 8:01 ` Matthieu Baerts
2022-09-27 8:02 ` [PATCH mptcp-next] mptcp: update misleading comments Matthieu Baerts
3 siblings, 1 reply; 6+ messages in thread
From: MPTCP CI @ 2022-09-26 23:07 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: 2 failed test(s): selftest_mptcp_join selftest_simult_flows 🔴:
- Task: https://cirrus-ci.com/task/4956489147219968
- Summary: https://api.cirrus-ci.com/v1/artifact/task/4956489147219968/summary/summary.txt
- KVM Validation: debug:
- Unstable: 1 failed test(s): packetdrill_add_addr 🔴:
- Task: https://cirrus-ci.com/task/6082389054062592
- Summary: https://api.cirrus-ci.com/v1/artifact/task/6082389054062592/summary/summary.txt
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/2474bd2962c4
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] 6+ messages in thread
* Re: mptcp: update misleading comments.: Tests Results
2022-09-26 23:07 ` mptcp: update misleading comments.: Tests Results MPTCP CI
@ 2022-09-27 8:01 ` Matthieu Baerts
0 siblings, 0 replies; 6+ messages in thread
From: Matthieu Baerts @ 2022-09-27 8:01 UTC (permalink / raw)
To: Davide Caratti; +Cc: mptcp
Hi Davide,
Please see below: it looks like Patchew is re-creating tags again after
someone sends a Reviewed-by, etc.
Yesterday, Mat sent its RvB tag and you can see it has been added in the
commit that has been tested here:
https://github.com/multipath-tcp/mptcp_net-next/commit/2474bd2962c4
Do you see what to change to avoid this? :)
Cheers,
Matt
On 27/09/2022 01:07, MPTCP CI wrote:
> Hi Paolo,
>
> Thank you for your modifications, that's great!
>
> Our CI did some validations and here is its report:
>
> - KVM Validation: normal:
> - Unstable: 2 failed test(s): selftest_mptcp_join selftest_simult_flows 🔴:
> - Task: https://cirrus-ci.com/task/4956489147219968
> - Summary: https://api.cirrus-ci.com/v1/artifact/task/4956489147219968/summary/summary.txt
>
> - KVM Validation: debug:
> - Unstable: 1 failed test(s): packetdrill_add_addr 🔴:
> - Task: https://cirrus-ci.com/task/6082389054062592
> - Summary: https://api.cirrus-ci.com/v1/artifact/task/6082389054062592/summary/summary.txt
>
> Initiator: Patchew Applier
> Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/2474bd2962c4
>
>
> 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)
>
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH mptcp-next] mptcp: update misleading comments.
2022-09-26 15:27 [PATCH mptcp-next] mptcp: update misleading comments Paolo Abeni
` (2 preceding siblings ...)
2022-09-26 23:07 ` mptcp: update misleading comments.: Tests Results MPTCP CI
@ 2022-09-27 8:02 ` Matthieu Baerts
3 siblings, 0 replies; 6+ messages in thread
From: Matthieu Baerts @ 2022-09-27 8:02 UTC (permalink / raw)
To: Paolo Abeni, mptcp
Hi Paolo, Mat,
On 26/09/2022 17:27, Paolo Abeni wrote:
> The MPTCP data path is quite complex and hard to understend even
> without some foggy comments referring to modified code and/or
> completely misleading from the beginning.
>
> Update a few of them to more accurately describing the current
> status.
Good idea, thank you for the patch and the review!
Now in our tree (feat. for net-next) with Mat's RvB tag:
New patches for t/upstream:
- 2bf3d5448679: mptcp: update misleading comments.
- Results: 676709a1f80b..34c4fe0fbc95 (export)
Tests are now in progress:
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220927T075707
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-27 8:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-26 15:27 [PATCH mptcp-next] mptcp: update misleading comments Paolo Abeni
2022-09-26 16:51 ` mptcp: update misleading comments.: Tests Results MPTCP CI
2022-09-26 21:14 ` [PATCH mptcp-next] mptcp: update misleading comments Mat Martineau
2022-09-26 23:07 ` mptcp: update misleading comments.: Tests Results MPTCP CI
2022-09-27 8:01 ` Matthieu Baerts
2022-09-27 8:02 ` [PATCH mptcp-next] mptcp: update misleading comments 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.