Differential D20550

Fix bugs in DCTCP on TCP events
ClosedPublic
Actions

Authored by rscheff on Jun 7 2019, 12:49 PM.

Details

Reviewers

tuexen
hiren
gnn
cc

Commits

rS351995: MFC r350404:
rS350404: When performing after_idle() or post_recovery(), don't disable the

Summary

DCTCP reactions to after_idle and post_recovery would disable
themselves after the first call.
Also, have consistent fall-through to NewReno for non-ECN
sessions.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

rscheff created this revision.Jun 7 2019, 12:49 PM

Harbormaster completed remote builds in B24749: Diff 58358.Jun 7 2019, 12:49 PM

Comment from Midori (original author of this code):

Thank you for your message and improving dctcp implementation. The description of two bugfix request makes sense. I have two comments. First, remove white space at line126 in the D20550 patch before submission. Second, you
should remove a change at line353 in the D20550 patch. If I understand correctly, it does not affect performance. Please point out if there is my misunderstanding.

rscheff added inline comments.Jun 11 2019, 8:36 AM

sys/netinet/cc/cc_dctcp.c
127 ↗	(On Diff #58358)	remove white space at line126 in the D20550 patch before submission
353 ↗	(On Diff #58358)	MidorI: remove a change at line353 in the D20550 patch. If I understand correctly, it does not affect performance R: If other parts of the code decided to send out an ACK right away (e.g. leaving zero-window episodes), we should not clear the ACKNOW flag here.

- missed rename of variable

Harbormaster completed remote builds in B24895: Diff 58729.Jun 17 2019, 2:32 PM

rscheff added inline comments.Jun 17 2019, 2:36 PM

sys/netinet/cc/cc_dctcp.c
127 ↗	(On Diff #58358)	Leaving whitespace, as that separates different ack types. using sacked data (CCV(ccv, sackhint.delivered_data) here depends on the RFC6675 chain of patched (D18892, D18985).