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

From: "Carles Gomez Montenegro" <[email-masked]>
Date: Sun, 10 Mar 2019 09:15:16 +0100

Dear Ilpo,

Thank you very much for your thorough review, and sorry for the late reply.

Please find below our inline responses:

> 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.

We believe that both your suggested ordering as well as the current
ordering are valid approches. In the last draft revision, we have left the
order unchanged as this does not affect the actual technical content.

> 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.

In the new draft version, in some cases, we have tried to be more specific
(e.g. “message overhead”, “memory overhead”, etc.). However, in some other
cases the context may help to better determine the scope of “overhead”, or
“overhead” relates with all the dimensions you listed (and for simplicity,
we prefer to keep just “overhead”).

> 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?

Because the middlebox will often be transparent to TCP (but not to other
protocols). Basically, the presence of such middleboxes is a major
motivation for use of TCP in IoT environments (e.g. RFC 8323).

> 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?

Yes, we believe that your suggestion improves the clarity and
effectiveness of the text. We have modified the draft as per your
suggestion.

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

Good point. We have now mentioned the benefits of a larger MSS.

> 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?

We understand Path MTU discovery will work also for a single-segment
window size, albeit with probably longer time until MTU discovery,
assuming that the time between two consecutive packets sent by the
constrained device is relatively high (as is typical in many IoT
applications).

However, single-MSS devices will tend to require minimal complexity, so we
understand that it is unlikely that many such devices will support Path
MTU Discovery.

> 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).

We have restructured the section, and have explicitly stated your point.

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

That sentence refers to detecting congestion earlier (when receiving the
ECN signal) than by RTO expiration.

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

Good point. We have added it to the paragraph.

> Does not mention that ECN increases implementation complexity.

Added a sentence in this regard now.

> 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?

We have updated the title to reflect that the section focuses only on the
single-MSS case.

> Sec 4.2.2 TCP options for single-MSS
>
> Wscale useful only if need window > 64KB should be mentioned.

We have added a note on this.

> 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.

We have extended the TFO discussion, explicitly mentioning its drawbacks.
Increased implementation complexity and TCB impact are now explicitly
mentioned. The TFO idempotency requirement is now discussed in the
document. However, we have made these updates in subsection 5.3.

> 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".

Done!

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

Single-MSS.

> 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).

In the current section, we focus on the main cases for single-MSS window
size.  Other relevant information, for larger window size, is present in
4.3.3. Nevertheless, to provide a proper introduction to the section, we
have added your notion that “In general, usefulness of DA depends heavily
on usage scenario”.

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

We have added text on Nagle.

> 4.2.4 RTO for 1-MSS
>
> 2nd para, 2nd sentence, "...size and cannot ..." -> "...size, it
> cannot..."

Done.

> 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.

Basically, what the section conveys is that the RTO may have a greater
impact on performance for single-MSS-based implementations (typical in IoT
scenarios) than for more general TCP implementations.

The reference to CoCoA is given just as an example that there is margin
for RTO tuning in IoT scenarios, with an impact on performance. (In
experimental work, CoCoA was found to work better than some TCP-specific
RTO algorithms.)

> 4.3.1 Error Recovery & CC & Flow control
>
> Title: use loss recovery, not error recovery

Done.

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

Done.

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

Yes, we agree with the suggestion, and prefer to make SACK a subsection of
4.3.1.

> 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).

Not sure if the current wording may lead to different understandings, but
out-of-order is mentioned here to denote the fact that a few segments may
be lost and the receiver will need to inform about the data blocks
actually received.

> No sender-side SACK aspect covered?

We currently have:
“a sender (having previously sent the SACK-Permitted
   option) can avoid performing unnecessary retransmissions, saving
   energy and bandwidth, as well as reducing latency.”
Is there any particular aspect you think should be added ?

> 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).

We’d welcome specific suggestions of document sections where your comment
applies.

> 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)?

We have added snd/rcv buffer as an example. There may be more, but also
this goes into TCP implementation internals.

> Mention 3-way handshake impact.

Done.

> 5.3 Connection lifetime
>
> 4 para starts with "This overhead", what overhead is meant?

This reads now as “The message and latency overhead that stems from using
a sequence of short-lived connections…”.

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

Done.

> 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.

We understand that the Security Considerations section needs to discuss
potential attacks and possible countermeasures. From this point of view,
using mechanisms intended to secure TCP (e.g. TLS, TCP-AO, etc.) fall in
the scope of countermeasures.

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

Yes, it is. We have added a sentence on this.

> 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.

We don’t see any important point to make about it in this document.

>
> 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).

We have fixed this. A clearer sentence would be “a send buffer is provided
by the application".

> 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?

Good point. “Multiple-segment” is a better term here.

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

We understand this is buffers.

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

Added now.

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

Done.

>
> 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"

All done.

> --
>  i.

Once again, thank you very much for your thorough review!

Cheers,

Carles (on behalf of all authors)