[storm] FW: MIB Doctor Review for iSCSI MIB, proposed changes, and list questions

Mark Bakke <Mark_Bakke@DELL.com> Wed, 11 July 2012 13:05 UTC

Return-Path: <Mark_Bakke@DELL.com>
X-Original-To: storm@ietfa.amsl.com
Delivered-To: storm@ietfa.amsl.com
Received: from localhost (localhost []) by ietfa.amsl.com (Postfix) with ESMTP id BE7D121F8685 for <storm@ietfa.amsl.com>; Wed, 11 Jul 2012 06:05:25 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -104.595
X-Spam-Status: No, score=-104.595 tagged_above=-999 required=5 tests=[BAYES_00=-2.599, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_MED=-4, TRACKER_ID=2.003, USER_IN_WHITELIST=-100]
Received: from mail.ietf.org ([]) by localhost (ietfa.amsl.com []) (amavisd-new, port 10024) with ESMTP id ORshkbq4wkNA for <storm@ietfa.amsl.com>; Wed, 11 Jul 2012 06:05:13 -0700 (PDT)
Received: from aussmtpmrkps320.us.dell.com (aussmtpmrkps320.us.dell.com []) by ietfa.amsl.com (Postfix) with ESMTP id 55D4B21F861E for <storm@ietf.org>; Wed, 11 Jul 2012 06:05:11 -0700 (PDT)
X-Loopcount0: from
X-IronPort-AV: E=Sophos; i="4.77,567,1336366800"; d="scan'208,217"; a="536988610"
Received: from mail.compellent.com ([]) by aussmtpmrkps320.us.dell.com with ESMTP; 11 Jul 2012 08:05:42 -0500
From: Mark Bakke <Mark_Bakke@DELL.com>
To: "Storm (storm@ietf.org)" <storm@ietf.org>
Date: Wed, 11 Jul 2012 08:05:31 -0500
Thread-Topic: MIB Doctor Review for iSCSI MIB, proposed changes, and list questions
Thread-Index: Ac1d7t+U2g2vOeQ/QtWQihiahn+w8QBdtJpw
Message-ID: <975552A94CBC0F4DA60ED7B36C949CBA03F17813D9@shandy.Beer.Town>
References: <975552A94CBC0F4DA60ED7B36C949CBA03F1781133@shandy.Beer.Town>
In-Reply-To: <975552A94CBC0F4DA60ED7B36C949CBA03F1781133@shandy.Beer.Town>
Accept-Language: en-US
Content-Language: en-US
acceptlanguage: en-US
Content-Type: multipart/alternative; boundary="_000_975552A94CBC0F4DA60ED7B36C949CBA03F17813D9shandyBeerTow_"
MIME-Version: 1.0
X-Mailman-Approved-At: Wed, 11 Jul 2012 08:02:15 -0700
Cc: "mrm@vmware.com" <mrm@vmware.com>, "prakashvn@hcl.com" <prakashvn@hcl.com>
Subject: [storm] FW: MIB Doctor Review for iSCSI MIB, proposed changes, and list questions
X-BeenThere: storm@ietf.org
X-Mailman-Version: 2.1.12
Precedence: list
List-Id: Storage Maintenance WG <storm.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/storm>, <mailto:storm-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/storm>
List-Post: <mailto:storm@ietf.org>
List-Help: <mailto:storm-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/storm>, <mailto:storm-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 11 Jul 2012 13:05:25 -0000


We are in the process of updating the iSCSI MIB module, to match the latest storm changes.  Mike MacFaden has also done a MIB Doctor review, which brought back a number of comments on things that are potentially missing from the original iSCSI MIB RFC.

Prakash, David, Tom and I have been reviewing the MIB doctor requests.  This message is intended to answer some of the MIB doctor comments as well as generate discussion on the list about possible changes.  We intend to post another draft (-02) before the July 16th deadline for the next round of discussion.

Here is a brief summary.  The full responses to comments are below.

*         Several editorial changes

*         A few new fields are added to objects that are either useful or were missed the first time around

