FCIP: review comments - 2

["Mallikarjun C." <cbm@rose.hp.com>]

Some more comments on rev9, excuse me if some/all these were earlier
discussed by the design team.
--
Mallikarjun

Mallikarjun Chadalapaka
Networked Storage Architecture
Network Storage Solutions Organization
Hewlett-Packard MS 5668 
Roseville CA 95747
cbm@rose.hp.com

> 6.6.1 FCIP Encapsulation of FC Frames
....
>    The CRCV (CRC Valid) Flag SHALL be set to 0.
> 
>    The CRC field SHALL be set to 0.

I am surprised that the FCIP encapsulated header is never protected
by a CRC.  The error detection section clearly acknowledges the
possibility that TCP checksum could be inadequate for certain 
deployments - and even suggests checking the FC frame CRC (sort
of like a data digest) on reception at the Encapsulated Frame Receiver 
Portal.

My recommendation is to require an FCIP Entity to employ the header
CRC if the SF that it receives asks for CRC - IOW, similar to iSCSI's 
approach.  So, I guess I am also advocating a new bit in the pFlags field 
to announce this.  When this CRC expectation is announced, the FC CRC
checking test in 6.6.2.2 should also be a mandatory test - from the 
"semi-optional" list it is currently in.

> 6.6.2 FCIP Data Engine Error Detection and Recover

The last word in the above should be "Recovery".
 
> 6.6.2.1 TCP Assistance With Error Detection and Recovery
.....
>Thus, the byte stream passed from TCP to
>    the FCIP_LEP will be in order and free of errors detectable by the
>    TCP checksum. If TCP did not perform these functions, the FCIP_LEP
>    would have to.

Suggest rewording the last sentence (TCP always performs these functions
to *its* satisfaction, the question is if FCIP feels the same; secondly, FCIP_LEP's
error mgmt behavior is not dynamically contingent upon TCP's behavior as this 
sentence implies) -
  "To address the possibility that TCP did not perform these functions adequately 
    in a given FCIP deployment context, the FCIP_LEP verifies if these expectations
    are met."

> 
> 6.6.2.2 Errors in FCIP Headers and Discarding FCIP Frames
.....

>    Further, some errors in the encapsulation will result in the FCIP_DE
>    losing synchronization with the FCIP frames in the byte stream

I don't see "frames" here with the uppercase "F" used everywhere else.

Also, one observation is that FCEncap document uses "frames" consistently 
throughout, whereas this document chooses to use uppercase "F" (mostly).

>    1)  Length field validation -- 15 < Length < 545;

I assume "Frame Length" is meant by "Length" above.

> 6.6.2.3 Synchronization Failures
.....
> 
>    If an FCIP_DE determines that it cannot find the next FCIP Frame
>    header in the byte stream entering through the Encapsulated Frame
>    Receiver Portal, the FCIP_DE SHALL either:

S/b "either" w/ "do one of the following".

>    b)  recover synchronization by searching the bytes delivered by the
>        Encapsulated Frame Receiver Portal for a valid FCIP Frame header
>        having the correct properties

Useful to refer to 6.6.2.2 here for "correct properties". 

> 7. Checking FC Frame Transit Times in the IP Network
....
>    requirement in the FC Entity is based on a desire to include the

S/b "in"  w/  "on"

>    Frame. If no synchronized time stamp value is available to accompany
>    the entering FC Frame a value of zero SHALL be supplied.

From this, it isn't clear to me if FCIP *requires* only Synchronized operation.
If so, the draft should also specify how implementations are expected to 
deal with "benign" transitions into and out of the Unsynchronized state (i.e.
transitions happening when no Encapsulated Frames are being received).

>    The FC Entity SHALL
>    verify that the FC Entity it is communicating with on an FCIP Link
>    is using the same synchronized time source as it is, either Fibre
>    Channel services or SNTP server.

I see a chicken-and-egg problem for some implementations:  Since Fibre 
Channel time services are not available until the FCIP Link is successfully 
set up and since timestamp is to be carried on every FC Frame (including 
those Fibre Channel time service exchanges) once the Link is set up, the only 
decent option for an implementation (assuming it suppots only Synchronized
operation) is to rely on SNTP server.  Is this correct?  

If Unsynchronized operation is intended to be disallowed (my earlier 
question), then it seems to me that SNTP is the only option.

> 8. The FCIP Special Frame
.....

> 
>    The FCIP Special Frame SHALL only be sent as the first bytes
>    transmitted in each direction on a newly formed TCP Connection and
>    only one FCIP Special Frame SHALL be transmitted in each direction.

A general comment on this wording (and others similar to this).   I would
suggest rewording (to be much stronger) to:

The FCIP Special Frame SHALL be the first application data exchanged on 
each newly formed TCP connection, and only one FCIP Special Frame SHALL 
be transmitted in each direction.

>    Note: The combination of the Source FC Entity World Wide Name and
>    Source FCIP Entity Identifier fields uniquely identifies every FC
>    Entity FCIP Entity pair in the IP Network.

- S/b "Source FCIP Entity Identifier" w/ "Source FC/FCIP Entity Identifier"

- Also I take it that the "FC/FCIP Entity Identifier" is unique only within the
  scope of the given FC Entity's WWN.  So, does the model allow multiple
  FCIP Entities to be associated with the FC Fabric Entity WWN?

