Page MenuHomeFreeBSD
Differential D28326

TCP PRR: Patch div/0 in tcp_prr_partialack
ClosedPublic
Actions

Authored by rscheff on Jan 24 2021, 9:26 PM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 27 2024, 7:46 AM
Unknown Object (File)
Jan 20 2024, 8:38 PM
Unknown Object (File)
Jan 20 2024, 8:03 PM
Unknown Object (File)
Dec 20 2023, 3:25 AM
Unknown Object (File)
Dec 10 2023, 12:41 PM
Unknown Object (File)
Nov 25 2023, 4:27 PM
Unknown Object (File)
Oct 10 2023, 7:13 PM
Unknown Object (File)
Oct 2 2023, 2:59 PM
View All 20 Files
Subscribers
imp
melifaro

Details

Reviewers
tuexen
kbowling
rrs
jtl
mm
Group Reviewers
transport
Commits
rG76dd854f47f4: TCP PRR: Patch div/0 in tcp_prr_partialack
rG6a376af0cd21: TCP PRR: Patch div/0 in tcp_prr_partialack
Summary

With clearing of recover_fs in bc7ee8e5bc555, div/0
was observed while processing partial_acks.

Suspect that rewind of an erraneous RTO may be
causing this - with the above change, recover_fs
would no longer retained at the last calculated
value, and reset. But CC_RTO_ERR can reenable
IN_RECOVERY(), without setting this again.

Adding a safety net prior to the division in that
function, which I missed in D28114.

Adding line wraps in that PRR block.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 36461
Build 33350: arc lint + arc unit

Event Timeline

rscheff created this revision.Jan 24 2021, 9:26 PM
Herald added a reviewer: transport. · View Herald TranscriptJan 24 2021, 9:26 PM
Herald added subscribers: melifaro, imp. · View Herald Transcript
rscheff requested review of this revision.Jan 24 2021, 9:26 PM
Harbormaster completed remote builds in B36455: Diff 82830.Jan 24 2021, 9:26 PM
tuexen added a comment.Jan 24 2021, 9:34 PM

Maybe mm@ can test this?

rscheff updated this revision to Diff 82831.Jan 24 2021, 9:41 PM
  • retain recover_fs past recovery, in case of RTO rollback
Harbormaster completed remote builds in B36456: Diff 82831.Jan 24 2021, 9:41 PM
rscheff added a reviewer: mm.Jan 24 2021, 9:41 PM
mm accepted this revision.Jan 25 2021, 8:47 AM

LGTM, both places where the division happens are now protected.
We are testing it now with public traffic (~ 8 hours), looks good so far.

tuexen accepted this revision.Jan 25 2021, 10:40 AM

I would suggest to separate the whitspace changes and the functional changes into two commits.

This revision is now accepted and ready to land.Jan 25 2021, 10:40 AM
rscheff updated this revision to Diff 82840.Jan 25 2021, 11:15 AM

Adjust line length in tcp_prr_partialack, no functional change.

This revision now requires review to proceed.Jan 25 2021, 11:15 AM
Harbormaster completed remote builds in B36461: Diff 82840.Jan 25 2021, 11:15 AM
rscheff updated this revision to Diff 82841.Jan 25 2021, 11:18 AM
  • separate out whitespace changes
Harbormaster completed remote builds in B36462: Diff 82841.Jan 25 2021, 11:19 AM
mm accepted this revision.Jan 25 2021, 1:25 PM

LGTM, now both places with the division are protected.
As mentioned earlier, our live test runs fine for now (>12 hours).

This revision is now accepted and ready to land.Jan 25 2021, 1:25 PM
tuexen accepted this revision.Jan 25 2021, 1:28 PM
rscheff closed this revision.Jan 26 2021, 3:10 PM
rscheff added a commit: rG6a376af0cd21: TCP PRR: Patch div/0 in tcp_prr_partialack.
rscheff added a commit: rG76dd854f47f4: TCP PRR: Patch div/0 in tcp_prr_partialack.Jan 29 2021, 11:02 AM

Revision Contents
Changeset List

PathSize
sys/
netinet/
13 lines

Diff 82840

View Options

sys/netinet/tcp_input.c

Loading...