Page MenuHomeFreeBSD
Differential D29524

tcp: Shouldn't drain empty sbuf
ClosedPublic
Actions

Authored by rscheff on Mar 31 2021, 11:01 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Feb 6, 8:07 AM
Unknown Object (File)
Mon, Feb 3, 2:03 PM
Unknown Object (File)
Fri, Jan 24, 5:47 PM
Unknown Object (File)
Fri, Jan 24, 5:43 PM
Unknown Object (File)
Fri, Jan 24, 2:29 PM
Unknown Object (File)
Fri, Jan 24, 9:45 AM
Unknown Object (File)
Fri, Jan 24, 8:29 AM
Unknown Object (File)
Sun, Jan 19, 8:31 PM
View All 90 Files
Subscribers
imp
melifaro

Details

Reviewers
tuexen
jhb
jtl
Group Reviewers
transport
Commits
rGcca0fba78e5f: tcp: Shouldn't drain empty sbuf
rG2a9ce2734f8b: tcp: Shouldn't drain empty sbuf
rG9f3835013032: tcp: Shouldn't drain empty sbuf
rG9aef4e7c2bd4: tcp: Shouldn't drain empty sbuf
Summary

Need a bool protection after all, to not hit a KASSERT

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rscheff created this revision.Mar 31 2021, 11:01 PM
Herald added a reviewer: transport. · View Herald TranscriptMar 31 2021, 11:02 PM
Herald added subscribers: melifaro, imp. · View Herald Transcript
rscheff requested review of this revision.Mar 31 2021, 11:02 PM
Harbormaster completed remote builds in B38230: Diff 86649.Mar 31 2021, 11:02 PM
rscheff updated this revision to Diff 86685.Apr 1 2021, 1:58 PM
  • fix typo in assert string
Harbormaster completed remote builds in B38247: Diff 86685.Apr 1 2021, 1:59 PM
tuexen added inline comments.Apr 1 2021, 2:00 PM
sys/netinet/tcp_hostcache.c
709

Maybe add a comment why you are doing this dance. It wasn't obvious for us...

rscheff updated this revision to Diff 86688.Apr 1 2021, 2:07 PM
  • add comment to explain why do_drain is needed
Harbormaster completed remote builds in B38249: Diff 86688.Apr 1 2021, 2:07 PM
tuexen added inline comments.Apr 1 2021, 2:21 PM
sys/netinet/tcp_hostcache.c
711

Not sure coordination is involved. Isn't it more that one should not call sbuf_drain() with an empty buffer. Please state that and finish the sentence with a full stop (.).

rscheff updated this revision to Diff 86691.Apr 1 2021, 2:34 PM
  • update comment
Harbormaster completed remote builds in B38251: Diff 86691.Apr 1 2021, 2:34 PM
tuexen accepted this revision.Apr 1 2021, 2:40 PM
This revision is now accepted and ready to land.Apr 1 2021, 2:40 PM
Closed by commit rG9aef4e7c2bd4: tcp: Shouldn't drain empty sbuf (authored by rscheff). · Explain WhyApr 1 2021, 3:21 PM
This revision was automatically updated to reflect the committed changes.
rscheff added a commit: rG9aef4e7c2bd4: tcp: Shouldn't drain empty sbuf.
jhb added a comment.Apr 1 2021, 5:38 PM

I would probably instead have changed sbuf_drain() to just return instead of asserting. Asserting made sense when it was a private implementation function, as a public API I think just returning instead of asserting is better behavior.

rscheff added a commit: rG9f3835013032: tcp: Shouldn't drain empty sbuf.Apr 16 2021, 8:47 PM
rscheff added a commit: rG2a9ce2734f8b: tcp: Shouldn't drain empty sbuf.Apr 16 2021, 9:34 PM
rscheff added a commit: rGcca0fba78e5f: tcp: Shouldn't drain empty sbuf.Apr 16 2021, 10:12 PM

Revision Contents
Changeset List

PathSize
sys/
netinet/
12 lines

Diff 86697

View Options

sys/netinet/tcp_hostcache.c

Loading...