Subject: [nfsv4] rfc5667bis open issues

From: Chuck Lever <[email-masked]>
Date: Thu, 22 Sep 2016 15:10:23 -0700

I'm preparing a new revision of rfc5667bis, and a couple of issues have
arisen:

1. Reporting RPC-over-RDMA decoding errors on NFSv4

The current text says the NFS server can return an RPC GARBAGE_ARGS
reply, or an RPC-over-RDMA level ERR_CHUNK reply.

Does this count as a dropped RPC reply, such that an NFSv4 server
would have to drop the connection?

When an NFS server returns one of these responses, does it have to
enter the reply in its DRC ? What if any implications are there for
an NFSv4.1 session (slot retired? ignored?)

2. Skipping Write chunks in an NFSv4 COMPOUND

The current text of rfc5666bis (Section 5.3.2) suggests that when
multiple Write chunks are provided for an RPC, and the responder
doesn't use one of them, it should use that chunk for the next
DDP-eligible XDR data item.

This could be a problem for READ_PLUS. The client has no way of
predicting whether the server will return a CONTENT_DATA arm (which
would consume a Write chunk) or a CONTENT_HOLE arm (which would not).

I think a better approach would be for the binding to specify that
if a Write chunk is provided for a READ_PLUS operation, the server
consumes that chunk:

- by returning CONTENT_DATA in that chunk

- by returning an empty chunk for CONTENT_HOLE

If the client provides an empty chunk in that position, the
READ_PLUS result is returned inline.

Opinions?

--
Chuck Lever

---

From: Chuck Lever <[email-masked]>
Date: Thu, 22 Sep 2016 15:36:39 -0700

> On Sep 22, 2016, at 3:10 PM, Chuck Lever <[email-masked]> wrote:
> 

The problematic text is actually this part of rfc5666bis:
 
4.4.6.2.  Unused Write Chunks

   There are occasions when a requester provides a Write chunk but the
   responder is not able to use it.

   For example, an Upper Layer Protocol may define a union result where
   some arms of the union contain a DDP-eligible data item while other
   arms do not.  The responder is REQUIRED to use requester-provided
   Write chunks in this case, but if the responder returns a result that
   uses an arm of the union that has no DDP-eligible data item, the
   Write chunk remains unconsumed.

   If there is a subsequent DDP-eligible data item, it MUST be placed in
   that unconsumed Write chunk.  The requester MUST provision each Write
   chunk so it can be filled with the largest DDP-eligible data item
   that can be placed in it.

--
Chuck Lever

---

From: David Noveck <[email-masked]>
Date: Thu, 22 Sep 2016 20:51:46 -0400

> would have to drop the connection?

I don't see any reason why it should.  Apart from RPC-over-RDMA, does this
apply to other XDR decode issues on the server? After all, it is getting a
reply, even if the content is essentially "I don't understand the request."

> When an NFS server returns one of these responses, does it
> have to enter the reply in its DRC ?

I should hope not.  The purpose of the DRC is to prevent repeated execution
of non-idempotent requests.  A request that you can't decode does not
change any state on the server.

> What if any implications are there for an NFSv4.1 session
> (slot retired? ignored?)

Why does this have to have to be addressed in 5667bis?  I don't see why XDR
decode errors need to be addressed differently in the RPC-over-RDMA case.

> DDP-eligible XDR data item.

It does say that and the treatment there is limited to the case of multiple
write chunks.  The treatment for the analogous case when there is a single
write chunk is addressed in 4.4.6.2.  Th treatment is the same:  each
DDP-eligible item is matched with the first available write chunk until
either there are no more write chunks or no more DDP-eligible items in the
reply.

> The problematic text is actually this part of rfc5666bis:

I don't see why this is described as "problematic".  Is there a suggestion
that we might change this text to something less problematic?  I don't see
how to do that.  I believe we should leave the text as it is.

Within Version One, there is no way to tie write chunks to particular
DDP-eligible items.  While one might think of that as a problem it is not
one that can be addressed in Version One or in rfc5667bis.

On Thu, Sep 22, 2016 at 6:36 PM, Chuck Lever <[email-masked]> wrote:

> _______________________________________________
> nfsv4 mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/nfsv4
>

---

From: Chuck Lever <[email-masked]>
Date: Fri, 23 Sep 2016 09:37:37 -0700

> On Sep 22, 2016, at 5:51 PM, David Noveck <[email-masked]> wrote:
> 
> I don't see any reason why it should.  Apart from RPC-over-RDMA, does this apply to other XDR decode issues on the server? After all, it is getting a reply, even if the content is essentially "I don't understand the request."
> 
> I should hope not.  The purpose of the DRC is to prevent repeated execution of non-idempotent requests.  A request that you can't decode does not change any state on the server.