*         More complex additions that we believe will need a separate revision and may affect the object model.  The latter changes would require support from those intending to implement or use them.

I will start separate discussion threads on a few items:

*         Heartbeat counters for NOP-In and NOP-Out usage

*         TCP MIB correlation for sessions and connections

*         iscsiInstanceSsnErrorStatsTable session details

For reference, we have:

*         The previous draft of the new iSCSI MIB - http://www.ietf.org/id/draft-ietf-storm-iscsimib-01.txt

*         The initial MIB doctor review - http://www.macfaden.com/ietf/iscsi-mib-1-reviewed.txt

*         An additional MIB doctor review with VMware engineers - http://macfaden.com/ietf/iscsi-mib-notes.txt

Detailed responses to MIB doctor review:

February 17, 2012 MIB DOCTOR Review: http://www.ietf.org/id/draft-ietf-storm-iscsimib-01.txt

Performed by Michael R. MacFaden, VMware, Inc
According to http://tools.ietf.org/html/rfc4181

This review only covers syntax, SMIv2 rules usage of the MIB module itself, not the draft.
Some of the things mentioned below exist the prior version of the mib, not this update.

1 boiler plate - check
2. Narrative Sections - check
3. Security Considerations - check
4. IANA Considerations - missing editors' note.
   Section 9 needs the following verbiage to be added.
       Editor's Note (to be removed prior to publication):  this draft
      makes no additional requests of the IANA.

Editors: We will fix this.

5. No new namespaces
6. References section - check
7. copyright notice - check
8. Intellectual Property section - missing

Editors: We will add the section:

            Copyright (c) 2011 IETF Trust and the persons identified as
            authors of the code.  All rights reserved.

            Redistribution and use in source and binary forms, with or
            without modification, is permitted pursuant to, and subject
            to the license terms contained in, the Simplified BSD
            License set forth in Section 4.c of the IETF Trust's Legal
            Provisions Relating to IETF Documents

9. This change updates existing mib rfc http://www.ietf.org/rfc/rfc4544.txt - check
   Backward compatibility checks: (oids, object-identities, remained the same)

Other items checked --
syntax check: smilint -l9 - no errors
smicng : not run
spelling: Aspell --- no misspellings found.
URLs validated: checked for valid, not-broken links:

References in MIB module  not found in normative (sec 10.1)
IscsiTransportProtocol TC ->  "RFC791, RFC1700

Editors: We will fix this.

enumerations: each element described
  - IscsiDigestMethod

Editors: We had this in the description - is there something else that needs to be added?

DiscontinuityTime is syntax: TimeStamp
All Counters: define their discontinuity timestamp.

Editors: We checked the descriptions in each of the Counter32 and Counter64 fields, and they all reference a discontinuity timestamp.  Isn't that enough?

Uncommon stuff:
1)  In IscsiPortalAttributesEntry has RowStatus as second item in the conceptual row,
not after iscsiPortalStorageType.

Editors: We currently don't put StorageType and RowStatus together in any of our objects, and didn't originally realize there was some tradition around this.  Changing numbering now would cause backward compatibility problems, so we cannot make this change.

2) iscsiPortalRoles does not have a DEFVAL when most others do in this conceptual table.
iscsiPortalAddrType has def val of IPV4? when the iscsiPortalAddr has no default to go with it?

Editors:  There isn't a reasonable DEFVAL for iscsiPortalRoles.  It has to be set when creating the row, so we will leave this one.

3) iscsiNodeAttributesEntry  sez:
   An entry (row) containing mana
   vs 'conceptual row' which is used inconsistently.

Editors: We will change this description from:

An entry (row) containing management information applicable

        to a particular iSCSI node.


A conceptual row containing management information applicable

        to a particular iSCSI node.

4)  (e.g. not created via this MIB).  vs MIB module. There is but one MIB as Dr Case would say.

Editors: We will fix this one.

