net/mptcp/protocol.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
Fix this divide error:
----
divide error: 0000 [#1] PREEMPT SMP KASAN NOPTI
CPU: 0 PID: 14336 Comm: syz-executor.6 Not tainted 6.1.0-rc1-00215-g47aa7f23f440 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
RIP: 0010:div_u64_rem include/linux/math64.h:29 [inline]
RIP: 0010:div_u64 include/linux/math64.h:128 [inline]
RIP: 0010:mptcp_subflow_get_send+0xa87/0x1200 net/mptcp/protocol.c:1486
----
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/314
Reported-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
net/mptcp/protocol.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index ddeb8b36a677..3a07c0d197d4 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1475,13 +1475,14 @@ struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk)
if (!ssk || !sk_stream_memory_free(ssk))
return NULL;
- burst = min_t(int, MPTCP_SEND_BURST_SIZE, mptcp_wnd_end(msk) - msk->snd_nxt);
- wmem = READ_ONCE(ssk->sk_wmem_queued);
- if (!burst) {
+ if (mptcp_wnd_end(msk) <= msk->snd_nxt) {
msk->last_snd = NULL;
return ssk;
}
+ burst = min_t(u32, MPTCP_SEND_BURST_SIZE, mptcp_wnd_end(msk) - msk->snd_nxt);
+ wmem = READ_ONCE(ssk->sk_wmem_queued);
+
subflow = mptcp_subflow_ctx(ssk);
subflow->avg_pacing_rate = div_u64((u64)subflow->avg_pacing_rate * wmem +
READ_ONCE(ssk->sk_pacing_rate) * burst,
--
2.35.3
On Sat, 22 Oct 2022, Geliang Tang wrote: > Fix this divide error: > > ---- > divide error: 0000 [#1] PREEMPT SMP KASAN NOPTI > CPU: 0 PID: 14336 Comm: syz-executor.6 Not tainted 6.1.0-rc1-00215-g47aa7f23f440 #1 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014 > RIP: 0010:div_u64_rem include/linux/math64.h:29 [inline] > RIP: 0010:div_u64 include/linux/math64.h:128 [inline] > RIP: 0010:mptcp_subflow_get_send+0xa87/0x1200 net/mptcp/protocol.c:1486 > ---- > > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/314 > Reported-by: Mat Martineau <mathew.j.martineau@linux.intel.com> > Signed-off-by: Geliang Tang <geliang.tang@suse.com> > --- > net/mptcp/protocol.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c > index ddeb8b36a677..3a07c0d197d4 100644 > --- a/net/mptcp/protocol.c > +++ b/net/mptcp/protocol.c > @@ -1475,13 +1475,14 @@ struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk) > if (!ssk || !sk_stream_memory_free(ssk)) > return NULL; > > - burst = min_t(int, MPTCP_SEND_BURST_SIZE, mptcp_wnd_end(msk) - msk->snd_nxt); > - wmem = READ_ONCE(ssk->sk_wmem_queued); > - if (!burst) { > + if (mptcp_wnd_end(msk) <= msk->snd_nxt) { > msk->last_snd = NULL; > return ssk; > } > > + burst = min_t(u32, MPTCP_SEND_BURST_SIZE, mptcp_wnd_end(msk) - msk->snd_nxt); > + wmem = READ_ONCE(ssk->sk_wmem_queued); > + > subflow = mptcp_subflow_ctx(ssk); > subflow->avg_pacing_rate = div_u64((u64)subflow->avg_pacing_rate * wmem + > READ_ONCE(ssk->sk_pacing_rate) * burst, > -- > 2.35.3 Hi Geliang - I haven't been able to reproduce the problem using commit id c3917837f483 (export/20221024T065054 without the "features other trees" patches), and the only change in mptcp_subflow_get_send() is the removal of the fallback check in "mptcp: add get_subflow wrappers". The stack trace shows that mptcp_subflow_get_send() is called directly by __mptcp_push_pending(): RIP: 0010:mptcp_subflow_get_send+0xa87/0x1200 net/mptcp/protocol.c:1486 ... Call Trace: <TASK> __mptcp_push_pending+0x19e/0x770 net/mptcp/protocol.c:1550 mptcp_sendmsg+0x694/0x19d0 net/mptcp/protocol.c:1813 So I think the divide by zero is due to the missing fallback check. Seems like the fallback check should be moved only when mptcp_subflow_get_send() is only called using the wrapper? WDYT? -- Mat Martineau Intel
Hi Geliang, 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): packetdrill_add_addr 🔴: - Task: https://cirrus-ci.com/task/5728997552685056 - Summary: https://api.cirrus-ci.com/v1/artifact/task/5728997552685056/summary/summary.txt - {"code":404,"message": - "Can't find artifacts containing file conclusion.txt"}: - Task: https://cirrus-ci.com/task/5166047599263744 - Summary: https://api.cirrus-ci.com/v1/artifact/task/5166047599263744/summary/summary.txt Initiator: Patchew Applier Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/c5b3f2e01f96 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)
No need to update info->limit, drop it.
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
net/mptcp/protocol.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 958c7fd6da2b..a21bf43472f6 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1244,8 +1244,8 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
pr_debug("msk=%p ssk=%p sending dfrag at seq=%llu len=%u already sent=%u",
msk, ssk, dfrag->data_seq, dfrag->data_len, info->sent);
- if (info->sent > info->limit ||
- info->limit > dfrag->data_len)
+ if (WARN_ON_ONCE(info->sent > info->limit ||
+ info->limit > dfrag->data_len))
return 0;
if (unlikely(!__tcp_can_send(ssk)))
@@ -1573,7 +1573,6 @@ static int __subflow_push_pending(struct sock *sk, struct sock *ssk,
}
info->sent += ret;
- info->limit -= ret;
copied += ret;
len -= ret;
--
2.35.3
On Sat, 22 Oct 2022, Geliang Tang wrote: > No need to update info->limit, drop it. > > Signed-off-by: Geliang Tang <geliang.tang@suse.com> Hi Geliang - Please include this in the next revision of the "BPF redundant scheduler" series. Thanks, Mat > --- > net/mptcp/protocol.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c > index 958c7fd6da2b..a21bf43472f6 100644 > --- a/net/mptcp/protocol.c > +++ b/net/mptcp/protocol.c > @@ -1244,8 +1244,8 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk, > pr_debug("msk=%p ssk=%p sending dfrag at seq=%llu len=%u already sent=%u", > msk, ssk, dfrag->data_seq, dfrag->data_len, info->sent); > > - if (info->sent > info->limit || > - info->limit > dfrag->data_len) > + if (WARN_ON_ONCE(info->sent > info->limit || > + info->limit > dfrag->data_len)) > return 0; > > if (unlikely(!__tcp_can_send(ssk))) > @@ -1573,7 +1573,6 @@ static int __subflow_push_pending(struct sock *sk, struct sock *ssk, > } > > info->sent += ret; > - info->limit -= ret; > copied += ret; > len -= ret; > > -- > 2.35.3 > > > -- Mat Martineau Intel
© 2016 - 2024 Red Hat, Inc.