From: Matthieu Baerts <matttbe@kernel.org>
To: Geliang Tang <geliang@kernel.org>
Cc: martin.lau@kernel.org, Geliang Tang <tanggeliang@kylinos.cn>,
Paolo Abeni <pabeni@redhat.com>,
mptcp@lists.linux.dev
Subject: Re: [PATCH mptcp-net v2] mptcp: fix the default value of scaling_ratio
Date: Tue, 7 May 2024 12:00:20 +0200 [thread overview]
Message-ID: <70899a25-80ff-4456-bc85-5bba1240bc27@kernel.org> (raw)
In-Reply-To: <Zjn1hZVgE9QHwn8X@T480>
Hi Geliang,
On 07/05/2024 11:34, Geliang Tang wrote:
> Hi Matt,
>
> On Tue, May 07, 2024 at 10:12:41AM +0200, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> (+cc Paolo who wrote the commit mentioned by the "Fixes" tag)
>>
>> On 07/05/2024 05:46, Geliang Tang wrote:
>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>>
>>> BPF tests fail sometimes with "bytes != total_bytes" errors:
>>>
>>> test_default:PASS:sched_init:default 0 nsec
>>> send_data:PASS:pthread_create 0 nsec
>>> send_data:FAIL:recv 936000 != 10485760 nr_recv:-1 errno:11
>>> default: 3041 ms
>>> server:FAIL:send 7579500 != 10485760 nr_sent:-1 errno:11
>>> send_data:FAIL:pthread_join thread_ret:-11 test_default:PASS: \
>>> has_bytes_sent addr_1 0 nsec
>>> test_default:PASS:has_bytes_sent addr_2 0 nsec
>>> close_netns:PASS:setns 0 nsec
>>>
>>> In this case mptcp_recvmsg() gets EAGAIN errors. This issue introduces
>>> by commit b8dc6d6ce931 ("mptcp: fix rcv buffer auto-tuning"). The default
>>> value of scaling_ratio should be TCP_DEFAULT_SCALING_RATIO, not U8_MAX.
>>
>> It looks like you are modifying the max value, not the default one.
>>
>> Also, please always explain the reason: why should it be X, not Y?
>>
>>>
>>> Fixes: b8dc6d6ce931 ("mptcp: fix rcv buffer auto-tuning")
>>> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/487
>>> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
>>> ---
>>> v2:
>>> - I finally found the root cause of this issue.
>>> - cc Martin too.
>>> ---
>>> net/mptcp/protocol.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>>> index 579031c60937..d00cd21e8d3f 100644
>>> --- a/net/mptcp/protocol.c
>>> +++ b/net/mptcp/protocol.c
>>> @@ -1981,9 +1981,9 @@ static int __mptcp_recvmsg_mskq(struct mptcp_sock *msk,
>>> */
>>> static void mptcp_rcv_space_adjust(struct mptcp_sock *msk, int copied)
>>> {
>>> + u8 scaling_ratio = TCP_DEFAULT_SCALING_RATIO;
>>
>> With this modification, you are limiting msk->scaling_ratio to
>> TCP_DEFAULT_SCALING_RATIO. That's probably not what you want. See below:
>>
>> (...)
>> mptcp_for_each_subflow(msk, subflow)
>> (...)
>> scaling_ratio = min(tp->scaling_ratio, scaling_ratio);
>>
>> (...)
>> msk->scaling_ratio = scaling_ratio;
>>
>>
>> Do you mean that in your case, msk->scaling_ratio was assigned to
>> U8_MAX? Or it was assigned to a too high value (>TCP_DEFAULT_SCALING_RATIO)?
>>
>> Please add such info in the commit message as well.
>
> Sure. Here's the new subject and commit log:
>
> '''
> Subject: mptcp: fix the max value of scaling_ratio
>
> BPF tests fail sometimes (a probability of approximately 1%) with
> "bytes != total_bytes" errors:
>
> test_burst:PASS:open_and_load:burst 0 nsec
> test_bpf_sched:PASS:Scheduler name too long 0 nsec
> test_bpf_sched:PASS:burst 0 nsec
> create_netns:PASS:ip netns add mptcp_ns 0 nsec
> create_netns:PASS:ip -net mptcp_ns link set dev lo up 0 nsec
> sched_init:PASS:create_netns 0 nsec
> endpoint_init:PASS:ip -net mptcp_ns link add veth1 type veth peer name
> endpoint_init:PASS:ip -net mptcp_ns addr add 10.0.1.1/24 dev veth1 0 nsec
> endpoint_init:PASS:ip -net mptcp_ns link set dev veth1 up 0 nsec
> endpoint_init:PASS:ip -net mptcp_ns addr add 10.0.1.2/24 dev veth2 0 nsec
> endpoint_init:PASS:ip -net mptcp_ns link set dev veth2 up 0 nsec
> endpoint_init:PASS:ip -net mptcp_ns mptcp endpoint add 10.0.1.2 subflow
> sched_init:PASS:endpoint_init 0 nsec
> test_bpf_sched:PASS:burst 0 nsec
> send_data_and_verify:PASS:burst 0 nsec
> send_data_and_verify:PASS:burst 0 nsec
> (network_helpers.c:613: errno: Resource temporarily unavailable) \
> send 5608500 expected 10485760
> (network_helpers.c:661: errno: None) recv 2755984 expected 10485760
> (network_helpers.c:669: errno: None) Failed in thread_ret -11
> send_data_and_verify:FAIL:send_recv_data unexpected error: -4 (errno 0)
> #162/9 mptcp/burst:FAIL
> #162 mptcp:FAIL
>
> In this case, mptcp_recvmsg() gets EAGAIN errors. This issue introduces
> by commit b8dc6d6ce931 ("mptcp: fix rcv buffer auto-tuning"). The max
> value of scaling_ratio should be TCP_DEFAULT_SCALING_RATIO (128), not
> U8_MAX (255). Otherwise, scaling_ratio is assigned to a too high value.
> '''
Sorry, I'm still not sure whether I understand the issue:
- Why limiting to the default value?
- Here, we want the higher scaling_ratio from all the available
subflows: is this value too high? What "too high value" you got that was
causing the issue you mentioned?
- What is a "too high value" for you and why?
- Or is it because scaling_ratio was assigned to U8_MAX and not changed
to the max value of the subflows by accident because there was a bypass
somehow?
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
prev parent reply other threads:[~2024-05-07 10:00 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-07 3:46 [PATCH mptcp-net v2] mptcp: fix the default value of scaling_ratio Geliang Tang
2024-05-07 4:33 ` MPTCP CI
2024-05-07 5:37 ` Geliang Tang
2024-05-07 8:12 ` Matthieu Baerts
2024-05-07 9:34 ` Geliang Tang
2024-05-07 10:00 ` Matthieu Baerts [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=70899a25-80ff-4456-bc85-5bba1240bc27@kernel.org \
--to=matttbe@kernel.org \
--cc=geliang@kernel.org \
--cc=martin.lau@kernel.org \
--cc=mptcp@lists.linux.dev \
--cc=pabeni@redhat.com \
--cc=tanggeliang@kylinos.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).