5) 64 bit counters with SNMPv1 support for 32 bit high/low? kinda
  iscsiSsnTxDataOctets, iscsiSsnRxDataOctets, provides only low word: iscsiSsnLCTxDataOctets, iscsiSsnLCRxDataOctets
  Compliance phrasing is a bit odd:
   "A Low Capacity shadow object of iscsiSsnTxDataOctets
        for those systems that don't support Counter64.
    instead of 'for those systems which are accessible via SNMPv1 only'

Editors:  We will fix the wording for SNMPv1.  We don't see a reason to add objects for high octets.

6) Notifications are required to have a rate limited implementation by a description,
   yet the number of times the failure occurred   is not conveyed in the notifcation varbind list.
    iscsiTgtLoginFailure, iscsiIntrLoginFailure, iscsiInstSessionFailure

    To avoid sending an excessive number of notifications due
        to multiple errors counted, an SNMP agent implementing this
        notification SHOULD NOT send more than 3 notifications of
        this type in any 10-second time period."

Editors:  We believe that this is covered by the counter "iscsiTgtLoginFailures":

iscsiTgtLoginFailure NOTIFICATION-TYPE








The following section addresses comments from the VMware iSCSI-MIB review:

Notes from VMware ISCIS-MIB review meeting
March 21, 2012
Attendees: Andy Banta, Kun Huang, Mike MacFaden

General comments:
1. Many objects lacking REFERENCE clause or offer only vague references.
For those managed objects having a Reference clause, nedd to fix up 'RFC cccc'
For example: "RFC cccc, Section 7.5, Connection Timeout Management"
Example of vague reference:

Editors: OK, we can update in the next draft.

2. Descriptions are short and simple but in some case seem a bit too short
for an IETF standard mib module.

Editors: We can look for opportunities to clarify but don't think we want to go through and rewrite a lot.  If there are particular descriptions that cause confusion please bring them up.

Here's the comments by object in the MIB module: draft-ietf-storm-iscsimib-01.txt

  |  |
  |  +--iscsiTgtLoginFailure(1) [iscsiTgtLoginFailures,iscsiTgtLastFailureType,iscsiTgtLastIntrFailureName,iscsiTgtLastIntrFailureAddrType,iscsiTgtLastIntrFailureAddr]

Consider adding port number.

Editors: LastFailurePortNumber would be easy enough to add.  Should we do an OrZero for implementations that can't get the port number?

  |  +--iscsiIntrLoginFailure(2) [iscsiIntrLoginFailures,iscsiIntrLastFailureType,iscsiIntrLastTgtFailureName,iscsiIntrLastTgtFailureAddrType,iscsiIntrLastTgtFailureAddr]

Can't determine if the failure is due to failure of the underlying transport or not.
When transport is TCP there should be some way to get to the TCP-MIB/TCP-ESTATS-MIB diagnostics.
See notes on iscsiInstanceSsnErrorStatsTable.

Consider adding port number.

Editors: Same as above.  On the failure, is it useful to have both local and remote addr and port number?  Will the TCP MIB keep these connections around long enough to go index it on a failed connection?  If we add these fields, will someone use them?

  |  +--iscsiInstSessionFailure(3) [iscsiInstSsnFailures,iscsiInstLastSsnFailureType,iscsiInstLastSsnRmtNodeName]

Consider handling the event "Target Unmapped"  by updatingiscsiInstLastSsnFailureType of a Session failure, as well.

Editors: FailureType is an OID pointing to a stat in iscsiInstSsnErrorStatsTable, so we would need to add to this:

    iscsiInstSsnTgtUnmappedErrors       Counter32

Are there others that would be needed?

Consider adding summary notification in the case of larger scales (number of targets)
what about large scales, # of targets, each target have a failure of similar sort.

