Elwyn,
Please see my responses inline.
Regards,
Steve
On 7/23/15 11:32 AM, Elwyn Davies wrote:
I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at
<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
Please resolve these comments along with any other Last Call comments
you may receive.
Document: draft-ietf-dime-ovli-08.txt
Reviewer: Elwyn Davies
Review Date: 2015/07/23
IETF LC End Date: 2015/07/27
IESG Telechat date: (if known) -
Summary: Ready with nits. A very well written document - thanks. Some
minor issues with out-of-date or OBE'd statements that need a bit of
tweaking.
SRD> Thanks, we had a good group of people working on it.
Major issues:
Minor issues:
s4.2:
Is there a need for and consequently how would one either cancel the
current abatement algorithm or select a different one? I guess this
might happen if the reacting node that was running the algorithm went
away or itself became overloaded. However I I have no idea whether
this is a reasonable question! OK.. well I see in s5.1.1 that the
abatement algorithm can be changed... a pointer in s4.2 would be
useful. But could it be turned off?
SRD> OK.
Appendix A.3: The statements in this appendix are not 'future proof'.
Referring to ongoing actions of the DIME WG will have little value in
the future. Maybe something like "There is an expectation that <these
developments> will occur and can be integrated with the features
described in this document."
SRD> Good suggestion.
Appendix C.1, para 1: There are some tense and past/future issues in
this section - It is now mid 2015 and this para refers in the future
to 'end of 2014'. Please rewrite to reflect current actualité and in
a way that will be relevant when this is published as an RFC.
SRD> I propose removing Appendix C in its entirety.
Appendix C.5:
C.5. No New Vulnerabilities
The working group believes that DOIC is compliant with the
requirement to avoid introducing new vulnerabilities. However, this
requirement may warrant an early security expert review.
Hmm! I fear that it would difficult to consider this draft 'ready' if
there is no reasonable consensus that it hasn't introduced any new
vulnerabilities. Has the security expert review actually happened?
SRD> No, a security expert review has not happened, other then Stephen's
review.
REQ 13: The solution MUST NOT introduce substantial additional work
for a node in an overloaded state. For example, a
requirement for an overloaded node to send overload
information every time it received a new request would
introduce substantial work.
*Not Compliant*. DOIC does in fact encourage an overloaded
node to send an OLR in every response. The working group
that other mechanisms to ensure that every relevant node
receives an OLR would create even more work. ****[Note: This
needs discussion.]****
Has this discussion occurred? Does the WG have consensus that this
derogation from the requirements is acceptable?
SRD> Yes, there is consensus on the mechanism as defined.
Nits/editorial comments:
General: s/i.e./i.e.,/ g; s/e.g./e.g.,/g
General: Consistent use of "non-supporting" vs "non supporting" (lots
of places)
SRD> Ok.
s2, Host-Routed Requests:
Expand acronym AVP (first use).
SRD> Ok.
s5.2.1.1:
A host-type OCS entry is identified by the pair of application-id
and
the node's DiameterIdentity.
A realm-type OCS entry is identified by the pair of application-id
and realm.
The application-id, DiameterIdentity and realm presumably come from
the various messages taht are being piggybacked on. A reference to
the relevant section of the RFC that gives definitions of these (and
which messages to get them from) would be helpful. Later... this
issue is partially resolved by s7.3. A pointer to that section would
be desirable. s7.3 needs a reference to where the relevant messages
are defined.
SRD> I agree that a pointer to section 7.3 would be helpful.
s5.2.1.4, last para:
A reporting node MUST keep an OCS entry with a validity duration of
zero ("0") for a period of time long enough to ensure that any non-
expired reacting node's OCS entry created as a result of the
overload
condition in the reporting node is deleted.
Is it possible to offer any guidance on what constitutes 'long enough'?
SRD> Long enough is based on when all of the expiration timers in OLRs
sent by the reporting node have expired. This is, in essence, what it
already says.
s5.2.2 (in Note):
Any extension
that defines a new abatement treatment must also defined the
interaction of the new abatement treatment with existing
treatments.
s/defined/define/
SRD> Agreed.
s5.3, paras 2, 3, 5, 7 (but probably not the MUST NOT in para 6):
In line with the text in the previous comment, I think s/MUST/must/,
s/MAY/may/ - they are not things the protocol does
SRD> I'm agreeable to this change if the rest of the DIME group is ok.
s7:
Pointers are needed to the RFC sections that define the data types and
the AVP syntax definition structure used in this section.
SRD> This is in the table in section 7.8.
s7.3 - see comments on s5.2.1.1 above
SRD> See above comment.
s8, para 6: s/Diameter identity/DiameterIdentity/ (possibly)
SRD> I'm don't see the need for the change. It's already talking about
the Destination-Host AVP, which carries the DiameterID. This is just
prose for saying that.
s9.2: It doesn't seem obvious to me that the values are supposed to
have names in the registry. Surely s/b
(e.g.)
Feature Vector Entry Name
Feature Vector Entry Value
Specification...
SRD> I'm open to guidance on this. I don't have a lot of experience in
defining IANA requirements.
s10.3, last para:
Requirement 28 [RFC7068] indicates that the overload control
solution
cannot assume that all Diameter nodes in a network are trusted, and
that malicious nodes not be allowed to take advantage of the
overload
control mechanism to get more than their fair share of service.
I can't parse the last phrase of this (from "and that malicious..."
onwards. I know what you mean..
Suggest s/trusted, and that/trusted. It also requires that/
SRD> Ok.
s10.4, last para:
At the time of this writing, the DIME working group is studying
requirements for adding end-to-end security features
[I-D.ietf-dime-e2e-sec-req] to Diameter.
This statement has pretty much been OBE'd. the draft is in WG last
call. I take it we can assume that the draft knows what is being
specified: thus
These features, when they
become available, might make it easier to establish trust in non-
adjacent nodes for overload control purposes.
It should be possible to make a more concrete statement here.
SRD> The work on e2e security is still in a requirements stage. As such,
we don't yet know what is being specified.
App C.1, para 1: s/normative references/a normative reference/
SRD> I am proposing removing all of appendix C.
App D.1, para 1: s/identity/entity/ (I think)
SRD> Agreed.
_______________________________________________
Gen-art mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/gen-art