The issue is that the language allows ERR_CHUNK to be returned
after a server has processed the RPC request when a client has
not provided adequate Write list or Reply chunk resources to
convey the reply.

In that case, it makes sense for the server to have added the
request to its DRC.

> Why does this have to have to be addressed in 5667bis?  I don't see why XDR decode errors need to be addressed differently in the RPC-over-RDMA case.

ERR_CHUNK is not necessarily an XDR decode error. See above.

If returning ERR_CHUNK in the cases where a result has been
generated but a reply can not be formed is not workable, the
alternative is for the server to drop the connection without
replying, IMO.

Some text could be introduced that suggests that servers take
care to ensure there are enough reply resources before they
begin processing an RPC. That may be challenging for some
server implementations, though.

> It does say that and the treatment there is limited to the case of multiple write chunks.  The treatment for the analogous case when there is a single write chunk is addressed in 4.4.6.2.  Th treatment is the same:  each DDP-eligible item is matched with the first available write chunk until either there are no more write chunks or no more DDP-eligible items in the reply.
> 
> I don't see why this is described as "problematic".  Is there a suggestion that we might change this text to something less problematic?  I don't see how to do that.  I believe we should leave the text as it is.

However, we can say that the default behavior for RPC-over-RDMA
is to skip to the next result, but that there is no prohibition
for ULBs to amend that default. I took that approach below.

> Within Version One, there is no way to tie write chunks to particular DDP-eligible items.  While one might think of that as a problem it is not one that can be addressed in Version One or in rfc5667bis.

There is a way to tie these together. Here is the text I have
adopted for rfc5667bis (revision not yet submitted). First:

2.2.1.  Empty Write Chunks

   Section 4.4.6.2 of [I-D.ietf-nfsv4-rfc5666bis] defines the concept of
   unused Write chunks.  An unused Write chunk is a Write chunk with
   either zero segments or where all segments in the Write chunk have
   zero length.  In this document these are referred to as "empty" Write
   chunks.  A "non-empty" Write chunk has at least one segment of non-
   zero length.

   An NFS client might wish an NFS server to return a DDP-eligible
   result inline.  If there is only one DDP-eligible result item in the
   reply, the NFS client simply specifies an empty Write list to force
   the NFS server to return that result inline.  If there are multiple
   DDP-eligible results, the NFS client specifies empty Write chunks for
   each DDP-eligible data item that it wishes to be returned inline.

   An NFS server might encounter an XDR union result where there are
   arms that have a DDP-eligible result, and arms that do not.  If the
   NFS client has provided a non-empty Write chunk that matches with a
   DDP-eligible result, but the response does not contain that result,
   the NFS server MUST return an empty Write chunk in that position in
   the Write list.

And then, in Section 4.3:
 
   The mechanism specified in Section 5.3.2 of
   [I-D.ietf-nfsv4-rfc5666bis]) is applied here, with some additional
   restrictions.  In the following list, a "READ" operation refers to
   either a READ, READ_PLUS, or READLINK operation.

   o  If an NFS client does not wish to use direct placement for any
      DDP-eligible item in an NFS reply, it leaves the Write list empty.

   o  The first chunk in the Write list MUST be used by the first READ
      operation in an NFS version 4 COMPOUND procedure.  The next Write
      chunk is used by the next READ operation, and so on.

   o  If an NFS client has provided a matching non-empty Write chunk,
      then the corresponding READ operation MUST return its data by
      placing data into that chunk.

   o  If an NFS client has provided an empty matching Write chunk, then
      the corresponding READ operation MUST return its result inline.

   o  If a READ operation returns a union arm which does not contain a
      DDP-eligible result, and the NFS client has provided a matching
      non-empty Write chunk, the NFS server MUST return an empty Write
      chunk in that Write list position.

   o  If there are more READ operations than Write chunks, then any
      remaining READ operations in the COMPOUND MUST return their
      results inline.

This problem has not arisen before for two reasons:

- So far NFSv4 clients do not build COMPOUNDs that contain more
DDP-eligible results than NFSv3 clients use (ie, only one per RPC).

- So far union results are used only in error cases; and when an
operation returns an error, it is always the last one in the
COMPOUND reply.

But now we have NFSv4.2 READ_PLUS. READ_PLUS can return:

