Subject: Re: [imss] FW: MIB Doctor review draft-ietf-imss-fc-fam-mib-01.txt

From: Keith McCloghrie <[email-masked]>
Date: Fri, 7 Oct 2005 10:59:50 -0700 (PDT)

Bert/Orly,

> WG, 
> here is the MIB doctor review.
> 
> Thanks to Orly for her review!
> 
> Pls copy her if you want her to see comments/questions/answers
> because she is not subscribed to your mailing list.
> 
> W.r.t. to the comment on th eexpired draft: it has been (or
> will be soon) resurrected by the secretariat!

My responses to Orly's comments are below.  I have put an updated
document at:

  ftp://ftpeng.cisco.com/ftp/kzm/draft-ietf-imss-fc-fam-mib-02.txt 

Let me know if/when I should submit it as the new I-D.

Thanks,
Keith.
=====================

My responses:

> The draft has expired, and has been deleted from the Internet-Drafts
> directory. 
> ----------------------------------------------------------------------
> Here are 9  issues I found as potential items for fix:
> 
> 1.)Format---line(738) is too long
 
Oops, one character too many.

> 2)  Conformance with RFC 3978/3979 boilerplate---
>       -Seems as if an old copy of those is  being used in the  "Status
> of this Memo".
> 
> Should be as follows:
> 
> Status of this Memo
> 
>    By submitting this Internet-Draft, each author represents that any
>    applicable patent or other IPR claims of which he or she is aware
>    have been or will be disclosed, and any of which he or she becomes
>    aware will be disclosed, in accordance with Section 6 of BCP 79.
> 
>    Internet-Drafts are working documents of the Internet Engineering
>    Task Force (IETF), its areas, and its working groups.  Note that
>    other groups may also distribute working documents as Internet-
>    Drafts.
> 
>    Internet-Drafts are draft documents valid for a maximum of six months
>    and may be updated, replaced, or obsoleted by other documents at any
>    time.  It is inappropriate to use Internet-Drafts as reference
>    material or to cite them other than as "work in progress."
> 
>    The list of current Internet-Drafts can be accessed at
>    http://www.ietf.org/ietf/1id-abstracts.txt.
> 
>    The list of Internet-Draft Shadow Directories can be accessed at
>    http://www.ietf.org/shadow.html.
> 
>    This Internet-Draft will expire on January 13, 2006.
> 
> Copyright Notice
> 
>    Copyright (C) The Internet Society (2005).
 
I myself get very frustrated at the number of times that the boilerplate
changes, but because of such changes, I think your replacement text is
incorrect, in two ways:

  a) I think the IETF's requirements (aside: how can they be called
  "Guidelines" when non-compliant submissions are refused!!) prohibit
  the inclusion of: "This Internet-Draft will expire on January 13,
  2006" in this section.

  b) idnits now complains about having a Copyright Notice near the 
  beginning of the document !!

If I had been clairvoyant enough to have submitted the I-D last
February containing the version of the boilerplate which is required
today, it would have been unacceptable back then.  However, since I do
now have to submit a revised version, I will have to change the I-D to
have today's version of the boilerplate rather than last February's :-(.

> 3)      IPR related ---
>         -Disclaimer of validity (15),  copyright statements (14) and IPR
> (7) are marked with numbers and 
>           listed in mixed order. 
>         - section 7 is redundant to 15 and seems as older version
> Should be as follows:
> 
> Intellectual Property Statement
> 
>    The IETF takes no position regarding the validity or scope of any
>    Intellectual Property Rights or other rights that might be claimed to
>    pertain to the implementation or use of the technology described in
>    this document or the extent to which any license under such rights
>    might or might not be available; nor does it represent that it has
>    made any independent effort to identify any such rights.  Information
>    on the procedures with respect to rights in RFC documents can be
>    found in BCP 78 and BCP 79.
> 
>    Copies of IPR disclosures made to the IETF Secretariat and any
>    assurances of licenses to be made available, or the result of an
>    attempt made to obtain a general license or permission for the use of
>    such proprietary rights by implementers or users of this
>    specification can be obtained from the IETF on-line IPR repository at
>    http://www.ietf.org/ipr.
> 
>    The IETF invites any interested party to bring to its attention any
>    copyrights, patents or patent applications, or other proprietary
>    rights that may cover technology that may be required to implement
>    this standard.  Please address the information to the IETF at
>    [email-masked].
> 
> 
> Disclaimer of Validity
> 
>    This document and the information contained herein are provided on an
>    "AS IS" basis and THE CONTRIBUTOR, THE ORGANIZATION HE/SHE REPRESENTS
>    OR IS SPONSORED BY (IF ANY), THE INTERNET SOCIETY AND THE INTERNET
>    ENGINEERING TASK FORCE DISCLAIM ALL WARRANTIES, EXPRESS OR IMPLIED,
>    INCLUDING BUT NOT LIMITED TO ANY WARRANTY THAT THE USE OF THE
>    INFORMATION HEREIN WILL NOT INFRINGE ANY RIGHTS OR ANY IMPLIED
>    WARRANTIES OF MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE.
> 
> 
> Copyright Statement
> 
>    Copyright (C) The Internet Society (2005).  This document is subject
>    to the rights, licenses and restrictions contained in BCP 78, and
>    except as set forth therein, the authors retain all their rights.
> 
> 
> Acknowledgment
> 
>    Funding for the RFC Editor function is currently provided by the
>    Internet Society.
 
