Last Call Review of draft-ietf-detnet-ip-05
review-ietf-detnet-ip-05-tsvart-lc-briscoe-2020-03-15-00

Request Review of draft-ietf-detnet-ip
Requested rev. no specific revision (document currently at 07)
Type Last Call Review
Team Transport Area Review Team (tsvart)
Deadline 2020-03-13
Requested 2020-02-28
Authors Balazs Varga, János Farkas, Lou Berger, Don Fedyk, Stewart Bryant
Draft last updated 2020-03-15
Completed reviews Rtgdir Last Call review of -04 by Stig Venaas (diff)
Tsvart Last Call review of -05 by Bob Briscoe (diff)
Genart Last Call review of -05 by Roni Even (diff)
Secdir Last Call review of -05 by Tero Kivinen (diff)
Assignment Reviewer Bob Briscoe 
State Completed
Review review-ietf-detnet-ip-05-tsvart-lc-briscoe-2020-03-15
Posted at https://mailarchive.ietf.org/arch/msg/tsv-art/foX3W-Zy6eEQBfEtgIWxJnTYqfo
Reviewed rev. 05 (document currently at 07)
Review result Ready with Issues
Review completed: 2020-03-15

Review
review-ietf-detnet-ip-05-tsvart-lc-briscoe-2020-03-15

This document has been reviewed as part of the transport area review team's
ongoing effort to review key IETF documents. These comments were written
primarily for the transport area directors, but are copied to the document's
authors and WG to allow them to address any issues raised and also to the IETF
discussion list for information.

When done at the time of IETF Last Call, the authors should consider this
review as part of the last-call comments they receive. Please always CC
tsv-art@ietf.org if you reply to or forward this review.

This is a review of draft-ietf-detnet-ip-05 published on 3-Feb-2020.
The IETF review system said a new revision had been submitted on 13-Mar-2020, but I couldn't access it, if indeed it existed.

In general the document was easy to understand. 
It seemed to say a lot, but commit to very little. More like an architecture document than a standards track data plane document.

This review is on the document's own terms. IOW, it reviews what the document says, not whether it is a good approach to the problem and so forth.

Review comments.

==Throughout==
   
   The use of normative text is uncharacteristic of IETF RFCs, where it is normally only used for interoperability requirements. For instance:
   "Quality of Service (QoS) for DetNet service flows carried in IP MUST
   be provided locally by the DetNet-aware hosts and routers supporting
   DetNet flows."
   This MUST is pretty meaningless, given it merely mandates that QoS has to be provided locally (how else could it be provided?). 
   Another example of a meaningless SHALL in 5.2 says:
   "implementations of this document SHALL use management and control
   information to select the one or more outgoing interfaces"
   And another example from 5.3:
   "Implementations of this document MUST ensure that a DetNet flow
   receives the traffic treatment that is provisioned for it..."
   
   There are many other examples of meaningless normative statements. I suggest that the authors go through reviewing whether each one actually deserves normative text.

==Section-by-Section==

1.  Introduction

   "The service sub-layer
   generally requires additional fields to provide its service; "
   It's not clear what "fields" are being talked about here.
   
   "Other than in the TSN case, the
   specific mechanisms used by a DetNet node to ensure DetNet service
   delivery requirements are met for supported DetNet flows is outside
   the scope of this document."
   I don't think the TSN case in in scope of this document either. I think this is meant to say that the TSN casse is the only one that's currently defined, not that it's the only one within the scope of this document.

4.1.  End-system-specific Considerations

   "this means that packets are appropriately shaped on
   transmission"
   From the application's perspective, doesn't this mean that the delay just moves from the network to the shaper?
   
   "In order to maximize reuse of existing mechanisms, DetNet-aware
   applications and end systems SHOULD NOT mix DetNet and non-DetNet
   traffic within a single 5-tuple."
   "maximize reuse of existing mechanisms" seems a strange rationale for not mixing (which is ironically the opposite of reuse). 
   
   
4.3.2 Quality of Service

   "From an encapsulation perspective, the combination
   of the 6-tuple i.e., the typical 5-tuple enhanced with the DSCP and
   previously mentioned optional field"

   The "optional field" has not been previously defined. The use of the IPv6 flow label was earlier described as optional. Is that the field meant here? It wasn't the name given to the field. If it is, pls don't. Pls unambiguously call it the flow label, not the optional field, throughout. When the flow label is used, the IPv6 next header field is not necessarily used. So the latter could also be described as optional.
   
   A few sentences further on it says 
   "the DSCP, or optional, field"
   
   Which implies the "optional field" is an alternative term for the "Diffserv field". But then it would not have said "DSCP and optional field" earlier.
   
4.4.  DetNet Flow Aggregation

   "an additional optional field defined in Section 5.1."
   There is no optional field defined in Section 5.1.
   
   "From a resource
   allocation perspective, DetNet nodes must provide service to an
   aggregate and not on a component flow basis."
   It seems wrong to specifically preclude a single component flow. Does this preclude an aggregate consisting of a single flow? Or was the intention to say "DetNet nodes /ought to/ provide service to an aggregate rather than on a component flow basis."?
   