Editors: Would such a notification really be used?  If there was a large scale failure of many targets, we believe the administrator would see enough of the notifications to notice it as a larger problem.


  |  |  +--iscsiInstanceSsnErrorStatsTable(2)
  |  |     |
  |  |     +--iscsiInstanceSsnErrorStatsEntry(1) [iscsiInstIndex]
  |  |        |
  |  |        +-- r-n Counter32 iscsiInstSsnDigestErrors(1)
  |  |        +-- r-n Counter32 iscsiInstSsnCxnTimeoutErrors(2)
  |  |        +-- r-n Counter32 iscsiInstSsnFormatErrors(3)

Related states for transport are not provided in this table. Either a RowPointer
to another mib module is needed or a generic transport counter to indicate failure
at that layer was detected by iscsiInstance.

Editors: If connection timeout errors are seen, wouldn't the user check transport connectivity via ping and other tools?  We are not sure that adding more objects here would add enough value.

Consider separate counters for HB vs data timeouts.
Consider specific heartbeat no-op timeout error countes.

Editors: We will start another thread for this discussion.  It does seem useful since most implementations use NOP for keep-alive heartbeats.

  |  +--iscsiPortal(2)
  |  |  |
  |  |  +--iscsiPortalAttributesTable(1)
  |  |     |
  |  |     +--iscsiPortalAttributesEntry(1) [iscsiInstIndex,iscsiPortalIndex]
  |  |        |
  |  |        +-- --- Unsigned32             iscsiPortalIndex(1)
  |  |        +-- rwn RowStatus              iscsiPortalRowStatus(2)
  |  |        +-- rwn Bits                   iscsiPortalRoles(3)
  |  |        +-- rwn InetAddressType        iscsiPortalAddrType(4)
  |  |        +-- rwn InetAddress            iscsiPortalAddr(5)
  |  |        +-- rwn IscsiTransportProtocol iscsiPortalProtocol(6)
  |  |        +-- rwn Unsigned32             iscsiPortalMaxRecvDataSegLength(7)
  |  |        +-- rwn IscsiDigestMethod      iscsiPortalPrimaryHdrDigest(8)
  |  |        +-- rwn IscsiDigestMethod      iscsiPortalPrimaryDataDigest(9)
  |  |        +-- rwn IscsiDigestMethod      iscsiPortalSecondaryHdrDigest(10)
  |  |        +-- rwn IscsiDigestMethod      iscsiPortalSecondaryDataDigest(11)
  |  |        +-- rwn TruthValue             iscsiPortalRecvMarker(12)
  |  |        +-- rwn StorageType            iscsiPortalStorageType(13)

Digests are negotiated and implementations may have additional levels (tertiary, quaternary)
as well as constraints that are not modelled here.

Editors: There's only one standardized digest method currently in use (CRC32C) that we know of.  Are there implementations that extend this beyond two?

Digests can also be configured in iscsiNodeAttributesTable. (Is there a prececence between node and portal?)

Editors: There is no digests field currently in NodeAttributesTable, so Portal is the only place to configure it.

Need to be able to set/show port number as well as iscsiPortalAddr.

Editors: We can add this one.

Regarding iscsiCxnRecvMarker, iscsiCxnSendMarker these are to be  deprecated and are redundant in session and connection.

Editors: These will be removed.  The V2 object groups in the draft do not contain the deprecated objects.

** note: I Checked with INET-ADDRESS-MIB about iscsiPortalAddrType, if set to 16
      dns(16)     A DNS domain name as defined by the
        InetAddressDNS textual convention.
      Then iscsiPortalAddr can be a dns hostname.

Appears the MIB Portal Attributes and Node Atributes are sort os a
mish-mash or Node, Lhba, Phba, various Portal properties.

Editors:  Not sure we understand this one - the MIB follows iSCSI node and portal definitions for portals and nodes.  HBAs are outside the scope of the MIB and of iSCSI.

Using a table rather than Primary and Secondary would be the right
approach for Header and Data digests, since the whole point of these
negotiations is to provide a list of what each side is willing to
settle on.

Editors: Technically, yes, but practically it seems over-complicated.

Additionally, a list for authentication negotiation would be useful,
also.  There are five different authentication methods defined by the
original RFC 3720, and a few additional ones that have since sprung

