Page MenuHomeFreeBSD
Differential D23230

Implement ECN++ (draft-generalized-ecn)
Needs ReviewPublic
Actions

Authored by rscheff on Jan 17 2020, 11:28 AM.
Tags
None
Referenced Files
F103128045: D23230.id111294.diff
Thu, Nov 21, 9:17 AM
F103120275: D23230.id102035.diff
Thu, Nov 21, 6:54 AM
F103097880: D23230.id87615.diff
Wed, Nov 20, 11:00 PM
F103090111: D23230.id87067.diff
Wed, Nov 20, 8:10 PM
F103089206: D23230.id83546.diff
Wed, Nov 20, 7:49 PM
Unknown Object (File)
Wed, Nov 20, 10:51 AM
Unknown Object (File)
Wed, Nov 20, 10:31 AM
Unknown Object (File)
Wed, Nov 20, 5:21 AM
View All Files
Subscribers
bjk
glebius
imp
melifaro
pauamma_gundo.com

Details

Reviewers
rgrimes
tuexen
jtl
cc
bcr
pauamma_gundo.com
Group Reviewers
transport
manpages
Summary

RFC3168 only allows regular data segments to be sent as
ECN-capable transport (ECT). Control (RST, FIN), pure ACK
and retransmissions are explicitly not to be sent with ECT.

However, it has been shown over the years, that this
approach stifles the usefulness of ECN, in particular when
deploying DCTCP in controlled environments.

Current work in the IETF (draft-generalized-ecn, also known
as ECN++) aims to allow all control and retransmission
packets to be marked ECT. In the case of retransmissions,
the only standardized recourse when one of those is dropped,
is to wait for the RTO timer to expire - including the
penalty imposed by having to suffer an RTO.

Furthermore, allowing control and pure ACKs to convey
additional congestion signals on the return path enables
features like AckCC (RFC5690).

Also, adding code to make non-session related control packets
(eg. RST to non-listening ports) appear idential with those in
response to recently used sessions, to prevent sidechannel
information leakage.

Test Plan

Attaching packetdrill scripts for full coverage

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 47828
Build 44715: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Harbormaster completed remote builds in B36805: Diff 83546.Feb 8 2021, 3:02 PM
rscheff retitled this revision from Implement ECN+ (RFC5562) and ECN++ feature (draft-generalized-ecn) to Implement ECN++ (draft-generalized-ecn) .Feb 8 2021, 3:05 PM
rscheff edited the summary of this revision. (Show Details)
rscheff updated this revision to Diff 84707.Feb 25 2021, 7:37 PM
rscheff retitled this revision from Implement ECN++ (draft-generalized-ecn) to Implement ECN++ (draft-generalized-ecn).
  • updated timestamp in man file
Harbormaster completed remote builds in B37381: Diff 84707.Feb 25 2021, 7:38 PM
rscheff updated this revision to Diff 87067.Apr 8 2021, 7:48 PM
  • rebase
  • streamline fastpath check
  • update man date
Harbormaster completed remote builds in B38429: Diff 87067.Apr 8 2021, 7:49 PM
rscheff updated this revision to Diff 87071.Apr 8 2021, 8:15 PM
  • have to explicitly check for ECN SYN
Harbormaster completed remote builds in B38433: Diff 87071.Apr 8 2021, 8:15 PM
rscheff updated this revision to Diff 87615.Apr 17 2021, 2:44 PM
  • add TF2_ECN_PLUSPLUS flag to reduce cacheline churn in fastpath when checking V_ecn_generalized
Harbormaster completed remote builds in B38645: Diff 87615.Apr 17 2021, 2:44 PM
rscheff updated this revision to Diff 88943.May 10 2021, 1:39 PM
  • rebase
Harbormaster completed remote builds in B39093: Diff 88943.May 10 2021, 1:39 PM
rscheff mentioned this in D32373: tcp: Add hystart-plus to cc_newreno and rack..Oct 19 2021, 5:48 PM
rscheff updated this revision to Diff 102035.Jan 28 2022, 8:43 AM
  • rebase
  • bump tcp.4 datestamp
Herald added a subscriber: glebius. · View Herald TranscriptJan 28 2022, 8:43 AM
Harbormaster completed remote builds in B44136: Diff 102035.Jan 28 2022, 8:44 AM
pauamma_gundo.com added a subscriber: pauamma_gundo.com.Jan 28 2022, 11:38 PM
pauamma_gundo.com added inline comments.
share/man/man4/tcp.4
543
549–550

