Subject: [nfsv4] Chunk error changes needed in RFC8166

From: David Noveck <[email-masked]>
Date: Tue, 20 Nov 2018 06:23:43 -0500

I've tried to summarize the needed changes below.  They appear too large
for an erratta and too small for a new RFC.  I think what makes most sense
is to file multiple erratta and see what happens.

The last paragraph of Section 3.4.4 needs to be revised.   As written it
causes confusion by drawing attention to the total rpc length despite the
fact that each chunk represents a single ddp-eligible data item.   Also, it
is not clear what the responder is to do if there is a failure of the
"sanity check".  Suggest replacing the pargraph by the following.

The RPC-over-RDMA version 1 transport protocol does not place a limit
on chunk size.  However, each ULP defines its own set of ddp-eligilble
data itemS and the transpport requires that the length of the data to
be transported must be no larger than the length of the data item to
be transfeered.   When the responder finds a write chunk that is
toOsmall to contain the data to be transferred or a read chunk whose
length does not match that of the corresponding data item, the error
ERR_CHUNK is returned.

 In the seventh paragraph of Section  3.4.5, a number of changes are necessary:

   - The second sentence needs to be replaced by the following:

The value in this field, which is always a multiple of four, MUST be
the byte offset  of a DDP-eligible data item (or zero for the case
discussed in Section 3.5.3).

   - An additional sentence needs to be added, as follows:

When the Position field in a read segment is not vallid, the responder
returns the error ERR_CHUNK.

In section 4.5.2, the first paragraph needs to be revised and clarified.
Suggest replacing it by the following paragrphs:

A receiver might encounter an XDR parsing error that prevents it from
processing the incoming Transport stream.  One examples of such an
error is an invalid value in the rdma_proc field. If the rdma_vers
field contains a recognized value, but an XDR parsing error occurs,
the Responder MUST reply with an RDMA_ERROR procedure and set the
rdma_err value to ERR_CHUNK.

In addition, a receiver might encounter a situation error that
prevents it from processing the incoming Transport stream, even when
the stream conforms to the XDR definition.  Examples of such issues
include an RDMA_NOMSG message where the Read list, Write list, and
Reply chunk are marked not present, a case in which the value of the
rdma_xid field does not match the value of the XID field in the
accompanying RPC message, a read chunk position which is not valid,
and  a chunk whose length is not valid.  In such cases, the Responder
MUST also reply with an RDMA_ERROR procedure and set the rdma_err
value to ERR_CHUNK.

---

From: spencer shepler <[email-masked]>
Date: Tue, 20 Nov 2018 08:19:45 -0800

My suggestion is that you submit multiple errata.  It appears that you have
3 based on the identified sections.

Once we have consensus on the WG alias, submit them and then we should be
able to "approve" the changes via the errata process.

Spencer

On Tue, Nov 20, 2018 at 3:24 AM David Noveck <[email-masked]> wrote:

> The RPC-over-RDMA version 1 transport protocol does not place a limit on chunk size.  However, each ULP defines its own set of ddp-eligilble data itemS and the transpport requires that the length of the data to be transported must be no larger than the length of the data item to be transfeered.   When the responder finds a write chunk that is toOsmall to contain the data to be transferred or a read chunk whose length does not match that of the corresponding data item, the error ERR_CHUNK is returned.
>
> The value in this field, which is always a multiple of four, MUST be the byte offset  of a DDP-eligible data item (or zero for the case discussed in Section 3.5.3)..
>
>
> When the Position field in a read segment is not vallid, the responder returns the error ERR_CHUNK.
>
> A receiver might encounter an XDR parsing error that prevents it from processing the incoming Transport stream.  One examples of such an error is an invalid value in the rdma_proc field. If the rdma_vers field contains a recognized value, but an XDR parsing error occurs, the Responder MUST reply with an RDMA_ERROR procedure and set the rdma_err value to ERR_CHUNK.
>
> In addition, a receiver might encounter a situation error that prevents it from processing the incoming Transport stream, even when the stream conforms to the XDR definition.  Examples of such issues include an RDMA_NOMSG message where the Read list, Write list, and Reply chunk are marked not present, a case in which the value of the rdma_xid field does not match the value of the XID field in the accompanying RPC message, a read chunk position which is not valid, and  a chunk whose length is not valid.  In such cases, the Responder MUST also reply with an RDMA_ERROR procedure and set the rdma_err value to ERR_CHUNK.
>
> _______________________________________________
> nfsv4 mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/nfsv4
>

---