Editors: Isn't the list of authorized entities sufficient, since the methods are included by reference?]

There's also the idea of mutual authentication, where each side
verfies the identity of the other, rathern than just a target
authenticatiing a host.

Editors: Is this comment referring to the iscsiSsnAuthIdentity?  We do see that this is just one per session, and we didn't identify it as an initiator or target identity (most people likely assumed initiator).

We'd need to change this name to iscsiSsnAuthIntrIdentity and add iscsiSsnAuthTgtIdentity to show both identities in a session where this is used.

Additional useful information at the Portal level might be at least
some identification of what type of initiator it is, and some
identifying information about it. Maybe not the level of detail you'll
find in IMA_PHBA_PROPERTIES, but the model, description and version.

Editors: this seems useful for troubleshooting.  We can add something similar to iscsiInstDescr:

iscsiPortalDescr OBJECT-TYPE
    SYNTAX        SnmpAdminString
    MAX-ACCESS    read-only
    STATUS        current
        "A UTF-8 string, determined by the implementation to
        describe the iSCSI portal.  When only a single instance
        is present, this object may be set to the zero-length
        string; with multiple iSCSI portals, it may be used in
        an implementation-dependent manner to describe the
        respective portal, and could include information such as
        HBA model, description and version or software driver and

I don't think that we'd want to try to structure it more than just a string.


  |  +--iscsiNode(5)
  |  |  |
  |  |  +--iscsiNodeAttributesTable(1)
  |  |     |
  |  |     +--iscsiNodeAttributesEntry(1) [iscsiInstIndex,iscsiNodeIndex]
  |  |        |
  |  |        +-- --- Unsigned32      iscsiNodeIndex(1)
  |  |        +-- r-n IscsiName       iscsiNodeName(2)
  |  |        +-- r-n SnmpAdminString iscsiNodeAlias(3)
  |  |        +-- r-n Bits            iscsiNodeRoles(4)
  |  |        +-- r-n RowPointer      iscsiNodeTransportType(5)
  |  |        +-- r-n TruthValue      iscsiNodeInitialR2T(6)
  |  |        +-- rwn TruthValue      iscsiNodeImmediateData(7)
  |  |        +-- rwn Unsigned32      iscsiNodeMaxOutstandingR2T(8)
  |  |        +-- rwn Unsigned32      iscsiNodeFirstBurstLength(9)
  |  |        +-- rwn Unsigned32      iscsiNodeMaxBurstLength(10)
  |  |        +-- rwn Unsigned32      iscsiNodeMaxConnections(11)
  |  |        +-- rwn TruthValue      iscsiNodeDataSequenceInOrder(12)
  |  |        +-- rwn TruthValue      iscsiNodeDataPDUInOrder(13)
  |  |        +-- rwn Unsigned32      iscsiNodeDefaultTime2Wait(14)
  |  |        +-- rwn Unsigned32      iscsiNodeDefaultTime2Retain(15)
  |  |        +-- rwn Unsigned32      iscsiNodeErrorRecoveryLevel(16)
  |  |        +-- r-n TimeStamp       iscsiNodeDiscontinuityTime(17)
  |  |        +-- rwn StorageType     iscsiNodeStorageType(18)

Digests as found in scsiPortalAttributesTable, may be provisioned at this level.

Editors: Is there a need to provision digest at this level via the MIB module?

  |  +--iscsiInitiator(8)
  |  |  |
  |  |  +--iscsiInitiatorAttributesTable(1)
  |  |  |  |
  |  |  |  +--iscsiInitiatorAttributesEntry(1) [iscsiInstIndex,iscsiNodeIndex]
  |  |  |     |
  |  |  |     +-- r-n Counter32       iscsiIntrLoginFailures(1)
  |  |  |     +-- r-n TimeStamp       iscsiIntrLastFailureTime(2)
  |  |  |     +-- r-n AutonomousType  iscsiIntrLastFailureType(3)
  |  |  |     +-- r-n IscsiName       iscsiIntrLastTgtFailureName(4)
  |  |  |     +-- r-n InetAddressType iscsiIntrLastTgtFailureAddrType(5)
  |  |  |     +-- r-n InetAddress     iscsiIntrLastTgtFailureAddr(6)

