RE: iSNS: Responses to David Black Technical Comments

[Black_David@emc.com]

Josh,

Most of the responses are fine, but there's one major issue
(iSNS Backup servers) and several minor issues, one of which is
unfortunately new and affects IP address formats.  Thanks, --David

-- iSNS Backup Servers --

> >    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.
> 
> Done.  Your comment is noted, and that is why I left the method for
> determining election/re-election of the primary server up to the specific
> implementation.  Yes, an HA cluster protocol would be ideal, but not
> necessarily required in all situations.

I remain concerned that the text here not only allows, but may encourage
building iSNS backup in a fashion that will fail split-brain with
potentially disastrous consequences.  On further reading, the thing to
do may be to modify the following sentence:

   Therefore, it is beyond the scope of this specification to
   facilitate more than a basic interoperability among failover
   mechanisms.

to add specifics about what is and is not specified for interoperability.
What is specified is:
- Client behavior and protocol interaction in the presence of multiple
	iSNS servers, some of which are Backup iSNS servers.
- Required failover behaviors of the collection of iSNS servers that
	includes Active and Backup servers.
What is not specified is:
- Complete functional failover requirements on each iSNS server and the
	protocols needed for a collection of iSNS servers to realize the
	required failover behaviors in an interoperable fashion.

While it's missing some aspects of interoperability, I think getting the
client foundation in place that allows failover to a backup server is
important enough to be worth including, even at the expense of not
mixing different iSNS implementations in a collection of Active/Backup
servers.

-- Minor Issues --

> > 
> > -- 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?
> 
> The following text has been added to the end of section 6.6.5.3:

Added text snipped - it's ok.  

Unless I missed it somewhere a sentence is needed
saying that the iSNS database MUST have an implicit internal linear
order of entries.  This is needed to give a meaning to words like
"after", "next" and "following" in this section.  It also might be
good to say than an iSNS database SHOULD NOT be reorganized with
respect to that implied order as a consequence of insertions and
deletions (i.e., relative order of entries that remain in the
database SHOULD be preserved so that GetNext encounters generally
reasonable behavior).
 
> > -- 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.
> 
> A new IANA Considerations section has been inserted as section 9, as shown
> below:

Good start, but instructions to IANA for handling additions are missing.
Please see RFC 2434 for information on what needs to be in this section.

> > -- 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.
> 
> Variable-length fields that are used as key attributes (i.e., EID, iSCSI
Name)
> need to be stringprep-ed.  Those that are not key attributes are used as
> descriptive fields, and so do not need to be normalized.  The following
text
> will be added to each variable-length field that is used as a 
> primary key:
> 
> This attribute MUST be normalized according to the stringprep template
> [STRINGPREP] before it is stored in the iSNS database.
> 
> Note that since we are now using the stringprep document generically, not
just
> for iSCSI names, then I would suggest removing the iSCSI label for that
> document.

Ok, let's consider that a late WG Last Call comment against the iSCSI
stringprep draft - please contact Mark Bakke directly about revising it.

> > -- 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?
> 
> Both IP address and TCP/UDP Port are required to identify a Portal.  The
> Portal object is therefore referenced by two attributes--its primary
> key has two fields.  The IP address alone is insigificant--it must be
> accompanied by the TCP/UDP Port Number to have any meaning in iSNS.

But this is the Management IP address and I don't see a Management IP Port -
did I miss something?  The phrases "used to manage" and "capable of being
managed" need to be defined - if this means SNMP, say so.

Also, I don't see any external way of indicating whether the address is
IPv4 or IPv6, and hence I believe Section 2.5.4 of RFC 2373 applies to
IP addresses in iSNS - unfortunately, the convention that's currently
being used for address formats implies that all IPv4 addresses in iSNS
are for IPv6-capable nodes, which is probably not what was intended
(sorry for the late catch on this one, but better now than to have one
of the ADs find it).

> > -- 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).
> 
> The text for 7.2.7 has been modified as follows:
> 
> The Entity Index is a 4-byte integer value that uniquely identifies each
> network entity registered in the iSNS server.  Upon initial registration
of
> an Entity, the iSNS server assigns an unused value for the Entity Index
for
> that Entity.  Each Entity in the iSNS database MUST be assigned a value
for
> the Entity Index that is not assigned to any other Entity.  The Entity
Index
> MAY be used to represent the Entity in situations when the Entity
Identifier
> is too long or otherwise inappropriate.

That takes care of assignment, what about reuse?  Short-term reuse SHOULD
NOT be done, as it's an invitation to confusion in places where the Entity
Index has been used.  Similarly for 7.3.7 and 7.4.5.