I don't know whether the above is correct without comparing it against
what the "guidelines" say.  To maximize the odds that my new text will
be acceptable, I will replace the above with a cut-and-paste from
the I-D that I submitted a couple of weeks ago which was acceptable
then (and hope that the "guidelines" haven't changed since).

> 4.) Abstract -- ok
> 
> 5.) MIB Boilerplate -- ok
 
Above you said "9 issues" -- if #4 and #5 are "-- ok", why are they
issues ?? 

> 6.) References -- 
> [FC-MGMT] draft-ietf-ips-fcmgmt-mib is RFC4044 by now "Fibre Channel
> Management MIB"  
> RFC2837,  was obsoleted by RFC4044, do we really need it? 
 
Yes, RFC2837 is referenced in the text because it's part of the history,
where it explains why RFC 2837 is obsolete.

> 7.) Security Considerations--ok
 
Again, why is this an issue ??

> 8.) Introduction--some redundancy exit with the abstract, maybe
> combination with section 3.  "Short Overview of Fibre Channel"
>      could have helped.
 
I agree it is redundant to have both an Abstract and an Intoduction,
but the IETF requires both, and even suggests they contain redundant
information (specifically, "a satisfactory abstract can be constructed
in part from material from the Introduction section").

> 9.) MIB objects--
> --there are few TCs in T11-FC-FABRIC-ADDR-MGR-MIB, why T11FabricIndex
> was placed in a separate MODULE and all others not with it ?

Because it was known in advance that there would be several follow-on
T11-XXX-MIB modules, which would need to IMPORT T11FabricIndex.
However, they would not need to IMPORT all of the TC's defined in
T11-FC-FABRIC-ADDR-MGR-MIB, and they would need to define their own
TC's.  So, the only 100%-consistent way to have spread the TC's over
the set of MIB modules would have been to delay the approval of
draft-ietf-imss-fc-fam-mib until all the other T11-XXX-MIB modules were
also ready to be approved, so that the T11-TC-MIB could contain all the
common TC's.  I did not (and still don't) believe such a delay was/is
warranted by such a small increase in consistency.

>   Also, based on its description it seems as the range should be
> (1..4095) , maybe there should be a word stating the consistency with
> [FC-SW-4]
>  regarding the value 0.
>   Maybe there are good reasons for the above that I simply not aware of.

Yes, I should and will add text to the DESCRIPTION, so that the
explanation which is presently in the Change Log doesn't get lost
when the Change Log is deleted.

