svn commit: r334804 - in head/sys: kern modules/tcp modules/tcp/rack netinet netinet/tcp_stacks sys

[Randall Stewart]

> On Jun 7, 2018, at 6:01 PM, hiren panchasara <hiren at strugglingcoder.info> wrote:
> 
> On 06/07/18 at 06:18P, Randall Stewart wrote:
>> Author: rrs
>> Date: Thu Jun  7 18:18:13 2018
>> New Revision: 334804
>> URL: https://svnweb.freebsd.org/changeset/base/334804
>> 
>> Log:
>>  This commit brings in a new refactored TCP stack called Rack.
>>  Rack includes the following features:
>>   - A different SACK processing scheme (the old sack structures are not used).
>>   - RACK (Recent acknowledgment) where counting dup-acks is no longer done
>>          instead time is used to knwo when to retransmit. (see the I-D)
>>   - TLP (Tail Loss Probe) where we will probe for tail-losses to attempt
>>          to try not to take a retransmit time-out. (see the I-D)
>>   - Burst mitigation using TCPHTPS
>>   - PRR (partial rate reduction) see the RFC.
>> 
>>  Once built into your kernel, you can select this stack by either
>>  socket option with the name of the stack is "rack" or by setting
>>  the global sysctl so the default is rack.
>> 
>>  Note that any connection that does not support SACK will be kicked
>>  back to the "default" base  FreeBSD stack (currently known as "default").
>> 
>>  To build this into your kernel you will need to enable in your
>>  kernel:
>>     makeoptions WITH_EXTRA_TCP_STACKS=1
>>     options TCPHPTS
>> 
>>  Sponsored by:	Netflix Inc.
>>  Differential Revision:		https://reviews.freebsd.org/D15525
>> 
>> Added:
>>  head/sys/modules/tcp/rack/
>>  head/sys/modules/tcp/rack/Makefile   (contents, props changed)
>>  head/sys/netinet/tcp_stacks/rack.c   (contents, props changed)
>>  head/sys/netinet/tcp_stacks/rack_bbr_common.h   (contents, props changed)
>>  head/sys/netinet/tcp_stacks/sack_filter.c   (contents, props changed)
>>  head/sys/netinet/tcp_stacks/sack_filter.h   (contents, props changed)
>>  head/sys/netinet/tcp_stacks/tcp_rack.h   (contents, props changed)
>> Modified:
>>  head/sys/kern/uipc_sockbuf.c
>>  head/sys/modules/tcp/Makefile
>>  head/sys/netinet/tcp.h
>>  head/sys/netinet/tcp_log_buf.h
>>  head/sys/netinet/tcp_output.c
>>  head/sys/netinet/tcp_stacks/fastpath.c
>>  head/sys/netinet/tcp_timer.c
>>  head/sys/netinet/tcp_timer.h
>>  head/sys/netinet/tcp_var.h
>>  head/sys/sys/mbuf.h
>>  head/sys/sys/queue.h
>>  head/sys/sys/sockbuf.h
>>  head/sys/sys/time.h
> 
> I thought we'd have more time to review/test this. Looks like BSDCan
> commit-spree in effect. :-)

The Phabricator review has been up since May 22nd. Thats over 2.5 weeks,
this was also discussed on the Thursday conference calls.
> 
> A few questions:
> 1) Does RACK work reliably without HPTS? If yes, has that config been
> tested?
> 
No it requires the pacer.

> 2) It looks like PRR is tied to RACK. Why did we go that route?
> Shouldn't it be easily used with the 'default' stack also?
> 

It is what I developed.. and I had no desire to work with the default stack. That
is a fifth rail that no one wants touched.

> 3) Can new SACK be used with the traditional stack?

Well if you want to rework the base stack you might be able to do that :)

It would be quite some effort.. I think Robert wants eventually the old
stack to be de-composed and then slowly work at getting more common
code between them until eventually you can have a diff and somehow
figure out how to integrate the two.

> 
> 4) Where should manpage like info for RACK go? a new man-page or
> extending tcp(4)? Info like how to enable system-wide or per socket
> should go here.
> 

The enable/disable or per-socket I think is in with the pluggable stack
stuff. We might want a Rack man page.. have to think about it.



> 5) Any perf numbers to go along with this commit? Synthetic or
> production numbers showing improvements in transfer speed or any other
> impact on CPU usage (specially with HPTS) that you can share?
> 

CPU will be more but we see close to a drop in rebuffers by about 12% I am told.

> 6) In your testing, have you found cases where RACK does poorly compared
> to the 'default' stack? Any recommendations on when should RACK be
> enabled? (Something like this could go in the manpage.)

Nope. 

R

> 
> Glad to finally see this in -head!
> 
> Cheers,
> Hiren

--------
Randall Stewart
rrs at netflix.com
803-317-4952