Subject: [Detnet] Tsvart last call review of draft-ietf-detnet-ip-05

From: Bob Briscoe via Datatracker <[email-masked]>
Date: Sun, 15 Mar 2020 15:57:31 -0700

Reviewer: Bob Briscoe
Review result: Ready with Issues

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
[email-masked] 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.