- User Since
- Oct 18 2018, 9:44 PM (17 w, 3 d)
Mon, Feb 11
- replace one division with a multiplication, with proper brackets. Taken from D8021
Replacing two divisions by one multiplication and one division in cubic.h looks good;
Fri, Feb 8
While splitting these overflow checks out from the other Diff, I found that there is another typecast overflow, and potential negative overflow in cubic_cwnd.
- removed overflow and boundary checks (see D19118 for details)
Thu, Feb 7
Splitting off the interger overflow changes into another Diff.
Wed, Feb 6
Tue, Feb 5
- prepare to land
Over the last two or three weeks, we have run a large number of performance regression tests including this patch, in particular again workloads with frequent app-stalls (no additional data to send for about an RTO interval). That type of workload very often causes burst to be transmitted, including self-inflicted packet drops.
Sun, Feb 3
- sys/libkern.h has implicit typecast to (int) for min/max. Need to use ulmin/ulmax for (long)
- fixing the limit check from D14141
Sat, Feb 2
According to this https://www.ietf.org/proceedings/88/slides/slides-88-tcpm-9.pdf Linux - at least at some point - also used a dynamic limit with pipe (inflight) as an input parameter: maxburst = pipe +3.
Thu, Jan 31
- consolidate snd_una adjustment for SYN bit
- set snd_wnd in the generic case in rack
RACK already increments snd_una to deal with FASTOPEN. Consolidate the snd_una++ in the code then?
RACK seems to have a similar code in rack_do_syn_recv.
Maybe a break in line 2433 is actually missing for the original issue (just another thought; browsing the history doesn't show that though). In any case, this change looks sensible enough.
Wed, Jan 30
So, what you are saying is that this issue here bascially masked the problem reported in PR235256 from happening when window scale was not negotiated.
At least that's what I'm observing, and what the code appears to be saying.
Tue, Jan 29
- prevent integer overflows of cwnd
- dragging ticks_last_cong along (restricting to INT_MAX)
For sufficiently large values of wmax, the calculation in w_tf suffers
for integer overflows in intermediate calculation steps
Mon, Jan 28
The validation scripts for RACK, which was actually similarly affected.
- adding RACK to the SYN bit sequence space fix
- adding RACK stack to ECN handshake check
- trailing whitespace
Here are two test scripts to validate both the proper behavior of ECN as a client, against a conformant RFC3168 client, as well as a non-conformant client which reflects back all header flags as received:
Sun, Jan 27
Ok, I've could not remember that there are already hardcoded exceptions for TCP options alreads. Since there is precedent that this is an accepted path to deal with special cases (such as ECN++ against a legacy Linux box, should the stack start doing ECN++, of course), i remove my objection.
I was just starting to validate something around RTO, and found, that the effective IW is now actually 11 SMSS, not 10 MSS, when NOT doing ABC (net.inet.tcp.rfc3465=0).
A packetdrill script to verfiy the proper operation of the 6675 pipe calculation.
Packetdrill script to validate both new functionalities of Rescue Retransmission, as well as PartialAck with 3 MSS Sack Block to enter Loss Recovery.
Sat, Jan 26
- missed to revert tcpcb variable name used during testing
This packetdrill script can reliably reproduce the observed issue.
- dangling else; operation validated w/ packetdrill unit testing
- update cwnd with w_tf only when it is larger (avoid transmission stalls)
packet drill test script for validating that RW == IW after idle
Fri, Jan 25
Here is one Patch, where the "wrong" variable name holding recovery has been used for SACK in the past: rS132417
For the application and client receive window limited case I would like to gather some feedback on my thoughs around this:
Include RACK in consolidation effort, tcp_compute_initcwnd and tcp_maxseg in rack_after_idle
Thu, Jan 24
Looking at D8225, that all seems to be code while in loss recovery. This patch is to restore a sane minimum cwnd once exiting loss recovery - so I don't see how these would be directly related.
Here is a sample siftr log, prior to the patch. A simple utility is transmitting 1MB once per second, with the pause in between transfers well above the idle time.
Should be adding RFC6298 to https://wiki.freebsd.org/TransportProtocols/tcp_rfc_compliance too...
Good that RFC2988 is marked obsolete there already - but i think that is only tracking the RFC state, not the state of which RFC the stack mostly conforms to.
Jan 18 2019
- line wrap6
- line wrap
- let us not forget on IPv6
- have sysctl check the port number range directly
Received a PM from tuexen. The Q was, why it is safe to nuke sack_newdata and completely replace it everywhere with snd_recover.
- remove port filter
- revert variable initialization (happening twice)
Shall I split off the port filter into a seperate Diff?
- removing stack variable
- adding check to avoid potential memory leak
- adding error value if enabling or disabling is not possible, if trying the action twice in a row
- fixing trailing whitespaces
- remove siftr patch
- fixing trailing whitespaces
- No need to check for uint32 smaller than zero
Here is the output of the now functional siftr, without and with the patch;
Jan 16 2019
Jan 10 2019
Jan 3 2019
Attached is a tcptrace of a real-world observed issue, where the lack of RFC6582 results in cwnd shrinking down to 1 MSS, followed by delayed ACK timeout and congestion avoidance growth of cwnd (1 MSS per RTT).
Dec 24 2018
The variable sack_newdata really is "RecoveryPoint" in RFC3517/RFC6675 when in SACK context, while RFC3782 calls this "recover" in the NewReno context.
Dec 22 2018
- Another accounting oversight. RFC6675 allows a partial ack with SACK info to immediately enter fast recovery, where this could result in inappropriate tracking of sacked_bytes.
- fixed accounting error with partial or cumulative acks
- lots of debugging code.
Dec 20 2018
- whitespace and comment changes
Dec 18 2018
This packetdrill script should complete without error, when IW10 and the above patch are applied, for a SACK session, or non-SACK session.
Dec 15 2018
Minor comment edit
and moving to GIT/Phabricator/ARC workflow
Nov 29 2018
Thanks for the updates comments!
Only nit in the comment to this patch: