Re: [Gen-art] [Last-Call] Genart last call review of draft-ietf-tcpm-2140bis-09

[Theresa Enghardt <[e-mail]>]

Thanks Joe, your replies look good to me.

Looking forward to the updated draft!

Best,
Theresa


On 3/3/21 4:54 PM, Joseph Touch wrote:
> Hi,Theresa,
>
> Many thanks for the review.
>
> I think we can address the issues below in the next rev easily. I’ve added some comments below to confirm the way forward.
>
> Please let us know if you have any concerns with the replies below, but otherwise we will have an update posted after the ID submission queue reopens.
>
> Joe
>
>> On Feb 19, 2021, at 10:50 AM, Theresa Enghardt via Datatracker <[e-mail]> wrote:
>>
>> Reviewer: Theresa Enghardt
>> Review result: Ready with Nits
>>
>> I am the assigned Gen-ART reviewer for this draft. The General Area
>> Review Team (Gen-ART) reviews all IETF documents being processed
>> by the IESG for the IETF Chair.  Please treat these comments just
>> like any other last call comments.
>>
>> For more information, please see the FAQ at
>>
>> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
>>
>> Document: draft-ietf-tcpm-2140bis-09
>> Reviewer: Theresa Enghardt
>> Review Date: 2021-02-19
>> IETF LC End Date: 2021-02-22
>> IESG Telechat date: Not scheduled for a telechat
>>
>> Summary: This document is fairly clear and concise, however there are a few
>> minor issues and nits that should be addressed before publishing.
>>
>> Major issues: None.
>>
>> Minor issues:
>>
>> Abstract:
>>
>> "This memo provides guidance to TCP implementers that are intended to
>>    help improve convergence to steady-state operation without affecting
>>    interoperability."
>> I think it'd be good to clarify what "steady-state operation" or "steady-state
>> behavior" means in this context, either here or in the introduction where this
>> term is used again. Assuming both terms refer to the same concept, please
>> consider unifying the terms.
> In both cases, it refers to the behavior of TCP over long connections, rather than during transient effects of changes in path behavior. It is a very typical term in the TCP community, but we will unify them and add a sentence to be explicit.
>
>> Introduction:
>>
>> "Path information shared across
>>    SYN destination port numbers assumes that TCP segments having the
>>    same host-pair experience the same path properties, irrespective of
>>    TCP port numbers."
>> Does this mean that one should rather avoid sharing information between
>> connections with different SYN destination port numbers? If there is a
>> recommendation, I think it'd be good to be explicit about it. If no
>> recommendation is being made, maybe the doc should state that these are
>> assumptions that implementers should be aware of.
> There are some networks that route traffic differently based on application type, inferred from the SYN destination port. We can clarify this issue.
>
> Note however that there are several other path properties that are already shared across port numbers, e.g., path MTU feedback per RFC1191 is cached at the IP layer, irrespective of port.
>
>> I think it's also worth
>> noting that, even if connections share the same host-pair and SYN destination
>> port numbers, paths and path conditions can always change over time. It'd be
>> good to put in a reference to Section 8.1 here, where there's more discussion
>> about these assumptions.
> We do note that things change over time; that’s where the rest of TCP’s mechanism comes into play. We can also add a reminder about age-out mechanisms in the temporal sharing section as well as sec 8.1
>
>> 4. The TCP Control Block (TCB)
>>
>> As Appendix B mentions that it may be unsafe to share other state, perhaps it's
>> good to add a sentence about this here and refer to Appendix B for more details.
> Will do.
>
>> "One class is clearly host-pair dependent (#, e.g., MSS, MMS, PMTU, RTT)..."
>> Please consider adding a brief explanation why it's "clearly" host-pair
>> dependent.
> Will do.
>
>> Discussion on this could also be added to Section 8.1 and then
>> refered to here. Also, isn't this information path-dependent rather than
>> host-dependent?
> Some of it is endpoint dependent only; some is path dependent. This will be clarified.
>
>> If the doc is making an assumption about host pairs and paths
>> here, please make it explicit.
> Agreed (see above).
>
>> 6.1 Initialization of the new TCB
>>
>> In the table "TEMPORAL SHARING - TCB Initialization", "or not cached" is added
>> for old_MMS_S and old_MMS_R, but not for other information in this table.
>> Please consider adding a brief explanation and/or clarifying this point here.
> Will do; they might not be cached when they are computed locally (MMS_R) or indicated in the SYN (MMS_S).
>
>> Is there any notion on information being outdated or expiring, as path
>> conditions might change over time?
> Yes - see note above.
>
>> "Note that PMTU feedback is cached at the IP layer" - Is this true even with
>> PLPMTUD?
> Yes,
>
>> I think it's worth putting in a brief clarification of whether this
>> depends on the mechanism, as PMTUD and PLPMTUD are explicitly introduced in
>> Section 3.
> Will do - both PMTUD and PLPMTUD cache PMTU info at the IP layer.
>
>> 7.2. Updates to the new TCB
>>
>> Here, the row with old_PMTU and curr_PMTU says "PMTUD+ / PLPMTUD". In Section
>> 6.2, a similar row only says "PMTUD+". Is this intentional, i.e., with PLPMTU,
>> can PMTU only be shared with Temporal Sharing and not with Ensemble Sharing?
> Both places should list both mechanisms.
>
>> 8. Compatibility Issues
>>
>> What kind of compatibility is meant here? Is it compatibility between
>> implementations? If not, please consider renaming this section, e.g., to
>> "Issues with TCB information sharing".
> Agreed.
>
>> 8.1. Traversing the same network path
>> "When TCB information is shared across different SYN destination ports,
>>    path-related information can be incorrect"
>> Please consider adding an explanation here. Why is this true specifically for
>> SYN destination ports?
> See earlier; this is because of port-sensitive routing.
>
>> 11. Updates to RFC 2140
>>
>> "addressing RSS in both the send and receive direction"
>> RSS is not defined in this doc.
> That is a typo; it should be MSS (as in MSS_R and MMS_S).
>
>> 12. Security Considerations
>> "These presented implementation methods do not have additional
>>    ramifications for explicit attacks."
>> What are explicit attacks, and are denial-of-service attacks not explicit?
> Explicit implies direct (which should be used instead), i.e., TCB sharing does not affect an attack on a single TCP connect, e.g., RST-flooding or SYN attacks.
>
>> "TCB sharing may be susceptible to denial-of-service attacks […]"
>> Please consider clarifying the attack scenario here: Is it that an attacker
>> opens many connections, and then each new connection gets state from the other
>> ones, so the denial-of-service attack's impact is increased due to TCB sharing?
> Right; this will be clarified. The idea is that the attacker is polluting the cached info.
>
>> Is there a potential for a denial-of-service attack preventing TCB sharing?
> Not really “preventing”, but just polluting the cached info.
>
> But even this pollution is temporary; it affects only the initial conditions of new connections. If an attacker is really opening many other TCP connections, they’re indistinguishable from legitimate connections that would be used to update the cache.
>
>> Can there be any privacy implications of sharing connection state, e.g.,
>> between connections of different applications, or within the same application
>> such as between browser tabs?
> The information that is shared affects the performance of TCP, not the content of its connections. I.e., the shared information is never intended to be private.
>
>> Nits/editorial comments:
>>
>> 3. Terminology:
>>
>> Some terms appear to be named inconsistenly: MMS_S uses "_S" suffix, but
>> sendMSS uses "send" prefix to denote send direction. Can these be unified?
> We’re using terms defined in other documents and prefer not to coin new terms or rename them. We’ll double-check these to confirm.
>
>> There is a definition for both "cwnd" and "sendcwnd". Are these the same?
> Cwnd is the term for the congestion window; there is a send cwnd and a receive cwnd.
>
>> Later, Section 4 uses "snd_cwnd". Should this be "sendcwnd" instead?
> Yes.
>
>> Typo:
>> "to discovery the PMTU" → "to discover the PMTU"
> Will do.
>
>> RWIN is spelled in all-caps, cwnd is lower case. Can these be unified?
> We are using conventions established in other documents. Again, we’ll double-check these.
>
>> 6.3 Discussion
>>
>> "Caching MMS_R and MMS_S may be of little direct value as they
>>    are reported by the local IP stack anyway."
>> The same statement has already been made at the very beginning of this
>> subsection. Please consider removing the redundant statement.
> Will do - that’s the reason they might not be cached.
>
>> For a better overview, maybe it would help if these discussion
>> points/considerations were grouped by the TCB value being discussed, and
>> possibly provided as a list?
> This is a useful idea; we’ll see what we can do in the next update.
>
>> 7.3 Discussion
>>
>> "Current TCP
>>    implementations initialize it to four segments as standard [rfc3390]
>>    and 10 segments experimentally [RFC6928]."
>> Duplicate information - the same statement has already been made in the
>> previous paragraph. Please consider removing the redundant statement and/or
>> restructuring the text here.
> Yes, thanks.
>
>> -- 
>> last-call mailing list
>> [e-mail]
>> https://www.ietf.org/mailman/listinfo/last-call
>