5.1.2.2.  IPsec AH and ESP

   Flows with the ESP header certainly have to be identified by the SPI, because the payload is encrypted. Whereas, although flows with the AH header could be identified by the SPI, this would unnecessarily lump together all the flows from one security principle on one IP to one security principle on another IP. This would be far less granular than the source and dest ports, which can be visible and accessible using the next header field of the AH header (because the AH payload is not encrypted).
   
   6. Management and Control Information Summary
   
   "IPv4 protocol field.  A limited set of values is allowed,..."
   "IPv6 next header field.  A limited set of values is allowed, ..."
   Section 5.1.1.3, where use of these fields is defined, does not mention that a limited set of values is allowed. And if the limited set is not defined, how do different nodes in a network interoperate? This is surely one role of the present document?   
      
   7. Security Considerations
   
   The following would seem to need to be addressed for an IP data plane:

   * Privacy - There seems to be a fundamental dilemma between privacy of the IP payload and flow identification. Use of port numbers is fine if privacy is not a concern. But if privacy is of concern, then you need encryption. However, the SPI in the ESP header is not meant to identify anything less than the full set of flows between two security principles on two IP nodes. But that is not granular enough for the QoS of each layer-4 flow. If that is solved by defining multiple SPIs, then that compromises privacy. This section at least needs to say that is a security Limitiation.

   * Authorization to use Detnet services. Would it be possible to restrict the use of DetNet services to particular nodes or users? Specifically how are the identifiers used for flow identification bound to those held in any authorization system. A particular concern with an IP data plane will be source address spoofing

   * Denial of Service mitigation. How is the data plane protected from:
     o deliberate fast churn of reservations?
     o excessive incoming traffic to all the policers?
     (For instance, Intserv had a blockade state facility at the edge that held reservations from reaching aggregated parts of the network.)

     I believe the above are IP-data-plane-specific issues. So section 7 should at least mention them, if only to admit that they haven't been considered.
   
   "From a data plane perspective this document does not add or modify
   any header information."
   Not true, the DSCP can be altered.
      
   
   "To prevent DetNet packets
   from being delayed by an entity external to a DetNet domain, DetNet
   technology definition can allow for the mitigation of Man-In-The-
   Middle attacks, for example through use of authentication and
   authorization of devices within the DetNet domain."
   Eh? What does mitigation of MITM attacks mean? Either they're prevented or they're not. Mitigated implies just slightly prevented. 
   How does mitigation of MITM attacks prevent delay? Seems a rather big jump.
   
   
==Nits==

1. Intro
s/DetNet provides these flows extremely low/
 /DetNet provides these flows with extremely low/

Repetition: functions as two sub-layers: functions into two sub-layers:

   s/group(referred/group (referred/

3. DetNet IP Data Plane Overview

s/however modification of these fields is allowed, for example to a DSCP value/
 /however modification of these fields is allowed, for example changing the DSCP value/
 
I don't undertand the sentence below, which seems to be a fusion of two or more sentences. 
May be s/those are/that is/. And maybe "relay nodes" is meant to start a new sentence.

 The DetNet enabled
   end systems originate IP encapsulated traffic those are identified
   within the DetNet domain as DetNet flows, relay nodes understand the
   forwarding requirements of the DetNet flow and ensure that node,
   interface and sub-network resources are allocated to ensure DetNet
   service requirements.

Pls expand PREOF and TSN on first use, not just in the Terminology section.

Missing word: "and ensures that the receives the proper traffic
   treatment"
   
   Fig 4: Top line of ASCII Art has been shifted to the right.
   
4.3.1 (and probably throughout)
   "differentiated services code point (DSCP) field"
   Should be "differentiated services field" or "Diffserv field". The DSCP is the value in the field [RFC3260].
   Also, in S.6. there is an incorrect mention of "IPv4 Type of Service", which means the combination of the Diffserv field and the ECN field. It is unlikely this is meant, so please correct to "Diffserv field"
   
5. DetNet IP Data Plane Procedures

   "these are referred in this section."
   I would say "referenced" or "referred to" But this might be valid US English? 
   
   s/since applies/since it applies/
   
6. Management and Control Information Summary

   "If the DSCP field is to be used in flow identification.
    Ignoring the DSCP filed is optional."
   Does the first sentence mean "Whether the Diffserv field is to be used"?
   The second sentence needs to be expressed in a way that's more relevant to how management info will express whether the field is ignored.
   
   s/DSCP filed [sic]/Diffserv field/
   
   "IPv6 flow label field.  This field can be optionally used for
      matching.  When used, can be used instead of matching against the
      Next Header field."
   The second sentence needs to be expressed in a way that's more relevant to how management info will say which is used or whether both are used.