RE: AD review of: draft-ietf-policy-qos-device-info-mode l-08.txt

Robert Moore <[email protected]> Tue, 20 May 2003 12:35:44 -0400
Newsgroups gmane.ietf.policy
Message-ID <[email protected]>



Hi Bert,

I didn't list these changes because I wasn't aware I had made them!
Checking back, it appears that I started the -09 edits on a slightly
out-of-date version of the -08 source file.  So I did an inadvertent
rollback on the very last changes that were made for -08.  I'm pretty sure
the set of such changes exactly matches these you've identified here.

I'll do a quick update to fix these items, and publish a -10.  Meanwhile,
if any of the authors who still do QoS for a living want to respond to your
other set of comments, I'll factor in any changes they propose.  I'm afraid
that network QoS is now so far in my past that it would be a bad idea for
me to even try to make any substantive changes to the document at this
point.

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:       Robert Moore/Raleigh/IBM@IBMUS                                                
                      <[email protected]        cc:       [email protected]                                                               
                      om>                      Subject:  RE: [Policy] AD review of: draft-ietf-policy-qos-device-info-mode             
                                                l-08.txt                                                                               
                      05/19/03 05:47 PM                                                                                                
                                                                                                                                       
                                                                                                                                       




Taking another look at the doc (rev 9) and checking for
changes since rev 08, I see that you did not list
that you removed section 4.3.20.2. That was not based
on my comments but for some other reason I guess.
So I am listing it just so thw WG knows and maybe Bob
can explain why it was removed.

In Sect 4.3.41 you changed  PrioritySchedulingElement
into PrioritySchedulerElement. Is that correct?
I wonder cause I do not see PrioritySchedulerElement
anywhere else in the document.

In sect 4.4.5 You changed cardinality 1..n into 1..1
Good for people to know that too. And why was it done?
Not based on my comments I assume.

In sect 4.4.6 you changed cardinality 1..1 into 0..1.
Explanation?

In sect 4.4.9 You changed cardinality 1..n into 1..1

people who want to see all the changes colored in the text
can take a look at: http://psg.com/~bwijnen/qddim.html


Thanks,
Bert

> -----Original Message-----
> From: Robert Moore [mailto:[email protected]]
> Sent: maandag 19 mei 2003 2:15
> To: Wijnen, Bert (Bert)
> Cc: [email protected]
> Subject: Re: [Policy] AD review of:
> draft-ietf-policy-qos-device-info-model-08.txt
>
>
>
>
>
>
> 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
>
>
>