No port number provided.

Editors: We will add iscsiIntrLastTgtFailurePort

  |  |  +--iscsiInitiatorLoginStatsTable(2)
  |  |  |  |
  |  |  |  +--iscsiInitiatorLoginStatsEntry(1) [iscsiInstIndex,iscsiNodeIndex]
  |  |  |     |
  |  |  |     +-- r-n Counter32 iscsiIntrLoginAcceptRsps(1)
  |  |  |     +-- r-n Counter32 iscsiIntrLoginOtherFailRsps(2)
  |  |  |     +-- r-n Counter32 iscsiIntrLoginRedirectRsps(3)
  |  |  |     +-- r-n Counter32 iscsiIntrLoginAuthFailRsps(4)
  |  |  |     +-- r-n Counter32 iscsiIntrLoginAuthenticateFails(5)
  |  |  |     +-- r-n Counter32 iscsiIntrLoginNegotiateFails(6)

No way to get to transport layer (tcp, etc) related errors not shown here or provided through RowPointer/etc.

Editors: We are not sure that this will add enough value.

  |  |  +--iscsiInitiatorLogoutStatsTable(3)
  |  |     |
  |  |     +--iscsiInitiatorLogoutStatsEntry(1) [iscsiInstIndex,iscsiNodeIndex]
  |  |        |
  |  |        +-- r-n Counter32 iscsiIntrLogoutNormals(1)
  |  |        +-- r-n Counter32 iscsiIntrLogoutOthers(2)

The counter iscsiIntrLogoutOthers does not cover cases where no ack is returned (timeout case) due
to failure of transport to deliver logout ack.

Editors: Adding a timeout counter for this seems to make sense.

  |  +--iscsiIntrAuthorization(9)
  |  |  |
  |  |  +--iscsiIntrAuthAttributesTable(1)
  |  |     |
  |  |     +--iscsiIntrAuthAttributesEntry(1) [iscsiInstIndex,iscsiNodeIndex,iscsiIntrAuthIndex]
  |  |        |
  |  |        +-- --- Unsigned32  iscsiIntrAuthIndex(1)
  |  |        +-- rwn RowStatus   iscsiIntrAuthRowStatus(2)
  |  |        +-- rwn RowPointer  iscsiIntrAuthIdentity(3)
  |  |        +-- rwn StorageType iscsiIntrAuthStorageType(4)

The reference for iscsiIntrAuthIdentity is too vague. Only provides the RFC?:
        "IPS-AUTH MIB, RFC 4545"

Editors: We will fix this.

  |  +--iscsiSession(10)
  |  |  |
  |  |  +--iscsiSessionAttributesTable(1)
  |  |  |  |
  |  |  |  +--iscsiSessionAttributesEntry(1) [iscsiInstIndex,iscsiSsnNodeIndex,iscsiSsnIndex]
  |  |  |     |
  |  |  |     +-- --- Unsigned32      iscsiSsnNodeIndex(1)
  |  |  |     +-- --- Unsigned32      iscsiSsnIndex(2)
  |  |  |     +-- r-n Enumeration     iscsiSsnDirection(3)
  |  |  |     +-- r-n IscsiName       iscsiSsnInitiatorName(4)
  |  |  |     +-- r-n IscsiName       iscsiSsnTargetName(5)
  |  |  |     +-- r-n Unsigned32      iscsiSsnTSIH(6)
  |  |  |     +-- r-n OctetString     iscsiSsnISID(7)
  |  |  |     +-- r-n SnmpAdminString iscsiSsnInitiatorAlias(8)
  |  |  |     +-- r-n SnmpAdminString iscsiSsnTargetAlias(9)
  |  |  |     +-- r-n TruthValue      iscsiSsnInitialR2T(10)
  |  |  |     +-- r-n TruthValue      iscsiSsnImmediateData(11)
  |  |  |     +-- r-n Enumeration     iscsiSsnType(12)
  |  |  |     +-- r-n Unsigned32      iscsiSsnMaxOutstandingR2T(13)
  |  |  |     +-- r-n Unsigned32      iscsiSsnFirstBurstLength(14)
  |  |  |     +-- r-n Unsigned32      iscsiSsnMaxBurstLength(15)
  |  |  |     +-- r-n Gauge32         iscsiSsnConnectionNumber(16)
  |  |  |     +-- r-n RowPointer      iscsiSsnAuthIdentity(17)
  |  |  |     +-- r-n TruthValue      iscsiSsnDataSequenceInOrder(18)
  |  |  |     +-- r-n TruthValue      iscsiSsnDataPDUInOrder(19)
  |  |  |     +-- r-n Unsigned32      iscsiSsnErrorRecoveryLevel(20)
  |  |  |     +-- r-n TimeStamp       iscsiSsnDiscontinuityTime(21)