From: Chuck Lever <[email-masked]>
Date: Tue, 20 Nov 2018 12:29:27 -0500

Hi Dave-

> On Nov 20, 2018, at 6:23 AM, David Noveck <[email-masked]> wrote:
> 
> I've tried to summarize the needed changes below.  They appear too large for an erratta and too small for a new RFC.  I think what makes most sense is to file multiple erratta and see what happens.
> 
> The last paragraph of Section 3.4.4 needs to be revised.   As written it causes confusion by drawing attention to the total rpc length despite the fact that each chunk represents a single ddp-eligible data item.   Also, it is not clear what the responder is to do if there is a failure of the "sanity check".  Suggest replacing the pargraph by the following.

IMO the proposed replacement text is not technically incorrect, but
I don't feel that it belongs in the introductory section (3.4.4).

Instead, consider placing the new "write chunk is too small"
requirement in Section 3.4.6, and the new "read chunk length mismatch"
requirement in Section 3.4.5.

(Also note that the terms Read and Write chunk are capitalized
throughout this document).

> 	• The second sentence needs to be replaced by the following:
>> The value in this field, which is always a multiple of four, MUST be the byte offset of a DDP-eligible data item (or zero for the case discussed in Section 3.5.3).

I think we need "MUST be a multiple of four and MUST be the byte offset
of a DDP-eligible data item", otherwise the replacement text weakens
the existing requirement that the value contained in the field must be
a multiple of four.

Here would be an appropriate place to state that the responder replies
with ERR_CHUNK if those conditions are not met.

> 	• An additional sentence needs to be added, as follows:

Or perhaps "the responder MUST return the error ERR_CHUNK." Otherwise
we are in the same position: there would still be no firm requirement
for the server to reject such a malformed Read segment.

> In section 4.5.2, the first paragraph needs to be revised and clarified.  Suggest replacing it by the following paragrphs:

The replacement is technically more precise, but I don't see any
significant error with the existing text. The existing paragraph
is not an exhaustive list of possible errors, but rather only
illustrative.

--
Chuck Lever

---

From: David Noveck <[email-masked]>
Date: Tue, 20 Nov 2018 15:00:35 -0500

>> The RPC-over-RDMA version 1 transport protocol does not place a limit on
chunk size.  However, each ULP defines its own set of ddp-eligilble data
itemS and the transpport requires that the length of the data to be
transported must be no larger than the length of the data item to be
transfeered.   When the responder finds a write chunk that is toOsmall to
contain the data to be transferred or a read chunk whose length does not
match that of the corresponding data item, the error ERR_CHUNK is returned.

> I don't feel that it belongs in the introductory section (3.4.4).

The question then is what do you do with the paragraph it replaces.  i
don't think it
can be left as it is since it is likely to lead to confusion.

> requirement in Section 3.4.5.

I'll look at that.

> throughout this document).

OK.

>> The value in this field, which is always a multiple of four, MUST be the
byte offset of a DDP-eligible data item (or zero for the case discussed in
Section 3.5.3).

> the existing requirement that the value contained in the field must be

I don't think it does.   If an offset is the starttng offset of a
DDp-elgible data item, there is no way it can have an offset that is not a
multuple of four.

How about:

The value in this field MUST be the byte offset of a DDP-eligible data item
(or zero for the case discussed in Section 3.5.3).  As a result, it can
never have a value not evenly divisible by four.

> with ERR_CHUNK if those conditions are not met.

Agree.

> Or perhaps "the responder MUST return the error ERR_CHUNK."

OK.

> Otherwise

OTOH, RFC 2119 says these terms should be used "sparingly" but I'll follow
the
style of the existing document.

>> In section 4.5.2, the first paragraph needs to be revised and
>> A receiver might encounter an XDR parsing error that prevents it from
processing the incoming Transport stream.  One examples of such an error is
an invalid value in the rdma_proc field. If the rdma_vers field contains a
recognized value, but an XDR parsing error occurs, the Responder MUST reply
with an RDMA_ERROR procedure and set the rdma_err value to ERR_CHUNK.
>> In addition, a receiver might encounter a situation error that prevents
it from processing the incoming Transport stream, even when the stream
conforms to the XDR definition.  Examples of such issues include an
RDMA_NOMSG message where the Read list, Write list, and Reply chunk are
marked not present, a case in which the value of the rdma_xid field does
not match the value of the XID field in the accompanying RPC message, a
read chunk position which is not valid, and  a chunk whose length is not
valid.  In such cases, the Responder MUST also reply with an RDMA_ERROR
procedure and set the rdma_err value to ERR_CHUNK.

