Subject: [Lwip] Review of draft-ietf-lwig-tcp-constrained-node-networks-04

From: Ilpo Järvinen <[email-masked]>
Date: Wed, 7 Nov 2018 14:14:12 +0200 (EET)

General Comments / Structure
----------------------------

I think this document is useful, however, I think that the current ordering
is somewhat confusing. The draft jumps too soon into 1 MSS case and only
later covers the tradeoffs in the use of some algorithms/mechanisms.
I'd rather make the order such that it starts from larger devices and
gradually strips features explaining the tradeoffs and if/when the
mechanism loses its usefulness at some point (or even becomes
counterproductive) and why, and how to mitigate the problems (if applicable).
There are two possible ways to realize this: either do this within each
mechanism section or have "general CNN recomendations", "small window 
rec", "1 MSS recomendations" and then repeat the mechanisms in these when 
there's something to say about them.

There's careless use of "overhead". In general, it should be explicitly
qualified whether it's processing, memory, or network overhead (can 
obviously impact more than one of them). Similar concern might apply to 
other similar words (also on the improvements side). It might even be
useful to explicitly list each of these per mechanism along these lines 
(E.g., for enabling ECN):

Implementation complexity: small increase
Processing overhead: small increase but may reduce # of wakeups
Memory/TCB impact: small increase
Network impact: when network is congested, reduces the number of
retransmission. Otherwise, no impact.

...but I realized it might be hard to word them.

Small Points
------------

Sec 3.2 Usage Scenarios, 3rd para: I fail to get the point of this
paragraph. There are two distinct points about the environment:
middleboxes on path and asymmetry in end point capabilities. No
implications about bringing these two in particular up in the same
paragraph are mentioned. That is, why I must note the asymmetry when
there's a middlebox?

Sec 4.1.1 MSS

Lots of discussion about MTU sizes (and why they have those numbers)
but the main recommendation seems to be just to use 1280/1220 bytes?
Perhaps it would be simpler to say that 1280/1220 B earlier and use the 
MTU stuff as examples that it works at least with these link layer techs?

Fails to mention that larger MSS may reduce processing and network
overhead (more work per wakeup / pkt).

While I admit I'm not very familiar with path MTU discovery in general
(except knowing very well how Linux TCP does it based on dup/cumulative 
ACKs after a larger probe packet), are there some additional challenges in 
the case where you have just window of one?

Sec 4.1.2 ECN

1st para, the list in the end of the para & 2nd para start: lacks the
point that IMHO is the most important for CNN: with ECN less rexmits
are needed because there's no need to drop to signal congestion regardless
of the congestion level (the original, now marked copy of the packet
makes to the destination instead of being dropped and requiring rexmit).

2nd para, the first sentence: earlier reaction is more a feature of
AQM (vs use of Taildrop) rather than just ECN.

2nd para: RTO also cause extra wakeups (vs ACK clock triggered sending).

Does not mention that ECN increases implementation complexity.

Sec 4.2 title says "small windows" but in practice the section talks
only about 1 MSS case. Both would be useful to cover, not just 1 MSS
case?

Sec 4.2.2 TCP options for single-MSS

Wscale useful only if need window > 64KB should be mentioned.

TFO implementation complexity, TCB impact and network overhead not
covered adequately (something mentioned in 5.3 but lacking). TFO
idempotency requirement missing from the document.

4.2.3 DA for 1-MSS

"The number of transferred bytes", you might want to rephrase to
something more precise: ACKs whose sending are avoided reduce
network "overhead".

Title says 1 MSS, content talks about "very small windows", which
way it is?

In general, usefulness of DA depends heavily on usage scenario:
unidirection, req-response, whether the receiver goes to zero
rwnd or not, size of the transaction (1 or more packets needed;
don't forget that e.g. CoAP over TCP sends the CSM messages
besides the actual payload), etc. This should either cover most
(/all) cases or not give other guidance than saying that setting
delayed ACK either may cause suboptimal performance (and list
them: potentially extra delays or extra ACKs).

Besides delayed ACK, I think that also Nagle should be covered
and has somewhat similar scenario dependant caveats as DA.

4.2.4 RTO for 1-MSS

2nd para, 2nd sentence, "...size and cannot ..." -> "...size, it
cannot..."

CoCoA is not specified for TCP (even as ID), does further work
type research / experimentation required things like this belong
into this kind of recommendations document? I think not. Then it
follows that there are not real recommendations in this section.

4.3.1 Error Recovery & CC & Flow control

Title: use loss recovery, not error recovery

Should mention the complexity & TCB impact (though both are
quite small).

4.3.2 SACK

IMHO, SACK should be subsection of loss recovery or 4.3.1
should be retitled to only FR/FR.

Out-of-order queue handling is unrelated to SACK, should be
covered somewhere else? There is implicit complexity & TCB
impact when the flow may have >1 MSS wnd, maybe group all these
(when not related to a particular mechanism that has its own
discussion somewhere) under a single section).

No sender-side SACK aspect covered?

In general, there's occassionally confusion within the document
whether some advice/description is for the receiving or sending
side (this is of course scenario dependant which the implementer
should consider in his/her own case but the document should cover
both cases where applicable).

5.2 Concurrent connections

1 para, last sentense: does this just rehash the previous sentence
or is there something besides TCB that should be mentioned here
(snd/rcv buffer come to my mind but perhaps there are other things
too to consider)?

Mention 3-way handshake impact.

5.3 Connection lifetime

4 para starts with "This overhead", what overhead is meant?

5 para, perhaps mention that detecting peer death timely may allow
saving some from overall TCB memory use.

6. Security considerations

Is the 1-2 para misuse of this section? If the point is to give
recommedations how to do CNN security, why not use "Recommendations
for TCP Flow Security in CNN" or something along those lines. I
thought Sec. considerations is reserved for the impact of the
suggested mechanisms. Since the document mostly does build on other
RFCs, the statement like in 3nd para is enough. The 4th para is
probably ok too.

8.1 uIP

I'm curious, is the buffer shared also between all TCP connections?

8.3 RIOT

Not being able to change MSS does not appear in the actual content
but only here in Annex. If there's some important point to make
about it, please add into main content.

8.4 TinyOS

Claims both: "The application is responsible for buffering" and
"A send buffer is provided [this is passive but I assumed by
OS/stack] so that the TCP implementaion can automatically
retransmit missing segments". So which way it is? I'd also remove
"can" and state without it that "The TCP implementation
automatically retransmits missing segments" (some rephrasing
may be needed anyway to fix the passive).

8.5 FreeRTOS & 8.6 uC/OS & Table in 8.7

"multiple-MSS" is unclear, is this multiple of MSS or multiple
segments (not necessary MSS sized)? Is this symmetric?

T3 "~1.2kB for each additional connection", is this "code" or
buffers?

One bonus, if available would be nice to include: TLS (in the
TCP implementations)?

Refs
----

- RFC 1323 obsoleted by 7323.
- Some IDs nowadays RFCs.
- RFC 1981 obsoleted by 8201.

Nits
----

Everywhere: ack vs ACK
4.3.2: "several MSSs" look weird, just use "several MSS".
5.3: rereceiving -> receiving
8.1: "uses same buffer both" -> "uses the same buffer for both" 
8.2: "SACK and Window Scale have" -> "SACK and Window Scale support have"

-- 
 i.