iSNS: More Last Call comments

[Black_David@emc.com]

Here are mine.  Sorry to be a bit tardy on these but
the powers that be decided that I had to spend most of
Friday and Saturday packing and unpacking to move to a
new office.  Thanks, --David

----- Technical -----

-- Section 3.3.2

   The iSNS server may refuse SCN service by returning a SCN
   Registration Rejected (Status Code 17).  The rejection might occur
   in situations where the network size or current number of SCN
   registrations, has passed an implementation-specific threshold.  A
   client not allowed to register for SCNs may decide to monitor its
   sessions with other storage devices directly.

The "may" needs to be upper case, and this text strikes me as being
a bit too cavalier about situations in which iSNS is unable to provide
the requested service.  A sentence should be added cautioning
implementers that both hardware and software resources should be
provided in sufficient quantities to make this refusal of service
a sufficiently rare case.

-- Section 3.4

Need a couple of "MUST"s in this section:
- One to require the existence of an administrative control interface for
	the listed parameters.
- One to require the default settings be used in the absence of other
	configuration (could be a "SHOULD").

-- Section 3.5.2

Will need to instruct IANA (and the RFC editor) to fill in the iSNS DHCP
option number when it is allocated.

-- Section 3.6

NAT discussion also needs to talk about using "global" addresses/port
numbers
for services/communication endpoints that need to be accessible.  This is
for an environment in which the administrator of iSNS knows about the NATs
and can configure appropriate static mappings in them so that iSNS operates
in the "global" domain and with a little care, everything "just works",
even though NATs are involved (in essence, nodes behind NATs have a local
address/port that they use for communication and a "global" one that
they report to iSNS, with the NAT[s] translating between them).  This is
a "no free lunch" discussion, as it implies extra overhead in configuring
the NAT[s] and care to deal with the fact that an endpoint now has two
<IP address, TCP port> address pairs.  There ought to be some useful
material on this in the NAT RFCs (and possibly drafts) somewhere.

-- Section 3.8

The discussion of iSNS database synchronization needs to mention distributed
transactions as a way of doing this, although it would be ok to discourage
them on complexity/overhead/latency grounds.

   A maximum of one backup iSNS server SHALL exist at any individual IP
   address.

Why?  At a minimum, this needs to be explained.

   Each backup server should note its relative precedence in the active
   server's list of backup servers.

That "should" needs to be at least "SHOULD" and it may need to be a "MUST"
as failure to do this could lead to more than one backup server believing
that it is primary after a failure of the active server.

   If a backup server establishes that it has lost connectivity to the
   active server and other backup servers of higher precedence, then it
   shall assume that it is the active server.  The method of
   determining whether connectivity has been lost is implementation-
   specific.  One possible approach is to assume that if the backup
   server does not receive iSNS hearbeat messages for a period of time,
   then connectivity to the active server has been lost.   Alternately,
   the backup server may establish TCP connections to the active server
   and other backup servers, and loss of connectivity determined
   through non-response to periodic echo messages (using iSNSP, SNMP,
   or other protocols).

