RE: AD review of draft-ietf-rap-feedback-fr-pib-04.txt

["Wijnen, Bert (Bert)" <bwijnen@lucent.com>]

Thanks for the revision.

My comments inline below.

Current real concerns:
- Section 3.2 start off with new text stating that SNMP Counter32 and
  Counter64 are not supported by SPPI. What is this based on?
  I do not understand that statement at all.
- And as I explain below in my answers/comments, I am not sure that
  your new definitions of Usage32/Usage64 make sense. Pls explain
  in relation to the above and to my comments inline below.

Minor things
- pls answer my questions inline below
- In the Abstract, it is probably better to change

     This document describes a Policy Information Base (PIB) to control 
     policy usage collection and reporting in a device. 
      
     The provisioning classes specified here allow a Policy Decision 
     Point (PDP) to select which policy objects should collect usage 
     information, what information should be collected and when it 
     should be reported. 
      
     This PIB requires the presence of other PIBs (defined elsewhere) 
     that provide the policy objects that usage information is collected 
     from. 
    
  into something aka:

     This document describes a portion of the Policy Information Base 
     (PIB) to control policy usage collection and reporting in a device. 
      
     The provisioning classes specified here allow a Policy Decision 
     Point (PDP) to select which policy objects should collect usage 
     information, what information should be collected and when it 
     should be reported. 
      
     This PIB module requires the presence of other PIB modules (defined
     elsewhere) that provide the policy objects that usage information 
     is collected from. 
    
  The idea being (similar as with the MIB), that there is only ONE PIB
  and several/multiple PIB Modules that together make up the PIB.

Could I have a response to this review today? Do you think you can fix
a new rev today, that would allow me to put it on IESG agenda for 27 Dec

My original comments to rev 03:

> > > the following draft to the IESG for consideration as a 
> > > standards track RFC:
> > > http://www.ietf.org/internet-drafts/draft-ietf-rap-feedback-fr-pib-03.txt
> > 
> > My main question is if we want to go through a similar process as we did
> > for the earlier PIBs, or if the WG rather decides that this PIB can be
> > also informational, just as the Framework PIB and the Diffserv PIB.
> >
> The discussion with WG chair and my co-AD leaves me at the position
> that I will not consider the PIB for STDs track, but instead will 
> handle it for Informational, with the same concerns as reaised with
> earlier PIBs. That situation has not changed.
> 
That has not changed, so we'll go for Informational.

> Further comments to this document:
> 
> - mmm.. the module-identity is named:  feedbackPolFrameworkPib
>   then all the rest of the PIB objects/classes/attrs are prefixed
>   with frwkFeedback. WOuld it no be best to keep the prefix of the
>   MIB module and the others the same.
I see that is done. Thanks

> - I am missing a REVISION clause in the MODULE-IDENTITY macro

You have added that now. Thanks.
But the LAST-UPDATED and the REVISION date/time need to be in sync.

> - I do not think that labels for enumerations canstart with Uppercase
>   and have underscores

Thanks for fixing

> - In some cases I see in a description clause of a table or entry
>   "class", or sometimes "table" or sometimes "pri" or "prc"...
>   I would strongly recommend to stick to one term and use that.
>   We had similar problems back when we reviewed diffserv and framework
>   PIBs. Pls check with those and use same terminology.

Thanks for tidying up.

> - frwkFeedbackActionListEntry in desxription clause:
>    "This class identifies a set of linkage instances..."
>   mmm.. I guess the table defines a "set", but am entry is one
>   instance of the class, no? probably just language.

thanks for better wording

> - Not sure if it is me, but I do not understand the DESCRIPTION
>   clauses of frwkFeedbackActionListGroup and frwkFeedbackActionListRefID
>   How does that work? Can you be more elaborate so I can understand
Mmm... I guess the WG members understand it... 
It's a tidbit better though.

> - can you explain (in the description clause) how the seOID is used
>   for the selection process for object/attribute 
>   frwkFeedbackSelUsageComboCapsSelection

it is a bit better

> - Is the example code in description clause of 
>    frwkFeedbackTrafficThresholdTable actually C code or is it
>    pseudo code? And where do these fields that you are checking
>    come from ?? Is NULL a NULL ptr or the zero value?
>    Would be best to be clear on those questions.

Thanks for better understandable text

> - I see: 
>    frwkFeedbackTrafficUsagePacketCount OBJECT-TYPE    
>         SYNTAX       Counter64 
>         STATUS       current    
> 
>         DESCRIPTION    
>                   "The count of packets handled by the associated 
>                    element during the reporting interval."    
>    but a Counter64 has Counter64 semantics and has nothing to do with
>    an interval. See RFC2578 definition of Counter64.
> -  A bit later in the PIB you do so again, e.g. another occurance

Mmm... I see that you now have defined two TCs Usage32 and Usage64
that look very much like a Counter32 or a Counter64, if not that,
then they look very much like a ZeroBasedCounter32 (as defined in RMON
MIB, RFC2021) and ZerobasedCounter64 (as defined in HNUM-TC, RFC2856)

Is this wise?

Anyway, your own TC says:
        Usage64 initial value is 0 and the object-type using 
        Usage64 needs to specify when it is initialized. 
But when the TC is used, your object DESCRIPTION clauses do not 
specify when it is initialized!!
May be it is implicit.. but would be good to specify

> - I see no MODULE COMPLIANCE statement in the PIB !!??

Thanks for adding those

> - the security considerations section does not prescribe which
>   of the possible security mechanisms MUST be supported. But
>   possibly is met by the specs RFC2748 and 3084. If you agree, then
>   you should instead say that that is the case and point to those
>   RFCs as normative references.
>   But the security section needs to list details about which classes
>   and or attributes are sensitive and why. Security ADs will look
>   for that.
> 
Thanks for adding the new text. That I think helps a lot for
the Sec ADs to approve too.

> - and last but not least, sending this to SMILINT for a compile
>   check I get a very long list of errors and warnings. Probably
>   some are no problem.. but I see that quite a few are srious
>   problems. Please fix.

SMILINT now compiles this PIB module clean. Great!

>   How can the WG have aggreed that this PIB doc is ready for 
>   consideration as PS ??
>  
Oh well.

> admin/bureaucratic/editorial/nits:
> - there are so called Redmond characters (e.g. non-ascii) in th
>   text. Pls fix to plain ascii
Seems fixed now, thanks

> - RFC Editor does not want acronyms in title and/or abstract
>   See: http://www.rfc-editor.org/policy.html
fixed thank you

> - I think it is more commom to have the "conventions used in this document"
>   (that is the use of MUST language and such) after the TOC
> - in the example on page 8-0, you use xxxEntry and xxx PRC terminology
>   Might be better to choose one and stick to that, no?

thanks for better consistency

> - sect 3.2 at the end (i.e. top of page 10)
>   missing 2) ??
aha, now no longer ordered lists ;-)
Anyway, a 1) does make people to expect a 2) and possibly more

> - would be good to add WG mailing list infor in CONTACT info
>   I also see only 4 of the 5 authors in the contact info.
>   that is fine with me... but ??

thansk for addition

> - frwkFeedbackActionEntry has in description:
>    "An instance of this class can indicates a action...'
>   proper english?
> - references must be split in normative and informative, again 
>   see: http://www.rfc-editor.org/policy.html

Thanks for doing so. I now am missing reference [RFC2119] ??!!

> - I see no IANA considerations to request assignment of the PIB oid

Thanks for adding that

> - I am missing an IPR section as per RFC2026 sect 10.
> 
I see you did not add it. If we go for Informational, that is OK (I think)
It would not be OK for STDS track

Bert