On Sun, 13 Dec 2020 21:59:45 +0800 Cambda Zhu wrote: > > On Dec 13, 2020, at 06:32, Jakub Kicinski <k...@kernel.org> wrote: > > On Tue, 8 Dec 2020 17:19:10 +0800 Cambda Zhu wrote: > >> For each TCP zero window probe, the icsk_backoff is increased by one and > >> its max value is tcp_retries2. If tcp_retries2 is greater than 63, the > >> probe0 timeout shift may exceed its max bits. On x86_64/ARMv8/MIPS, the > >> shift count would be masked to range 0 to 63. And on ARMv7 the result is > >> zero. If the shift count is masked, only several probes will be sent > >> with timeout shorter than TCP_RTO_MAX. But if the timeout is zero, it > >> needs tcp_retries2 times probes to end this false timeout. Besides, > >> bitwise shift greater than or equal to the width is an undefined > >> behavior. > > > > If icsk_backoff can reach 64, can it not also reach 256 and wrap? > > If tcp_retries2 is set greater than 255, it can be wrapped. But for TCP > probe0, > it seems to be not a serious problem. The timeout will be icsk_rto and backoff > again. And considering icsk_backoff is 8 bits, not only it may always be > lesser > than tcp_retries2, but also may always be lesser than tcp_orphan_retries. And > the icsk_probes_out is 8 bits too. So if the max_probes is greater than 255, > the connection won’t abort even if it’s an orphan sock in some cases. > > We can change the type of icsk_backoff/icsk_probes_out to fix these problems. > But I think maybe the retries greater than 255 have no sense indeed and the > RFC > only requires the timeout(R2) greater than 100s at least. Could it be better > to > limit the min/max ranges of their sysctls?
All right, I think the patch is good as is, applied for 5.11, thank you!