Typo and move full stop to a more natural place.

bjk added a subscriber: bjk.Jan 31 2022, 5:00 AM

Current work in the IETF (draft-generalized-ecn, also known as ECN++)

http://datatracker.ietf.org/doc/draft-generalized-ecn does not return a document.
The IETF TSV WG is where I would expect any such work to be done, but
https://datatracker.ietf.org/wg/tsvwg/documents/ does not appear to show
any documents that might be given the moniker "ECN++". Could you provide
a better reference or link for the draft specification that is being implemented?

rscheff added a comment.Jan 31 2022, 7:20 AM

Interesting. I could find this right away: ECN++: Adding Explicit Congestion Notification (ECN) to TCP Control Packets

Here is the history

rscheff updated this revision to Diff 102150.Jan 31 2022, 7:26 AM
  • fix man page typos
Harbormaster completed remote builds in B44185: Diff 102150.Jan 31 2022, 7:26 AM
rscheff marked 2 inline comments as done.Jan 31 2022, 7:27 AM

Thanks!

bjk added a comment.Jan 31 2022, 7:28 AM
In D23230#771332, @rscheff wrote:

Interesting. I could find this right away: ECN++: Adding Explicit Congestion Notification (ECN) to TCP Control Packets

Here is the history

Ah, a TCP-specific draft, not generic to all ECN usage. So I was looking in the wrong place :)
Thanks for the pointer.

pauamma_gundo.com added a comment.Jan 31 2022, 1:44 PM

LGTM now.

pauamma_gundo.com added a comment.Jan 31 2022, 1:46 PM
In D23230#771360, @pauamma_gundo.com wrote:

LGTM now.

By which I mean, the language in the manual page change does. Can't say more.

bcr added a comment.Jan 31 2022, 2:36 PM

Typo in man page.

share/man/man4/tcp.4
555

s/identially/identically/

rscheff updated this revision to Diff 102159.Jan 31 2022, 2:50 PM
  • fix typo in man page
Harbormaster completed remote builds in B44190: Diff 102159.Jan 31 2022, 2:50 PM
bcr accepted this revision as: manpages.Feb 1 2022, 2:55 PM

OK for the manpages part of the change.

rscheff updated this revision to Diff 103223.Feb 25 2022, 1:07 PM
  • rebase
  • simplify the fastpath check if ECT should be set.
  • streamline fastpath check
  • update man date
  • have to explicitly check for ECN SYN
  • add TF2_ECN_PLUSPLUS flag to reduce cacheline churn in fastpath when checking V_ecn_generalized
  • bump tcp.4 datestamp
  • fix man page typos
  • fix typo in man page
  • update to use the common tcp_ecn.c sourcefile
  • fix types
Harbormaster completed remote builds in B44574: Diff 103223.Feb 25 2022, 1:07 PM
pauamma_gundo.com added inline comments.Feb 25 2022, 4:57 PM
share/man/man4/tcp.4
554–555

Can you explain what you mean by that? I'm arguably not the indended audience, but if I don't understand it, there's a chance some of the intended audience and or some interested readers won't either.

rscheff added inline comments.Feb 25 2022, 5:13 PM
share/man/man4/tcp.4
554–555

When a host receives a TCP packet to a port which is not listening, or no connection exists, or the header information is in some other way not acceptable, the host may respond with a RST (reset) packet. Some of these RST packets are sent from "regular" TCP processing (e.g. outside the sequence window) and others from non-open ports. Making these distinctable by carrying different markings - depending which code path was sending them - would give clues as to what ports/services may be reachable, and give rise to more targetted attacks.
As part if this change, when introducing this new capability (modified TCP header markings for sessions), the irregular codepath is addressed too, such that a 3rd party can not deduct which of the two different paths was sending out the RST, reducing an information side-channel.

pauamma_gundo.com added inline comments.Feb 26 2022, 12:09 AM
share/man/man4/tcp.4
554–555

Gotcha, thanks. Would "This value also uses ECN for RST replies to probes of non-open ports." mean the same? It looks clearer to me.

