Page MenuHomeFreeBSD

Simplify cubic_ack_received
AcceptedPublic

Authored by kmacy on Sep 21 2016, 2:19 AM.

Details

Reviewers
hiren
lstewart
gnn
Group Reviewers
transport
Summary

The logic remains unchanged (with one exception) but the branching is much easier to read.

The one exception is that - if it is possible to reach a state where min_rtt_ticks is zero but we are *not* in slowstart the ack will be processed whereas before it would have been ignored.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 5244
Build 5398: CI src buildJenkins

Event Timeline

kmacy updated this revision to Diff 20561.Sep 21 2016, 2:19 AM
kmacy retitled this revision from to Simplify cubic_ack_received.
kmacy updated this object.
kmacy edited the test plan for this revision. (Show Details)
kmacy added a reviewer: transport.
kmacy set the repository for this revision to rS FreeBSD src repository.
kmacy added a subscriber: lstewart.
hiren edited edge metadata.Oct 9 2016, 8:04 AM

@kmacy, about the exception you noted, I am a little puzzled.
a state where min_rtt_ticks is zero but we are *not* in slowstart - could still be processed in the old code if other rfc3465 conditions align as the guarding 'if' looks like this:

(!V_tcp_do_rfc3465 ||
CCV(ccv, snd_cwnd) <= CCV(ccv, snd_ssthresh) || ​
(V_tcp_do_rfc3465 && ccv->flags & CCF_ABC_SENTAWND))

sys/netinet/cc/cc_cubic.c
147

Can we move this 'if' condition above the prev 'if' to keep the flow closer to that of the original file? or does it break the logic in a way I am not understanding?

hiren added a comment.Oct 17 2016, 8:03 PM

@kmacy Can you please looks at my comments? I like the changes otherwise.

Anyone else want to weigh in?

kmacy added a comment.Oct 27 2016, 1:15 AM

With regards to:

The one exception is that - if it is possible to reach a state where min_rtt_ticks is zero but we are *not* in slowstart the ack will be processed whereas before it would have been ignored.

I think this change is actually a functional improvement.

sys/netinet/cc/cc_cubic.c
147

It breaks the logic. We would be skipping newreno slow start handling.

gnn accepted this revision.Nov 28 2016, 5:32 PM
gnn edited edge metadata.

LGTM

This revision is now accepted and ready to land.Nov 28 2016, 5:32 PM