Subject: [nfsv4] preliminary review of draft-cel-nfsv4-reminv-design

From: David Noveck <[email-masked]>
Date: Thu, 4 Aug 2016 10:48:34 -0400

The first issue concerns the structure rpcrdma2_chunk_lists.  Although this
structure is defined in draft-cel-nfsv4-rpcrdma-version-two-01, there is
nothing that uses it.  The structure that RDMA_MSG and RDMA_NOMSG include
is rpcrdma2_chunks, which is not defined in the XDR.  It appear that this
issue has existed since draft-cel-nfsv4-rpcrdma-version-two-00.

Although that issue can and should be fixed, I think we need to have a more
complete discussion of the extension model for version two and for
RPC-over-RDMA as a whole in the near term.

As I understand things, the original intention was that Version Two be a
compatible extension of Version One.  Now, with the inclusion on the
direction indication added in -01, and now the R-key proposed in
reminv-design, Version One and Version Two messages will be
OTW-incompatible, and I think are better off with a model in which Version
Two consists of  subset which equal to Version One and a set of additions,
primarily optional.  I think we should diverge from that model only if the
benefits are sufficiently important to justify this discontinuity.

With these changes, the implementation barrier to convert a Version One
implementation to be compatible Version Two becomes significant.  I think
we would be better off, if most Version One implementations could be made
compatible with very easily, making the decision to do so more or less
automatic.

To get back to remote invalidation, I would prefer your section 3.3 as a
baseline to be made accessible with no XDR changes in the base Version
Two.  The additional functionality provided by 3.4, while desirable, is not
of sufficient benefit to justify a non-compatible XDR change.  I feel that
this functionality should be available as an OPTIONAL extension.

When I say that it should be an "OPTIONAL Extension", I don't mean to imply:

   - That it needs to be implemented as a subcase of RDMA_OPTIONAL.
   - That it should be documented in a separate document, as opposed to
   being documented (eventually) in draft-ietf-nfsv4-rpcrdma-version-two.

What I do mean is that we should define new message type for extensions (so
that we maintain Version One as a subset of  Version Two) and that we
should (not "SHOULD" :-) make these new message types "OPTIONAL" (in the
RFC2119 sense).  I can see, if there is sufficient reason, making an
extension REQUIRED, but I don't see a reason to change an existing message
type in an incompatible way.

One way to do this is to define new message types RDMA_MESSAGEX and
RDMA_NOMSGX which include direction and rdma_handle but there are other
ways to do this.  To make it easier to determine whether support for
OPTIONAL message types is present, we could define a transport
characteristic/attribute that provides a bit mask of supported message
types.

---

From: Chuck Lever <[email-masked]>
Date: Thu, 4 Aug 2016 12:35:39 -0400

Hi Dave-

Thanks for your review comments.

> On Aug 4, 2016, at 10:48 AM, David Noveck <[email-masked]> wrote:
> 
> The first issue concerns the structure rpcrdma2_chunk_lists.  Although this structure is defined in draft-cel-nfsv4-rpcrdma-version-two-01, there is nothing that uses it.  The structure that RDMA_MSG and RDMA_NOMSG include is rpcrdma2_chunks, which is not defined in the XDR.  It appear that this issue has existed since draft-cel-nfsv4-rpcrdma-version-two-00. 
> 
> Although that issue can and should be fixed, I think we need to have a more complete discussion of the extension model for version two and for RPC-over-RDMA as a whole in the near term.
> 
> As I understand things, the original intention was that Version Two be a compatible extension of Version One.  Now, with the inclusion on the direction indication added in -01, and now the R-key proposed in reminv-design, Version One and Version Two messages will be OTW-incompatible, and I think are better off with a model in which Version Two consists of  subset which equal to Version One and a set of additions, primarily optional.  I think we should diverge from that model only if the benefits are sufficiently important to justify this discontinuity.

Yes, the original intention was to be as compatible as possible,
and any XDR divergence with Version One would not be undertaken
lightly. That preference is why we have elected to defer changes
like this one to extensions when it makes sense.

However, with a version number bump, we are no longer entirely
shackled by existing implementations or XDR definitions. It's
very easy to take that too far, of course, which is why the
default choice is to extend rather than alter the base XDR.