Consider reporting the digest negotiated for this session.
Consider reporting the error-recovery-level (ERL).
Consider reporting type(?) negotiated.

  |  |  +--iscsiSessionStatsTable(2)
  |  |  |  |
  |  |  |  +--iscsiSessionStatsEntry(1) [iscsiInstIndex,iscsiSsnNodeIndex,iscsiSsnIndex]
  |  |  |     |
  |  |  |     +-- r-n Counter32 iscsiSsnCmdPDUs(1)
  |  |  |     +-- r-n Counter32 iscsiSsnRspPDUs(2)
  |  |  |     +-- r-n Counter64 iscsiSsnTxDataOctets(3)
  |  |  |     +-- r-n Counter64 iscsiSsnRxDataOctets(4)
  |  |  |     +-- r-n Counter32 iscsiSsnLCTxDataOctets(5)
  |  |  |     +-- r-n Counter32 iscsiSsnLCRxDataOctets(6)

Consider reporting missing PDU.
Break down counters for data pdus in and out.
Add counter for async pdus in.

Editors: The above seem like good ideas - will anyone use these?  Also, breaking down the existing counters might break current implementations.  We would likely have to create new ones.

  |  |  +--iscsiSessionCxnErrorStatsTable(3)
  |  |     |
  |  |     +--iscsiSessionCxnErrorStatsEntry(1) [iscsiInstIndex,iscsiSsnNodeIndex,iscsiSsnIndex]
  |  |        |
  |  |        +-- r-n Counter32 iscsiSsnCxnDigestErrors(1)
  |  |        +-- r-n Counter32 iscsiSsnCxnTimeoutErrors(2)

Description should discuss when these counters are most likely provided such as when the error-recovery-level
is 1 or 2.

Editors: We will update the descriptions.

  |  +--iscsiConnection(11)
  |     |
  |     +--iscsiConnectionAttributesTable(1)
  |        |
  |        +--iscsiConnectionAttributesEntry(1) [iscsiInstIndex,iscsiSsnNodeIndex,iscsiSsnIndex,iscsiCxnIndex]
  |           |
  |           +-- --- Unsigned32             iscsiCxnIndex(1)
  |           +-- r-n Unsigned32             iscsiCxnCid(2)
  |           +-- r-n Enumeration            iscsiCxnState(3)
  |           +-- r-n InetAddressType        iscsiCxnAddrType(4)
  |           +-- r-n InetAddress            iscsiCxnLocalAddr(5)
  |           +-- r-n IscsiTransportProtocol iscsiCxnProtocol(6)
  |           +-- r-n InetPortNumber         iscsiCxnLocalPort(7)
  |           +-- r-n InetAddress            iscsiCxnRemoteAddr(8)
  |           +-- r-n InetPortNumber         iscsiCxnRemotePort(9)
  |           +-- r-n Unsigned32             iscsiCxnMaxRecvDataSegLength(10)
  |           +-- r-n Unsigned32             iscsiCxnMaxXmitDataSegLength(11)
 |           +-- r-n IscsiDigestMethod      iscsiCxnHeaderIntegrity(12)
  |           +-- r-n IscsiDigestMethod      iscsiCxnDataIntegrity(13)
  |           +-- r-n TruthValue             iscsiCxnRecvMarker(14)
  |           +-- r-n TruthValue             iscsiCxnSendMarker(15)
  |           +-- r-n Unsigned32             iscsiCxnVersionActive(16)

