lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 24 May 2022 13:22:49 -0700 From: Eric Dumazet <edumazet@...gle.com> To: Neal Cardwell <ncardwell@...gle.com> Cc: David Laight <David.Laight@...lab.com>, Oleksandr Natalenko <oleksandr@...alenko.name>, Yuchung Cheng <ycheng@...gle.com>, Yousuk Seung <ysseung@...gle.com>, Soheil Hassas Yeganeh <soheil@...gle.com>, Adithya Abraham Philip <abrahamphilip@...gle.com>, "David S. Miller" <davem@...emloft.net>, Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>, David Ahern <dsahern@...nel.org>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Konstantin Demin <rockdrilla@...il.com> Subject: Re: [RFC] tcp_bbr2: use correct 64-bit division On Tue, May 24, 2022 at 1:06 PM Neal Cardwell <ncardwell@...gle.com> wrote: > > On Tue, May 24, 2022 at 4:01 AM David Laight <David.Laight@...lab.com> wrote: > > > > From: Oleksandr Natalenko > > > Sent: 22 May 2022 23:30 > > > To: Neal Cardwell <ncardwell@...gle.com> > > > > > > Hello Neal. > > > > > > It was reported to me [1] by Konstantin (in Cc) that BBRv2 code suffers from integer division issue on > > > 32 bit systems. > > > > Do any of these divisions ever actually have 64bit operands? > > Even on x86-64 64bit divide is significantly slower than 32bit divide. > > > > It is quite clear that x * 8 / 1000 is the same as x / (1000 / 8). > > So promoting to 64bit cannot be needed. > > > > David > > The sk->sk_pacing_rate can definitely be bigger than 32 bits if the > network path can support more than 34 Gbit/sec (a pacing rate of 2^32 > bytes per sec is roughly 34 Gibt/sec). This definitely happens. > > So this one seems reasonable to me (and is only in debug code, so the > performance is probably fine): > - (u64)sk->sk_pacing_rate * 8 / 1000, > + div_u64((u64)sk->sk_pacing_rate * 8, 1000), > > For the other two I agree we should rework them to avoid the 64-bit > divide, since we don't need it. > > There is similar logic in mainline Linux in tcp_tso_autosize(), which > is currently using "unsigned long" for bytes. > Not sure I follow. sk_pacing_rate is also 'unsigned long' So tcp_tso_autosize() is correct on 32bit and 64bit arches. There is no forced 64bit operation there. > Eric, what do you advise? > > thanks, > neal
Powered by blists - more mailing lists