Re: [tcpm] WGLC for draft-ietf-tcpm-converters-08

[<[e-mail]>]

Hi Phil,

Thank you for the review. Much appreciated.

Changes to address almost all your comments (except the protocol model one) can be seen at:
https://github.com/obonaventure/draft-tcp-converters/pull/62/files 

Please see inline.

Cheers,
Med

> -----Message d'origine-----
> De : tcpm [mailto:[e-mail]] De la part de
> [e-mail]
> Envoyé : jeudi 27 juin 2019 17:58
> À : [e-mail]; [e-mail]
> Cc : [e-mail]
> Objet : Re: [tcpm] WGLC for draft-ietf-tcpm-converters-08
> 
> I support this document. Thanks to the authors for the nice work.
> 
> Here are some comments /questions and suggested clarifications.
> 
> Questions:
> Section 4.1
> "    The Client and the Transport Converter MUST send the fixed-sized
>    header, shown in Figure 9, as the first four bytes of the bytestream."
> Are there any other protocols that claim rights on the first bytes of the
> bytestream? Even in the context of signalling as indicated by the port#
> (to be assigned), is this certain?

[Med] I'm not aware of any. The point here is to unambiguously identify Convert data. 

> 
> " Data added by the Convert protocol to the TCP bytestream in the
>    upstream connection is unambiguously distinguished from payload data
>    in the downstream connection by the Total Length field in the Convert
>    messages."
> Is it required that the Convert data is at a fixed point in the
> bytestream? Think you've assumed it's at the start?

[Med] Yes, we assume that Convert data is at the start. 

> (Also, I think you could delete 'upstream' and 'downstream' in the quoted
> sentence - in any case, presumably they should both be one or the other)
> 

[Med] Fixed. 

> Convert protocol:
> How do the messages with the Supported TCP Extensions TLV and Connect TLV
> relate to each other? Is it required to learn through the "Supported TCP
> Extensions" that a Converter supports a particular TCP option before
> sending a Connect with that Option? I assume not - and that it's just a
> MAY to send the Supported TCP Extensions - as it seems to me like an
> optimisation and not too much harm happens if you send a Connect for an
> unsupported extension.

[Med] Yes, this is optional. The expected behavior is covered by this text: 

   The Info TLV (Figure 12) is an optional TLV which can be sent by a
   Client to request the TCP extensions that are supported by a
   Transport Converter.  It is typically sent on the first connection                         
   that a Client establishes with a Transport Converter to learn its
   capabilities.  Assuming a Client is entitled to invoke the Transport
   Converter, the latter replies with the Supported TCP Extensions TLV
   described in Section 4.2.4.

> 
> --------------
> Suggested clarifications - significant:-
> 

[Med] Snipped. Will cover this one in a separate message

> ----------------
> Suggested clarifications - other:-
> Section 1:
> 
> Para " There are some situations..." I think you could slightly adjust the
> mention of PEPs. For example, a PEP that re-spaces TCP ACKs doesn't have
> anything to do with the problem mentioned in the first part of the para
> (about the difficulty of ensuring both clients and servers are upgraded)

[Med] What about updating it as follows: 

OLD:
   Performance Enhancing Proxies [RFC3135], and other
   service functions have been deployed as solutions to improve TCP
   performance over links with specific characteristics.

NEW:
   Some assistance from the network to make use of these features 
   is valuable. For example, Performance Enhancing Proxies [RFC3135], 
   and other service functions have been deployed as solutions 
   to improve TCP performance over links with specific characteristics.

> 
> Para " More recently, experimentation" I found the first sentence hard to
> parse. 

[Med] Will reword. 

The para seems to use latency to refer to two things (without
> pointing this out): packet forwarding latency and set-up signalling
> latency
> 
> List of bullets about what a transport converter achieves. I 'd add one to
> say it achieves 0-RTT - and to explain what 0-RTT means (quite a bit later
> before the reader discovers this) (and expand the RTT abbreviation)
> 

[Med] Good point. This will be introduced before that bullet list since that list is built in reference to RFC1919.


> Section 3:
> Figure 2: explain "R"

[Med] Fixed. This refers to "router".

> 
> " Further, the architecture allows for making use of new TCP extensions
>    even if those are not supported by a given server."
> I find the sentence a bit awkward. Maybe: Further, the architecture
> enables the use of new TCP extensions between the client and Transport
> Converter when they are not supported by a server.