- NFS4_CONTENT_DATA, which has an opaque DDP-eligible result
- NFS4_CONTENT_HOLE, which has no DDP-eligible result
- an error status, which has no DDP-eligible result

So we now have a situation where there are operations that can follow
a result that might or might not be returned in a Write chunk.

Suppose you have a COMPOUND that looks like this:

   { SEQUENCE, PUTFH, READ_PLUS(2048), READ_PLUS(9000), GETATTR }

Using the currently proposed scheme, the client must set up two Write
chunks that can receive 9000 bytes, to handle the case where the first
READ_PLUS returns NFS4_CONTENT_HOLE. After the reply is received, the
client has to ensure that the first returned Write chunk is matched to
the second XDR result, which could be troublesome for some
implementations.

What I propose is that if the first READ_PLUS returns NFS4_CONTENT_HOLE,
the server would return an empty first Write chunk. Then the second
READ_PLUS result always lines up with the second Write chunk, which IMO
is much better for clients.

--
Chuck Lever

---

From: David Noveck <[email-masked]>
Date: Sat, 24 Sep 2016 08:11:10 -0400

>The issue is that the language allows ERR_CHUNK to be
> returned after a server has processed the RPC request when
> a client has not provided adequate Write list or Reply chunk
> resources to convey the reply.

I forgot that we made ERR_CHUNK ambiguous, apparently because of reluctance
to add things to the XDR.  There is no pressing need to do this since the
responder is aware of the difference and his management of the DRC can
follow based on his knowledge.

However, in the context of Version Two it would be better if we avoided
those ambiguities, given that we have lots of space
for distinct error codes.

> In that case, it makes sense for the server to have added
> the request to its DRC.

Agree that in the case in which the server has executed the request, it
should add the request to the DRC.  In practical terms, there are not
likely to be cases in which there is a non-idempotent request with a reply
longer than 1K.

As far as the session case, the server will consider the request executed
but the client does not have a reply containing the slot and sequence.  To
deal with that case, he would have to use rdma_xid in the message with the
ERR_CHUNK to get the needed context and so conclude that the slot was
available for reuse.

> What I propose is that if the first READ_PLUS returns
> NFS4_CONTENT_HOLE, the server would return an empty first
> Write chunk. Then the second READ_PLUS result always
> lines up with the second Write chunk, which IMO is much better > for
clients.

I'm OK with this but I think you will need to adjust the text to reflect
the fact that READ_PLUS can return an array of read_plus_content's,
although, in practice, those that return more than one are extremely rare.

On Fri, Sep 23, 2016 at 12:37 PM, Chuck Lever <[email-masked]>
wrote:

> > I don't see any reason why it should.  Apart from RPC-over-RDMA, does
> this apply to other XDR decode issues on the server? After all, it is
> getting a reply, even if the content is essentially "I don't understand the
> request."
> >
> > I should hope not.  The purpose of the DRC is to prevent repeated
> execution of non-idempotent requests.  A request that you can't decode does
> not change any state on the server.
>
> > Why does this have to have to be addressed in 5667bis?  I don't see why
> XDR decode errors need to be addressed differently in the RPC-over-RDMA
> case.
>
> > It does say that and the treatment there is limited to the case of
> multiple write chunks.  The treatment for the analogous case when there is
> a single write chunk is addressed in 4.4.6.2.  Th treatment is the same:
> each DDP-eligible item is matched with the first available write chunk
> until either there are no more write chunks or no more DDP-eligible items
> in the reply.
> >
> > I don't see why this is described as "problematic".  Is there a
> suggestion that we might change this text to something less problematic?  I
> don't see how to do that.  I believe we should leave the text as it is.
>

---

From: Chuck Lever <[email-masked]>
Date: Mon, 26 Sep 2016 13:20:11 -0400

> On Sep 24, 2016, at 8:11 AM, David Noveck <[email-masked]> wrote:
> 
> I forgot that we made ERR_CHUNK ambiguous, apparently because of reluctance to add things to the XDR.  There is no pressing need to do this since the responder is aware of the difference and his management of the DRC can follow based on his knowledge.
> 
> However, in the context of Version Two it would be better if we avoided those ambiguities, given that we have lots of space for distinct error codes.
> 
> Agree that in the case in which the server has executed the request, it should add the request to the DRC.  In practical terms, there are not likely to be cases in which there is a non-idempotent request with a reply longer than 1K.

For NFSv3, that is largely true.

For NFSv4, I believe a large reply to a non-idempotent request is
possible, and may even be common. Any time a client does something
like this:

  { SEQUENCE, PUTFH, SETATTR, GETATTR }