"shall" --> "SHOULD", but this is a disaster waiting to happen, as different
iSNS implementations will get this just different enough that the result
will be split-brain (more than one server believing it's primary) on loss
of communication.  This area is *hard* to get right (what's needed here
is an HA cluster coordination protocol) - an expedient way out for now
may be to limit support to one primary and one backup.

   When a backup server becomes the active server, it shall assume all
   active server responsibilities, including (if used) transmission of
   the iSNS heartbeat message.

"shall" --> "SHALL", although this is probably editorial.

-- Section 5.1.1, 5.1.3, 5.2.1, and 5.2.3

   The following attributes are available to support iSCSI.  Attributes
   indicated in the REQUIRED TO IMPLEMENT column MUST be supported by
   an iSNS server used to support iSCSI. Attributes indicated in the
   REQUIRED TO USE column MUST be supported by an iSCSI device that
   elects to use the iSNS.

Suggest retitling the columns as "REQUIRED for Server" and "REQUIRED for
Client", and noting that the attributes required for a Client will always
be used in practice.  Similar comments apply to the other 3 requirements
tables.

   All iSCSI user-specified and vendor-specified attributes are
   optional to implement and use.

"optional" --> "OPTIONAL", and add corresponding versions of this to 
the other 3 requirements tables (this is a recent IESG "hot button").

-- Section 5.1.3 and 5.2.3

What is the difference between NOT USED and RESERVED?  A possible guess is
that NOT USED will never be used, whereas RESERVED is for possible future
use.  This needs to be explained, or substitute RESERVED for "NOT USED".

-- Section 6.1.1

   The iSNSP version described in this document is 0x0001.

Say that all other values are reserved.

-- Section 6.5

The use of "Multicast" at this juncture will be assumed to be IP Multicast,
and needs to be explained earlier.  Add an explanation of iSNS
Multicast and Broadcast somewhere in Section 3 and reference it here.
Need to explain which messages can be Multicast/Broadcast.

-- Section 6.6.5.3

Add a discussion of what happens if the iSNS database is updated during
a sequence of GetNext operations.  Is it possible to lose one's place
if an object is deleted?  How is a client expected to cope with this?

-- Section 7

This looks like it should be retitled to IANA Considerations, with all of
this stuff going into registries, and the section needs to be expanded to
include things like the BSD from Section 6.5.  Some of the material in
the table in Section 7.1 may not be appropriate for an IANA registry,
though.

-- Section 7.2.1

7.2.1   Entity Identifier (EID)

   The Entity Identifier (EID) is a variable-length field containing
   user-readable UTF-8 text, and is terminated with at least one NULL
   character. variable length identifier   This field uniquely
   identifies each network entity registered in the iSNS server.

Does this need to be stringprep'ed?  "variable length identifier" looks
like it escaped from previous editing.  Check all other UTF-8 attributes
for possible needs for stringprep.

   By convention, EIDs generated by the iSNS server begin with
   the string "iSNS:".  iSNS clients MUST NOT generate and register
   EIDs beginning with the string "iSNS:".

Replace "By convention" with a "MUST" requirement on server generation
of EIDs.

-- Section 7.2.3

   This field contains the IP Address used to manage the Network Entity
   and all Storage Nodes contained therein.  The Management IP Address
   is a 16-byte field that may contain either a 32-bit IPv4 or 128-bit
   IPv6 address.  When this field contains an IPv4 value, the most
   significant 12 bytes are set to 0x00.  When this field contains an
   IPv6 value, the entire 16-byte field is used.  If the network entity
   is capable of being managed and this field is not set, then in-band
   management through the IP address of one of the Portals of the
   Network Entity is assumed.

What is this IP address used for?  Why is an IP address, as opposed to
an IP address plus TCP or UDP port?

-- Section 7.2.7

   The Entity Index is a 4-byte integer value that uniquely identifies
   each network entity registered in the iSNS server.  The Entity Index
   is assigned by the iSNS server during the initial registration of an
   Entity.  It can be used to represent a registered entity in
   situations where the Entity Identifier is too long to be used.

Explain the requirements on "uniquely identifies", in particular rules
about if/when an iSNS server can reuse an Entity Index value.
Same comment applies to Portal Index (Section 7.3.7) and iSCSI Node Index
(Section 7.4.5).

-- Section 7.4.7

Remove DH-CHAP value.

-- Section 7.5.3

Remove mFCP value.

-- Section 7.7.1

   This is a 4-byte field, and is used to provide a FC-4 type during a
   FC-4 Type query.  The FC-4 types are consistent with the FC-4 Types
   as defined in FC-PH.  Byte 0 contains the FC-4 type.  All other
   bytes are reserved.

Update FC-PH reference as T11 is obsoleting FC-PH.

-- Section 7.9

   To avoid
   misinterpreting proprietary attributes, it is RECOMMENDED that the
   vendor's own OUI (Organizationally Unique Identifier) be placed in
   the upper three bytes of the attribute field itself.  If the OUI is
   not used, then some other unique marker recognizable by the vendor
   SHOULD be used.

Should these be REQUIRED/MUST rather than RECOMMENDED/SHOULD?  The second
sentence is an invitation to problems, and needs to be rethought -
Enterprise
numbers (as used for MIBs) are a possibility, and there needs to be a way
to distinguish whether an OUI or something else is in this field.

-- Section 7.11     Standards-Based Extensions

   These attributes are reserved for future work by other standards
   bodies.

Which attributes are these?  I don't see any corresponding entry in the
table in Section 7.1.

-- Section 8.2

   If the iSNS server is used to distribute authorizations for
   communications between iFCP and iSCSI peer devices, IPsec ESP with
   null transform MUST be implemented, and non-null transform MAY be
   implemented.  If a non-null transform is implemented, then the DES
   encryption algorithm MUST NOT be used.

"SHOULD NOT be used" is sufficient for DES, also applies to next text:

   If the iSNS server is used to distribute security policy for iFCP
   and iSCSI devices, then authentication, data integrity, and
   confidentiality must be supported and used.  Where confidentiality
   is desired or required, IPSec ESP with non-null transform SHOULD be
   used, and the DES encryption algorithm MUST NOT be used.

"must" --> "MUST", and there are a number of other places in this section
where RFC 2119 words need to be in upper-case.

   Note that the
   authentication block is used only for iSNS broadcast or multicast
   messages, and SHOULD NOT be used in unicast iSNS messages.

"SHOULD NOT" --> "MUST NOT" here and in earlier discussion of this block
to avoid any need for recipients to deal with it in unicast messages.

-- Section 8.6

   When IPSec security is enabled, each iSNS client that is registered
   in the iSNS database SHALL maintain at least one phase-1 and one
   phase-2 security association with the iSNS server.  All iSNS
   protocol messages between iSNS clients and the iSNS server SHALL be
   protected by a phase-2 security association.

This prohibits discarding and recreating the phase 2 associations, e.g.,
to deal with limited crypto resources.  If that was intended, explain
why, as keeping the phase-1 ought to be sufficient to maintain the 
security context.