rscheff updated this revision to Diff 111188.Sep 29 2022, 2:58 PM
  • rebase
  • make tcp_ecn_get_ace static inline
  • include accecn for ecn++
  • fix ecn bits in timewait
  • fix syn-sent ip_ecn marking
Harbormaster completed remote builds in B47606: Diff 111188.Sep 29 2022, 2:58 PM
rscheff marked 3 inline comments as done.Sep 29 2022, 3:07 PM
pauamma_gundo.com accepted this revision.Oct 1 2022, 5:05 AM

Mnor wording nit, can be fixed on commit.

share/man/man4/tcp.4
513
rscheff updated this revision to Diff 111294.Oct 1 2022, 8:55 AM
rscheff marked an inline comment as done.
  • update man page wording
Harbormaster completed remote builds in B47665: Diff 111294.Oct 1 2022, 8:55 AM
pauamma_gundo.com accepted this revision.Oct 5 2022, 4:56 PM
rscheff updated this revision to Diff 111754.Oct 13 2022, 8:43 AM
  • include ect0 for tcp_respond()
Harbormaster completed remote builds in B47828: Diff 111754.Oct 13 2022, 8:43 AM
rscheff updated this revision to Diff 111756.Oct 13 2022, 8:50 AM
  • remove logging
Harbormaster completed remote builds in B47829: Diff 111756.Oct 13 2022, 8:51 AM
pauamma_gundo.com accepted this revision.Oct 13 2022, 5:20 PM

Manual page unchanged besides the bumped Dd.

rscheff updated this revision to Diff 112774.Nov 8 2022, 12:11 PM
  • rebase
  • bump man date
  • remove duplicate code
Harbormaster completed remote builds in B48231: Diff 112774.Nov 8 2022, 12:11 PM
pauamma_gundo.com added inline comments.Nov 9 2022, 12:58 AM
share/man/man4/tcp.4
773–789

Was removing this intended as part of this review?

rscheff added inline comments.Nov 9 2022, 9:12 AM
share/man/man4/tcp.4
773–789

I'm confused, there is no change here on reviews.freebsd.org; it may be that this diff was uploaded just prior to a flurry of tcp related (including man page) changes, which I have not yet rebased the patch to...

rscheff updated this revision to Diff 112797.Nov 9 2022, 10:38 AM
  • rebase
  • use ect1 if session is using that
Harbormaster completed remote builds in B48238: Diff 112797.Nov 9 2022, 10:38 AM
rscheff marked an inline comment as done.Nov 9 2022, 10:41 AM
rscheff updated this revision to Diff 112811.Nov 9 2022, 4:55 PM
  • use switch statement instead of if else
Harbormaster completed remote builds in B48244: Diff 112811.Nov 9 2022, 4:56 PM
pauamma_gundo.com accepted this revision.Nov 9 2022, 11:23 PM
rscheff updated this revision to Diff 133195.Jan 23 2024, 2:27 PM
  • rebase and align
Harbormaster completed remote builds in B55568: Diff 133195.Jan 23 2024, 2:27 PM
rscheff updated this revision to Diff 133384.Jan 26 2024, 2:09 AM
  • rebase main
Harbormaster completed remote builds in B55631: Diff 133384.Jan 26 2024, 2:09 AM
rscheff updated this revision to Diff 134553.Feb 18 2024, 10:29 AM
  • rebase main
Harbormaster completed remote builds in B56083: Diff 134553.Feb 18 2024, 10:29 AM

Revision Contents
Changeset List

PathSize
share/
man/
man4/
21 lines
sys/
netinet/
5 lines
105 lines
55 lines
30 lines
tcp_stacks/
28 lines
34 lines
18 lines
4 lines
3 lines

Diff 111754

View Options

share/man/man4/tcp.4

Loading...
View Options

sys/netinet/tcp_ecn.h

Loading...
View Options

sys/netinet/tcp_ecn.c

Loading...
View Options

sys/netinet/tcp_input.c

Loading...
View Options

sys/netinet/tcp_output.c

Loading...
View Options

sys/netinet/tcp_stacks/rack.c

Loading...
View Options

sys/netinet/tcp_subr.c

Loading...
View Options

sys/netinet/tcp_syncache.c

Loading...
View Options

sys/netinet/tcp_usrreq.c

Loading...
View Options

sys/netinet/tcp_var.h

Loading...