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

[Chuck Lever <[e-mail]>]

Hi Dave-

Thanks for your review comments.


> On Aug 4, 2016, at 10:48 AM, David Noveck <[e-mail]> 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?


> 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.

(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