Re: [nfsv4] Barry Leiba's Discuss on draft-ietf-nfsv4-rpcrdma-cm-pvt-data-07: (with DISCUSS and COMMENT)

[Chuck Lever <[e-mail]>]

Hi Barry-


> On Feb 13, 2020, at 12:45 AM, Barry Leiba via Datatracker <[e-mail]> wrote:
> 
> Barry Leiba has entered the following ballot position for
> draft-ietf-nfsv4-rpcrdma-cm-pvt-data-07: Discuss
> 
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
> 
> 
> Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> for more information about IESG DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-nfsv4-rpcrdma-cm-pvt-data/
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> Thanks for this document.  This is a simple DISCUSS point that should be very
> easy to resolve:
> 
> — Section 5.2 —
> 
>   A sender computes the encoded
>   value by dividing the buffer size, in octets, by 1024 and subtracting
>   one from the result.
> 
> Is the buffer size necessarily a multiple of 1024?  If so, where is that
> specified?  If not, what is the encoded value when the buffer size is, say,
> 2000?  Is it zero?  Or one?

Good catch! Buffer sizes are not constrained to 1024-byte length alignment.

Further, if a sender posts, say, a 2032-byte message to a receiver that uses
a 2000-byte buffer, a Receive error occurs that typically results in
connection loss.

IMO Section 5.2 should instruct the sender to round the actual buffer length
down to the nearest 1024-byte multiple before encoding. Would that clarify
the issue for you?


> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> Some purely editorial comments:
> 
> — Abstract —
> The abstract needs to stand alone, so you should expand the term RDMA-CM in the
> abstract.  (RPC doesn’t need expanding, so once you’ve expanded RDMA-CM,
> RPC-over-RDMA should be OK.)
> 
> — Introduction —
> Please expand “XDR” on first use.
> 
>   Section 7 of the current document
> 
> “of this document” is better, I think.
> 
> — Section 3.2 —
> Please expand “RNIC” and “STag”.
> 
>   invalidation without the need for additional protocol to be defined.
> 
> Either “an additional protocol” or “additional protocols”.
> 
> — Section 4.1 —
> 
>   Realizing these goals
>   require that implementations of this extension follow the practices
> 
> The subject is “realizing”, which is singular.  So, “requires’.

Agreed to all.


> — Section 5.1 —
> 
>   Bits 14 - 8:  These bits are reserved and are always zero when the
>      Version field contains 1.
> 
> In other protocols, leaving it unspecified as to what happens if not all
> reserved bits are zero has caused interoperability problems.  If you know
> that’s not a concern here, that’s fine.  Otherwise, it might be good to say
> explicitly that either they are ignored on receipt or non-zero bits result in
> an error.

Gotcha. I can add a sentence that states "When the Version field contains 1,
receivers MUST ignore reserved bits".

I will address all these comments in a subsequent revision.

--
Chuck Lever