Subject: [nfsv4] Clean up for rpcrdma-version-two Read chunks

From: Chuck Lever <[email-masked]>
Date: Mon, 4 May 2020 12:13:01 -0400

Two weeks ago, I presented on the status of draft-ietf-nfsv4-rpcrdma-version-two.
The slides are here:

https://datatracker.ietf.org/meeting/interim-2020-nfsv4-01/materials/slides-interim-2020-nfsv4-01-sessa-ietf-107-rpc-rdma-v2.pdf

On slide 18, I described three situations involving Read chunks that do not appear
to be adequately covered by RFC 8166 ("Remote Direct Memory Access Transport for
Remote Procedure Call Version 1"). I'd like to propose modifications to the
rpcrdma-version-two draft to address these cases:

> RPC/RDMA v1 allows a position zero Read chunk to appear in an RDMA_MSG type Call.
> Where does a Responder put the inline portion of such a message?

I propose that in RPC/RDMA version 2, a Responder MUST return RDMA2_ERR_BAD_XDR if
a Requester sends a Read list containing a position zero Read chunk as part of
header type other than RDMA2_NOMSG.

> RPC/RDMA v1 does not explicitly require an RDMA_NOMSG type Call to have a position
> zero Read chunk. Does such a message have gaps? Are they zero-filled?

I propose that in RPC/RDMA version 2, a Responder MUST return RDMA2_ERR_BAD_XDR if
a Requester sends an RDMA2_NOMSG header type whose Read list does not include a
position zero Read chunk.

> RPC/RDMA v1 does not prevent or prohibit overlapping Read chunks. Is the correct
> response ERR_CHUNK?

A protocol change would be needed to totally prevent the expression of overlapping
Read chunks. Maybe it's a little too late to address that in RPC/RDMA version 2.

I propose that in RPC/RDMA version 2, a Responder MUST return RDMA2_ERR_BAD_XDR if
a Requester sends a Read list with chunks whose offsets and lengths result in the
same message byte position appearing in more than one Read chunk.

Comments/concerns?

--
Chuck Lever

---

From: David Noveck <[email-masked]>
Date: Mon, 4 May 2020 20:59:29 -0400

> header type other than RDMA2_NOMSG.

Agree.

> position zero Read chunk.

As stated, this would forbid NOMSG bring used to send a long reply.  I
think the text to address this needs to be careful not to foreclose that.
Your text above uses the word "requester" assuming this is sufficient but
the only way a peer receiving message could determine whether it was sent
by a requester or responder is my looking at the message, which,in this
case, does not exist.

> Read chunks. Maybe it's a little too late to address that in RPC/RDMA

I think you mean version 1.  Nobody seems up to do rfc8166bis.

> same message byte position appearing in more than one Read chunk.

This would require sorting by the receiver. It might be better to place
responsibility on the sender to sort these.

> Comments/concerns?
>
> --
> Chuck Lever
>
>
>
> _______________________________________________
> nfsv4 mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/nfsv4
>

---

From: Chuck Lever <[email-masked]>
Date: Tue, 5 May 2020 02:21:02 -0400

> On May 4, 2020, at 8:59 PM, David Noveck <[email-masked]> wrote:
> 
> As stated, this would forbid NOMSG bring used to send a long reply.

Nit: rpcrdma-version-two no longer uses the term Long message, see
Section 4.4.3.

> I think the text to address this needs to be careful not to foreclose that. Your text above uses the word "requester" assuming this is sufficient but the only way a peer receiving message could determine whether it was sent by a requester or responder is my looking at the message, which, in this case, does not exist.

The RPC/RDMA version two header has the RDMA2_F_RESPONSE flag (Section
6.2.2.1) which was introduced to enable a receiver to distinguish the
roles of the sender and receiver peers without sniffing the RPC layer
payload.

A while back I had envisioned using Read chunks in Responder-to-
Requester messages, and even wrote an I-D about it. But now that we
have both Reply chunks and Message chaining, it seems unnecessary to
hold the door open for using a Read chunk for a Reply, especially
given how arcane Read chunks are. Did you have a particular use case in
mind?

Also, I think the proposed text above would prevent the use of the
RDMA2_NOMSG type for asynchronous credit grants (Section 4.2.1.2) so I
ought to restate the requirement as:

>> A Responder MUST return RDMA2_ERR_BAD_XDR if a Requester sends an
>> RDMA2_NOMSG header type with a non-empty Read list that does not
>> include a position zero Read chunk.

> I think you mean version 1.

No, I meant version 2. My sense of the virtual room two weeks ago was
that no-one had the stomach for major surgery on the RPC/RDMA version 2
data structures at this point. That's why I've limited the above proposals
to simple requirements that the Responder recognize badly formed Read
lists and respond to them as errors.

If we were to "go there," my thought about how to address the gap/overlap
issue would be to eliminate Read chunks and structure the Read list the
same way that the Write list is structured; ie, as a list of arrays of
RDMA segments, but each array would have a position field.

It might be interesting to use position fields in the Write list as well,
filled in by the Responder, to help disambiguate the position of result
data items in a Reply message.

But at this point we have escaped the orbit of RPC/RDMA version one entirely.

> Nobody seems up to do rfc8166bis.
> 
> This would require sorting by the receiver.

It would indeed. The alternative is kind of disastrous.

Gaps would probably contain data from a previous use of the Read sink
buffer.

The exact contents of overlap regions would depend on the order that the
RNIC completed the RDMA Read operations.

Thus IMO a good quality Responder has to perform some sanity checking on
the position values and lengths in incoming Read segments. I'm not sure
how it could avoid some sorting-like behavior to perform this check.

> It might be better to place responsibility on the sender to sort these.

IMO the goal of RPC/RDMA is to reduce host processing on the Requester.
Sorting on the Responder follows that paradigm more closely than the
converse.

For a given position value, RPC/RDMA already requires that Read segments
have to be in the order they appear in the reconstructed RPC message. But
there's currently no requirement that the Read list's position values have
to appear in monotonically increasing order. In fact I think RPC/RDMA
permits a Requester to interleave Read segments at different positions, as
long as they are in the Read list in the order they should be used to
reconstruct the RPC Call.

I'm working on some changes to the Linux NFS/RDMA implementation that might
perform Responder-side Read list sorting in order to deal properly with
Read lists that contain segments with more than one Position value. For
example, a Read list that contains Read segments with position zero and
Read segments with a non-zero position could be re-sorted so that all of
the segments are in byte order at position zero. This enables the Responder
to set up the Read sink buffer pages so at Read completion the message is
already in proper segment and byte order.

--
Chuck Lever

---

From: David Noveck <[email-masked]>
Date: Tue, 5 May 2020 15:04:05 -0400

>It would indeed.

It would require that if they could arrive unsorted.

> The alternative is kind of disastrous.

I think a reasonable alternative is to ask the sender to send them in
order.  I understand you don't agree with that but can't see how it is
"disastrous".

> Gaps would probably contain data from a previous use of the Read sink

I don't see how.  If you have multiple read chunks, it is probably due to
multiple operations, each of which contain a DDP-eligible data area, since
there is no op with more than one DDp-eligible argument. In this cse the
non-DDP_eligible data appears inline.

I guess you are focused on the case of a mix of PZRCS and PNZRCS, which is
kind of a special case.

> RNIC completed the RDMA Read operations.

I think we agree about overlapping chunks being bad but only disagree about
how best to prevent them.

> the position values and lengths in incoming Read segments.

I Agree.

> I'm not sure

That's true only if it is valld to send them out-of-order which I don't
think
should be the case.

> It might be better to place responsibility on the sender to sort these.

I should have said "send these in sorted order".  Realistically, requesters
will send these in sorted order and no actual sort would be required.

> IMO the goal of RPC/RDMA is to reduce host processing on the Requester.

That's an important goal but don't agree that it is *the* goal.

> Sorting on the Responder follows that paradigm more closely than the

I think requring them being in sorted order avoids a lot of complexity.
I haven't seen any case where it makes sense to send them other than in
sorted order.

With regard to this goal/paradigm, alll I can say is that T. S. Kuhn has
lot to answer for :-)