In this case, however, I think the benefits outweigh the costs
of altering the base XDR.

> With these changes, the implementation barrier to convert a Version One implementation to be compatible Version Two becomes significant.  I think we would be better off, if most Version One implementations could be made compatible with very easily, making the decision to do so more or less automatic.

In Linux, it's simply a matter of adding an "if vers == 2,
insert (or expect) two u32 fields here." With an rpcgen-based
implementation, the change is also fairly trivial (in fact most
of it is handled by machine-generated code).

Speaking as an implementer, having to support two new RDMA
message types (in addition to RDMA_OPT_INIT_XCHAR) is much more
effort than having to deal with one or two extra fields in
rpcrdma2_chunk_lists.

> To get back to remote invalidation, I would prefer your section 3.3 as a baseline to be made accessible with no XDR changes in the base Version Two.  The additional functionality provided by 3.4, while desirable, is not of sufficient benefit to justify a non-compatible XDR change.  I feel that this functionality should be available as an OPTIONAL extension.

The benefit of the new field can be described this way:

RPC-over-RDMA allows multiple handles per RPC.

The burden of selecting a handle to invalidate remotely should
be the client's. I believe one existing client implementation
does mix persistently registered handles with dynamically
registered handles in the same RPC. A client can't use RI at
all if it can't tolerate an arbitrary choice of which handle
in an RPC is invalidated remotely.

The "big switch" approach is simply not generic enough when
multiple handles are in play and we don't have control over
client implementation choices. It would exclude a portion of
implementations, limiting the appeal of RPC-over-RDMA Version
Two.

In the case of SMB Direct, all implementers decided that they
would go with all FRWR registration. A big switch works in
that scenario. In fact, the switch is always "on".

Providing rdma_inv_handle in each RPC-over-RDMA header goes
along with the design of having lists of segments, each with
their own handle.

Should the protocol be designed to discourage implementations
that need to communicate handles on a per-RPC basis in favor
of ones that can work with just an exchange of a transport
characteristic?

> 	• That it needs to be implemented as a subcase of RDMA_OPTIONAL.
> 	• That it should be documented in a separate document, as opposed to being documented (eventually) in draft-ietf-nfsv4-rpcrdma-version-two.
> What I do mean is that we should define new message type for extensions (so that we maintain Version One as a subset of  Version Two) and that we should (not "SHOULD" :-) make these new message types "OPTIONAL" (in the RFC2119 sense).  I can see, if there is sufficient reason, making an extension REQUIRED, but I don't see a reason to change an existing message type in an incompatible way.
> 
> One way to do this is to define new message types RDMA_MESSAGEX and RDMA_NOMSGX which include direction and rdma_handle but there are other ways to do this.  To make it easier to determine whether support for OPTIONAL message types is present, we could define a transport characteristic/attribute that provides a bit mask of supported message types.

(An xchar that carries a bitmap of supported message types seems
appropriate in the initial set of supported characteristics.)

IIRC, earlier we had decided that message types RDMA_MSG and
RDMA_NOMSG would be REQUIRED. Thus to support Remote Invalidation
in full, an implementation would need to support RDMA_MSG,
RDMA_NOMSG, RDMA_OPT_INIT_XCHAR, RDMA_MESSAGEX, and RDMA_NOMSGX.

But now we are talking about significant XDR changes, and a
significant implementation effort.

--
Chuck Lever

---

From: David Noveck <[email-masked]>
Date: Thu, 4 Aug 2016 16:49:40 -0400

> of altering the base XDR.

Whether this is the case depends on the proportion of implementations that
are
adequately supported by a simpler approach that does not provide the
extended
functionality of allowing per-request selection of invalidation approach.

If 80% or 90% or 95% of implementations are OK with the simpler
implementation,
the balance of costs and benefits are different than they would be in 30%,
50%, or
60% need the'more flexible approach.  I think we need to hear from the rest
of the
working group about what they think is important here.

> A client can't use RI at

The question is how common such clients are.

> insert (or expect) two u32 fields here."

In lots of places.

> With an rpcgen-based

Essentially you have two implementation rather than one extensible one.