- From this statement, it implies to me that the Destination FC/FCIP Entity
  Identifier must be present in the special frame to ensure that the TCP connection
  is indeed established to the right "FC/FCIP Entity" under the scope of the 
  given Destination FC Fabric Entity WWN.  What am I missing?

>    The Destination FC Fabric Entity World Wide Name field MAY contain

Why isn't the above requirement a SHALL?

> 
> 9.1.2.2 Dynamic Creation of New TCP Connections
...

>     - The expected Destination FC Fabric Entity World Wide Name of the
>       FC Entity FCIP Entity pair to which the TCP Connection is being
>       made
>     - TCP Connection Parameters (see section 9.3)
>     - Quality of Service Information (see section 11)
> 
>    Based on this information, the FCIP Entity SHALL generate a TCP
>    connect request [8] to the FCIP Well-Known Port of 3225 (or other
>    configuration specific port number) at the IP Address specified by
>    the service advertisement. If the TCP connect request is rejected,
>    act to limit unnecessary repetition of attempts to establish similar
>    connections. If the TCP connect request is accepted, the FCIP Entity
>    SHALL follow the steps described in section 9.1.2.3 to complete the
>    establishment of a new FCIP_DE.
> 
>    It is recommended that an FCIP Entity not initiate TCP connect
>    requests to another FCIP Entity if incoming TCP connect requests
>    from that FCIP Entity have already been accepted.

This entire text is duplicated from previous section 9.1.2.1.  Seems like 
we could do with one instance (possibly in a new subsection).

> 
> 9.1.2.3 Connection Setup After a Successful TCP Connect Request
>
......
>All fields in the FCIP Special
>    Frame SHALL be filled in as described in section 8, particularly:

While the sentence above is unequivocally clear, I don't understand the need
for all the text that follows "particularly"....  It is confusingly repetitive since as far 
as I can tell, all these are covered in more or less the same language in section 8.

>    After the FCIP Special Frame bytes are sent on the newly formed
>    connection, the FCIP Entity SHALL wait for the FCIP Special Frame to
>    be echoed as the first bytes received on the newly formed connection.

- S/b "bytes are" w/ "is"
- S/b "first bytes" w/ "first TCP segment data"
- I take it that the onus is on the FCIP Entity that did the TCP active open 
  to send the SF.  That leads me to: What if there's a TCP simultaneous open?
  Would not each end end up sending the SF and waiting for the echo?  Also,
  would not the earlier rule on only one frame being transferred in each
  direction be violated then?
 
> 
>    If the echoed FCIP Special Frame bytes do not exactly match the FCIP
>    Special Frame bytes sent (words 7 through 17 inclusive), the FCIP
>    Entity SHALL close the TCP Connection and notify the FC Entity with
>    the reason for the closure.

Seems like all the 11 words are required to be compared. If so, what is the
Ch bit being used for?  IOW, why SHALL it be set by the responder?

> 9.1.3 Processing Incoming TCP Connect Requests
....
> 
>    The FCIP Entity SHALL listen for new TCP Connection requests [8] on
>    the FCIP Well-Known Port (3225). An FCIP Entity MAY also accept and
>    establish TCP Connections to a TCP port number other than the FCIP
>    Well-Known Port, as configured by the network administrator.

It may be useful to add that this usage is outside the scope of this document.

> 
>    The FCIP Entity SHALL determine the following information about the
>    requested connection:
> 
>     - Whether the requested connection is allowed

I take it that by means not specified in this spec?  If so, it's useful to call 
it out.

>    abort the TCP connect request [8]. If the requested connection is

I was told that "abort" isn't available on all implementations - perhaps
"abort/close" would be better....

>    The FCIP Entity MAY apply a timeout of not less than 90 seconds to
>    the waiting for the FCIP Special Frame bytes and if the timeout
>    expires the FCIP Entity SHALL close the TCP Connection and notify
>    the FC Entity with the reason for the closure.

I am not clear why this notification is required, since the local FC Entity did
not motivate the TCP connection establishment.  If it is intended for logging,
perhaps notifying the Control & Services module would be more appropriate.

>    If the Connection Nonce field contains a value identical to the most
>    recently received Connection Nonce from the same IP Address, the
>    FCIP Entity SHALL close the TCP Connection and notify the FC Entity
>    with the reason for the closure.

Same comment.

>    1)  Leave the Destination FC Fabric Entity World Wide Name field and
>        Ch bit both 0;

So allow the FCIP Link to be established?  It is unclear to me how implementations
adopting this option would be able to prevent unintended FCIP Link formation.

>    2)  Change the Destination FC Fabric Entity World Wide Name field to
>        match FC Fabric Entity World Wide Name associated with the FCIP
>        Entity that received the TCP connect request and change the Ch
>        bit to 1; or

In effect, this is being indirectly allowed as a legal discovery process.  Is
there a DoS concern here?  Also, would revealing the FC WWN be acceptable
in confidentiality terms?

>    3)  Close the TCP Connection without sending any response.

I like this option best, :-)

> 10.2 FC Fabric and IP Network Deployment Models
.....
>        Entities in an equal manner. This varies from a true Client-

S/b "varies" w/ "differs".