> significant error with the existing text.

Perhaps it is not an error per se but I feels that the existing text is
unnecessarily confusing in that combines XDR errors wth a number of other
constraints that the protocol imposes on the XDR stream even though the XDR
deffinition is respected.

> The existing paragraph

I think that makes sense for true XDR errors.  it makes sense to have a set
of illustrative examples and it is difficult to be exhaustive.

with regard to the addtional constraints on the transport stream, we have a
set of important constaints and it makes sense to list them, so that
someone who gets this error knows what potential issues the server might be
objecting to, in addition to somethings that does not match the XDR
definition of the transport stream.

On Tue, Nov 20, 2018 at 12:29 PM Chuck Lever <[email-masked]> wrote:

> on chunk size.  However, each ULP defines its own set of ddp-eligilble data
> itemS and the transpport requires that the length of the data to be
> transported must be no larger than the length of the data item to be
> transfeered.   When the responder finds a write chunk that is toOsmall to
> contain the data to be transferred or a read chunk whose length does not
> match that of the corresponding data item, the error ERR_CHUNK is returned.
>
> the byte offset of a DDP-eligible data item (or zero for the case discussed
> in Section 3.5.3).
>
> > In section 4.5.2, the first paragraph needs to be revised and
> clarified.  Suggest replacing it by the following paragrphs:
> processing the incoming Transport stream.  One examples of such an error is
> an invalid value in the rdma_proc field. If the rdma_vers field contains a
> recognized value, but an XDR parsing error occurs, the Responder MUST reply
> with an RDMA_ERROR procedure and set the rdma_err value to ERR_CHUNK.
> >> In addition, a receiver might encounter a situation error that prevents
> it from processing the incoming Transport stream, even when the stream
> conforms to the XDR definition.  Examples of such issues include an
> RDMA_NOMSG message where the Read list, Write list, and Reply chunk are
> marked not present, a case in which the value of the rdma_xid field does
> not match the value of the XID field in the accompanying RPC message, a
> read chunk position which is not valid, and  a chunk whose length is not
> valid.  In such cases, the Responder MUST also reply with an RDMA_ERROR
> procedure and set the rdma_err value to ERR_CHUNK.
>

---

From: Chuck Lever <[email-masked]>
Date: Tue, 20 Nov 2018 15:19:00 -0500

> On Nov 20, 2018, at 3:00 PM, David Noveck <[email-masked]> wrote:
> 
> The question then is what do you do with the paragraph it replaces.  i don't think it

Perhaps not, but I don't believe it's the place to introduce
new requirements, especially when both requirements are
specific to chunk types that are detailed in later sections.
If this section does have to change, let's keep the tone
general and introductory.

I'm not really clear on the scope of an errata. Is it similar
to the kind of rewriting you and I have done in the trunking
documents, or is it narrower?

> I don't think it does.   If an offset is the starttng offset of a DDp-elgible data item, there is no way it can have an offset that is not a multuple of four.
> 
> How about:
> 
> The value in this field MUST be the byte offset of a DDP-eligible data item (or zero for the case discussed in Section 3.5.3).  As a result, it can never have a value not evenly divisible by four.

Makes sense to me.

As I've thought more about this, it occurs to me that the existing
RFC 8166 has precedents for several possible responses to these
problems.

When reporting an error, ERR_CHUNK is one possibility, but some
server implementations may prefer to respond instead with
GARBAGE_ARGS. Perhaps the replacement text should allow for that
additional possibility.

The original purpose of 8166 was to document the behavior of
existing implementations. If those implementations do not check
for these conditions, then we are perhaps venturing beyond the
original mission of this document. In that case, not reporting
the error (and/or not documenting a particular required response)
is another choice we should consider.

> OTOH, RFC 2119 says these terms should be used "sparingly" but I'll follow the

Right, I believe that style is to use MUST when defining the
server's response to error conditions. OK.

> Perhaps it is not an error per se but I feels that the existing text is unnecessarily confusing in that combines XDR errors wth a number of other constraints that the protocol imposes on the XDR stream even though the XDR deffinition is respected. 
> 
> > The existing paragraph
> I think that makes sense for true XDR errors.  it makes sense to have a set of illustrative examples and it is difficult to be exhaustive.
> 
> with regard to the addtional constraints on the transport stream, we have a set of important constaints and it makes sense to list them, so that someone who gets this error knows what potential issues the server might be objecting to, in addition to somethings that does not match the XDR definition of the transport stream.

OK, makes sense.

--
Chuck Lever