> rpcrdma2_chunk_lists.e problems with the simpler approach.

That would be the case if every implementer had to support those OPTIONAL
messages types.  If only a few would, the situation would be different.

Those who did not need the more extensive support would not have to deal
with any
XDR change at all.

> Thus to support Remote Invalidation

so this all boils down to the question of how many implementations want or
need to support
this extended form of remote invalidation.

On Thu, Aug 4, 2016 at 12:35 PM, Chuck Lever <[email-masked]> wrote:

> > The first issue concerns the structure rpcrdma2_chunk_lists.  Although
> this structure is defined in draft-cel-nfsv4-rpcrdma-version-two-01,
> there is nothing that uses it.  The structure that RDMA_MSG and RDMA_NOMSG
> include is rpcrdma2_chunks, which is not defined in the XDR.  It appear
> that this issue has existed since draft-cel-nfsv4-rpcrdma-version-two-00.
> >
> > Although that issue can and should be fixed, I think we need to have a
> more complete discussion of the extension model for version two and for
> >       • That it should be documented in a separate document, as opposed
> to being documented (eventually) in draft-ietf-nfsv4-rpcrdma-version-two.
> > What I do mean is that we should define new message type for extensions
> (so that we maintain Version One as a subset of  Version Two) and that we

---

From: Chuck Lever <[email-masked]>
Date: Fri, 5 Aug 2016 12:45:56 -0400

> On Aug 4, 2016, at 4:49 PM, David Noveck <[email-masked]> wrote:
> 
> Whether this is the case depends on the proportion of implementations that are 
> adequately supported by a simpler approach that does not provide the extended
> If 80% or 90% or 95% of implementations are OK with the simpler implementation, 
> the balance of costs and benefits are different than they would be in 30%, 50%, or 
> 60% need the'more flexible approach.  I think we need to hear from the rest of the 

What I'd like to hear from others on the WG and other implementers
is: XDR churn aside, are there any other technical reasons that
the per-RPC approach is inferior to the "big switch" approach?

> The question is how common such clients are.

Not exactly. There are two confounding factors that make the
real answer slippery:

- Future innovation
- Clients that can use "big switch" but prefer to use per-RPC

But OK, let's consider only current client implementations:

Today there are three well-known diaspora clients: Linux,
Solaris, and Oracle's dNFS. As far as I know, as they are
currently implemented, two of these would have to use a
per-connection switch to disable RI entirely.

In V1, in any RPC, a requester is free to provide one or more of:

Persistently registered rkeys:
1. the local DMA rkey
2. an rkey that is created for that connection, but that
    doesn't change from RPC to RPC (for a utility buffer, say)
2a. an rkey for a fixed memory region that is updated before
     each time it is advertised

Dynamically registered rkeys:
3. an rkey that is registered not using FRWR just for that RPC
4. an rkey that is registered using FRWR just for that RPC

Any combination is allowed in one RPC, or during a stream of
RPCs on a connection, but only type 4 rkeys may be invalidated
remotely.

Linux clients can use 1, 3, or 4, and do not mix these modes
in one RPC or in a stream of RPCs on one connection. Thus a
per-connection switch happens to be adequate for Linux to
support RI.

Solaris uses 2 or 3 or both in one RPC or in a stream of RPCs.
To use RI, Solaris would need to use 4 instead of 3, but even
then a per-connection switch would not allow Solaris to use RI.

Oracle dNFS uses FMR, and runs only in user space. I think
that means it can't use RI at all. But that's OK since it
uses RDMA only for large payloads, thus RI wouldn't save much.
A per-connection switch works here too, to disable RI.

But I don't believe it's a matter of percentages here. Of the
two current implementations that can use RI, Solaris, a premier
implementation of NFS/RDMA, cannot tolerate responder's choice,
and thus a per-connection switch is not adequate for it to use
Remote Invalidation at all.

> > insert (or expect) two u32 fields here." 

In a smart implementation, lots = 2. Forward and backchannel
call, and forward and backchannel reply.

--
Chuck Lever

---

From: karen deitke <[email-masked]>
Date: Wed, 10 Aug 2016 13:12:51 -0600