I'll also take this opportunity to make the DESCRIPTION more 
definitive (i.e., change it to "the forthcoming standard, FC-SW-4,
defines how ...") which is now true, rather than using the speculative
text which needed to be written previously.

I'll also take this opportunity to delete the Change Log.

> --T11FamState description could be easier to follow if the order of the
> values-explanation  was as the order in the enumeration

OK.

> --line:260:  element #18 `t11FamPrincipalSwitchSelections' does not
> match order of 
>   columnar objects under `t11FamEntry',
> 't11FamLocalPrincipalSwitchSlctns' is listed as #18 and affect the rest
> that follow it. 
>   Better have the right order as the `t11FamEntry' listed and the
> corresponding OBJECT -GROUP
 
I had already discovered this and fixed it in my source, but had not
re-submitted a new version (so as to try to avoid being disruptive to
the process).

> --MIB layout is not as usually recommended:
>          +-- xxxNotifications(0)
>          +-- xxxObjects(1)
>          +-- xxxConformance(2)
> 
> It is recommended to change 
> t11FamNotifications   OBJECT IDENTIFIER ::= { t11FamMIBObjects 0 }
> 
> TO:
> t11FamNotifications   OBJECT IDENTIFIER ::= { t11FabricAddrMgrMIB 0 }
 
Sigh, ... OK.

_______________________________________________
imss mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/imss

---

From: Keith McCloghrie <[email-masked]>
Date: Mon, 10 Oct 2005 09:44:35 -0700 (PDT)

I've put a new version at:

> 1-Checking nits according to
> http://www.ietf.org/ietf/1id-guidelines.txt:
>   - There might be pages with length exceeding 58 lines per page, (page
> 3) being 59 lines.
> PLS check.

I have checked, and my text conforms to the requirement in
draft-rfc-editor-rfc2223bis-08.txt which says:

           Each page must be limited to 58 lines followed by a Form Feed
           (FF) character, followed by an EOL sequence.  The 58 line
           limit includes the headers and footers specified below.

i.e., there appears to be a bug in 'idnits'.

> 2-Was the abstract got shorter on purpose omitting the reference to T11
> group? It is in the Ack section but I wonder.

I believe my text conforms to http://www.ietf.org/ietf/1id-guidelines.html
which says:

   Every Internet-Draft must have an Abstract section. The Abstract
   should provide a concise and comprehensive overview of the purpose
   and contents of the entire document. Its purpose is to give a
   technically knowledgeable reader a general overview of the function
   of the document, to decide whether reading it will be useful. In
   addition to its function in the document, the Abstract section is
   used as a summary in publication announcements and in the online
   index of Internet-Drafts.

   Composing a useful Abstract is a non-trivial writing task. Often, a
   satisfactory abstract can be constructed in part from material from
   the Introduction section, but a good abstract will be shorter, less
   detailed, and broader in scope than the Introduction. Simply copying
   and pasting the first few paragraphs of the Introduction is
   tempting, but it generally results in an Abstract that is both
   incomplete and redundant.

   An Abstract will typically be 5-10 lines, but an Abstract of more
   than 20 lines or less than 3 lines is generally not acceptable.

   An Abstract should be complete in itself, so it should contain no
   citations unless they are completely defined within the Abstract.
   Abbreviations appearing in the Abstract should generally be expanded
   in parentheses. There is a small set of reasonable exceptions to
   this rule; for example, readers don't need to be reminded of what
   "IP" or "TCP" or "MIB" means. In the end, therefore, this is a
   judgment call, but please err on the side of explicitness.

> 3-in issue 6 of version 01 I mentioned two points, the second one is
> fine. The first one I think you have missed. It should be addressed:
> [FC-MGMT]
>      K. McCloghrie, "Fibre Channel Management MIB", Internet-Draft
>      (draft-ietf-ips-fcmgmt-mib-nn.txt), work-in-progress.
> Should be changed to:
> [RFC4044]
>       K. McCloghrie, "Fibre Channel Management MIB",RFC 4044, May 2005

OK, but please note that you incorrectly omitted the period at the
end of your text, and you also incorrectly omitted a space character
in 'MIB",RFC'.

> 4-Following #3 above, any reference to [FC-MGMT] in the text (like sec.
> 4, 5 etc.) better be replaced with [RFC4044]

The use of "[FC-MGMT]" complies with draft-rfc-editor-rfc2223bis-08.txt
which states:

      We recommend enclosing citations in square brackets ("[ ]").
      Simple numeric citations ("[53]") can cause confusing gaps when
      the list of references is split between normative and informative.
      A good alternative is to have two separate series, "[n1]", "[n2]",
      ... "[i1]", "[i2]" for citations to normative and informative
      references.  Other choices include author abbreviations, possibly
      a year ("[Smith93]"), and some brief encoding of the title and
      year ("[MPLS99a]").