> reconstruct the RPC Call.

True but I don't see why any requester would find it necessary/helpful
 to interleave read segments in this way, except perhaps in the special case
a PZRC/PNZRC mix.

> the segments are in byte order at position zero.

You are talking about doing some thing that goes beyond sorting, even though
sorting is a part of it and it appears to me that you are sorting by
someting
apporoximating chunk position.
You are reorganizing all the read chunks including
changing non-PZ chunks to position-zero chunks.   In addition the sorting
is not by
the position field of the chunk but by expected position.  I can see why
you moght
do if you receive somethning like that that but I can't see why the
reqester is using a
PZRC to send this, given that, in version two, you can avoid the PZRC and
use
message continuation to send the request.

> This enables the Responder

The problem with this is that when some of non-position-zero read chunks
are to be written
to file offsets not a multiple of the page size.  the alignment of the data
in serrver memory
is not what you would want itto be.   I think it is better, when allocating
the pages to be
read-into and ten page flipped as part ofthe WRITEs, to be aware of the
within-file offsets
of the WRITEs.

On Tue, May 5, 2020 at 2:21 AM Chuck Lever <[email-masked]> wrote:

> > I think the text to address this needs to be careful not to foreclose
> that. Your text above uses the word "requester" assuming this is sufficient
> but the only way a peer receiving message could determine whether it was
> sent by a requester or responder is my looking at the message, which, in
> this case, does not exist.
>

