Last Call Review of draft-ietf-storm-iscsi-sam-
review-ietf-storm-iscsi-sam-genart-lc-thomson-2012-07-26-00

I have been selected as the General Area Review Team (Gen-ART)
reviewer for this draft (for background on Gen-ART, please see


http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html

).

Please wait for direction from your document shepherd
or AD before posting a new version of the draft.

Document: draft-ietf-storm-iscsi-sam-06
Reviewer: Martin Thomson
Review Date: 2012-07-24
IESG Telechat date: ?

Summary: This document has a few issues that would need to be
addressed before it is published as a Standards Track RFC.

Major Issues: None

Minor Issues:

Is the intent to reference RFC 3720 or the new -cons draft (as
published)?  Given that -cons is intended to obsolete 3720, I would
have expected no reference to 3720 at all.

Section 5.1 / 5.1.1 does not define how many bits the PRI field
consumes.  I might infer from the diagram that is the four most
significant bits of the second byte, but the diagram is unclear.  In
any case, the diagram should only be used for illustrative purposes
and the text should provide the definitive answer.

Section 5.2.1 also relies on the diagram.  Though in this case it is
clearer and I can easily infer that the status qualifier is one byte,
I'm still relying on the diagram.

Why is Section 6.2.1 not Section 5.1.2?  Same for 6.2.2 and 6.2.3.
Aren't these new (?) additions to the Command PDU?  Or is there
another PDU that these apply to?

Section 7.1.1 What does it mean to not claim support for any
particular level?  Obviously you can't use the features described
here, but what else?

Section 8 makes a bold claim that needs substantiation.  Given the
global nature of task identifiers, is it necessary to prevent one user
from querying a task that another created?  What about triggering
reset on a I_T nexus that another user is using?

Questions (for -cons):

Section 6.2 makes the following requirement:
   If the connection is still active (it is not undergoing an
   implicit or explicit logout), QUERY TASK MUST be issued on the
   same connection to which the task to be queried is allegiant at
   the time the Task Management request is issued. If the
   connection is implicitly or explicitly logged out (i.e., no other
   request will be issued on the failing connection and no other
   response will be received on the failing connection), then a
   QUERY TASK function request may be issued on another connection.
   This Task Management request will then establish a new allegiance
   for the command being queried.

The comment is probably more for of -cons (Section 4.2.5.1 and Section
7.2.2).  Obviously, this design is long-standing, and I'm only reading
parts of the specification.

There is an implication that tasks have identification that is
globally unique, even if the normal behaviour is to bind (ally) a
request with a connection.  The reasons for allowing this are not
explained, but it imposes some fragility on the system.  For instance,
the requirement that the old connection be closed with a "remove the
connection for recovery" seems counter to the goal of failure
recovery.  If the point of this is for failure recovery, then a
primary failure mode would be network failure - in which case this
mechanism cannot be used.

What purpose does this allegiance feature address?

What are the security implications of using allegiance?

Nits:

I don't know what tooling was used to generate this document, but
there are some strange artifacts.  In particular, diagrams and tables
are misaligned in a number of places.

There are a few terms like "I_T nexus" that are not explained prior to
use.  These are defined in -cons.  However, I find the existence of a
terminology mapping table in this draft to be strange.  Wouldn't that
table be more useful in the "core" document?

Section 4.1:

This seems unnecessarily convoluted:

      Note that an operational value of "2" or higher for this key on
      an iSCSI session does not influence the SCSI level features in
      any way on that I_T nexus. An operational value of "2" or higher
      for this key permits the iSCSI-related features defined in this
      document to be used on all connections on this iSCSI session.
      SCSI level hand-shakes (e.g. commands, mode pages) eventually
      determine the existence or lack of various SAM-5 features
      available for the I_T nexus between the two SCSI end points). To
      summarize, negotiation of this key to "2" or higher is a
      necessary but not a sufficient condition of SAM-5 compliant
      feature usage at the SCSI protocol level.

How about:

  In addition to negotiating a value of "2" or higher, support for an individual
  MUST also be signaled using SCSI level hand-shakes prior to use.

I know that adds 2119 language, but if the goal is to prevent someone
from attempting to use a feature prior to getting positive
confirmation of support, then this should be ok.

Section 6.2 describes multiple tasks.  It would be good if the
description of each of the tasks were given a sub-section so that they
could be cross referenced and more readily found.

--Martin