Subject: Re: [Tsv-art] Tsvart last call review of draft-ietf-detnet-ip-05

From: Bob Briscoe <[email-masked]>
Date: Sun, 25 Oct 2020 22:55:04 +0000

Bala'zs

Only 6 months later :|
But I think it will still be useful if I respond to your responses. 
Inline tagged [BB]

On 17/04/2020 15:32, Balázs Varga A wrote:
> Hi Bob,
>
> Thanks for the review. I am working on the update of the
> document to address the received comments. Proposed
> resolutions are inline below.
>
> Let me know if any of them needs further discussion.
>
> Many thanks
> Bala'zs
>
> -----Original Message-----
> From: Bob Briscoe via Datatracker <[email-masked]>
> Sent: Sunday, March 15, 2020 11:58 PM
> To: [email-masked]
> Cc: [email-masked]; [email-masked]; [email-masked]
> Subject: Tsvart last call review of draft-ietf-detnet-ip-05
>
> 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.
>
> <BV> OK, we will review and omit meaningless parts.

[BB] Having checked the three examples I picked out, for two of them,  I 
notice 'MUST' has been demoted to 'must'. That's sort-of OK, but it's 
always preferable to avoid using lower case words that can be confused 
with normative words. However, that's just my personal preferred style, 
not essential.

However, I think my point has been misunderstood regarding the middle one:
                 "implementations of this document SHALL use
                   management and control information to select the one 
or more outgoing
                   interfaces"
I notice the following has been added:
                 Specific
                 management and control information will be defined in 
future
                 documents, e.g., [I-D.ietf-detnet-yang].

The problem with the original was not that it didn't say exactly what 
information implementations SHALL use. It was that it doesn't affect 
interoperability. So it's surely only necessary to write it as informative:
                 "implementations of this document use
                   management and control information to select the one 
or more outgoing
                   interfaces"

>
> ==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.
>
> <BV> OK, it means "header fields", so "header" will be inserted in the text.

[BB] it was obvious that it meant header fields. I was asking for it to 
say which protocol you are talking about here (altho I haven't read 
other detnet drafts, this doc is where one would surely go to find the 
answer to this question).

>
>     "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.
>
> <BV> OK, sentence was too busy. Intention was to refer that TSN and DetNet
> are using the same mechanisms.
> OLD
>     "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."
> NEW
>    "Other mechanisms than the ones used in the TSN case are outside the
>    scope of this document."
> END
>
> 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?
>
> <BV> Yes, it means that.

[BB] I notice no mention of this has been written into the doc. It's a 
very important point for the end-system, which is surely the purpose of 
this section...

If all traffic is shaped independently on ingress, the network can 
guarantee a certain maximum queuing delay for all users. But the 
converse is not true. That is, if ingress traffic is not shaped at all, 
max delay guarantees for all users will not necessarily be broken. 
Guarantees are most likely to survive without shaping when there are 
high levels of aggregation, but any level of aggregation might be 
sufficient. In such cases, pushing delay back into a shaper will create 
a delay problem when it probably would not have been a problem if it had 
not been shaped.

Instead of shaping, out of contract traffic could be re-marked. Then, if 
there is a delay problem once all the flows are aggregated, the out of 
contract traffic could be preferentially discarded.

>
>     "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).
>
> <BV> This intends to say that existing mechanism using a 5-tuple to match traffic
> cannot differentiate DetNet and non-DetNet traffic.

[BB] I notice this text hasn't changed. Perhaps:
"In order to maximize reuse of existing mechanisms, the 5-tuple is used 
to match traffic, even though it cannot differentiate DetNet and 
non-DetNet traffic. Therefore, end systems SHOULD NOT mix DetNet and 
non-DetNet traffic within a single 5-tuple."

>
> 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.
>
> <BV> Yes, it means the flow label. I will change text.
>
>     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.
>
> <BV> No, "optional field" is not an alternative term for the "Diffserv field"
>
>
> 4.4.  DetNet Flow Aggregation
>
>     "an additional optional field defined in Section 5.1."
>     There is no optional field defined in Section 5.1.
>
> <BV> It means the flow label, defined in 5.1.1.5. I will change text.
>
>     "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."?
>
> <BV> Yes, your assumption is correct. I will change text.
>
> 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).
>
> <BV> Yes. The decision is at the end user. He/She may decide to use
> IPSec over UDP as well.

[BB] I didn't mean the other way up. I meant the 'normal' case of a 
transport protocol over AH. When over AH alone, the transport is still 
in the clear. So it would be crazy for DetNet not to use the port 
numbers of the higher layer transport just because it was over AH.

>
>     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?
>
> <BV> Intention was to say, that supporting protocols listed in 5.1.2 is mandatory (UDP, TCP, IPSec).
> Other protocol values may be implemented, but not discussed by this document. This is what was
> meant by the "limited set".

[BB] I think you've missed my point. If some nodes implement other 
protocols, this draft needs to define the mechanism for one node to 
check that other nodes implement the same other protocol, which is 
necessary to know before any node can use that other protocol. If the 
mechanism is by administrative configuration, the draft just needs to 
say that.

>
>     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.
>
> <BV> Many thanks for the security related comments. We have a draft dedicated
> to detnet security [I-D.ietf-detnet-security] covering most of these aspects. We
> intend to discuss what is really needed in this document to avoid unnecessary
> overlaps.

[BB] Please tell me when detnet-security should have taken on these 
points, as I should probably then review it.

Regards

Bob

>
> ==Nits==
>
> <BV> Many thanks. All nits will be addressed in the next version.
>
> 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.
>
>
>

-- 
________________________________________________________________
Bob Briscoe                               http://bobbriscoe.net/