[Ips] MIB Doctor review of: draft-ietf-ips-scsi-mib-06.txt

Compiles clean with SMICng.
Smilint causes soem warnings, see below.

More or less serious:

- I see in ScsiIdentifier DESCRIPTION clause:
      The format depends on the transport used:
      - SPI: only bits:0-3 for a port identifier (MSB is 0 and LSB
      is 3). Other bits must be zero.
      - SPI: identifier of a device is a zero-length octet string.
  So how does one know if it is one or the other?
  Same for the other transports

- I see HrSWInstalledIndexOrZero TC.
  This runs the risk of name clash in the future.
  Would it be so bad to name it ScsiHrSWInstalledIndexOrZero

- I see:
    scsiObjects       OBJECT IDENTIFIER ::= { scsiModule 1 }
    scsiNotifications OBJECT IDENTIFIER ::= { scsiModule 2 }
    scsiConformance   OBJECT IDENTIFIER ::= { scsiModule 3 }

    scsiTransportTypes   OBJECT IDENTIFIER ::= { scsiObjects 1 }
    scsiGeneral       OBJECT IDENTIFIER ::= { scsiObjects 2 }
    scsiInitiator     OBJECT IDENTIFIER ::= { scsiObjects 3 }
    scsiTarget        OBJECT IDENTIFIER ::= { scsiObjects 4 }
    scsiLogicalUnit   OBJECT IDENTIFIER ::= { scsiObjects 5 }
  I think I would put the transportTypes under an ADMIN tree,
  so I would define it somethink aka:
    scsiAdmin         OBJECT IDENTIFIER ::= { scsiModule 1 }
    scsiObjects       OBJECT IDENTIFIER ::= { scsiModule 2 }
    scsiNotifications OBJECT IDENTIFIER ::= { scsiModule 3 }
    scsiConformance   OBJECT IDENTIFIER ::= { scsiModule 4 }

    scsiTransportTypes   OBJECT IDENTIFIER ::= { scsiAdmin 1 }
    scsiGeneral       OBJECT IDENTIFIER ::= { scsiObjects 1 }
    scsiInitiator     OBJECT IDENTIFIER ::= { scsiObjects 2 }
    scsiTarget        OBJECT IDENTIFIER ::= { scsiObjects 3 }
    scsiLogicalUnit   OBJECT IDENTIFIER ::= { scsiObjects 4 }
  Maybe it is just me.

- scsiInstanceTable
  I do not see any text that specifies persistency behaviour
  over a restart or reboot. We need to specify that.

  Same for scsiDeviceTable... and possibly other tables, pls check!

- scsiInstScsiNotificationsEnable DESCRIPTION clause claims:
     " This object indicates whether notifications defined in this
      MIB will be sent."
  which is not true. The actual sending is under control of the 
  MIB modules in RFC3413. This object indicates (I think) if 
  notifications get generated.

- scsiDeviceRole and scsiPortRole
  WOuld it make sense to define a TC for ScsiRole ??

- scsiIntrPortOutCommands and scsiIntrPortHSOutCommands
  If I understood the introductory text correctly, then the 
  first one will always be the low order 32 bit of the second one.
  If so, pls specify so in the DESCRIPTION clause.

  Same question for other such combinations

- For all Counter32 and Counter64 objects I do not see any mention
  of a discontinuity timer as required/recommended by RFC2578.

- scsiDscTgtRowStatus
  DESCRIPTION clause should express if any columns can be changed 
  while row is active.
  It should also describe which objects MUST have valid values
  before a row can be made active
  All as per RowStatus TC in RFC2579

  No discussion of persistency after restart/reboot for this table

  Same question for (at least some) other RowStatus objects,
  for example scsiAuthIntrRowStatus

- For scsiAuthIntrTgtPortIndex
  What if there is a entry/row with this index as zero and one or more
  with a non-zero index. Do the counters than get incremented in both
  the zero-indexed enrty and in the specific entry?
  May want to add some words on the fact if this situation is valid
  and if so what the behaviour is.

- scsiLuPeripheralType
  not having quick access to the ANSI spec, does it make sense to 
  enumerate the values?

- Did you do this consciously:
   -- scsiNotifications OBJECT IDENTIFIER ::= { scsiModule 2 }
   scsiNotificationsPrefix OBJECT IDENTIFIER ::= { scsiNotifications 0 }
  I myslef probably would have used
   scsiNotifications OBJECT IDENTIFIER ::= { scsiModule 0 }
  and then the notifications can go underneath that right away.
  Not a showstopper. Just asking.

- I think that this is a typical MIB Module where I would specify
  two MODULE-COMPLIANCE statements, one for FullCompliance amd one for
  ReadOnlyCompliance (RFC3289 is a good example of how to do it).

Kind of nit picking, yet for consistency in MIB documents it would
be really good to address these:

- In the abstract:
   This memo defines a Management Information Base (MIB) for Small
  should read:
   This memo defines a portion of the Management Information Base (MIB),
   namely managed objects for Small

- In various places where you speak of "The SCSI MIB" it is better to
  speak of "The SCSI MIB Module".  There is only one MIB which consists
  of many MIB Modules.

- Section 5, s/the SNMP tables/the MIB tables/ !?

- I would add DISPLAY-HINTS for the TEXTUAL CONVENTIONs
  In fact smilint gives these warnings because of them being absent:

.\SCSI-MIB:87: [5] {type-without-format} warning: type `ScsiIndexValue' has no format specification
.\SCSI-MIB:94: [5] {type-without-format} warning: type `ScsiPortIndexValueOrZero' has no format specification
.\SCSI-MIB:108: [5] {type-without-format} warning: type `ScsiIndexValueOrZero' has no format specification
.\SCSI-MIB:194: [5] {type-without-format} warning: type `ScsiIdCodeSet' has no format specification
.\SCSI-MIB:208: [5] {type-without-format} warning: type `ScsiIdAssociation' has no format specification
.\SCSI-MIB:223: [5] {type-without-format} warning: type `ScsiIdType' has no format specification
.\SCSI-MIB:254: [5] {type-without-format} warning: type `HrSWInstalledIndexOrZero' has no format specification
Real nits:

- I think I would change
      scsiModule MODULE-IDENTITY
  into
      scsiMIB MODULE-IDENTITY
  it much more common practice.

- I see:      SYNTAX Unsigned32(1..4294967295)
  we normally leave space before the open parenthesis

- I see the use of "octets" and "bytes" used, even in the same
  DESCRIPTION clause. I can live with it, but it seems inconsistent
  does it not?

Admin issues

- I see references to RFC2573, RFC2575... 
  These have been obsoleted by RFC3413 and 3415 respectively
  Actually I do not see that you have a citation to these.
  To 3413, you probably SHOULD have a citation near text that
  explains that the ultimate control on sending of notifications
  is controled by the MIB modules in that RFC.

- I think you must add RFC3411 as a normative reference, 
  you IMPORT from it

Last ... I have not (yet) checked if sect 11 is correct.

Thanks,
Bert 

_______________________________________________
Ips mailing list
Ips@ietf.org
https://www1.ietf.org/mailman/listinfo/ips