---

From: Chuck Lever <[email-masked]>
Date: Tue, 5 May 2020 15:43:27 -0400

> On May 5, 2020, at 3:04 PM, David Noveck <[email-masked]> wrote:
> 

Even if the Requester is "REQUIRED to send these in order" there can always be Requester implementation bugs
which mis-order the segments. That's why the Responder implementation has to be defensive anyway.

> I think a reasonable alternative is to ask the sender to send them in order.  I understand you don't agree with that but can't see how it is "disastrous".

I don't have a problem with suggesting or requiring the sender to organize the Read list in a particular order. I just don't
think it's a full solution. The possibility of Requester bugs or malicious intent necessitates that Responders be protective
anyway. A Read list ordering requirement doesn't make the need for Responder defensiveness go away. At the very
least the Responder still has to recognize mis-sorted Read lists and bat them down.

It's the difference between "the protocol requires receivers to recognize and reject badness" versus "the protocol makes
it impossible to express badness".

> I don't see how.  If you have multiple read chunks, it is probably due to multiple operations, each of which contain a DDP-eligible data area, since there is no op with more than one DDp-eligible argument. In this cse the non-DDP_eligible data appears inline.
> 
> I guess you are focused on the case of a mix of PZRCS and PNZRCS, which is kind of a special case.

For the foreseeable future, it is a common special case. This case arises frequently for krb5, for example.

> That's true only if it is valld to send them out-of-order which I don't think
> should be the case.
> 

True enough for Requesters we know of with sane implementations. However,
as we well know, that doesn't mean this will always be the case. We need to
somehow document our assumptions and requirements here.

> I haven't seen any case where it makes sense to send them other than in sorted order.
> 
> With regard to this goal/paradigm, alll I can say is that T. S. Kuhn has lot to answer for :-)
> 

Again, that's the particular case that is likely to be frequent and impact RPC/RDMA v1
as well as v2.

> sorting is a part of it and it appears to me that you are sorting by someting
> changing non-PZ chunks to position-zero chunks.   In addition the sorting is not by
> the position field of the chunk but by expected position.  I can see why you moght
> do if you receive somethning like that that but I can't see why the reqester is using a 
> PZRC to send this, given that, in version two, you can avoid the PZRC and use 

