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

[David Noveck <[e-mail]>]

> In this case, however, I think the benefits outweigh the costs
> 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
> all if it can't tolerate an arbitrary choice of which handle
> in an RPC is invalidated remotely.

The question is how common such clients are.

> In Linux, it's simply a matter of adding an "if vers == 2,
> insert (or expect) two u32 fields here."

In lots of places.

> With an rpcgen-based
> implementation, the change is also fairly trivial (in fact most
> of it is handled by machine-generated code).

Essentially you have two implementation rather than one extensible one.

> 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.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
> in full, an implementation would need to support RDMA_MSG,
> RDMA_NOMSG, RDMA_OPT_INIT_XCHAR, RDMA_MESSAGEX, and
> RDMA_NOMSGX.

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 <[e-mail]> wrote:

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