> The question is how common such clients are.
Solaris can represent one such client.  Currently we do not implement 
frwr which is needed for RI, however when we do, we would not be able to 
utilize RI if the client can not identify which chunks should be 
invalidate.  Random invalidation by the server would be detrimental on 
solaris as it would potentially also invalidate other memory buffers 
sharing the same  stag. Additionally it could invalidate other memory we 
intend to keep registered.

> Those who did not need the more extensive support would not have to 
> deal with any
> XDR change at all.
>
> so this all boils down to the question of how many implementations 
> want or need to support
This is a need for solaris.

Karen
>
>
>
>
> On Thu, Aug 4, 2016 at 12:35 PM, Chuck Lever <[email-masked] 
> <mailto:[email-masked]>> wrote:
>
>     Hi Dave-
>
>     > On Aug 4, 2016, at 10:48 AM, David Noveck <[email-masked]
>     <mailto:[email-masked]>> wrote:
>     >
>     > The first issue concerns the structure rpcrdma2_chunk_lists. 
>     Although this structure is defined in
>     draft-cel-nfsv4-rpcrdma-version-two-01, there is nothing that uses
>     it.  The structure that RDMA_MSG and RDMA_NOMSG include is
>     rpcrdma2_chunks, which is not defined in the XDR.  It appear that
>     this issue has existed since draft-cel-nfsv4-rpcrdma-version-two-00.
>     >
>     > Although that issue can and should be fixed, I think we need to
>     have a more complete discussion of the extension model for version
>     two and for RPC-over-RDMA as a whole in the near term.
>     >
>     > As I understand things, the original intention was that Version
>     Two be a compatible extension of Version One.  Now, with the
>     inclusion on the direction indication added in -01, and now the
>     R-key proposed in reminv-design, Version One and Version Two
>     messages will be OTW-incompatible, and I think are better off with
>     a model in which Version Two consists of  subset which equal to
>     Version One and a set of additions, primarily optional.  I think
>     we should diverge from that model only if the benefits are
>     sufficiently important to justify this discontinuity.
>
>     > With these changes, the implementation barrier to convert a
>     Version One implementation to be compatible Version Two becomes
>     significant.  I think we would be better off, if most Version One
>     implementations could be made compatible with very easily, making
>     the decision to do so more or less automatic.
>
>     > To get back to remote invalidation, I would prefer your section
>     3.3 as a baseline to be made accessible with no XDR changes in the
>     base Version Two.  The additional functionality provided by 3.4,
>     while desirable, is not of sufficient benefit to justify a
>     non-compatible XDR change.  I feel that this functionality should
>     be available as an OPTIONAL extension.
>
>     >       • That it should be documented in a separate document, as
>     opposed to being documented (eventually) in
>     draft-ietf-nfsv4-rpcrdma-version-two.
>     > What I do mean is that we should define new message type for
>     extensions (so that we maintain Version One as a subset of 
>     Version Two) and that we should (not "SHOULD" :-) make these new
>     message types "OPTIONAL" (in the RFC2119 sense).  I can see, if
>     there is sufficient reason, making an extension REQUIRED, but I
>     don't see a reason to change an existing message type in an
>     incompatible way.
>     >
>     > One way to do this is to define new message types RDMA_MESSAGEX
>     and RDMA_NOMSGX which include direction and rdma_handle but there
>     are other ways to do this.  To make it easier to determine whether
>     support for OPTIONAL message types is present, we could define a
>     transport characteristic/attribute that provides a bit mask of
>     supported message types.
>
> _______________________________________________
> nfsv4 mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/nfsv4

---

From: karen deitke <[email-masked]>
Date: Thu, 11 Aug 2016 09:10:40 -0600

Thanks.  The ietf email was dropped, adding it back so others can also 
reply.

Karen