Consider adding initial remote  ip addr and port to this table.

Editors:  Not sure what the value in initial remote IP addr and port would be - if they change wouldn't this be a new connection?

  |  +--iscsiTarget(6)
  |  |  |
  |  |  +--iscsiTargetAttributesTable(1)
  |  |  |  |
  |  |  |  +--iscsiTargetAttributesEntry(1) [iscsiInstIndex,iscsiNodeIndex]
  |  |  |     |
  |  |  |     +-- r-n Counter32       iscsiTgtLoginFailures(1)
  |  |  |     +-- r-n TimeStamp       iscsiTgtLastFailureTime(2)
  |  |  |     +-- r-n AutonomousType  iscsiTgtLastFailureType(3)
  |  |  |     +-- r-n IscsiName       iscsiTgtLastIntrFailureName(4)
  |  |  |     +-- r-n InetAddressType iscsiTgtLastIntrFailureAddrType(5)
  |  |  |     +-- r-n InetAddress     iscsiTgtLastIntrFailureAddr(6)

  |  |  +--iscsiTargetLoginStatsTable(2)
  |  |  |  |
  |  |  |  +--iscsiTargetLoginStatsEntry(1) [iscsiInstIndex,iscsiNodeIndex]
  |  |  |     |
  |  |  |     +-- r-n Counter32 iscsiTgtLoginAccepts(1)
  |  |  |     +-- r-n Counter32 iscsiTgtLoginOtherFails(2)
  |  |  |     +-- r-n Counter32 iscsiTgtLoginRedirects(3)
  |  |  |     +-- r-n Counter32 iscsiTgtLoginAuthorizeFails(4)
  |  |  |     +-- r-n Counter32 iscsiTgtLoginAuthenticateFails(5)
  |  |  |     +-- r-n Counter32 iscsiTgtLoginNegotiateFails(6)

  |  |  +--iscsiTargetLogoutStatsTable(3)
  |  |     |
  |  |     +--iscsiTargetLogoutStatsEntry(1) [iscsiInstIndex,iscsiNodeIndex]
  |  |        |
  |  |        +-- r-n Counter32 iscsiTgtLogoutNormals(1)
  |  |        +-- r-n Counter32 iscsiTgtLogoutOthers(2)

  |  +--iscsiTgtAuthorization(7)
  |  |  |
  |  |  +--iscsiTgtAuthAttributesTable(1)
  |  |     |
  |  |     +--iscsiTgtAuthAttributesEntry(1) [iscsiInstIndex,iscsiNodeIndex,iscsiTgtAuthIndex]
  |  |        |
  |  |        +-- --- Unsigned32  iscsiTgtAuthIndex(1)
  |  |        +-- rwn RowStatus   iscsiTgtAuthRowStatus(2)
  |  |        +-- rwn RowPointer  iscsiTgtAuthIdentity(3)
  |  |        +-- rwn StorageType iscsiTgtAuthStorageType(4)

Additional Comments:

There's no information in any of this regarding iSCSI discovery
information.  That would be potentially useful information,
especially in the case of discovered targets that have no sessions.
That could be an indication of a network problem of a
mis-configuration.  The type of discovery performed and the discovered
targets could be put in there and provide this information.

Editors: This MIB module currently represents only the initiators and targets provided by the entity being queried.  The remote objects do not appear here and are part of someone else's entity.  So if you had a target-only device, it wouldn't have objects for the initiators connecting to it and vise-versa.

Since we don't currently support remote objects, adding this capability would be a significant new MIB development by the WG, rather than a fix given the current review.

End of File.