[Med] What about the following: 

  Furthermore, the architecture enables the use of 
  new TCP extensions by the clients even if those are 
  not supported by a given server. This is achieved 
  owing to the assistance of a Transport Converter.

> 
> Figure 3: I didn't understand why the note is needed "* TLS messages
> exhanged between the Client and the Server are not shown."

[Med] This is to make it clear that the TLS connection is handled directly between the client and the server. The converter does not interfere with that. 

> 
> Para under figure 3: This mentions Connect TLVs, which is a mystery
> concept at this stage.

[Med] changes to "Convert messages".

> 
> Section 1 mostly says TCP extensions, with an occasional TCP options. Be
> consistent?

[Med] OK

> 
> Section 3.2:
> Figure 5: in the figure title there's "(1)" - what does this mean or is it
> a typo?

[Med] Fixed. We used to have two figures with the same title (1) and (2).

> 
> Para " A Transport Converter MAY operate"
> "external address" - what does 'external' mean?

[Med] Changed to "source". 

"external" was used in reference to the source address of a packet as relayed by a converter. This is further discussed in 3.4.

> 
> Section 4
> Should there be a new Section 4.1 that explains the use of port # (to be
> assigned) to identify Convert protocol messages?

[Med] Fixed. 

> 
> S4.1 title
> "fixed header" - is "fixed" needed?

[Med] It is fixed because it is present in all convert messages. Do you prefer "common" ?

> 
> Figure 10:
> I think the first "(optional)" should be deleted (ie in bytes 3 & 4)

[Med] Deleted the second one. 

> 
> " In general
>    zero padding MUST be added if the value's length in bytes can not be
>    expressed as 2+(4 * n)."
> better, something like: If necessary, Value MUST be padded with zeroes so
> that the length of the TLV is a multiple of 32 bits.
> 

[Med] That is better. Thanks. 

> " As a general rule, when an error is encountered an Error TLV with the
>    appropriate error code MUST be returned by the Transport Converter."
> 'As a general rule' seems to conflict with 'MUST'
> 

[Med] Fixed. 

> " Transport Converter SHOULD include in this
>    list the TCP options that it accepts from Clients and that it
>    includes the SYN packets that it sends to initiate connections."
> I couldn't parse the second part of the sentence ("and that it...")
> 

[Med] Changed to:

" A Transport Converter SHOULD include in
this list the TCP options that it accepts from Clients; these options are 
included by the Transport Converter in the SYN packets that it sends to initiate connections."

Better? 

> " The list of Supported TCP Extension is padded with 0 to end on a 32
>    bits boundary."
> Replace: The list of Supported TCP Extensions MUST be padded with 0 to end
> on a 32
>    bits boundary.
> 

[Med] Deal!

> Section 6
> This section actually only discusses one type of middlebox (removes SYN).
> Can the discussion be widened slightly

[Med] This section only focuses on SYN/SYN-ACK because these are used by the Convert protocol.

Do you have in mind a particular case (specific to the Convert protocol) that may be problematic?

 - or maybe simply say that if
> middlebox interference detected, then MUST stop using the Converter (with
> SYN removal given as an example)?
> 
> Not yet read Section 7 onwards, comments to follow.
> 
> Section 10 Change log
> There's lots of really useful info about why the design is as it is (eg
> the use of TLV rather than Options). It would be a shame if this was lost
> from the rfc - please can info be extracted into a section "how
> implementation experiences have influenced design decisions" or similar?
> 
> Thanks
> phil
> 
> -----Original Message-----
> From: tcpm [mailto:[e-mail]] On Behalf Of Scharf, Michael
> Sent: 25 June 2019 17:39
> To: [e-mail] Extensions <[e-mail]>
> Cc: [e-mail]
> Subject: [tcpm] WGLC for draft-ietf-tcpm-converters-08
> 
> Hi all,
> 
> draft-ietf-tcpm-converters has been discussed and reviewed quite a bit.
> 
> Therefore, this e-mail starts a working group last call (WGLC) for draft-
> ietf-tcpm-converters-08 that will run until ***July 14, 2019***.
> 
> Please let us know if there are any remaining open issues regarding this
> document. Statements supporting publication are also welcome. In absence
> of feedback we will assume that the TCPM consensus is to move the document
> to the IESG. As discussed in the past, the intended status of the document
> is experimental.
> 
> Thanks a lot
> 
> Michael
> (TCPM co-chair)
> 
> _______________________________________________
> tcpm mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/tcpm
> 
> _______________________________________________
> tcpm mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/tcpm