> 5-It is easier to follow when OBJECT-GROUP has the same order of objects
> as they appear in their corresponding Entries.
> t11FamGroup  is not fully aligned with its various object including the
> change you made for object 18.

These days, when all the "nits" are being written down so that they can be
enforced, it seems that "nits" are more important than "easier to follow".
So, because there is no SMI requirement, and no Guidelines requirement,
and no "nit" requirement regarding the order of objects listed in an
OBJECT-GROUP definition, I see no need to change this. 

> 6-Any reason to remove the statement --to be assigned by IANA from the
> t11FabricAddrMgrMIB ? Actually the guide recommend the following:
> 
>    -- RFC Ed.: replace nn with IANA-assigned number & remove this note
 
Actually, the guide recommends:

   -- RFC Ed.: replace XXX with IANA-assigned number & remove this note

My personal belief is that this is redundant now that the IETF requires
an "IANA Considerations" section, but sigh, I'll add it.

> Now--Since there are two such cases in this ID, I would recommend to
> name them differently. Like nn and mm,
> Or simply the common XXX, YYY.
 
I can't use "YYY" because RFC 4181 recommends "XXX".

> 7-in section 11 I would extend the request to reference both locations
> rather than just say
> "IANA is requested to make a MIB OID assignment under the appropriate  
>     under the appropriate subtree. "
> The guide recommend the following:
> Editor's Note (to be removed prior to publication):  the IANA is
>       requested to assign a value for "XXX" under the 'mib-2' subtree
>       and to record the assignment in the SMI Numbers registry.  When
>       the assignment has been made, the RFC Editor is asked to replace
>       "XXX" (here and in the MIB module) with the assigned value and to
>       remove this note.
> I suppose you can make it shorter but include  both MIB modules in this
> ID.

I'll change it to:

   IANA is requested to make two MIB OID assignments, one for the
   T11-TC-MIB module and one for the T11-FC-FABRIC-ADDR-MGR-MIB module,
   under the appropriate subtree(s).

Keith.

---

From: Keith McCloghrie <[email-masked]>
Date: Mon, 10 Oct 2005 09:46:16 -0700 (PDT)

> Keith, I also did a quick check on rev 02 that you pointed to.
> Here are some additional nits that I found, and that you might
> as well tidy up while working on it:
> 
> - issues with citations/references:
>   !! Missing Reference for citation: [RFC2741]
>   P005 L031:    single SNMP agent having multiple AgentX [RFC2741] sub-agents, with
 
I have removed "[RFC2741]".

>   !! Missing Reference for citation: [RFC2863]
>   P004 L044:    extensions to the standard IF-MIB [RFC2863] for Fibre Channel
>   (you seem to be using [IF-MIB] and also [RFC2863]. I prefer the latter)
 
I have changed "[RFC2863]" to "[IF-MIB]"

> - I would change (2 times):
>     REVISION    "200510070000Z"
>     DESCRIPTION
>            "Initial version of this MIB module."
>   into:
>     REVISION    "200510070000Z"
>     DESCRIPTION
>            "Initial version of this MIB module, published as RFCyyyy."
>            -- RFC-Editor, pls fill in xxxx for RFCyyyy
   
How is the RFC Editor going to know what "xxxx" is ??   I have changed it
to :

  -- RFC Editor: replace yyyy with actual RFC number & remove this note

> - For consistency, I think I would change 
>      t11FabricAddrMgrMIB MODULE-IDENTITY
>   into
>      t11FamMIB MODULE-IDENTITY
>   but as I said, it is a nit.
 
Thanks for catching this, but I believe that consistency requires it to be:

  t11FcFabricAddrMgrMIB MODULE-IDENTITY

> - indentation for various MIB objects and descriptions is inconsistent
 
I've fixed those that I could find.

> Could you also comment on:
> 
>    W: f(t11fc.mi2), (699,13) Row "t11FamIfEntry" does not have a consistent indexing
>       scheme - cannot specify an index item from additional "base row" fcmInstanceEntry,
>       since can have only one "base row" which is ifEntry
> 
> I think it is OK, but would like to hear/read your comments.
 