Where the GETATTR requests an ACL or security label, is problematic
if the client does not estimate the reply buffer size correctly.

However, this case is for when the client has a bug; it's not a
case where we expect one or the other side to perform heroic
recovery. ERR_CHUNK would terminate the RPC on the client, which
would very likely return EIO to the application. I think that's
about the sanest outcome we can expect.

> As far as the session case, the server will consider the request executed but the client does not have a reply containing the slot and sequence.  To deal with that case, he would have to use rdma_xid in the message with the ERR_CHUNK to get the needed context and so conclude that the slot was available for reuse.

This feels like something that belongs in 5667bis. I'm no real
expert on session behavior. Can you sketch some text that can be
added?

> > lines up with the second Write chunk, which IMO is much better > for clients.
> 
> I'm OK with this but I think you will need to adjust the text to reflect the fact that READ_PLUS can return an array of read_plus_content's, although, in practice, those that return more than one are extremely rare.

I hadn't realized READ_PLUS returned an array.

If an NFS server is allowed to structure its reply in a way that
the client cannot predict, then I think we'll have to limit the
way READ_PLUS uses DDP. I propose these rules:

- The client can provide no more than one Write chunk if it expects
NFS4_CONTENT_DATA. (No Write chunk or an empty Write chunk, following
the previous rules, would be for when the client predicts that the
reply can go inline).

- If that Write chunk is non-empty, it MUST be large enough to
receive all expected payload bytes in a single NFS4_CONTENT_DATA
element.

- The server uses that Write chunk for the first array element that
has an NFS4_CONTENT_DATA arm.

Then we have a choice, depending on whether it is more desirable
to return data in a single round-trip, or more desirable to preserve
holes. Either:

- If the server finds that the array has grown larger than can be
returned inline or via the supplied Reply chunk, it MUST return
the payload data in a single NFS4_CONTENT_DATA element via the
provided Write chunk.

Or:

- The server MUST return as much payload as it can fit within the
resources provided by the client, and return it as a short READ
result. The client is responsible for retrying the READ_PLUS to
read the remaining payload.

Somehow we have to deal with the case where the server cannot fit
any of the payload in the client-provided resources.

READ_PLUS is actually a poor fit for offloaded DDP anyway. The whole
point of offload is that the client has to do no work; the payload
arrives in its memory without any effort on its part.

I would just as soon require that, on RDMA transports, READ_PLUS
returns only a hole or exactly one contiguous piece of content.

--
Chuck Lever

---

From: Chuck Lever <[email-masked]>
Date: Mon, 26 Sep 2016 13:27:24 -0400

> On Sep 26, 2016, at 1:20 PM, Chuck Lever <[email-masked]> wrote:
> 
>> 
> result.

Sorry, that's not clear. I mean a "short read result": In other
words, the server indicates that it didn't return all the bytes
the client requested.

> The client is responsible for retrying the READ_PLUS to

--
Chuck Lever

---

From: David Noveck <[email-masked]>
Date: Mon, 26 Sep 2016 22:39:01 -0400

> > As far as the session case, the server will consider the request
executed but the client does not have a reply containing the slot and
sequence.  To deal with that case, he would have to use rdma_xid in the
message with the ERR_CHUNK to get the needed context and so conclude that
the slot was available for reuse.

> expert on session behavior. Can you sketch some text that can > be added?

I'll write it assuming it will constitute a new subsection 4.x dealing with
session-related issues.

On Mon, Sep 26, 2016 at 1:20 PM, Chuck Lever <[email-masked]> wrote:

> > I forgot that we made ERR_CHUNK ambiguous, apparently because of
> reluctance to add things to the XDR.  There is no pressing need to do this
> since the responder is aware of the difference and his management of the
> DRC can follow based on his knowledge.
> >
> those ambiguities, given that we have lots of space for distinct error
> codes.
> >
> > As far as the session case, the server will consider the request
> executed but the client does not have a reply containing the slot and
> sequence.  To deal with that case, he would have to use rdma_xid in the
> message with the ERR_CHUNK to get the needed context and so conclude that
> the slot was available for reuse.
>

---

From: Chuck Lever <[email-masked]>
Date: Wed, 28 Sep 2016 11:22:34 -0400

> On Sep 26, 2016, at 10:39 PM, David Noveck <[email-masked]> wrote:
> 
> > expert on session behavior. Can you sketch some text that can be added?
> 
> I'll write it assuming it will constitute a new subsection 4.x dealing with session-related issues.

Thanks, that sounds appropriate. I'll hold the next document
revision until you send me text.

--
Chuck Lever