Page MenuHomeFreeBSD
Differential D60460

tcp: count only the SACK blocks that fit in tcp_maxseg()
Needs ReviewPublic
Actions

Authored by nick_spun.io on Thu, Oct 8, 5:43 AM.
Tags
None
Referenced Files
F175644004: D60460.id189278.diff
Mon, Oct 12, 6:14 AM
F175608536: D60460.id189278.diff
Mon, Oct 12, 12:36 AM
F175608199: D60460.id189279.diff
Mon, Oct 12, 12:33 AM
F175608022: D60460.id189026.diff
Mon, Oct 12, 12:31 AM
F175589458: D60460.id189279.diff
Sun, Oct 11, 9:21 PM
F175585816: sent-too-much-964x960_tcp_log.bin
Sun, Oct 11, 8:46 PM
F175585789: sent-too-much-sack-optlen.pkt
Sun, Oct 11, 8:46 PM
F175578278: D60460.id189278.diff
Sun, Oct 11, 7:40 PM
View All 21 Files
Subscribers
glebius
imp
melifaro

Details

Reviewers
tuexen
rscheff
glebius
adrian
Group Reviewers
transport
Summary

tcp_addoptions() sends only the SACK blocks that fit in TCP_MAXOLEN.
tcp_maxseg() counted every pending block and clamped the total, so
without timestamps and with five or more blocks pending it came out 4
bytes below the segment tcp_output() sends. Limited transmit after an
RTO then sent one full segment and failed the "sent too much" KASSERT.

PR: 282605

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 77915
Build 74798: arc lint + arc unit

Event Timeline

nick_spun.io created this revision.Thu, Oct 8, 5:43 AM
Herald added a reviewer: transport. · View Herald TranscriptThu, Oct 8, 5:43 AM
Herald added subscribers: glebius, melifaro, imp. · View Herald Transcript
nick_spun.io requested review of this revision.Thu, Oct 8, 5:43 AM
Harbormaster completed remote builds in B77839: Diff 189026.Thu, Oct 8, 5:43 AM
glebius accepted this revision.Thu, Oct 8, 10:20 PM
glebius added reviewers: tuexen, rscheff.

Looks correct, but let's check with @rscheff. Thanks!

sys/netinet/tcp_subr.c
3806–3807

I'd suggest to declare sockblks in this scope. Up to you.

tuexen added a comment.Fri, Oct 9, 5:03 AM

Let me write a packetdrill-test to make sure we catch an KASSERT-issue.

nick_spun.io updated this revision to Diff 189278.Fri, Oct 9, 6:37 PM

change variable scope

Harbormaster completed remote builds in B77914: Diff 189278.Fri, Oct 9, 6:37 PM
nick_spun.io marked an inline comment as done.Fri, Oct 9, 6:37 PM
nick_spun.io updated this revision to Diff 189279.Fri, Oct 9, 6:39 PM

fix spacing

Harbormaster completed remote builds in B77915: Diff 189279.Fri, Oct 9, 6:39 PM
nick_spun.io added a reviewer: adrian.Sun, Oct 11, 7:28 PM
tuexen added a comment.Sun, Oct 11, 7:45 PM

I am trying to trigger a "sent too much" KASSERT but was not successful up to now. Can you provide a BBLog file of such a situation?

nick_spun.io added a comment.Sun, Oct 11, 8:46 PM
In D60460#1390029, @tuexen wrote:

I am trying to trigger a "sent too much" KASSERT but was not successful up to now. Can you provide a BBLog file of such a situation?

Here's a bblog file and a packetdrill repro

sent-too-much-sack-optlen.pkt2 KBDownload

sent-too-much-964x960_tcp_log.bin7 KBDownload

tuexen added a comment.Sun, Oct 11, 9:23 PM
In D60460#1390041, @nick_spun.io wrote:
In D60460#1390029, @tuexen wrote:

I am trying to trigger a "sent too much" KASSERT but was not successful up to now. Can you provide a BBLog file of such a situation?

Here's a bblog file and a packetdrill repro

sent-too-much-sack-optlen.pkt2 KBDownload

sent-too-much-964x960_tcp_log.bin7 KBDownload

How can I access these files?

nick_spun.io added a comment.Sun, Oct 11, 9:25 PM
In D60460#1390073, @tuexen wrote:
In D60460#1390041, @nick_spun.io wrote:
In D60460#1390029, @tuexen wrote:

I am trying to trigger a "sent too much" KASSERT but was not successful up to now. Can you provide a BBLog file of such a situation?

Here's a bblog file and a packetdrill repro

sent-too-much-sack-optlen.pkt2 KBDownload

sent-too-much-964x960_tcp_log.bin7 KBDownload

How can I access these files?

Sorry, I forgot that attached files are still private by default - you should be able to see them now.

nick_spun.io added a comment.Sun, Oct 11, 9:26 PM

Uploaded them here as well in case it still doesn't work

https://people.freebsd.org/~nprice/sent-too-much

Revision Contents
Changeset List

PathSize
sys/
netinet/
9 lines

Diff 189279

View Options

sys/netinet/tcp_subr.c

Loading...