A v2 Requester is now able to use Message chaining but it is currently not /required/ to use it.
A v1 Requester is still allowed to send a PZRC and a normal Read chunk.

RPC/RDMA v2 would have to explicitly forbid the use of PZRCs with normal Read chunks.
Is there a sane way to do that?

We could require that RDMA2_NOMSG includes only position zero Read segments.

For situations that call for normal Read chunks with an inline body that is larger than the
transport's inline threshold, the transport is required then to use RDMA2_MSG and Message
chaining.

If the transport's Message chaining limits can't accommodate that arrangement, then the
Requester has to use a PZRC (ie, no normal Read chunks).

--
Chuck Lever

---

From: Chuck Lever <[email-masked]>
Date: Thu, 7 May 2020 11:54:53 -0400

Hi Dave-

I've mocked up the proposed changes in the Editor's Copy of this document.
The changes add similar requirements for NOMSG and Reply chunks.

Here is the full text:

https://chucklever.github.io/i-d-rpcrdma-version-two/draft-ietf-nfsv4-rpcrdma-version-two.html

Here is the raw text of the new requirements:

Requirement A: the Read list must be in ascending order

Added to Section 4.3.5

   If a Responder receives a Read list whose RDMA segment position
   values do not appear in monotonically increasing order, it MUST
   discard the message without processing it and respond with an
   RDMA2_ERROR message with the rdma_xid field set to the XID of the
   malformed message and the rdma_err field set to RDMA2_ERR_BAD_XDR.
   If a Requester receives a Read list whose RDMA segment position
   values do not appear in monotonically increasing order, it MUST
   discard the message without processing it and terminate the RPC
   transaction that corresponds to the XID value in the rdma_xid
   field of the malformed message.

Requirement B: NOMSG is the only header type that can carry a PZRC or Reply chunk

Added to Section 4.4.3

   If a Responder receives an RPC-over-RDMA message that carries a
   Position Zero Read chunk and whose RDMA2_F_RESPONSE flag is clear but
   the message does not use the RDMA2_NOMSG header type, it MUST discard
   that message without processing it and respond with an RDMA2_ERROR
   message with the rdma_xid field set to the XID of the malformed
   message and the rdma_err field set to RDMA2_ERR_BAD_XDR.  When a
   Requester receives an RPC-over-RDMA message that carries a Reply
   chunk and whose RDMA2_F_RESPONSE flag is set but the message does not
   use the RDMA2_NOMSG header type, it MUST discard that message without
   processing it and terminate the RPC transaction corresponding to the
   XID value in the message's rdma_xid field.

Requirement C: A NOMSG header type with a non-empty Read list must have a PZRC

Added to Section 6.3.2

   When a Responder receives an RDMA2_NOMSG header type with a non-empty
   Read list that does not carry a Position Zero Read chunk, it MUST
   discard that message without processing it and send an RDMA2_ERROR
   message with the rdma_err field set to RDMA2_ERR_BAD_XDR.  When a
   Requester receives an RDMA2_NOMSG header type with an empty Reply
   chunk, it discards that message and terminates the RPC transaction
   represented by the XID value in the rdma_xid field of the malformed
   message.

I have not addressed this one yet:

> On May 4, 2020, at 12:13 PM, Chuck Lever <[email-masked]> wrote:
> 

There is still a need, IMO, for the receiver to watch out for overlapping
read segments when there is more than one Read chunk in the list. As an
example:

Given an RPC Call with two normal Read chunks, each chunk has one read
segment, presented in ascending order of position:

Inline
Length: 512 bytes

Read segment A
Position: byte 32
Length: 128 bytes

Read segment B
Position: byte 144
Length: 64 bytes

Read segment B's starting position (144) is inside Read segment A.

Perhaps a way to address overlaps would be to change the segment position
value to mean the byte offset within the inline body. A value of zero
still would have the special meaning of "this chunk replaces the inline
body".

--
Chuck Lever