Re: AD review of: draft-ietf-policy-qos-device-info-model-08.txt
Robert Moore <[email protected]> Sun, 18 May 2003 20:15:05 -0400
| Newsgroups | gmane.ietf.policy |
|---|---|
| Message-ID | <[email protected]> |
I've submitted QDDIM09 to the I-Ds repository, so it should be published in a day or two. I've summarized below what I did to address each of Bert's points. In a separate note, Bert asked whether QDDIM should still be standards track. I'm not sure how to answer that question. My inclination, though, would be to say that it should remain a standards track document, since (1) we've established the precedent that Information Models (specifically, PCIM and PCIMe) are standards track documents, and (2) of the two Data Models corresponding to QDDIM, the DiffServ MIB is a proposed standard, and the DiffServ PIB is informational because of SPPI / COPS reasons, not because of DiffServ reasons. Regards, Bob Bob Moore WebSphere Advanced Design and Technology WebSphere Platform System House IBM Software Group +1-919-254-4436 [email protected] "Wijnen, Bert (Bert)" To: [email protected] <[email protected] cc: om> Subject: [Policy] AD review of: draft-ietf-policy-qos-device-info-model-08.txt Sent by: policy-admin@ietf .org 04/09/2003 12:42 PM OK, here is my review (finally... blush) - Needs to be checked against ID-NITS, see: http://www.ietf.org/ID-nits.html Also pls check against RFC2223bis see draft-rfc-editor-rfc2223bis-04.txt Things that fall under this category: - no citations in abstract <fixed> - split references in normative and informative <fixed> - expand Acronyms when used for the first time <fixed> - use proper references in the references section <fixed> - is the reference to cim 2.5 correct? <yes, I believe that it is; certainly, on the DMTF side, CIM 2.5 is there to be referenced, permanently and immutably> - not sure if the IP address as on page 7 is allowed according to ID-NITs <I changed these> - I see some Msoft characters in the doc (page 12 and 64 are examples) <I found and fixed the on on p. 12, but I couldn't find one on p.64; however, I'm finding these solely by eyeball, so I may have missed some> - many referenced documents have become RFCs now. You may want to update those. <done> - I suspect that security area is too weak. Specifically if you tell people to use IPSEC, you have to explain how that is done. But... maybe you can refer to PCIM and PCIMe, similar to how you did it for QPIM. Maybe there are some extra concerns since you also derive a lot directly from CIM ?? <I did not change anything here, since I wasn't sure what to say; if the Security Area has some specific text, we can certainly include it> - In the abstract, I think that the last piece needs to be removed. That is: A separate draft could be written to provide a mapping of the data contained in this document to a form suitable for implementation in a directory that uses (L)DAP as its access protocol. Similarly, a draft could be written to provide a mapping of the data in [QPIM] to a directory. Together, these four drafts (information models and directory schema mappings) would then describe how to write QoS policy rules that can be used to store information in directories to configure device QoS mechanisms. Cause that tells what "could be done" but that cannot be the "abstract" for this document, can it? <fixed> - The document speaks about "this draft" a lot. I think it should be changed throughout the document into "this document". So that once it is published we do not see "this draft" anymore. <fixed> - Page 11. It might help if in the figure you indicate where we find CIM, PCIM, PCIMe, QPIM, QDDIM. etc <this is actually a fairly deep question that we never got to the bottom of; since I don't think that it's crucial to have these labels here, I made no changes.> - I see (as an example on page 16/17) QoSService as a class. Then the doc talks about the plural QoSServices, and I am not sure if it always means QoSService Instances or subclasses of QoSService. At other places it talks about "QoS services" which seems better to me <fixed> - Page 17 I see: Note that this work is not yet completely aligned, as there are differences among the DiffServ Informal Management Model, the DiffServ MIB, the DiffServ PIB, and this draft. Work to finish aligning these drafts is in progress, and will be reflected in the next revision of this draft. Does this mean you were expecting a new rev of QDDIM anyway? Anyway, such text probably does not belong in a doc that goes for RFC publication. <I removed this text> - AS an example: I see on page 21 Implementations may recognize other <type>'s in addition to these. If collisions of implementation-specific <type>'s become a problem, it is possible that <type>'s may become an IANA- administered range in a future revision of this standard. This document itself should not claim that it is a standard. Or at least not in the text. If it gets approved as stds track then RFC-Editor will add that in the footing. I suggest to use "this document" instead of "this standard". Pls check if there are more occurences. <fixed -- the document no longer refers to itself as a standard> - The figures on pages 26 to 32 are not consistent in their use of class names and such. I can understand that you need to abbreviate becuase of space constraints, but it might be good to do so in a consistent manner, and to list the abbreviations and explain which exact Class they represent. <I didn't change anything here -- I think the reader can tell quite easily which class an abbreviation is referring to.> - On page 37 you claim (at top) that "is shown in figure 9" but I think it is actually figure 10. On page 38 it should be figure 11 instead of 10 (or so I think) <fixed> - I am a bit surprised to see how the descriptions of Properties is done. In the PCIM (RFC3060) it was done pretty formal, for example: NAME CN DESCRIPTION A user-friendly name of a policy-related object. SYNTAX string Another one: NAME Mandatory DESCRIPTION A flag indicating that the evaluation of the PolicyConditions and execution of PolicyActions (if the condition list evaluates to TRUE) is required. SYNTAX boolean DEFAULT VALUE TRUE Or yet anbother one: NAME SequencedActions DESCRIPTION An enumeration indicating how to interpret the action ordering indicated via the PolicyActionInPolicyRule aggregation. SYNTAX uint16 VALUES mandatory(1), recommended(2), dontCare(3) DEFAULT VALUE dontCare(3) In RFC3460 I see it done in a similar way: NAME PolicyDecisionStrategy DESCRIPTION The evaluation method used for policies contained in the PolicySet. FirstMatching enforces the actions of the first rule that evaluates to TRUE; All Matching enforces the actions of all rules that evaluate to TRUE. SYNTAX uint16 VALUES 1 [FirstMatching], 2 [AllMatching] DEFAULT VALUE 1 [FirstMatching] So why is that not followed in this document? <in PCIM and PCIMe, many of the descriptions came from pre-existing CIM MOF files. In many cases these files weren't there when QDDIM was being written. I believe that the necessary modeling information is all here in QDDIM; as always, though, implementation experience may prove me wrong.> - Does Class TockenBucketMeterService not need a deltaInterval property? Otherwise what does the AverageRate property mean? <this is the first comment that really touches on the QDDIM model itself. I've made no changes in response to these comments, because what's in the document now reflects a consensus reached after *long* debates among the authors and other in the WG. If anyone wants to reopen the debates, feel free. But I'm going to pass.> - Do we still want/need a class TosMarkerService and a property of ToSValue? Has ToS not been obsoleted? <another comment on the model itself> - Class REDDropperService derives from DropperService. Does the ingeritedDropperType always have the value "Random" ?? <yes> - sect 4.3.21.2 does not even tell the datatype of the property <fixed> - Sect 4.3.26 I am being told that the proper name for a flow label is FlowLabel and not FlowID. Not sure how serious this is. <I've left the property name as it was, but inserted descriptive text that talks about "FlowLabel".> - Section 4.3.34 I do not see any of the properties described, do I? <I've removed the definition from this section, and referred the reader to CIM for the definition. This is how it should have been all along, just as it is for the other CIM classes that QDDIM references.> - sect 4.3.37.2 Would it not be better to use a 32bit unsigned? <another comment on the model itself> - sect 4.3.37.5 May I assume that the buffers are being shared? That is not so clear from the description (at least not to me). <I'm not sure I understand your point here, since the name of the property is "SharedBuffers".> - Sect 4.3.40 Explain what WRR stands for <fixed> - sect 4.3.40.1 Here I wonder if a 16 bit unsigned would not be more than sufficient Its units are "thousands" ?? Thousands of what? <it's thousandths, not thousands. Since it's just a multiplier, it has ^^ no units> - sect 4.3.4.2 Does this property not add just extra complexity? Or is that just me thinking so? And... what happens if both WeightingFactor and Priority are equal? <another comment on the model> <you missed it, but there was some editorial cleanup in section 4.4.17 that I had missed in -08. It's fixed now.> - Sect 4.4.18 and 4.4.19 Am I missing Property descriptions? <fixed> - sec 4.4.28 Am I missing Property descriptions? <fixed> Thanks, Bert _______________________________________________ Policy mailing list [email protected] https://www1.ietf.org/mailman/listinfo/policy