On 8/11/2016 4:44 AM, David Noveck wrote:
> Msg received but I think we need to close the door at some point on 
> incompatible XDR changes in Version Two.  We already have two.  I 
> would have preferred zero but I can live with two that can be tested 
> for together, as described in Chuck's I-D.
>
> If there is anybody else who has a need for such an XDR change in 
> version two, I think it is time to raise the issue now and get issues 
> relating to potential XDR changes resolved before we get Version Two 
> ready to be a working group document, which I hope will be soon.
>
> On Wed, Aug 10, 2016 at 3:12 PM, karen deitke <[email-masked] 
> <mailto:[email-masked]>> wrote:
>
>
>
>     Solaris can represent one such client.  Currently we do not
>     implement frwr which is needed for RI, however when we do, we
>     would not be able to utilize RI if the client can not identify
>     which chunks should be invalidate.  Random invalidation by the
>     server would be detrimental on solaris as it would potentially
>     also invalidate other memory buffers sharing the same  stag.
>     Additionally it could invalidate other memory we intend to keep
>     registered.
>
>>
>>     Those who did not need the more extensive support would not have
>>     to deal with any
>>     XDR change at all.
>>
>>     so this all boils down to the question of how many
>>     implementations want or need to support
>>     On Thu, Aug 4, 2016 at 12:35 PM, Chuck Lever
>>     <[email-masked] <mailto:[email-masked]>> wrote:
>>
>>         Hi Dave-
>>
>>         > On Aug 4, 2016, at 10:48 AM, David Noveck
>>         <[email-masked] <mailto:[email-masked]>> wrote:
>>         >
>>         > The first issue concerns the structure
>>         rpcrdma2_chunk_lists.  Although this structure is defined in
>>         draft-cel-nfsv4-rpcrdma-version-two-01, there is nothing that
>>         uses it.  The structure that RDMA_MSG and RDMA_NOMSG include
>>         is rpcrdma2_chunks, which is not defined in the XDR.  It
>>         appear that this issue has existed since
>>         draft-cel-nfsv4-rpcrdma-version-two-00.
>>         >
>>         > Although that issue can and should be fixed, I think we
>>         need to have a more complete discussion of the extension
>>         model for version two and for RPC-over-RDMA as a whole in the
>>         near term.
>>         >
>>         > As I understand things, the original intention was that
>>         Version Two be a compatible extension of Version One.  Now,
>>         with the inclusion on the direction indication added in -01,
>>         and now the R-key proposed in reminv-design, Version One and
>>         Version Two messages will be OTW-incompatible, and I think
>>         are better off with a model in which Version Two consists of 
>>         subset which equal to Version One and a set of additions,
>>         primarily optional.  I think we should diverge from that
>>         model only if the benefits are sufficiently important to
>>         justify this discontinuity.
>>
>>         Version One implementation to be compatible Version Two
>>         becomes significant.  I think we would be better off, if most
>>         Version One implementations could be made compatible with
>>         very easily, making the decision to do so more or less automatic.
>>
>>         > To get back to remote invalidation, I would prefer your
>>         section 3.3 as a baseline to be made accessible with no XDR
>>         changes in the base Version Two.  The additional
>>         functionality provided by 3.4, while desirable, is not of
>>         sufficient benefit to justify a non-compatible XDR change.  I
>>         feel that this functionality should be available as an
>>         OPTIONAL extension.
>>
>>         >       • That it should be documented in a separate
>>         document, as opposed to being documented (eventually) in
>>         > What I do mean is that we should define new message type
>>         for extensions (so that we maintain Version One as a subset
>>         of  Version Two) and that we should (not "SHOULD" :-) make
>>         these new message types "OPTIONAL" (in the RFC2119 sense).  I
>>         can see, if there is sufficient reason, making an extension
>>         REQUIRED, but I don't see a reason to change an existing
>>         message type in an incompatible way.
>>         >
>>         > One way to do this is to define new message types
>>         RDMA_MESSAGEX and RDMA_NOMSGX which include direction and
>>         rdma_handle but there are other ways to do this.  To make it
>>         easier to determine whether support for OPTIONAL message
>>         types is present, we could define a transport
>>         characteristic/attribute that provides a bit mask of
>>         supported message types.
>>
>>     [email-masked] <mailto:[email-masked]>
>>     <https://www.ietf.org/mailman/listinfo/nfsv4>
>     _______________________________________________ nfsv4 mailing list
>     [email-masked] <mailto:[email-masked]>
>     <https://www.ietf.org/mailman/listinfo/nfsv4> 
>