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 <noreply@xxxxxxxx>
Sent: Sunday, March 15, 2020 11:58 PM
To: tsv-art@xxxxxxxx
Cc: detnet@xxxxxxxx; draft-ietf-detnet-ip.all@xxxxxxxx; last-call@xxxxxxxx
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 tsv-art@xxxxxxxx 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/
--
last-call mailing list
last-call@xxxxxxxx
https://www.ietf.org/mailman/listinfo/last-call