Dave Perkins suggested (about seven or eight years ago) that this be
included in RFC 2578, but the WG rejected it.  It is still in SMICng,
but there a switch to ignore this "warning".  I forget which switch it is.

> Further, I have these somehwat more serious comments:
> 
> 1. I see in the t11FamTable a number of writable objects, but I seem to be
>    missing any writeup as to what the expected persistency behaviour is
>    for these objects. Am I not looking at the correct place?
>    Same for t11FamIfTable. Maybe other places as well, pls check.

I have inserted into the MODULE-IDENTITY's DESCRIPTION

           After an agent reboot, the values of read-write objects
           defined in this MIB module are implementation-dependent.

and near the end of each read-write object's DESCRIPTION:

           For the persistence of values across reboots, see the
           MODULE-IDENTITY's DESCRIPTION clause."

> 2. In that same table, I see Counter32 objects, but nothing about possible
>    discontinuities and if any, which is the discontinuity timer.

RFC 2578 says:
                                          ...  Discontinuities
   in the monotonically increasing value normally occur at re-
   initialization of the management system, and at other times as
   specified in the description of an object-type using this ASN.1 type.
   If such other times can occur, for example, the creation of an object
   instance at times other than re-initialization, then a corresponding
   object should be defined, with an appropriate SYNTAX clause, to
   indicate the last discontinuity.

Note:  "... at other times as specified in the description ...".  RFC 4181
does not alter this because it says:

     It is OK to use Counter32/64 for counters that may/will be reset
     when the management subsystem is re-initialized or when other
     unusual/irregular events occur ...    However, if it is
     possible for such other unusual/irregular events to occur, the
     DESCRIPTION clause MUST state that this is so and MUST describe
     those other unusual/irregular events in sufficient detail that it
     is possible for a management application to determine whether a
     reset has occurred since the last time the counter was polled.  The
     RECOMMENDED way to do this is to provide a discontinuity indicator
     as described in RFC 2578 Sections 7.1.6 and 7.1.10.

Thus, the DESCRIPTION clause is required to mention discontinuities and have
a discontinuity indicator *if* they can occur, not just at reboot, but at
other times as well.

So, it is appropriate for you to ask the question, but my answer is:
no, there are no discontinuities other than at reboot, and thus, the text
I have is fine.

> 3. For t11FamIfRowStatus, I think you need to state if other writable column(s)
>    can be changed or not when in active state.
 
The rule in RFC 4181 is broken -- it says:

       The DESCRIPTION clause of the status column MUST specify whether
       or not it is possible to modify other columns in the same
       conceptual row when the status value is active(1).  Note that in
       many cases it will be possible to modify some writable columns
       when the row is active but not others.  In such cases, the
       DESCRIPTION clause for each writable column SHOULD state whether
       or not that column can be modified when the row is active, and
       the DESCRIPTION clause for the status column SHOULD state that
       modifiability of other columns when the status value is active(1)
       is specified in the DESCRIPTION clauses for those columns (rather
       than listing the modifiable columns individually).

Note that the first sentence says what MUST be in the DESCRIPTION clause.
The second and third sentences give an example of when the first sentence
is *wrong*, and a SHOULD applies instead.

Further, it is possible for any table, T, defined in any MIB module, that
a future MIB module could define new read-write objects in a new table which
AUGMENTS T.  Thus, the "In such cases" in third sentence above is a future
possibility for all tables in all MIB modules.  So, *all* RowStatus objects:

       ... SHOULD state that modifiability of other columns when the status
       value is active(1) is specified in the DESCRIPTION clauses for
       those columns (rather than listing the modifiable columns
       individually).

i.e., this is boilerplate, and effectively redundant.  Why do we require
redundant text in DESCRIPTIONs ??

Furthermore, in this MIB, the MODULE-COMPLIANCE clause says that the
MIN-ACCESS for both t11FamIfRcfReject and t11FamIfRowStatus is
read-only, i.e., the implementation gets to choose whether write
access is to be supported: a) at all times, b) at some times but not
other times, or c) never.  Thus, the OBJECT-TYPE cannot require that
writable columns be change-able/not change-able at any specific times.

In light of the above arguments, I propose to add:

           Implementations which support write-access to this object
           can do so under whatever conditions they choose."

to the t11FamIfRcfReject's DESCRIPTION.

Keith.